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

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

[Pali Rohár]

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@kernel.org>
---
Tested on P2020 board. It allowed me do debug and implement this patch series:
https://lore.kernel.org/linuxppc-dev/20220819191557.28116-1-pali@kernel.org/
---
 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
+
+config PPC_EARLY_DEBUG_16550_STRIDE
+	int "Early debug Serial 16550 stride"
+	depends on PPC_EARLY_DEBUG_16550
+	default 1
+
 config FAIL_IOMMU
 	bool "Fault-injection capability for IOMMU"
 	depends on FAULT_INJECTION
diff --git a/arch/powerpc/include/asm/udbg.h b/arch/powerpc/include/asm/udbg.h
index b4aa0d88ce2c..20b5a37ab772 100644
--- a/arch/powerpc/include/asm/udbg.h
+++ b/arch/powerpc/include/asm/udbg.h
@@ -53,6 +53,7 @@ extern void __init udbg_init_ehv_bc(void);
 extern void __init udbg_init_ps3gelic(void);
 extern void __init udbg_init_debug_opal_raw(void);
 extern void __init udbg_init_debug_opal_hvsi(void);
+extern void __init udbg_init_debug_16550(void);
 
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_UDBG_H */
diff --git a/arch/powerpc/kernel/udbg.c b/arch/powerpc/kernel/udbg.c
index b1544b2f6321..92b3fc258d11 100644
--- a/arch/powerpc/kernel/udbg.c
+++ b/arch/powerpc/kernel/udbg.c
@@ -67,6 +67,8 @@ void __init udbg_early_init(void)
 	udbg_init_debug_opal_raw();
 #elif defined(CONFIG_PPC_EARLY_DEBUG_OPAL_HVSI)
 	udbg_init_debug_opal_hvsi();
+#elif defined(CONFIG_PPC_EARLY_DEBUG_16550)
+	udbg_init_debug_16550();
 #endif
 
 #ifdef CONFIG_PPC_EARLY_DEBUG
diff --git a/arch/powerpc/kernel/udbg_16550.c b/arch/powerpc/kernel/udbg_16550.c
index d3942de254c6..46f2d831d7c9 100644
--- a/arch/powerpc/kernel/udbg_16550.c
+++ b/arch/powerpc/kernel/udbg_16550.c
@@ -8,6 +8,7 @@
 #include <asm/udbg.h>
 #include <asm/io.h>
 #include <asm/reg_a2.h>
+#include <asm/early_ioremap.h>
 
 extern u8 real_readb(volatile u8 __iomem  *addr);
 extern void real_writeb(u8 data, volatile u8 __iomem *addr);
@@ -335,3 +336,35 @@ void __init udbg_init_debug_microwatt(void)
 }
 
 #endif /* CONFIG_PPC_EARLY_DEBUG_MICROWATT */
+
+#ifdef CONFIG_PPC_EARLY_DEBUG_16550
+
+static void __iomem *udbg_uart_early_addr;
+
+void __init udbg_init_debug_16550(void)
+{
+	udbg_uart_early_addr = early_ioremap(CONFIG_PPC_EARLY_DEBUG_16550_PHYSADDR, 0x1000);
+	udbg_uart_init_mmio(udbg_uart_early_addr, CONFIG_PPC_EARLY_DEBUG_16550_STRIDE);
+}
+
+static int __init udbg_init_debug_16550_ioremap(void)
+{
+	void __iomem *addr;
+
+	if (!udbg_uart_early_addr)
+		return 0;
+
+	addr = ioremap(CONFIG_PPC_EARLY_DEBUG_16550_PHYSADDR, 0x1000);
+	if (WARN_ON(!addr))
+		return -ENOMEM;
+
+	udbg_uart_init_mmio(addr, CONFIG_PPC_EARLY_DEBUG_16550_STRIDE);
+	early_iounmap(udbg_uart_early_addr, 0x1000);
+	udbg_uart_early_addr = NULL;
+
+	return 0;
+}
+
+early_initcall(udbg_init_debug_16550_ioremap);
+
+#endif /* CONFIG_PPC_EARLY_DEBUG_16550 */
-- 
2.20.1

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

[Randy Dunlap]

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@kernel.org>
> ---
> Tested on P2020 board. It allowed me do debug and implement this patch series:
> https://lore.kernel.org/linuxppc-dev/20220819191557.28116-1-pali@kernel.org/
> ---
>  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?

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...

> +
> +config PPC_EARLY_DEBUG_16550_STRIDE
> +	int "Early debug Serial 16550 stride"
> +	depends on PPC_EARLY_DEBUG_16550
> +	default 1
> +
>  config FAIL_IOMMU
>  	bool "Fault-injection capability for IOMMU"
>  	depends on FAULT_INJECTION

Thanks.
-- 
~Randy

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

[Pali Rohár]

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@kernel.org>
> > ---
> > Tested on P2020 board. It allowed me do debug and implement this patch series:
> > https://lore.kernel.org/linuxppc-dev/20220819191557.28116-1-pali@kernel.org/
> > ---
> >  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.

