| Message ID | 1539562189-3137-1-git-send-email-vabhav.sharma@nxp.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot] drivers: serial: probe all serial devices | expand |
On Mon, Oct 15, 2018 at 8:15 PM Vabhav Sharma <vabhav.sharma@nxp.com> wrote: > > Serial subsystem search and probe only one first serial > device and unable to use remaining available UART devices > > This patch changes the logic to probe all available serial devices > using platform data or device tree in DM model in order to use all > UART devices > > Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com> > --- > drivers/serial/Kconfig | 12 ++++++++++++ > drivers/serial/serial-uclass.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 54 insertions(+) > Looks more and more devices have requirement to be probed during boot. Guess we should handle such in a unified way? Regards, Bin
On 10/15/2018 02:09 AM, Vabhav Sharma wrote: > Serial subsystem search and probe only one first serial > device and unable to use remaining available UART devices The serial devices are bound and you can switch to them. What is the real problem ? > This patch changes the logic to probe all available serial devices > using platform data or device tree in DM model in order to use all > UART devices Get rid of the ifdeffery and copied code please. > Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com> > --- > drivers/serial/Kconfig | 12 ++++++++++++ > drivers/serial/serial-uclass.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 54 insertions(+) > > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig > index 597db4b..d6451b1 100644 > --- a/drivers/serial/Kconfig > +++ b/drivers/serial/Kconfig > @@ -133,6 +133,18 @@ config SERIAL_SEARCH_ALL > > If unsure, say N. > > +config SERIAL_PROBE_ALL > + bool "Probe all available serial devices" > + depends on DM_SERIAL > + help > + The serial subsystem only probe for single serial device, but does > + not probe for remaining available devices. > + With this option set,we make probing for all available devices > + mandatory. > + > + If probing is not required for all remaining available > + devices other than default current console device, say N. > + > config SPL_DM_SERIAL > bool "Enable Driver Model for serial drivers in SPL" > depends on DM_SERIAL && SPL_DM > diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c > index e50f0aa..405e60e 100644 > --- a/drivers/serial/serial-uclass.c > +++ b/drivers/serial/serial-uclass.c > @@ -82,6 +82,13 @@ static void serial_find_console_or_panic(void) > uclass_first_device(UCLASS_SERIAL, &dev); > if (dev) { > gd->cur_serial_dev = dev; > +#ifdef CONFIG_SERIAL_PROBE_ALL > + /* Scanning uclass to probe all devices */ > + for (; > + dev; > + uclass_next_device(&dev)) > + ; > +#endif > return; > } > } else if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) { > @@ -92,11 +99,25 @@ static void serial_find_console_or_panic(void) > if (np && !uclass_get_device_by_ofnode(UCLASS_SERIAL, > np_to_ofnode(np), &dev)) { > gd->cur_serial_dev = dev; > +#ifdef CONFIG_SERIAL_PROBE_ALL > + /* Scanning uclass to probe all devices */ > + for (; > + dev; > + uclass_next_device(&dev)) > + ; > +#endif > return; > } > } else { > if (!serial_check_stdout(blob, &dev)) { > gd->cur_serial_dev = dev; > +#ifdef CONFIG_SERIAL_PROBE_ALL > + /* Scanning uclass to probe all devices */ > + for (; > + dev; > + uclass_next_device(&dev)) > + ; > +#endif > return; > } > } > @@ -121,6 +142,13 @@ static void serial_find_console_or_panic(void) > !uclass_get_device(UCLASS_SERIAL, INDEX, &dev)) { > if (dev->flags & DM_FLAG_ACTIVATED) { > gd->cur_serial_dev = dev; > +#ifdef CONFIG_SERIAL_PROBE_ALL > + /* Scanning uclass to probe all devices */ > + for (; > + dev; > + uclass_next_device(&dev)) > + ; > +#endif > return; > } > } > @@ -132,6 +160,13 @@ static void serial_find_console_or_panic(void) > if (!ret) { > /* Device did succeed probing */ > gd->cur_serial_dev = dev; > +#ifdef CONFIG_SERIAL_PROBE_ALL > + /* Scanning uclass to probe all devices */ > + for (; > + dev; > + uclass_next_device(&dev)) > + ; > +#endif > return; > } > } > @@ -140,6 +175,13 @@ static void serial_find_console_or_panic(void) > !uclass_get_device(UCLASS_SERIAL, INDEX, &dev) || > (!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) { > gd->cur_serial_dev = dev; > +#ifdef CONFIG_SERIAL_PROBE_ALL > + /* Scanning uclass to probe all devices */ > + for (; > + dev; > + uclass_next_device(&dev)) > + ; > +#endif > return; > } > #endif >
> -----Original Message----- > From: Marek Vasut <marex@denx.de> > Sent: Tuesday, October 16, 2018 12:29 PM > To: Vabhav Sharma <vabhav.sharma@nxp.com>; u-boot@lists.denx.de; u-boot- > dm@lists.denx.de; sjg@chromium.org > Cc: yamada.masahiro@socionext.com; bmeng.cn@gmail.com > Subject: Re: [U-Boot-DM] [PATCH] drivers: serial: probe all serial devices > > On 10/15/2018 02:09 AM, Vabhav Sharma wrote: > > Serial subsystem search and probe only one first serial device and > > unable to use remaining available UART devices > > The serial devices are bound and you can switch to them. What is the real > problem ? Yes, I understand switch is possible with change in DTS or platform data for choosing UART device to be used as boot console(e.g. UART0) with gd->cur_serial_dev is updated to chosen UART device. The problem was stated in email(attached 23 may) " [U-Boot-DM] QUERY:U-boot DM:SERIAL:Multiple On-chip UART Controller Support" with suggested solution from Simon which is sent as patch for review. Similar issue was faced by Andreas > > > This patch changes the logic to probe all available serial devices > > using platform data or device tree in DM model in order to use all > > UART devices > > Get rid of the ifdeffery and copied code please. This is case for using uclass_next_device() to iterate through them, Having CONFIG option doesn't make it mandatory for every platform to probe all serial devices. > > > Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com> > > --- > > drivers/serial/Kconfig | 12 ++++++++++++ > > drivers/serial/serial-uclass.c | 42 > > ++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 54 insertions(+) > > > > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index > > 597db4b..d6451b1 100644 > > --- a/drivers/serial/Kconfig > > +++ b/drivers/serial/Kconfig > > @@ -133,6 +133,18 @@ config SERIAL_SEARCH_ALL > > > > If unsure, say N. > > > > +config SERIAL_PROBE_ALL > > + bool "Probe all available serial devices" > > + depends on DM_SERIAL > > + help > > + The serial subsystem only probe for single serial device, but does > > + not probe for remaining available devices. > > + With this option set,we make probing for all available devices > > + mandatory. > > + > > + If probing is not required for all remaining available > > + devices other than default current console device, say N. > > + > > config SPL_DM_SERIAL > > bool "Enable Driver Model for serial drivers in SPL" > > depends on DM_SERIAL && SPL_DM > > diff --git a/drivers/serial/serial-uclass.c > > b/drivers/serial/serial-uclass.c index e50f0aa..405e60e 100644 > > --- a/drivers/serial/serial-uclass.c > > +++ b/drivers/serial/serial-uclass.c > > @@ -82,6 +82,13 @@ static void serial_find_console_or_panic(void) > > uclass_first_device(UCLASS_SERIAL, &dev); > > if (dev) { > > gd->cur_serial_dev = dev; > > +#ifdef CONFIG_SERIAL_PROBE_ALL > > + /* Scanning uclass to probe all devices */ > > + for (; > > + dev; > > + uclass_next_device(&dev)) > > + ; > > +#endif > > return; > > } > > } else if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) { @@ -92,11 > +99,25 > > @@ static void serial_find_console_or_panic(void) > > if (np > && !uclass_get_device_by_ofnode(UCLASS_SERIAL, > > np_to_ofnode(np), &dev)) { > > gd->cur_serial_dev = dev; > > +#ifdef CONFIG_SERIAL_PROBE_ALL > > + /* Scanning uclass to probe all devices */ > > + for (; > > + dev; > > + uclass_next_device(&dev)) > > + ; > > +#endif > > return; > > } > > } else { > > if (!serial_check_stdout(blob, &dev)) { > > gd->cur_serial_dev = dev; > > +#ifdef CONFIG_SERIAL_PROBE_ALL > > + /* Scanning uclass to probe all devices */ > > + for (; > > + dev; > > + uclass_next_device(&dev)) > > + ; > > +#endif > > return; > > } > > } > > @@ -121,6 +142,13 @@ static void serial_find_console_or_panic(void) > > !uclass_get_device(UCLASS_SERIAL, INDEX, &dev)) { > > if (dev->flags & DM_FLAG_ACTIVATED) { > > gd->cur_serial_dev = dev; > > +#ifdef CONFIG_SERIAL_PROBE_ALL > > + /* Scanning uclass to probe all devices */ > > + for (; > > + dev; > > + uclass_next_device(&dev)) > > + ; > > +#endif > > return; > > } > > } > > @@ -132,6 +160,13 @@ static void serial_find_console_or_panic(void) > > if (!ret) { > > /* Device did succeed probing */ > > gd->cur_serial_dev = dev; > > +#ifdef CONFIG_SERIAL_PROBE_ALL > > + /* Scanning uclass to probe all devices */ > > + for (; > > + dev; > > + uclass_next_device(&dev)) > > + ; > > +#endif > > return; > > } > > } > > @@ -140,6 +175,13 @@ static void serial_find_console_or_panic(void) > > !uclass_get_device(UCLASS_SERIAL, INDEX, &dev) || > > (!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) { > > gd->cur_serial_dev = dev; > > +#ifdef CONFIG_SERIAL_PROBE_ALL > > + /* Scanning uclass to probe all devices */ > > + for (; > > + dev; > > + uclass_next_device(&dev)) > > + ; > > +#endif > > return; > > } > > #endif > > > > > -- > Best regards, > Marek Vasut
> -----Original Message----- > From: Bin Meng <bmeng.cn@gmail.com> > Sent: Monday, October 15, 2018 5:58 PM > To: Vabhav Sharma <vabhav.sharma@nxp.com> > Cc: U-Boot Mailing List <u-boot@lists.denx.de>; u-boot-dm@lists.denx.de; > Simon Glass <sjg@chromium.org>; dannenberg@ti.com; Masahiro Yamada > <yamada.masahiro@socionext.com>; Stefan Roese <sr@denx.de> > Subject: Re: [PATCH] drivers: serial: probe all serial devices > > On Mon, Oct 15, 2018 at 8:15 PM Vabhav Sharma <vabhav.sharma@nxp.com> > wrote: > > > > Serial subsystem search and probe only one first serial device and > > unable to use remaining available UART devices > > > > This patch changes the logic to probe all available serial devices > > using platform data or device tree in DM model in order to use all > > UART devices > > > > Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com> > > --- > > drivers/serial/Kconfig | 12 ++++++++++++ > > drivers/serial/serial-uclass.c | 42 > > ++++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 54 insertions(+) > > > > Looks more and more devices have requirement to be probed during boot. > Guess we should handle such in a unified way? I understand, For serial devices config is included to make it optional Generic way might make it mandatory which also increase boot time. > > Regards, > Bin
On 10/16/2018 09:20 AM, Vabhav Sharma wrote: > > >> -----Original Message----- >> From: Marek Vasut <marex@denx.de> >> Sent: Tuesday, October 16, 2018 12:29 PM >> To: Vabhav Sharma <vabhav.sharma@nxp.com>; u-boot@lists.denx.de; u-boot- >> dm@lists.denx.de; sjg@chromium.org >> Cc: yamada.masahiro@socionext.com; bmeng.cn@gmail.com >> Subject: Re: [U-Boot-DM] [PATCH] drivers: serial: probe all serial devices >> >> On 10/15/2018 02:09 AM, Vabhav Sharma wrote: >>> Serial subsystem search and probe only one first serial device and >>> unable to use remaining available UART devices >> >> The serial devices are bound and you can switch to them. What is the real >> problem ? > Yes, I understand switch is possible with change in DTS or platform data for choosing UART device to be used as boot console(e.g. UART0) with gd->cur_serial_dev is updated to chosen UART device. > The problem was stated in email(attached 23 may) " [U-Boot-DM] QUERY:U-boot DM:SERIAL:Multiple On-chip UART Controller Support" with suggested solution from Simon which is sent as patch for review. > Similar issue was faced by Andreas You can also use setenv stdin/stdout/stderr to alternate between stdio devices. So what is the problem ? >>> This patch changes the logic to probe all available serial devices >>> using platform data or device tree in DM model in order to use all >>> UART devices >> >> Get rid of the ifdeffery and copied code please. > This is case for using uclass_next_device() to iterate through them, Having CONFIG option doesn't make it mandatory for every platform to probe all serial devices. Is this also the case for having 6 copies of exactly the same code ? You can turn it into a function if needed. >>> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com> >>> --- >>> drivers/serial/Kconfig | 12 ++++++++++++ >>> drivers/serial/serial-uclass.c | 42 >>> ++++++++++++++++++++++++++++++++++++++++++ >>> 2 files changed, 54 insertions(+) >>> >>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index >>> 597db4b..d6451b1 100644 >>> --- a/drivers/serial/Kconfig >>> +++ b/drivers/serial/Kconfig >>> @@ -133,6 +133,18 @@ config SERIAL_SEARCH_ALL >>> >>> If unsure, say N. >>> >>> +config SERIAL_PROBE_ALL >>> + bool "Probe all available serial devices" >>> + depends on DM_SERIAL >>> + help >>> + The serial subsystem only probe for single serial device, but does >>> + not probe for remaining available devices. >>> + With this option set,we make probing for all available devices >>> + mandatory. >>> + >>> + If probing is not required for all remaining available >>> + devices other than default current console device, say N. >>> + >>> config SPL_DM_SERIAL >>> bool "Enable Driver Model for serial drivers in SPL" >>> depends on DM_SERIAL && SPL_DM >>> diff --git a/drivers/serial/serial-uclass.c >>> b/drivers/serial/serial-uclass.c index e50f0aa..405e60e 100644 >>> --- a/drivers/serial/serial-uclass.c >>> +++ b/drivers/serial/serial-uclass.c >>> @@ -82,6 +82,13 @@ static void serial_find_console_or_panic(void) >>> uclass_first_device(UCLASS_SERIAL, &dev); >>> if (dev) { >>> gd->cur_serial_dev = dev; >>> +#ifdef CONFIG_SERIAL_PROBE_ALL >>> + /* Scanning uclass to probe all devices */ >>> + for (; >>> + dev; >>> + uclass_next_device(&dev)) >>> + ; >>> +#endif >>> return; >>> } >>> } else if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) { @@ -92,11 >> +99,25 >>> @@ static void serial_find_console_or_panic(void) >>> if (np >> && !uclass_get_device_by_ofnode(UCLASS_SERIAL, >>> np_to_ofnode(np), &dev)) { >>> gd->cur_serial_dev = dev; >>> +#ifdef CONFIG_SERIAL_PROBE_ALL >>> + /* Scanning uclass to probe all devices */ >>> + for (; >>> + dev; >>> + uclass_next_device(&dev)) >>> + ; >>> +#endif >>> return; >>> } >>> } else { >>> if (!serial_check_stdout(blob, &dev)) { >>> gd->cur_serial_dev = dev; >>> +#ifdef CONFIG_SERIAL_PROBE_ALL >>> + /* Scanning uclass to probe all devices */ >>> + for (; >>> + dev; >>> + uclass_next_device(&dev)) >>> + ; >>> +#endif >>> return; >>> } >>> } >>> @@ -121,6 +142,13 @@ static void serial_find_console_or_panic(void) >>> !uclass_get_device(UCLASS_SERIAL, INDEX, &dev)) { >>> if (dev->flags & DM_FLAG_ACTIVATED) { >>> gd->cur_serial_dev = dev; >>> +#ifdef CONFIG_SERIAL_PROBE_ALL >>> + /* Scanning uclass to probe all devices */ >>> + for (; >>> + dev; >>> + uclass_next_device(&dev)) >>> + ; >>> +#endif >>> return; >>> } >>> } >>> @@ -132,6 +160,13 @@ static void serial_find_console_or_panic(void) >>> if (!ret) { >>> /* Device did succeed probing */ >>> gd->cur_serial_dev = dev; >>> +#ifdef CONFIG_SERIAL_PROBE_ALL >>> + /* Scanning uclass to probe all devices */ >>> + for (; >>> + dev; >>> + uclass_next_device(&dev)) >>> + ; >>> +#endif >>> return; >>> } >>> } >>> @@ -140,6 +175,13 @@ static void serial_find_console_or_panic(void) >>> !uclass_get_device(UCLASS_SERIAL, INDEX, &dev) || >>> (!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) { >>> gd->cur_serial_dev = dev; >>> +#ifdef CONFIG_SERIAL_PROBE_ALL >>> + /* Scanning uclass to probe all devices */ >>> + for (; >>> + dev; >>> + uclass_next_device(&dev)) >>> + ; >>> +#endif >>> return; >>> } >>> #endif >>> >> >> >> -- >> Best regards, >> Marek Vasut
Hi Bin, On 15 October 2018 at 06:28, Bin Meng <bmeng.cn@gmail.com> wrote: > On Mon, Oct 15, 2018 at 8:15 PM Vabhav Sharma <vabhav.sharma@nxp.com> wrote: >> >> Serial subsystem search and probe only one first serial >> device and unable to use remaining available UART devices >> >> This patch changes the logic to probe all available serial devices >> using platform data or device tree in DM model in order to use all >> UART devices >> >> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com> >> --- >> drivers/serial/Kconfig | 12 ++++++++++++ >> drivers/serial/serial-uclass.c | 42 ++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 54 insertions(+) >> > > Looks more and more devices have requirement to be probed during boot. > Guess we should handle such in a unified way? How about: - in dm_init(), after binding, we move on to an auto-probe step - a uclass flag indicates that all devices in that uclass must be probed (that will be useful for PCI) - a way to tell DM to probe all devices in a particular uclass (e.g. a call to dm_set_uclass_autoprobe(enum uclass_id, bool) (that will be useful for this serial case) Regards, Simon
Hi Marek Vasut, Apology for delayed reply, Occupied with other work > -----Original Message----- > From: Marek Vasut <marex@denx.de> > Sent: Tuesday, October 16, 2018 2:39 PM > To: Vabhav Sharma <vabhav.sharma@nxp.com>; u-boot@lists.denx.de; u- > boot-dm@lists.denx.de; sjg@chromium.org > Cc: yamada.masahiro@socionext.com; bmeng.cn@gmail.com; Andreas > Dannenberg <dannenberg@ti.com> > Subject: Re: [U-Boot-DM] [PATCH] drivers: serial: probe all serial devices > > On 10/16/2018 09:20 AM, Vabhav Sharma wrote: > > > > > >> -----Original Message----- > >> From: Marek Vasut <marex@denx.de> > >> Sent: Tuesday, October 16, 2018 12:29 PM > >> To: Vabhav Sharma <vabhav.sharma@nxp.com>; u-boot@lists.denx.de; > >> u-boot- dm@lists.denx.de; sjg@chromium.org > >> Cc: yamada.masahiro@socionext.com; bmeng.cn@gmail.com > >> Subject: Re: [U-Boot-DM] [PATCH] drivers: serial: probe all serial > >> devices > >> > >> On 10/15/2018 02:09 AM, Vabhav Sharma wrote: > >>> Serial subsystem search and probe only one first serial device and > >>> unable to use remaining available UART devices > >> > >> The serial devices are bound and you can switch to them. What is the > >> real problem ? > > Yes, I understand switch is possible with change in DTS or platform data for > choosing UART device to be used as boot console(e.g. UART0) with gd- > >cur_serial_dev is updated to chosen UART device. > > The problem was stated in email(attached 23 may) " [U-Boot-DM] > QUERY:U-boot DM:SERIAL:Multiple On-chip UART Controller Support" with > suggested solution from Simon which is sent as patch for review. > > Similar issue was faced by Andreas > > You can also use setenv stdin/stdout/stderr to alternate between stdio > devices. So what is the problem ? Problem is seen with PL011 driver using DM model, Only boot console baud rate is set. Tried modifying the environment variable but seems readonly (## Error inserting "stdout" variable, errno=22) Multiple UART enablement is required to use all console. > > >>> This patch changes the logic to probe all available serial devices > >>> using platform data or device tree in DM model in order to use all > >>> UART devices > >> > >> Get rid of the ifdeffery and copied code please. > > This is case for using uclass_next_device() to iterate through them, Having > CONFIG option doesn't make it mandatory for every platform to probe all > serial devices. > > Is this also the case for having 6 copies of exactly the same code ? You can > turn it into a function if needed. Ok, I understand > > >>> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com> > >>> --- > >>> drivers/serial/Kconfig | 12 ++++++++++++ > >>> drivers/serial/serial-uclass.c | 42 > >>> ++++++++++++++++++++++++++++++++++++++++++ > >>> 2 files changed, 54 insertions(+) > >>> > >>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index > >>> 597db4b..d6451b1 100644 > >>> --- a/drivers/serial/Kconfig > >>> +++ b/drivers/serial/Kconfig > >>> @@ -133,6 +133,18 @@ config SERIAL_SEARCH_ALL > >>> > >>> If unsure, say N. > >>> > >>> +config SERIAL_PROBE_ALL > >>> + bool "Probe all available serial devices" > >>> + depends on DM_SERIAL > >>> + help > >>> + The serial subsystem only probe for single serial device, but does > >>> + not probe for remaining available devices. > >>> + With this option set,we make probing for all available devices > >>> + mandatory. > >>> + > >>> + If probing is not required for all remaining available > >>> + devices other than default current console device, say N. > >>> + > >>> config SPL_DM_SERIAL > >>> bool "Enable Driver Model for serial drivers in SPL" > >>> depends on DM_SERIAL && SPL_DM > >>> diff --git a/drivers/serial/serial-uclass.c > >>> b/drivers/serial/serial-uclass.c index e50f0aa..405e60e 100644 > >>> --- a/drivers/serial/serial-uclass.c > >>> +++ b/drivers/serial/serial-uclass.c > >>> @@ -82,6 +82,13 @@ static void serial_find_console_or_panic(void) > >>> uclass_first_device(UCLASS_SERIAL, &dev); > >>> if (dev) { > >>> gd->cur_serial_dev = dev; > >>> +#ifdef CONFIG_SERIAL_PROBE_ALL > >>> + /* Scanning uclass to probe all devices */ > >>> + for (; > >>> + dev; > >>> + uclass_next_device(&dev)) > >>> + ; > >>> +#endif > >>> return; > >>> } > >>> } else if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) { @@ -92,11 > >> +99,25 > >>> @@ static void serial_find_console_or_panic(void) > >>> if (np > >> && !uclass_get_device_by_ofnode(UCLASS_SERIAL, > >>> np_to_ofnode(np), &dev)) { > >>> gd->cur_serial_dev = dev; > >>> +#ifdef CONFIG_SERIAL_PROBE_ALL > >>> + /* Scanning uclass to probe all devices */ > >>> + for (; > >>> + dev; > >>> + uclass_next_device(&dev)) > >>> + ; > >>> +#endif > >>> return; > >>> } > >>> } else { > >>> if (!serial_check_stdout(blob, &dev)) { > >>> gd->cur_serial_dev = dev; > >>> +#ifdef CONFIG_SERIAL_PROBE_ALL > >>> + /* Scanning uclass to probe all devices */ > >>> + for (; > >>> + dev; > >>> + uclass_next_device(&dev)) > >>> + ; > >>> +#endif > >>> return; > >>> } > >>> } > >>> @@ -121,6 +142,13 @@ static void serial_find_console_or_panic(void) > >>> !uclass_get_device(UCLASS_SERIAL, INDEX, &dev)) { > >>> if (dev->flags & DM_FLAG_ACTIVATED) { > >>> gd->cur_serial_dev = dev; > >>> +#ifdef CONFIG_SERIAL_PROBE_ALL > >>> + /* Scanning uclass to probe all devices */ > >>> + for (; > >>> + dev; > >>> + uclass_next_device(&dev)) > >>> + ; > >>> +#endif > >>> return; > >>> } > >>> } > >>> @@ -132,6 +160,13 @@ static void serial_find_console_or_panic(void) > >>> if (!ret) { > >>> /* Device did succeed probing */ > >>> gd->cur_serial_dev = dev; > >>> +#ifdef CONFIG_SERIAL_PROBE_ALL > >>> + /* Scanning uclass to probe all devices */ > >>> + for (; > >>> + dev; > >>> + uclass_next_device(&dev)) > >>> + ; > >>> +#endif > >>> return; > >>> } > >>> } > >>> @@ -140,6 +175,13 @@ static void serial_find_console_or_panic(void) > >>> !uclass_get_device(UCLASS_SERIAL, INDEX, &dev) || > >>> (!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) { > >>> gd->cur_serial_dev = dev; > >>> +#ifdef CONFIG_SERIAL_PROBE_ALL > >>> + /* Scanning uclass to probe all devices */ > >>> + for (; > >>> + dev; > >>> + uclass_next_device(&dev)) > >>> + ; > >>> +#endif > >>> return; > >>> } > >>> #endif > >>> > >> > >> > >> -- > >> Best regards, > >> Marek Vasut > > > -- > Best regards, > Marek Vasut
> -----Original Message----- > From: sjg@google.com <sjg@google.com> On Behalf Of Simon Glass > Sent: Friday, October 19, 2018 8:56 AM > To: Bin Meng <bmeng.cn@gmail.com> > Cc: Vabhav Sharma <vabhav.sharma@nxp.com>; U-Boot Mailing List <u- > boot@lists.denx.de>; u-boot-dm@lists.denx.de; Andreas Dannenberg > <dannenberg@ti.com>; Masahiro Yamada > <yamada.masahiro@socionext.com>; Stefan Roese <sr@denx.de> > Subject: Re: [PATCH] drivers: serial: probe all serial devices > > Hi Bin, > > On 15 October 2018 at 06:28, Bin Meng <bmeng.cn@gmail.com> wrote: > > On Mon, Oct 15, 2018 at 8:15 PM Vabhav Sharma > <vabhav.sharma@nxp.com> wrote: > >> > >> Serial subsystem search and probe only one first serial device and > >> unable to use remaining available UART devices > >> > >> This patch changes the logic to probe all available serial devices > >> using platform data or device tree in DM model in order to use all > >> UART devices > >> > >> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com> > >> --- > >> drivers/serial/Kconfig | 12 ++++++++++++ > >> drivers/serial/serial-uclass.c | 42 > >> ++++++++++++++++++++++++++++++++++++++++++ > >> 2 files changed, 54 insertions(+) > >> > > > > Looks more and more devices have requirement to be probed during boot. > > Guess we should handle such in a unified way? > > How about: > > - in dm_init(), after binding, we move on to an auto-probe step > - a uclass flag indicates that all devices in that uclass must be probed (that > will be useful for PCI) For devices class respective UCLASS to be provided with auto-probe, How many class to be included? > > - a way to tell DM to probe all devices in a particular uclass (e.g. a call to > dm_set_uclass_autoprobe(enum uclass_id, bool) > (that will be useful for this serial case) Do you mean to verify and send patch for serial uclass with this approach after device_probe in dm_init > > Regards, > Simon
Dear Vabhav, In message <VI1PR04MB48001E65CD3F477EC8CE6EB9F3F70@VI1PR04MB4800.eurprd04.prod.outlook.com> you wrote: > > > You can also use setenv stdin/stdout/stderr to alternate between stdio > > devices. So what is the problem ? > Problem is seen with PL011 driver using DM model, Only boot console baud rate is set. U-Boot can talk to only one serial device at a time anyway, so why should it intiaalize othe rports than the one used for the (then current) console? U-Boot does lazy initializationintentionally. It is wrong to initialize devices which are not actually used. > Tried modifying the environment variable but seems readonly (## > Error inserting "stdout" variable, errno=22) How exactly did you try to do that? > Multiple UART enablement is required to use all console. I have a feeling that you attempt to do the Wong Thing. Best regards, Wolfgang Denk
> -----Original Message----- > From: Wolfgang Denk <wd@denx.de> > Sent: Thursday, October 25, 2018 7:35 PM > To: Vabhav Sharma <vabhav.sharma@nxp.com> > Cc: Marek Vasut <marex@denx.de>; u-boot@lists.denx.de; u-boot- > dm@lists.denx.de; sjg@chromium.org; yamada.masahiro@socionext.com; > bmeng.cn@gmail.com > Subject: Re: [U-Boot-DM] [PATCH] drivers: serial: probe all serial devices > > Dear Vabhav, > > In message > <VI1PR04MB48001E65CD3F477EC8CE6EB9F3F70@VI1PR04MB4800.eurprd04. > prod.outlook.com> you wrote: > > > > > You can also use setenv stdin/stdout/stderr to alternate between > > > stdio devices. So what is the problem ? > > Problem is seen with PL011 driver using DM model, Only boot console > baud rate is set. > > U-Boot can talk to only one serial device at a time anyway, so why should it > intiaalize othe rports than the one used for the (then > current) console? > > U-Boot does lazy initializationintentionally. It is wrong to initialize devices > which are not actually used. There is configuration option to enable the device using platform data or device tree using DM model. For e.g: 2 UART controllers are enabled in device tree but DM model initialized only one, This is limitation My suggestion is to initialize all devices which are enabled, E.g. use case is using UART1 for uboot consoled and UART2 for linux boot On NXP SoC, We also use UART3 for ethernet firmware logging but using DM model all enabled devices are not probed posing limitation > > > Tried modifying the environment variable but seems readonly (## Error > > inserting "stdout" variable, errno=22) > > How exactly did you try to do that? #editenv stdout > > > Multiple UART enablement is required to use all console. > > I have a feeling that you attempt to do the Wong Thing. I quoted the reason above and also discussed on in email(23 may) " [U-Boot-DM] QUERY:U-boot DM:SERIAL:Multiple On-chip UART Controller Support" with suggested solution from Simon which is sent as patch for review. Similar issue was faced by Andreas > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de I > am an atheist, thank God!
Hi Simon, On Fri, Oct 19, 2018 at 11:26 AM Simon Glass <sjg@chromium.org> wrote: > > Hi Bin, > > On 15 October 2018 at 06:28, Bin Meng <bmeng.cn@gmail.com> wrote: > > On Mon, Oct 15, 2018 at 8:15 PM Vabhav Sharma <vabhav.sharma@nxp.com> wrote: > >> > >> Serial subsystem search and probe only one first serial > >> device and unable to use remaining available UART devices > >> > >> This patch changes the logic to probe all available serial devices > >> using platform data or device tree in DM model in order to use all > >> UART devices > >> > >> Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com> > >> --- > >> drivers/serial/Kconfig | 12 ++++++++++++ > >> drivers/serial/serial-uclass.c | 42 ++++++++++++++++++++++++++++++++++++++++++ > >> 2 files changed, 54 insertions(+) > >> > > > > Looks more and more devices have requirement to be probed during boot. > > Guess we should handle such in a unified way? > > How about: > > - in dm_init(), after binding, we move on to an auto-probe step > - a uclass flag indicates that all devices in that uclass must be probed > (that will be useful for PCI) > > - a way to tell DM to probe all devices in a particular uclass (e.g. a > call to dm_set_uclass_autoprobe(enum uclass_id, bool) > (that will be useful for this serial case) Sounds good to me. Thanks for checking. Regards, Bin
Dear Vabhav, In message <VI1PR04MB48005B3CB205F14C4670CC2BF3F70@VI1PR04MB4800.eurprd04.prod.outlook.com> you wrote: > > > U-Boot does lazy initialization intentionally. It is wrong to initialize devices > > which are not actually used. > There is configuration option to enable the device using platform > data or device tree using DM model. "enable" means we have all required information needed to initialize them, if we need them. The need arises only when someone wants to transfer data over that UART. If nobody uses the UArt, there is no need to ever initialize it. Lazy initialization is a good thing - it saves resources, especially memory and time. Think for example of boot time optimizations... > For e.g: 2 UART controllers are enabled in device tree but DM > model initialized only one, This is limitation In which way is this a limitation? We use only one UART (for the serial console), so it works as supposed: only the used devices are fully initialized. > My suggestion is to initialize all devices which are enabled, E.g. > use case is using UART1 for uboot consoled and UART2 for linux boot What do you mean "for Linux boot"? If Linux needs the UART, it will initialize it itself in the Linux dvice driver. > On NXP SoC, We also use UART3 for ethernet firmware logging but > using DM model all enabled devices are not probed posing > limitation. Again, this is intentional, and not a limitation. And what exactly do you mean by "UART3 for ethernet firmware logging"? Maybe you should rethink your software concept. We don't want to have a zillion of ports in used if not really necessary. Either you use standard channels (STDOUT, STDERR) for logging. Also, there used to be a syslog compatible logging driver. > > I have a feeling that you attempt to do the Wong Thing. > I quoted the reason above and also discussed on in email(23 may) " > [U-Boot-DM] QUERY:U-boot DM:SERIAL:Multiple On-chip UART > Controller Support" with suggested solution from Simon which is > sent as patch for review. But Simon also explained "U-Boot only probes things in a 'lazy' manner so far", and there should be really good reasons for deviating from this principle. In no way should a "probe all devices" be made the default. And Andreas also wrote how to solve this issue in a board-specific way if it is really, really needed on some board. However, you still fail to explain why you ned multiple serial ports in U-Boot running in parallel. Best regards, Wolfgang Denk
> -----Original Message----- > From: Wolfgang Denk <wd@denx.de> > Sent: Friday, October 26, 2018 4:57 PM > To: Vabhav Sharma <vabhav.sharma@nxp.com> > Cc: Marek Vasut <marex@denx.de>; u-boot@lists.denx.de; u-boot- > dm@lists.denx.de; sjg@chromium.org; yamada.masahiro@socionext.com; > bmeng.cn@gmail.com > Subject: Re: [U-Boot-DM] [PATCH] drivers: serial: probe all serial devices > > Dear Vabhav, > > In message > <VI1PR04MB48005B3CB205F14C4670CC2BF3F70@VI1PR04MB4800.eurprd04. > prod.outlook.com> you wrote: > > > > > U-Boot does lazy initialization intentionally. It is wrong to > > > initialize devices which are not actually used. > > > There is configuration option to enable the device using platform data > > or device tree using DM model. > > "enable" means we have all required information needed to initialize them, > if we need them. The need arises only when someone wants to transfer > data over that UART. If nobody uses the UArt, there is no need to ever > initialize it. On u-boot, only one console is used There is need to use multiple console for network firmware logging, linux boot > > Lazy initialization is a good thing - it saves resources, especially memory and > time. Think for example of boot time optimizations... I understand > > > For e.g: 2 UART controllers are enabled in device tree but DM model > > initialized only one, This is limitation > > In which way is this a limitation? We use only one UART (for the serial > console), so it works as supposed: only the used devices are fully initialized. Limitation for aforesaid example > > > > My suggestion is to initialize all devices which are enabled, E.g. > > use case is using UART1 for uboot consoled and UART2 for linux boot > > What do you mean "for Linux boot"? If Linux needs the UART, it will > initialize it itself in the Linux dvice driver. Example is PL011 UART linux driver which expect initialization(integer and fractional baud date) done by bootloader > > > On NXP SoC, We also use UART3 for ethernet firmware logging but using > > DM model all enabled devices are not probed posing limitation. > > Again, this is intentional, and not a limitation. > > And what exactly do you mean by "UART3 for ethernet firmware logging"? Different console than linux boot console. > > Maybe you should rethink your software concept. We don't want to have a > zillion of ports in used if not really necessary. Either you use standard > channels (STDOUT, STDERR) for logging. Also, there used to be a syslog > compatible logging driver. Agree > > > > I have a feeling that you attempt to do the Wong Thing. > > > I quoted the reason above and also discussed on in email(23 may) " > > [U-Boot-DM] QUERY:U-boot DM:SERIAL:Multiple On-chip UART Controller > > Support" with suggested solution from Simon which is sent as patch for > > review. > > But Simon also explained "U-Boot only probes things in a 'lazy' > manner so far", and there should be really good reasons for deviating from > this principle. In no way should a "probe all devices" be made the default. I see. Not default and config will be selected as required Sent patch has config option > > And Andreas also wrote how to solve this issue in a board-specific way if it is > really, really needed on some board. > > However, you still fail to explain why you ned multiple serial ports in U-Boot > running in parallel. I am trying to convince you for the discussed problem faced recently in DM model > > Best regards, > > Wolfgang Denk > > -- > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de > Well, the way I see it, logic is only a way of being ignorant by num- > bers. - Terry Pratchett, _Small Gods_
Dear Vabhav, In message <VI1PR04MB480032AF6B3C36FACA4318B6F3CF0@VI1PR04MB4800.eurprd04.prod.outlook.com> you wrote: > > > "enable" means we have all required information needed to initialize them, > > if we need them. The need arises only when someone wants to transfer > > data over that UART. If nobody uses the UArt, there is no need to ever > > initialize it. > On u-boot, only one console is used Right. And in any case, you can ALWAYS use only one serial driver at a time, as U-Boot is strictly single-tasking. It does not make a difference whether you initialize one or twenty UARTS - you can use always exactly one at any point of time. > There is need to use multiple console for network firmware logging, linux boot Yes, you repeat that. But you don't explain why you think this is needed. For example, why would you need a UART for "network logging"? Why would LInux need a different console port than U-Boot? This all makes no sense to me... > > In which way is this a limitation? We use only one UART (for the serial > > console), so it works as supposed: only the used devices are fully initialized. > Limitation for aforesaid example You give no comprehensible example. You say you need it, but don't explain why, or how you would actually use it with only one serial port active at any time. > > What do you mean "for Linux boot"? If Linux needs the UART, it will > > initialize it itself in the Linux dvice driver. > Example is PL011 UART linux driver which expect initialization(integer and fractional baud date) done by bootloader In this case the Linux driver is broken and needs to be fixed. Alternatively, just _use_ this port in U-Boot before booting. No need to initialize other ports, though. > > And what exactly do you mean by "UART3 for ethernet firmware logging"? > Different console than linux boot console. I don't see why this would make sense. But as metioned several times before, you can always only use a single port at any time in U-Boot. Best regards, Wolfgang Denk
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index 597db4b..d6451b1 100644 --- a/drivers/serial/Kconfig +++ b/drivers/serial/Kconfig @@ -133,6 +133,18 @@ config SERIAL_SEARCH_ALL If unsure, say N. +config SERIAL_PROBE_ALL + bool "Probe all available serial devices" + depends on DM_SERIAL + help + The serial subsystem only probe for single serial device, but does + not probe for remaining available devices. + With this option set,we make probing for all available devices + mandatory. + + If probing is not required for all remaining available + devices other than default current console device, say N. + config SPL_DM_SERIAL bool "Enable Driver Model for serial drivers in SPL" depends on DM_SERIAL && SPL_DM diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index e50f0aa..405e60e 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -82,6 +82,13 @@ static void serial_find_console_or_panic(void) uclass_first_device(UCLASS_SERIAL, &dev); if (dev) { gd->cur_serial_dev = dev; +#ifdef CONFIG_SERIAL_PROBE_ALL + /* Scanning uclass to probe all devices */ + for (; + dev; + uclass_next_device(&dev)) + ; +#endif return; } } else if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) { @@ -92,11 +99,25 @@ static void serial_find_console_or_panic(void) if (np && !uclass_get_device_by_ofnode(UCLASS_SERIAL, np_to_ofnode(np), &dev)) { gd->cur_serial_dev = dev; +#ifdef CONFIG_SERIAL_PROBE_ALL + /* Scanning uclass to probe all devices */ + for (; + dev; + uclass_next_device(&dev)) + ; +#endif return; } } else { if (!serial_check_stdout(blob, &dev)) { gd->cur_serial_dev = dev; +#ifdef CONFIG_SERIAL_PROBE_ALL + /* Scanning uclass to probe all devices */ + for (; + dev; + uclass_next_device(&dev)) + ; +#endif return; } } @@ -121,6 +142,13 @@ static void serial_find_console_or_panic(void) !uclass_get_device(UCLASS_SERIAL, INDEX, &dev)) { if (dev->flags & DM_FLAG_ACTIVATED) { gd->cur_serial_dev = dev; +#ifdef CONFIG_SERIAL_PROBE_ALL + /* Scanning uclass to probe all devices */ + for (; + dev; + uclass_next_device(&dev)) + ; +#endif return; } } @@ -132,6 +160,13 @@ static void serial_find_console_or_panic(void) if (!ret) { /* Device did succeed probing */ gd->cur_serial_dev = dev; +#ifdef CONFIG_SERIAL_PROBE_ALL + /* Scanning uclass to probe all devices */ + for (; + dev; + uclass_next_device(&dev)) + ; +#endif return; } } @@ -140,6 +175,13 @@ static void serial_find_console_or_panic(void) !uclass_get_device(UCLASS_SERIAL, INDEX, &dev) || (!uclass_first_device(UCLASS_SERIAL, &dev) && dev)) { gd->cur_serial_dev = dev; +#ifdef CONFIG_SERIAL_PROBE_ALL + /* Scanning uclass to probe all devices */ + for (; + dev; + uclass_next_device(&dev)) + ; +#endif return; } #endif
Serial subsystem search and probe only one first serial device and unable to use remaining available UART devices This patch changes the logic to probe all available serial devices using platform data or device tree in DM model in order to use all UART devices Signed-off-by: Vabhav Sharma <vabhav.sharma@nxp.com> --- drivers/serial/Kconfig | 12 ++++++++++++ drivers/serial/serial-uclass.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+)