| Message ID | 20180116134741.4103-2-agraf@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RPi: Properly handle dynamic serial configuration | expand |
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@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 */. > 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? Best regards Heinrich > + continue; > + } > + > gd->cur_serial_dev = dev; > return; > } >
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@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
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 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 */ + continue; + } + gd->cur_serial_dev = dev; return; }
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@suse.de> --- drivers/serial/serial-uclass.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)