From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vabhav Sharma (OSS) Date: Mon, 19 Oct 2020 08:19:25 +0000 Subject: [PATCH v2] drivers: serial: probe all uart devices In-Reply-To: References: <1601400385-11854-1-git-send-email-vabhav.sharma@oss.nxp.com> <5c9bdfee-897c-e230-d6a8-8c2dbedd1201@denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, Stefan, Thank you for feedback and time > -----Original Message----- > From: Simon Glass > Sent: Thursday, October 15, 2020 8:35 PM > To: Stefan Roese > Cc: Vabhav Sharma (OSS) ; > andre.przywara at arm.com; u-boot at lists.denx.de > Subject: Re: [PATCH v2] drivers: serial: probe all uart devices > > Hi, > > On Wed, 14 Oct 2020 at 06:47, Stefan Roese wrote: > > > > Hi Vabhav, > > > > On 14.10.20 13:15, Vabhav Sharma (OSS) wrote: > > > Hi Stefan, > > > Sorry for delayed reply, Occupied with high priority task > > > > > >> -----Original Message----- > > >> From: Stefan Roese > > >> Sent: Wednesday, September 30, 2020 10:46 AM > > >> To: Vabhav Sharma (OSS) ; > > >> andre.przywara at arm.com; u-boot at lists.denx.de; sjg at chromium.org > > >> Cc: Vabhav Sharma > > >> Subject: Re: [PATCH v2] drivers: serial: probe all uart devices > > >> > > >> On 29.09.20 19:26, Vabhav Sharma wrote: > > >>> From: Vabhav Sharma > > >>> > > >>> 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 > > >>> --- > > >>> 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. > > > > So you did measure a penalty in bootup time with this code moved into > > a function? I would have thought that this is neglectable. > > > > > Tradeoff is to choose between Function overhead Vs Binary Size, What > > > is your suggestion > > > > My vote is for the function. Mainly not because of the smaller image > > size, but because of the smaller source code base, being easier to > > maintain (IMHO). > > Same for me. Sure, I understand. > > Regards, > Simon