[U-Boot] [PATCH 1/7] serial: Use next serial device if probing fails

[Alexander Graf]

On 16.01.18 22:32, Heinrich Schuchardt wrote:
> On 01/16/2018 02:47 PM, Alexander Graf wrote:
>> Currently our serial device search chokes on the fact that the serial
>> probe function could fail. If it does, instead of searching for the next
>> usable serial device, it just quits.
>>
>> This patch changes the fallback logic so that even when a serial device
>> was not probed correctly, we just try the next ones until we find one that
>> works.
>>
>> Signed-off-by: Alexander Graf <agraf at suse.de>
>> ---
>>  drivers/serial/serial-uclass.c | 23 +++++++++++++++++++----
>>  1 file changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
>> index 2e5116f7ce..637544d1a7 100644
>> --- a/drivers/serial/serial-uclass.c
>> +++ b/drivers/serial/serial-uclass.c
>> @@ -74,6 +74,7 @@ static void serial_find_console_or_panic(void)
>>  {
>>  	const void *blob = gd->fdt_blob;
>>  	struct udevice *dev;
>> +	int ret;
>>  
>>  	if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
>>  		uclass_first_device(UCLASS_SERIAL, &dev);
>> @@ -104,8 +105,8 @@ static void serial_find_console_or_panic(void)
>>  		 * from 1!).
>>  		 *
>>  		 * Failing that, get the device with sequence number 0, or in
>> -		 * extremis just the first serial device we can find. But we
>> -		 * insist on having a console (even if it is silent).
>> +		 * extremis just the first working serial device we can find.
>> +		 * But we insist on having a console (even if it is silent).
>>  		 */
>>  #ifdef CONFIG_CONS_INDEX
>>  #define INDEX (CONFIG_CONS_INDEX - 1)
>> @@ -113,8 +114,22 @@ static void serial_find_console_or_panic(void)
>>  #define INDEX 0
>>  #endif
> 
> Why would you try probing by INDEX if CONFIG_CONS_INDEX is not defined?
> You could place the else before /* Search for any working device */.

I don't know, but I didn't want to touch that legacy code, in case
something breaks :). Also that explicit search succeeds even if probing
failed, so in a way it's different from the search for a working device.

> 
>>  		if (!uclass_get_device_by_seq(UCLASS_SERIAL, INDEX, &dev) ||
>> -		    !uclass_get_device(UCLASS_SERIAL, INDEX, &dev) ||
>> -		    (!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) {
>> +		    !uclass_get_device(UCLASS_SERIAL, INDEX, &dev)) {
>> +			if (dev->flags & DM_FLAG_ACTIVATED) {
>> +				gd->cur_serial_dev = dev;
>> +				return;
>> +			}
>> +		}
>> +
>> +		/* Search for any working device */
>> +		for (ret = uclass_first_device_check(UCLASS_SERIAL, &dev);
>> +		     dev;
>> +		     ret = uclass_next_device_check(&dev)) {
>> +			if (ret) {
>> +				/* Device did succeed probing */
> 
> Above you wrote that you want the first and not the last device.
> 
> If this line is reached upon success, wouldn't you set
> gd->cur_serial_dev here and return?

It's just a typo in the comment. It should read "Device did not succeed
probing" or maybe rather "Device failed probing".

I agree that it's slightly confusing though, so maybe I'll just swap the
logic around.


Alex