[PATCH v4 2/2] drivers: serial: probe all uart devices

[Sean Anderson]

On 11/23/20 2:14 AM, Vabhav Sharma (OSS) wrote:
> 
> 
>> -----Original Message-----
>> From: Sean Anderson <seanga2 at gmail.com>
>> Sent: Thursday, November 19, 2020 9:01 AM
>> To: Vabhav Sharma (OSS) <vabhav.sharma at oss.nxp.com>;
>> sjg at chromium.org; sr at denx.de
>> Cc: u-boot at lists.denx.de; Varun Sethi <V.Sethi at nxp.com>;
>> andre.przywara at arm.com; Vabhav Sharma <vabhav.sharma at nxp.com>
>> Subject: Re: [PATCH v4 2/2] drivers: serial: probe all uart devices
>>
>> On 11/17/20 10:00 AM, 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>
>>> Reviewed-by: Stefan Roese <sr at denx.de>
>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>> --
>>> v3:
>>>    Incorporated Simon and Stephan review comment
>>>    - Define generic function uclass_probe_all(enum uclass_id)
>>>      in drivers/core/uclass.c
>>>    - Added the function in caller of serial_find_console_or_panic()
>>>    - Removed repeated sequence with generic function call uclass_probe_all()
>>>    - Dependent on other patch [PATCH] dm: core: add function
>> uclass_probe_all()
>>>      to probe all devices
>>>
>>> v2:
>>>    Incorporated Stefan review comment, Update #ifdef with macro
>>>    if (IS_ENABLED)..
>>> ---
>>>   drivers/serial/Kconfig         | 17 +++++++++++++++++
>>>   drivers/serial/serial-uclass.c |  4 ++++
>>>   2 files changed, 21 insertions(+)
>>>
>>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig index
>>> b4805a2..af8779b 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,
>>
>> nit: probes for a
> Ok
>>
>>> +	  but does not probe for other remaining serial devices.
>>> +	  With this option set,we make probing and searching for
>>
>> nit: set, we
> Sure
>>
>>> +	  all available devices optional.
>>> +	  Normally, U-Boot talk to one serial port at a time but SBSA
>>
>> nit: talks
>> nit: time, but
> Agree
>>
>>> +	  compliant UART devices like PL011 require initialization
>>> +	  by firmware and let the kernel use serial port for sending
>>
>> nit: to let the kernel use the
> I understand
>>
>>> +	  and receiving the characters.
>>> +
>>> +	  If probing is not required for all remaining available
>>> +	  devices other than default current console device, say N.
>>
>> Perhaps use the "If unsure, say N." wording like other Kconfigs.
> Got it
>>
>>> +
>>>   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 f3c25d4..417d3af 100644
>>> --- a/drivers/serial/serial-uclass.c
>>> +++ b/drivers/serial/serial-uclass.c
>>> @@ -172,6 +172,10 @@ int serial_init(void)
>>>   /* Called after relocation */
>>>   int serial_initialize(void)
>>>   {
>>> +	/* Scanning uclass to probe devices */
>>> +	if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL))
>>> +		uclass_probe_all(UCLASS_SERIAL);
>>> +

Note that you will also need to check the return value here. E.g.

if (IS_ENABLED(CONFIG_SERIAL_PROBE_ALL))
	int ret = uclass_probe_all(UCLASS_SERIAL);
	if (ret)
		return ret;

>>>   	return serial_init();
>>>   }
>>>
>>>
>