> 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?

> > +
> > +config PPC_EARLY_DEBUG_16550_STRIDE
> > +	int "Early debug Serial 16550 stride"
> > +	depends on PPC_EARLY_DEBUG_16550
> > +	default 1
> > +
> >  config FAIL_IOMMU
> >  	bool "Fault-injection capability for IOMMU"
> >  	depends on FAULT_INJECTION
> 
> Thanks.
> -- 
> ~Randy

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

[Randy Dunlap]

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@kernel.org>
>>> ---
>>> Tested on P2020 board. It allowed me do debug and implement this patch series:
>>> https://lore.kernel.org/linuxppc-dev/20220819191557.28116-1-pali@kernel.org/
>>> ---
>>>  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...

>> 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.
-- 
~Randy

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

[Christophe Leroy]

Le 20/08/2022 à 00:34, Randy Dunlap a écrit :
> 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@kernel.org>
>> ---
>> Tested on P2020 board. It allowed me do debug and implement this patch series:
>> https://lore.kernel.org/linuxppc-dev/20220819191557.28116-1-pali@kernel.org/
>> ---
>>   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?

I did a few tests and it seems that a default doesn't help. I tried 
setting 'default n' to PPC_EARLY_DEBUG_16550 but I still get a prompt 
when doing an oldconfig. But only if you have CONFIG_PPC_EARLY_DEBUG=y 
in your old config. But that's expected behaviour when doing 'make 
oldconfig', isn't it ?

Or are you meaning a defconfig ? Because I get what you describe when 
doing 83xx/mpc836x_rdk_defconfig. And I agree with you this is going to 
create problems.

> 
> 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...

Maybe a solution is to make PPC_EARLY_DEBUG_MEMCONS the default, doing 
that I get no change from before. But that changes the behaviour for 
mpc885_ads_defconfig, so it is not the solution.

So I think the only solution is to provide a default value to 
PPC_EARLY_DEBUG_16550_PHYSADDR, just like it is done for 
PPC_EARLY_DEBUG_CPM_ADDR.


> 
>> +
>> +config PPC_EARLY_DEBUG_16550_STRIDE
>> +	int "Early debug Serial 16550 stride"
>> +	depends on PPC_EARLY_DEBUG_16550
>> +	default 1
>> +
>>   config FAIL_IOMMU
>>   	bool "Fault-injection capability for IOMMU"
>>   	depends on FAULT_INJECTION
> 
> Thanks.

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

[Christophe Leroy]

Le 20/08/2022 à 00:38, Pali Rohár a écrit :
> 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@kernel.org>
>>> ---
>>> Tested on P2020 board. It allowed me do debug and implement this patch series:
>>> https://lore.kernel.org/linuxppc-dev/20220819191557.28116-1-pali@kernel.org/
>>> ---
>>>   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).

The exact same problem exists with EARLY_DEBUG_CPM. Nevertheless there 
is a default CONFIG_PPC_EARLY_DEBUG_CPM_ADDR. This value is wrong for 
some people, but at least it avoids prompt during the build of existing 
defconfig.

Other solution is to update the defconfigs with the good value to avoid 
getting an empty value.

> 
> User _has to_ specify correct physical address if want to use early
> debug 16550 console.

By the wait the build robot doesn't mind.

> 
>> 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?

The only solution I see is to put it at the end of the list, so that the 
previous default value which is PPC_EARLY_DEBUG_MEMCONS gets selected.


> 
>>> +
>>> +config PPC_EARLY_DEBUG_16550_STRIDE
>>> +	int "Early debug Serial 16550 stride"
>>> +	depends on PPC_EARLY_DEBUG_16550
>>> +	default 1
>>> +
>>>   config FAIL_IOMMU
>>>   	bool "Fault-injection capability for IOMMU"
>>>   	depends on FAULT_INJECTION
>>
>> Thanks.
>> -- 
>> ~Randy

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@kernel.org>
>>>> ---
>>>> Tested on P2020 board. It allowed me do debug and implement this patch series:
>>>> https://lore.kernel.org/linuxppc-dev/20220819191557.28116-1-pali@kernel.org/
>>>> ---
>>>>   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.

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

[Christophe Leroy]

Le 19/08/2022 à 23:12, Pali Rohár a écrit :
> 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@kernel.org>
> ---
> Tested on P2020 board. It allowed me do debug and implement this patch series:
> https://lore.kernel.org/linuxppc-dev/20220819191557.28116-1-pali@kernel.org/

Build failure if I select it on mpc885_ads_defconfig :

   LD      vmlinux.o
   MODPOST vmlinux.symvers
   MODINFO modules.builtin.modinfo
   GEN     modules.builtin
   CC      .vmlinux.export.o
   LD      .tmp_vmlinux.kallsyms1
