On Monday 22 August 2022 14:25:57 Christophe Leroy wrote:

Le 19/08/2022 à 23:12, Pali Rohár a écrit :

quoted

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/ (local)

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'

quoted

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

quoted

  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.

quoted

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

quoted

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