2022-08-19 21:42:17

by Pali Rohár

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

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 <[email protected]>
---
Tested on P2020 board. It allowed me do debug and implement this patch series:
https://lore.kernel.org/linuxppc-dev/[email protected]/
---
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


2022-08-19 22:48:01

by Randy Dunlap

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

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 <[email protected]>
> ---
> Tested on P2020 board. It allowed me do debug and implement this patch series:
> https://lore.kernel.org/linuxppc-dev/[email protected]/
> ---
> 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

2022-08-19 22:58:06

by Pali Rohár

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

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 <[email protected]>
> > ---
> > Tested on P2020 board. It allowed me do debug and implement this patch series:
> > https://lore.kernel.org/linuxppc-dev/[email protected]/
> > ---
> > 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

2022-08-19 23:14:18

by Randy Dunlap

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



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 <[email protected]>
>>> ---
>>> Tested on P2020 board. It allowed me do debug and implement this patch series:
>>> https://lore.kernel.org/linuxppc-dev/[email protected]/
>>> ---
>>> 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

2022-08-22 14:28:23

by Christophe Leroy

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



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 <[email protected]>
>>>> ---
>>>> Tested on P2020 board. It allowed me do debug and implement this patch series:
>>>> https://lore.kernel.org/linuxppc-dev/[email protected]/
>>>> ---
>>>> 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.

2022-08-22 14:29:12

by Christophe Leroy

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



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 <[email protected]>
>>> ---
>>> Tested on P2020 board. It allowed me do debug and implement this patch series:
>>> https://lore.kernel.org/linuxppc-dev/[email protected]/
>>> ---
>>> 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

2022-08-22 14:29:20

by Christophe Leroy

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



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 <[email protected]>
>> ---
>> Tested on P2020 board. It allowed me do debug and implement this patch series:
>> https://lore.kernel.org/linuxppc-dev/[email protected]/
>> ---
>> 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.

2022-08-22 14:51:00

by Christophe Leroy

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



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 <[email protected]>
> ---
> Tested on P2020 board. It allowed me do debug and implement this patch series:
> https://lore.kernel.org/linuxppc-dev/[email protected]/

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 */

2022-08-22 15:43:42

by Pali Rohár

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

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.

2022-08-22 16:06:09

by Pali Rohár

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

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 <[email protected]>
> > ---
> > Tested on P2020 board. It allowed me do debug and implement this patch series:
> > https://lore.kernel.org/linuxppc-dev/[email protected]/
>
> 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 */

2022-08-22 17:40:51

by Gabriel Paubert

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

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 <[email protected]>
> > > ---
> > > Tested on P2020 board. It allowed me do debug and implement this patch series:
> > > https://lore.kernel.org/linuxppc-dev/[email protected]/
> >
> > 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 */


2022-08-22 23:53:26

by Pali Rohár

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

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 <[email protected]>
---
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/[email protected]/
---
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

2022-09-24 12:39:04

by Pali Rohár

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

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 <[email protected]>
> ---
> 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/[email protected]/
> ---
> 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
>