powerpc64-linux-ld: arch/powerpc/kernel/udbg.o: in function 
`udbg_early_init':
/home/chleroy/linux-powerpc/arch/powerpc/kernel/udbg.c:71: undefined 
reference to `udbg_init_debug_16550'



> ---
>   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
> +

Putting it before EARLY_DEBUG_MEMCONS means that configs that were 
previously selectiong EARLY_DEBUG_MEMCONS will now select 
EARLY_DEBUG_16550 instead.

Add a dependency to PPC_UDBG_16550 to avoid the build failure I mentionned ?

>   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

A default value is necessary here to avoid prompts during defconfig builds.

> +
> +config PPC_EARLY_DEBUG_16550_STRIDE
> +	int "Early debug Serial 16550 stride"
> +	depends on PPC_EARLY_DEBUG_16550
> +	default 1
> +
>   config FAIL_IOMMU
>   	bool "Fault-injection capability for IOMMU"
>   	depends on FAULT_INJECTION
> diff --git a/arch/powerpc/include/asm/udbg.h b/arch/powerpc/include/asm/udbg.h
> index b4aa0d88ce2c..20b5a37ab772 100644
> --- a/arch/powerpc/include/asm/udbg.h
> +++ b/arch/powerpc/include/asm/udbg.h
> @@ -53,6 +53,7 @@ extern void __init udbg_init_ehv_bc(void);
>   extern void __init udbg_init_ps3gelic(void);
>   extern void __init udbg_init_debug_opal_raw(void);
>   extern void __init udbg_init_debug_opal_hvsi(void);
> +extern void __init udbg_init_debug_16550(void);

'extern' keywork is pointless and deprecated for function prototypes, 
please don't add new ones.

Checkpatch reports:

CHECK: extern prototypes should be avoided in .h files
#77: FILE: arch/powerpc/include/asm/udbg.h:56:
+extern void __init udbg_init_debug_16550(void);


>   
>   #endif /* __KERNEL__ */
>   #endif /* _ASM_POWERPC_UDBG_H */
> diff --git a/arch/powerpc/kernel/udbg.c b/arch/powerpc/kernel/udbg.c
> index b1544b2f6321..92b3fc258d11 100644
> --- a/arch/powerpc/kernel/udbg.c
> +++ b/arch/powerpc/kernel/udbg.c
> @@ -67,6 +67,8 @@ void __init udbg_early_init(void)
>   	udbg_init_debug_opal_raw();
>   #elif defined(CONFIG_PPC_EARLY_DEBUG_OPAL_HVSI)
>   	udbg_init_debug_opal_hvsi();
> +#elif defined(CONFIG_PPC_EARLY_DEBUG_16550)
> +	udbg_init_debug_16550();
>   #endif
>   
>   #ifdef CONFIG_PPC_EARLY_DEBUG
> diff --git a/arch/powerpc/kernel/udbg_16550.c b/arch/powerpc/kernel/udbg_16550.c
> index d3942de254c6..46f2d831d7c9 100644
> --- a/arch/powerpc/kernel/udbg_16550.c
> +++ b/arch/powerpc/kernel/udbg_16550.c
> @@ -8,6 +8,7 @@
>   #include <asm/udbg.h>
>   #include <asm/io.h>
>   #include <asm/reg_a2.h>
> +#include <asm/early_ioremap.h>
>   
>   extern u8 real_readb(volatile u8 __iomem  *addr);
>   extern void real_writeb(u8 data, volatile u8 __iomem *addr);
> @@ -335,3 +336,35 @@ void __init udbg_init_debug_microwatt(void)
>   }
>   
>   #endif /* CONFIG_PPC_EARLY_DEBUG_MICROWATT */
> +
> +#ifdef CONFIG_PPC_EARLY_DEBUG_16550
> +
> +static void __iomem *udbg_uart_early_addr;
> +
> +void __init udbg_init_debug_16550(void)
> +{
> +	udbg_uart_early_addr = early_ioremap(CONFIG_PPC_EARLY_DEBUG_16550_PHYSADDR, 0x1000);
> +	udbg_uart_init_mmio(udbg_uart_early_addr, CONFIG_PPC_EARLY_DEBUG_16550_STRIDE);
> +}
> +
> +static int __init udbg_init_debug_16550_ioremap(void)
> +{
> +	void __iomem *addr;
> +
> +	if (!udbg_uart_early_addr)
> +		return 0;
> +
> +	addr = ioremap(CONFIG_PPC_EARLY_DEBUG_16550_PHYSADDR, 0x1000);
> +	if (WARN_ON(!addr))
> +		return -ENOMEM;
> +
> +	udbg_uart_init_mmio(addr, CONFIG_PPC_EARLY_DEBUG_16550_STRIDE);
> +	early_iounmap(udbg_uart_early_addr, 0x1000);
> +	udbg_uart_early_addr = NULL;
> +
> +	return 0;
> +}
> +
> +early_initcall(udbg_init_debug_16550_ioremap);
> +
> +#endif /* CONFIG_PPC_EARLY_DEBUG_16550 */

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

[Pali Rohár]

