[U-Boot] [PATCH] drivers: serial: probe all serial devices
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(+)
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
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
-----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
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
-----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
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
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 doesnot probe for remaining available devices.With this option set,we make probing for all available devicesmandatory.If probing is not required for all remaining availabledevices 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 doesnot probe for remaining available devices.With this option set,we make probing for all available devicesmandatory.If probing is not required for all remaining availabledevices 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
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 doesnot probe for remaining available devices.With this option set,we make probing for all available devicesmandatory.If probing is not required for all remaining availabledevices 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 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 doesnot probe for remaining available devices.With this option set,we make probing for all available devicesmandatory.If probing is not required for all remaining availabledevices 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
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!
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
participants (5)
-
Bin Meng -
Marek Vasut -
Simon Glass -
Vabhav Sharma -
Wolfgang Denk