Re: [PATCH] powerpc: Add support for early debugging via Serial 16550 console

[Christophe Leroy]

Le 20/08/2022 à 00:43, Randy Dunlap a écrit :
> 
> 
> On 8/19/22 15:38, Pali Rohár wrote:
>> On Friday 19 August 2022 15:34:14 Randy Dunlap wrote:
>>> Hi--
>>>
>>> On 8/19/22 14:12, Pali Rohár wrote:
>>>> Currently powerpc early debugging contains lot of platform specific
>>>> options, but does not support standard UART / serial 16550 console.
>>>>
>>>> Later legacy_serial.c code supports registering UART as early debug console
>>>> from device tree but it is not early during booting, but rather later after
>>>> machine description code finishes.
>>>>
>>>> So for real early debugging via UART is current code unsuitable.
>>>>
>>>> Add support for new early debugging option CONFIG_PPC_EARLY_DEBUG_16550
>>>> which enable Serial 16550 console on address defined by new option
>>>> CONFIG_PPC_EARLY_DEBUG_16550_PHYSADDR and by stride by option
>>>> CONFIG_PPC_EARLY_DEBUG_16550_STRIDE.
>>>>
>>>> With this change it is possible to debug powerpc machine descriptor code.
>>>> For example this early debugging code can print on serial console also
>>>> "No suitable machine description found" error which is done before
>>>> legacy_serial.c code.
>>>>
>>>> Signed-off-by: Pali Rohár <pali@xxxxxxxxxx>
>>>> ---
>>>> Tested on P2020 board. It allowed me do debug and implement this patch series:
>>>> https://lore.kernel.org/linuxppc-dev/20220819191557.28116-1-pali@xxxxxxxxxx/
>>>> ---
>>>>   arch/powerpc/Kconfig.debug       | 14 ++++++++++++++
>>>>   arch/powerpc/include/asm/udbg.h  |  1 +
>>>>   arch/powerpc/kernel/udbg.c       |  2 ++
>>>>   arch/powerpc/kernel/udbg_16550.c | 33 ++++++++++++++++++++++++++++++++
>>>>   4 files changed, 50 insertions(+)
>>>>
>>>> diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
>>>> index 9f363c143d86..a4e7d90a45d2 100644
>>>> --- a/arch/powerpc/Kconfig.debug
>>>> +++ b/arch/powerpc/Kconfig.debug
>>>> @@ -276,6 +276,11 @@ config PPC_EARLY_DEBUG_OPAL_HVSI
>>>>   	  Select this to enable early debugging for the PowerNV platform
>>>>   	  using an "hvsi" console
>>>>   
>>>> +config PPC_EARLY_DEBUG_16550
>>>> +	bool "Serial 16550"
>>>> +	help
>>>> +	  Select this to enable early debugging via Serial 16550 console
>>>> +
>>>>   config PPC_EARLY_DEBUG_MEMCONS
>>>>   	bool "In memory console"
>>>>   	help
>>>> @@ -355,6 +360,15 @@ config PPC_EARLY_DEBUG_CPM_ADDR
>>>>   	  platform probing is done, all platforms selected must
>>>>   	  share the same address.
>>>>   
>>>> +config PPC_EARLY_DEBUG_16550_PHYSADDR
>>>> +	hex "Early debug Serial 16550 physical address"
>>>> +	depends on PPC_EARLY_DEBUG_16550
>>>
>>> Is there any chance that you could provide a default value here
>>> so that 'make olddefconfig' does not end up like it does
>>> without having a default value?
>>
>> No. Because there is not any default value. Physical address of 16550 is
>> hardly HW dependent. Possibly also bootloader dependent (if it can remap
>> peripherals to different physical addresses).
>>
>> User _has to_ specify correct physical address if want to use early
>> debug 16550 console.
> 
> OK, we'll see if it ever causes a real problem then...

It does, for instance with 83xx/mpc836x_rdk_defconfig

> 
>>> CONFIG_PPC_EARLY_DEBUG_16550=y
>>> # CONFIG_PPC_EARLY_DEBUG_MEMCONS is not set
>>> CONFIG_PPC_EARLY_DEBUG_16550_PHYSADDR=
>>> CONFIG_PPC_EARLY_DEBUG_16550_STRIDE=1
>>>
>>> which then causes a kconfig prompt when starting
>>> the build...
>>
>> Cannot we set somehow that PPC_EARLY_DEBUG_16550 would be disabled by
>> default when upgrading defconfig?
> 
> Don't know about that.
> 
> Thanks.