On Monday 22 August 2022 14:13:30 Christophe Leroy wrote:
> >> 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?
> 
> The only solution I see is to put it at the end of the list, so that the 
> previous default value which is PPC_EARLY_DEBUG_MEMCONS gets selected.

Does it work for all cases? If yes, then it looks like an elegant solution.

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

[Pali Rohár]

On Monday 22 August 2022 14:25:57 Christophe Leroy wrote:
> Le 19/08/2022 à 23:12, Pali Rohár a écrit :
> > 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@kernel.org>
> > ---
> > Tested on P2020 board. It allowed me do debug and implement this patch series:
> > https://lore.kernel.org/linuxppc-dev/20220819191557.28116-1-pali@kernel.org/
> 
> Build failure if I select it on mpc885_ads_defconfig :
> 
>    LD      vmlinux.o
>    MODPOST vmlinux.symvers
>    MODINFO modules.builtin.modinfo
>    GEN     modules.builtin
>    CC      .vmlinux.export.o
>    LD      .tmp_vmlinux.kallsyms1
> powerpc64-linux-ld: arch/powerpc/kernel/udbg.o: in function 
> `udbg_early_init':
> /home/chleroy/linux-powerpc/arch/powerpc/kernel/udbg.c:71: undefined 
> reference to `udbg_init_debug_16550'
> 
> 
> 
> > ---
> >   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
> > +
> 
> Putting it before EARLY_DEBUG_MEMCONS means that configs that were 
> previously selectiong EARLY_DEBUG_MEMCONS will now select 
> EARLY_DEBUG_16550 instead.
> 
> Add a dependency to PPC_UDBG_16550 to avoid the build failure I mentionned ?

Yea, there is really missing dependency. I will fix it.

> >   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
> 
> A default value is necessary here to avoid prompts during defconfig builds.
> 
> > +
> > +config PPC_EARLY_DEBUG_16550_STRIDE
> > +	int "Early debug Serial 16550 stride"
> > +	depends on PPC_EARLY_DEBUG_16550
> > +	default 1
> > +
> >   config FAIL_IOMMU
> >   	bool "Fault-injection capability for IOMMU"
> >   	depends on FAULT_INJECTION
> > diff --git a/arch/powerpc/include/asm/udbg.h b/arch/powerpc/include/asm/udbg.h
> > index b4aa0d88ce2c..20b5a37ab772 100644
> > --- a/arch/powerpc/include/asm/udbg.h
> > +++ b/arch/powerpc/include/asm/udbg.h
> > @@ -53,6 +53,7 @@ extern void __init udbg_init_ehv_bc(void);
> >   extern void __init udbg_init_ps3gelic(void);
> >   extern void __init udbg_init_debug_opal_raw(void);
> >   extern void __init udbg_init_debug_opal_hvsi(void);
> > +extern void __init udbg_init_debug_16550(void);
> 
> 'extern' keywork is pointless and deprecated for function prototypes, 
> please don't add new ones.

I used extern keyword to follow existing coding style.

> Checkpatch reports:
> 
> CHECK: extern prototypes should be avoided in .h files
> #77: FILE: arch/powerpc/include/asm/udbg.h:56:
> +extern void __init udbg_init_debug_16550(void);
> 
> 
> >   
> >   #endif /* __KERNEL__ */
> >   #endif /* _ASM_POWERPC_UDBG_H */
> > diff --git a/arch/powerpc/kernel/udbg.c b/arch/powerpc/kernel/udbg.c
> > index b1544b2f6321..92b3fc258d11 100644
> > --- a/arch/powerpc/kernel/udbg.c
> > +++ b/arch/powerpc/kernel/udbg.c
> > @@ -67,6 +67,8 @@ void __init udbg_early_init(void)
> >   	udbg_init_debug_opal_raw();
> >   #elif defined(CONFIG_PPC_EARLY_DEBUG_OPAL_HVSI)
> >   	udbg_init_debug_opal_hvsi();
> > +#elif defined(CONFIG_PPC_EARLY_DEBUG_16550)
> > +	udbg_init_debug_16550();
> >   #endif
> >   
> >   #ifdef CONFIG_PPC_EARLY_DEBUG
> > diff --git a/arch/powerpc/kernel/udbg_16550.c b/arch/powerpc/kernel/udbg_16550.c
> > index d3942de254c6..46f2d831d7c9 100644
> > --- a/arch/powerpc/kernel/udbg_16550.c
> > +++ b/arch/powerpc/kernel/udbg_16550.c
> > @@ -8,6 +8,7 @@
> >   #include <asm/udbg.h>
> >   #include <asm/io.h>
> >   #include <asm/reg_a2.h>
> > +#include <asm/early_ioremap.h>
> >   
> >   extern u8 real_readb(volatile u8 __iomem  *addr);
> >   extern void real_writeb(u8 data, volatile u8 __iomem *addr);
> > @@ -335,3 +336,35 @@ void __init udbg_init_debug_microwatt(void)
> >   }
> >   
> >   #endif /* CONFIG_PPC_EARLY_DEBUG_MICROWATT */
> > +
> > +#ifdef CONFIG_PPC_EARLY_DEBUG_16550
> > +
> > +static void __iomem *udbg_uart_early_addr;
> > +
> > +void __init udbg_init_debug_16550(void)
> > +{
> > +	udbg_uart_early_addr = early_ioremap(CONFIG_PPC_EARLY_DEBUG_16550_PHYSADDR, 0x1000);
> > +	udbg_uart_init_mmio(udbg_uart_early_addr, CONFIG_PPC_EARLY_DEBUG_16550_STRIDE);
> > +}
> > +
> > +static int __init udbg_init_debug_16550_ioremap(void)
> > +{
> > +	void __iomem *addr;
> > +
> > +	if (!udbg_uart_early_addr)
> > +		return 0;
> > +
> > +	addr = ioremap(CONFIG_PPC_EARLY_DEBUG_16550_PHYSADDR, 0x1000);
> > +	if (WARN_ON(!addr))
> > +		return -ENOMEM;
> > +
> > +	udbg_uart_init_mmio(addr, CONFIG_PPC_EARLY_DEBUG_16550_STRIDE);
> > +	early_iounmap(udbg_uart_early_addr, 0x1000);
> > +	udbg_uart_early_addr = NULL;
> > +
> > +	return 0;
> > +}
> > +
> > +early_initcall(udbg_init_debug_16550_ioremap);
> > +
> > +#endif /* CONFIG_PPC_EARLY_DEBUG_16550 */

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

