[PATCH v2] drivers: serial: probe all uart devices

[Vabhav Sharma (OSS)]

Hi Stefan,
Sorry for delayed reply, Occupied with high priority task

> -----Original Message-----
> From: Stefan Roese <sr at denx.de>
> Sent: Wednesday, September 30, 2020 10:46 AM
> To: Vabhav Sharma (OSS) <vabhav.sharma at oss.nxp.com>;
> andre.przywara at arm.com; u-boot at lists.denx.de; sjg at chromium.org
> Cc: Vabhav Sharma <vabhav.sharma at nxp.com>
> Subject: Re: [PATCH v2] drivers: serial: probe all uart devices
> 
> On 29.09.20 19:26, Vabhav Sharma wrote:
> > From: Vabhav Sharma <vabhav.sharma at nxp.com>
> >
> > U-Boot DM model probe only single device at a time which is enabled
> > and configured using device tree or platform data method.
> >
> > PL011 UART IP is SBSA compliant and firmware does the serial port
> > set-up, initialization and let the kernel use UART port for sending
> > and receiving characters.
> >
> > Normally software talk to one serial port time but some LayerScape
> > platform require all the UART devices enabled in Linux for various use
> > case.
> >
> > Adding support to probe all enabled serial devices like SBSA compliant
> > PL011 UART ports probe and initialization by firmware.
> >
> > Signed-off-by: Vabhav Sharma <vabhav.sharma at nxp.com>
> > ---
> > v2:
> >     Incorporated Stefan review comment, Update #ifdef with macro
> >     if (IS_ENABLED)..
> 
> Better, thanks. But...
> 
> > ---
> > ---
> >   drivers/serial/Kconfig         | 17 +++++++++++++++++
> >   drivers/serial/serial-uclass.c | 30 ++++++++++++++++++++++++++++++
> >   2 files changed, 47 insertions(+)
> >
> > diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index
> > e344677..b2e30f1 100644
> > --- a/drivers/serial/Kconfig
> > +++ b/drivers/serial/Kconfig
> > @@ -134,6 +134,23 @@ config SERIAL_SEARCH_ALL
> >
> >   	  If unsure, say N.
> >
> > +config SERIAL_PROBE_ALL
> > +	bool "Probe all available serial devices"
> > +	depends on DM_SERIAL
> > +	default n
> > +	help
> > +	  The serial subsystem only probe for single serial device,
> > +	  but does not probe for other remaining serial devices.
> > +	  With this option set,we make probing and searching for
> > +	  all available devices optional.
> > +	  Normally, U-Boot talk to one serial port at a time but SBSA
> > +	  compliant UART devices like PL011 require initialization
> > +	  by firmware and let the kernel use serial port for sending
> > +	  and receiving the characters.
> > +
> > +	  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 0027625..d303a66 100644
> > --- a/drivers/serial/serial-uclass.c
> > +++ b/drivers/serial/serial-uclass.c
> > @@ -86,6 +86,11 @@ static void serial_find_console_or_panic(void)
> >   		uclass_first_device(UCLASS_SERIAL, &dev);
> >   		if (dev) {
> >   			gd->cur_serial_dev = dev;
> > +			if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
> > +				/* Scanning uclass to probe all devices */
> > +				for (; dev; uclass_next_device(&dev))
> > +					;
> > +			}
> >   			return;
> >   		}
> >   	} else if (CONFIG_IS_ENABLED(OF_CONTROL) && blob) { @@ -96,11
> > +101,21 @@ 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;
> > +				if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL))
> {
> > +					/* Scanning uclass to probe devices
> */
> > +					for (; dev; uclass_next_device(&dev))
> > +						;
> > +				}
> 
> This code sequence (incl. gd->cur_serial_dev = dev;) is repeated multiple
> times (below as well). Could you instead move this into a function and call
> this function to reduce code and binary size?
I understand.
Checked and found that 56 bytes of code is added (including enabling the macro on LS platform)

Intended to define it earlier but defining one line of code in a function cause more overhead (function call, Stack operations, Pointer arithmetic) in execution time.

Tradeoff is to choose between Function overhead Vs Binary Size, What is your suggestion
> 
> Thanks,
> Stefan
> 
> >   				return;
> >   			}
> >   		} else {
> >   			if (!serial_check_stdout(blob, &dev)) {
> >   				gd->cur_serial_dev = dev;
> > +				if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL))
> {
> > +					/* Scanning uclass to probe devices
> */
> > +					for (; dev; uclass_next_device(&dev))
> > +						;
> > +				}
> >   				return;
> >   			}
> >   		}
> > @@ -125,6 +140,11 @@ 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;
> > +				if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL))
> {
> > +					/* Scanning uclass to probe devices
> */
> > +					for (; dev; uclass_next_device(&dev))
> > +						;
> > +				}
> >   				return;
> >   			}
> >   		}
> > @@ -136,6 +156,11 @@ static void serial_find_console_or_panic(void)
> >   			if (!ret) {
> >   				/* Device did succeed probing */
> >   				gd->cur_serial_dev = dev;
> > +				if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL))
> {
> > +					/* Scanning uclass to probe devices
> */
> > +					for (; dev; uclass_next_device(&dev))
> > +						;
> > +				}
> >   				return;
> >   			}
> >   		}
> > @@ -144,6 +169,11 @@ 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;
> > +			if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL)) {
> > +				/* Scanning uclass to probe all devices */
> > +				for (; dev; uclass_next_device(&dev))
> > +					;
> > +			}
> >   			return;
> >   		}
> >   #endif
> >
> 
> 
> Viele Grüße,
> Stefan
> 
> --
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de