[Gabriel Paubert]

On Mon, Aug 22, 2022 at 05:33:35PM +0200, Pali Rohár wrote:
> On Monday 22 August 2022 14:25:57 Christophe Leroy wrote:
> > Le 19/08/2022 à 23:12, Pali Rohár a écrit :
> > > 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@kernel.org>
> > > ---
> > > Tested on P2020 board. It allowed me do debug and implement this patch series:
> > > https://lore.kernel.org/linuxppc-dev/20220819191557.28116-1-pali@kernel.org/
> > 
> > Build failure if I select it on mpc885_ads_defconfig :
> > 
> >    LD      vmlinux.o
> >    MODPOST vmlinux.symvers
> >    MODINFO modules.builtin.modinfo
> >    GEN     modules.builtin
> >    CC      .vmlinux.export.o
> >    LD      .tmp_vmlinux.kallsyms1
> > powerpc64-linux-ld: arch/powerpc/kernel/udbg.o: in function 
> > `udbg_early_init':
> > /home/chleroy/linux-powerpc/arch/powerpc/kernel/udbg.c:71: undefined 
> > reference to `udbg_init_debug_16550'
> > 
> > 
> > 
> > > ---
> > >   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
> > > +
> > 
> > Putting it before EARLY_DEBUG_MEMCONS means that configs that were 
> > previously selectiong EARLY_DEBUG_MEMCONS will now select 
> > EARLY_DEBUG_16550 instead.
> > 
> > Add a dependency to PPC_UDBG_16550 to avoid the build failure I mentionned ?
> 
> Yea, there is really missing dependency. I will fix it.
> 
> > >   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
> > 
> > A default value is necessary here to avoid prompts during defconfig builds.
> > 
> > > +
> > > +config PPC_EARLY_DEBUG_16550_STRIDE
> > > +	int "Early debug Serial 16550 stride"
> > > +	depends on PPC_EARLY_DEBUG_16550
> > > +	default 1
> > > +
> > >   config FAIL_IOMMU
> > >   	bool "Fault-injection capability for IOMMU"
> > >   	depends on FAULT_INJECTION
> > > diff --git a/arch/powerpc/include/asm/udbg.h b/arch/powerpc/include/asm/udbg.h
> > > index b4aa0d88ce2c..20b5a37ab772 100644
> > > --- a/arch/powerpc/include/asm/udbg.h
> > > +++ b/arch/powerpc/include/asm/udbg.h
> > > @@ -53,6 +53,7 @@ extern void __init udbg_init_ehv_bc(void);
> > >   extern void __init udbg_init_ps3gelic(void);
> > >   extern void __init udbg_init_debug_opal_raw(void);
> > >   extern void __init udbg_init_debug_opal_hvsi(void);
> > > +extern void __init udbg_init_debug_16550(void);
> > 
> > 'extern' keywork is pointless and deprecated for function prototypes, 
> > please don't add new ones.
> 
> I used extern keyword to follow existing coding style.

In this case it's better to remove existing extern specifications in the
surrounding lines. Increasing a bit the footprint of the patch is
justified, and does not significantly increase the risk of conflicts
with other patches, unlike the gratuitous churn we see sometimes.

	Gabriel
> 
> > Checkpatch reports:
> > 
> > CHECK: extern prototypes should be avoided in .h files
> > #77: FILE: arch/powerpc/include/asm/udbg.h:56:
> > +extern void __init udbg_init_debug_16550(void);
> > 
> > 
> > >   
> > >   #endif /* __KERNEL__ */
> > >   #endif /* _ASM_POWERPC_UDBG_H */
> > > diff --git a/arch/powerpc/kernel/udbg.c b/arch/powerpc/kernel/udbg.c
> > > index b1544b2f6321..92b3fc258d11 100644
> > > --- a/arch/powerpc/kernel/udbg.c
> > > +++ b/arch/powerpc/kernel/udbg.c
> > > @@ -67,6 +67,8 @@ void __init udbg_early_init(void)
> > >   	udbg_init_debug_opal_raw();
> > >   #elif defined(CONFIG_PPC_EARLY_DEBUG_OPAL_HVSI)
> > >   	udbg_init_debug_opal_hvsi();
> > > +#elif defined(CONFIG_PPC_EARLY_DEBUG_16550)
> > > +	udbg_init_debug_16550();
> > >   #endif
> > >   
> > >   #ifdef CONFIG_PPC_EARLY_DEBUG
> > > diff --git a/arch/powerpc/kernel/udbg_16550.c b/arch/powerpc/kernel/udbg_16550.c
> > > index d3942de254c6..46f2d831d7c9 100644
> > > --- a/arch/powerpc/kernel/udbg_16550.c
> > > +++ b/arch/powerpc/kernel/udbg_16550.c
> > > @@ -8,6 +8,7 @@
> > >   #include <asm/udbg.h>
> > >   #include <asm/io.h>
> > >   #include <asm/reg_a2.h>
> > > +#include <asm/early_ioremap.h>
> > >   
> > >   extern u8 real_readb(volatile u8 __iomem  *addr);
> > >   extern void real_writeb(u8 data, volatile u8 __iomem *addr);
> > > @@ -335,3 +336,35 @@ void __init udbg_init_debug_microwatt(void)
> > >   }
> > >   
> > >   #endif /* CONFIG_PPC_EARLY_DEBUG_MICROWATT */
> > > +
> > > +#ifdef CONFIG_PPC_EARLY_DEBUG_16550
> > > +
> > > +static void __iomem *udbg_uart_early_addr;
> > > +
> > > +void __init udbg_init_debug_16550(void)
> > > +{
> > > +	udbg_uart_early_addr = early_ioremap(CONFIG_PPC_EARLY_DEBUG_16550_PHYSADDR, 0x1000);
> > > +	udbg_uart_init_mmio(udbg_uart_early_addr, CONFIG_PPC_EARLY_DEBUG_16550_STRIDE);
> > > +}
> > > +
> > > +static int __init udbg_init_debug_16550_ioremap(void)
> > > +{
> > > +	void __iomem *addr;
> > > +
> > > +	if (!udbg_uart_early_addr)
> > > +		return 0;
> > > +
> > > +	addr = ioremap(CONFIG_PPC_EARLY_DEBUG_16550_PHYSADDR, 0x1000);
> > > +	if (WARN_ON(!addr))
> > > +		return -ENOMEM;
> > > +
> > > +	udbg_uart_init_mmio(addr, CONFIG_PPC_EARLY_DEBUG_16550_STRIDE);
> > > +	early_iounmap(udbg_uart_early_addr, 0x1000);
> > > +	udbg_uart_early_addr = NULL;
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +early_initcall(udbg_init_debug_16550_ioremap);
> > > +
> > > +#endif /* CONFIG_PPC_EARLY_DEBUG_16550 */

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

[Pali Rohár]

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@kernel.org>
---
Changes in v2:
* Move PPC_EARLY_DEBUG_16550 after PPC_EARLY_DEBUG_MEMCONS, so memcons stay default
* Add missing dependency on PPC_UDBG_16550
---
Tested on P2020 board. It allowed me do debug and implement this patch series:
https://lore.kernel.org/linuxppc-dev/20220819191557.28116-1-pali@kernel.org/
---
 arch/powerpc/Kconfig.debug       | 15 +++++++++++++++
 arch/powerpc/include/asm/udbg.h  |  1 +
 arch/powerpc/kernel/udbg.c       |  2 ++
 arch/powerpc/kernel/udbg_16550.c | 33 ++++++++++++++++++++++++++++++++
 4 files changed, 51 insertions(+)

diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
index 9f363c143d86..ad7238d28fa9 100644
--- a/arch/powerpc/Kconfig.debug
+++ b/arch/powerpc/Kconfig.debug
@@ -283,6 +283,12 @@ config PPC_EARLY_DEBUG_MEMCONS
 	  This console provides input and output buffers stored within the
 	  kernel BSS and should be safe to select on any system. A debugger
 	  can then be used to read kernel output or send input to the console.
+
+config PPC_EARLY_DEBUG_16550
+	bool "Serial 16550"
+	depends on PPC_UDBG_16550
+	help
+	  Select this to enable early debugging via Serial 16550 console
 endchoice
 
 config PPC_MEMCONS_OUTPUT_SIZE
@@ -355,6 +361,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
+
+config PPC_EARLY_DEBUG_16550_STRIDE
+	int "Early debug Serial 16550 stride"
+	depends on PPC_EARLY_DEBUG_16550
+	default 1
+
 config FAIL_IOMMU
 	bool "Fault-injection capability for IOMMU"
 	depends on FAULT_INJECTION
diff --git a/arch/powerpc/include/asm/udbg.h b/arch/powerpc/include/asm/udbg.h
index b4aa0d88ce2c..20b5a37ab772 100644
--- a/arch/powerpc/include/asm/udbg.h
+++ b/arch/powerpc/include/asm/udbg.h
@@ -53,6 +53,7 @@ extern void __init udbg_init_ehv_bc(void);
 extern void __init udbg_init_ps3gelic(void);
 extern void __init udbg_init_debug_opal_raw(void);
 extern void __init udbg_init_debug_opal_hvsi(void);
+extern void __init udbg_init_debug_16550(void);
 
 #endif /* __KERNEL__ */
 #endif /* _ASM_POWERPC_UDBG_H */
diff --git a/arch/powerpc/kernel/udbg.c b/arch/powerpc/kernel/udbg.c
index b1544b2f6321..92b3fc258d11 100644
--- a/arch/powerpc/kernel/udbg.c
+++ b/arch/powerpc/kernel/udbg.c
@@ -67,6 +67,8 @@ void __init udbg_early_init(void)
 	udbg_init_debug_opal_raw();
 #elif defined(CONFIG_PPC_EARLY_DEBUG_OPAL_HVSI)
 	udbg_init_debug_opal_hvsi();
+#elif defined(CONFIG_PPC_EARLY_DEBUG_16550)
+	udbg_init_debug_16550();
 #endif
 
 #ifdef CONFIG_PPC_EARLY_DEBUG
diff --git a/arch/powerpc/kernel/udbg_16550.c b/arch/powerpc/kernel/udbg_16550.c
index d3942de254c6..46f2d831d7c9 100644
--- a/arch/powerpc/kernel/udbg_16550.c
+++ b/arch/powerpc/kernel/udbg_16550.c
@@ -8,6 +8,7 @@
 #include <asm/udbg.h>
 #include <asm/io.h>
 #include <asm/reg_a2.h>
+#include <asm/early_ioremap.h>
 
 extern u8 real_readb(volatile u8 __iomem  *addr);
 extern void real_writeb(u8 data, volatile u8 __iomem *addr);
@@ -335,3 +336,35 @@ void __init udbg_init_debug_microwatt(void)
 }
 
 #endif /* CONFIG_PPC_EARLY_DEBUG_MICROWATT */
+
+#ifdef CONFIG_PPC_EARLY_DEBUG_16550
+
+static void __iomem *udbg_uart_early_addr;
+
+void __init udbg_init_debug_16550(void)
+{
+	udbg_uart_early_addr = early_ioremap(CONFIG_PPC_EARLY_DEBUG_16550_PHYSADDR, 0x1000);
+	udbg_uart_init_mmio(udbg_uart_early_addr, CONFIG_PPC_EARLY_DEBUG_16550_STRIDE);
+}
+
+static int __init udbg_init_debug_16550_ioremap(void)
+{
+	void __iomem *addr;
+
+	if (!udbg_uart_early_addr)
+		return 0;
+
+	addr = ioremap(CONFIG_PPC_EARLY_DEBUG_16550_PHYSADDR, 0x1000);
+	if (WARN_ON(!addr))
+		return -ENOMEM;
+
+	udbg_uart_init_mmio(addr, CONFIG_PPC_EARLY_DEBUG_16550_STRIDE);
+	early_iounmap(udbg_uart_early_addr, 0x1000);
+	udbg_uart_early_addr = NULL;
+
+	return 0;
+}
+
+early_initcall(udbg_init_debug_16550_ioremap);
+
+#endif /* CONFIG_PPC_EARLY_DEBUG_16550 */
-- 
2.20.1

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

[Pali Rohár]

Hello! Is something else needed for this v2 patch?

On Tuesday 23 August 2022 01:15:01 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@kernel.org>
> ---
> Changes in v2:
> * Move PPC_EARLY_DEBUG_16550 after PPC_EARLY_DEBUG_MEMCONS, so memcons stay default
> * Add missing dependency on PPC_UDBG_16550
> ---
> Tested on P2020 board. It allowed me do debug and implement this patch series:
> https://lore.kernel.org/linuxppc-dev/20220819191557.28116-1-pali@kernel.org/
> ---
>  arch/powerpc/Kconfig.debug       | 15 +++++++++++++++
>  arch/powerpc/include/asm/udbg.h  |  1 +
>  arch/powerpc/kernel/udbg.c       |  2 ++
>  arch/powerpc/kernel/udbg_16550.c | 33 ++++++++++++++++++++++++++++++++
>  4 files changed, 51 insertions(+)
> 
> diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug
> index 9f363c143d86..ad7238d28fa9 100644
> --- a/arch/powerpc/Kconfig.debug
> +++ b/arch/powerpc/Kconfig.debug
> @@ -283,6 +283,12 @@ config PPC_EARLY_DEBUG_MEMCONS
>  	  This console provides input and output buffers stored within the
>  	  kernel BSS and should be safe to select on any system. A debugger
>  	  can then be used to read kernel output or send input to the console.
> +
> +config PPC_EARLY_DEBUG_16550
> +	bool "Serial 16550"
> +	depends on PPC_UDBG_16550
> +	help
> +	  Select this to enable early debugging via Serial 16550 console
>  endchoice
>  
>  config PPC_MEMCONS_OUTPUT_SIZE
> @@ -355,6 +361,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
> +
> +config PPC_EARLY_DEBUG_16550_STRIDE
> +	int "Early debug Serial 16550 stride"
> +	depends on PPC_EARLY_DEBUG_16550
> +	default 1
> +
>  config FAIL_IOMMU
>  	bool "Fault-injection capability for IOMMU"
>  	depends on FAULT_INJECTION
> diff --git a/arch/powerpc/include/asm/udbg.h b/arch/powerpc/include/asm/udbg.h
> index b4aa0d88ce2c..20b5a37ab772 100644
> --- a/arch/powerpc/include/asm/udbg.h
> +++ b/arch/powerpc/include/asm/udbg.h
> @@ -53,6 +53,7 @@ extern void __init udbg_init_ehv_bc(void);
>  extern void __init udbg_init_ps3gelic(void);
>  extern void __init udbg_init_debug_opal_raw(void);
>  extern void __init udbg_init_debug_opal_hvsi(void);
> +extern void __init udbg_init_debug_16550(void);
>  
>  #endif /* __KERNEL__ */
>  #endif /* _ASM_POWERPC_UDBG_H */
> diff --git a/arch/powerpc/kernel/udbg.c b/arch/powerpc/kernel/udbg.c
> index b1544b2f6321..92b3fc258d11 100644
> --- a/arch/powerpc/kernel/udbg.c
> +++ b/arch/powerpc/kernel/udbg.c
> @@ -67,6 +67,8 @@ void __init udbg_early_init(void)
>  	udbg_init_debug_opal_raw();
>  #elif defined(CONFIG_PPC_EARLY_DEBUG_OPAL_HVSI)
>  	udbg_init_debug_opal_hvsi();
> +#elif defined(CONFIG_PPC_EARLY_DEBUG_16550)
> +	udbg_init_debug_16550();
>  #endif
>  
>  #ifdef CONFIG_PPC_EARLY_DEBUG
> diff --git a/arch/powerpc/kernel/udbg_16550.c b/arch/powerpc/kernel/udbg_16550.c
> index d3942de254c6..46f2d831d7c9 100644
> --- a/arch/powerpc/kernel/udbg_16550.c
> +++ b/arch/powerpc/kernel/udbg_16550.c
> @@ -8,6 +8,7 @@
>  #include <asm/udbg.h>
>  #include <asm/io.h>
>  #include <asm/reg_a2.h>
> +#include <asm/early_ioremap.h>
>  
>  extern u8 real_readb(volatile u8 __iomem  *addr);
>  extern void real_writeb(u8 data, volatile u8 __iomem *addr);
> @@ -335,3 +336,35 @@ void __init udbg_init_debug_microwatt(void)
>  }
>  
>  #endif /* CONFIG_PPC_EARLY_DEBUG_MICROWATT */
> +
> +#ifdef CONFIG_PPC_EARLY_DEBUG_16550
> +
> +static void __iomem *udbg_uart_early_addr;
> +
> +void __init udbg_init_debug_16550(void)
> +{
> +	udbg_uart_early_addr = early_ioremap(CONFIG_PPC_EARLY_DEBUG_16550_PHYSADDR, 0x1000);
> +	udbg_uart_init_mmio(udbg_uart_early_addr, CONFIG_PPC_EARLY_DEBUG_16550_STRIDE);
> +}
> +
> +static int __init udbg_init_debug_16550_ioremap(void)
> +{
> +	void __iomem *addr;
> +
> +	if (!udbg_uart_early_addr)
> +		return 0;
> +
> +	addr = ioremap(CONFIG_PPC_EARLY_DEBUG_16550_PHYSADDR, 0x1000);
> +	if (WARN_ON(!addr))
> +		return -ENOMEM;
> +
> +	udbg_uart_init_mmio(addr, CONFIG_PPC_EARLY_DEBUG_16550_STRIDE);
> +	early_iounmap(udbg_uart_early_addr, 0x1000);
> +	udbg_uart_early_addr = NULL;
> +
> +	return 0;
> +}
> +
> +early_initcall(udbg_init_debug_16550_ioremap);
> +
> +#endif /* CONFIG_PPC_EARLY_DEBUG_16550 */
> -- 
> 2.20.1
>