ARM: OMAP: debug_ll: Use Kconfig UART base address

PATCH In review Alexander Shiyan 4 participants 6 messages May 5, 2022

Subsystems (3)

Arm (32-bit) Arm sub-architectures Arm/actions semi architecture

lore

Share & tools

lore

Alexander Shiyan

author

2022-05-05 07:37 UTC #0 lore raw

PATCH 1/1 ARM: OMAP: debug_ll: Use Kconfig UART base address

Signed-off-by Alexander Shiyan <[email protected]>

---
 arch/arm/boards/afi-gf/lowlevel.c            |  2 +-
 arch/arm/boards/beagle/lowlevel.c            |  2 +-
 arch/arm/boards/beaglebone/lowlevel.c        |  2 +-
 arch/arm/boards/myirtech-x335x/lowlevel.c    |  2 +-
 arch/arm/boards/phytec-som-am335x/lowlevel.c |  2 +-
 arch/arm/boards/vscom-baltos/lowlevel.c      |  2 +-
 arch/arm/boards/wago-pfc-am35xx/lowlevel.c   |  2 +-
 arch/arm/mach-omap/include/mach/debug_ll.h   | 43 ++++++++++++--------
 8 files changed, 33 insertions(+), 24 deletions(-)

arch/arm/boards/afi-gf/lowlevel.c +1 -1

index de40f6c5af..9e86b29b2d 100644
@@ -230,7 +230,7 @@ static noinline int gf_sram_init(void)
 
 	am33xx_uart_soft_reset((void *)AM33XX_UART2_BASE);
 	am33xx_enable_uart2_pin_mux();
-	omap_uart_lowlevel_init((void *)AM33XX_UART2_BASE);
+	omap_uart_lowlevel_init();
 	putc_ll('>');
 
 	barebox_arm_entry(0x80000000, SZ_256M, fdt);

arch/arm/boards/beagle/lowlevel.c +1 -1

index 683ab552f4..c64a2114d4 100644
@@ -196,7 +196,7 @@ static noinline int beagle_board_init(void)
 
 	mux_config();
 
-	omap_uart_lowlevel_init((void *)OMAP3_UART3_BASE);
+	omap_uart_lowlevel_init();
 
 	/* Dont reconfigure SDRAM while running in SDRAM! */
 	if (!in_sdram)

arch/arm/boards/beaglebone/lowlevel.c +1 -1

index 544e396e03..365ba64a79 100644
@@ -139,7 +139,7 @@ static noinline int beaglebone_sram_init(void)
 
 	am33xx_uart_soft_reset((void *)AM33XX_UART0_BASE);
 	am33xx_enable_uart0_pin_mux();
-	omap_uart_lowlevel_init((void *)AM33XX_UART0_BASE);
+	omap_uart_lowlevel_init();
 	putc_ll('>');
 
 	barebox_arm_entry(0x80000000, sdram_size, fdt);

arch/arm/boards/myirtech-x335x/lowlevel.c +1 -1

index 1a883da203..da47300d81 100644
@@ -98,7 +98,7 @@ ENTRY_FUNCTION(start_am33xx_myirtech_sram, bootinfo, r1, r2)
 	if (IS_ENABLED(CONFIG_DEBUG_LL)) {
 		am33xx_uart_soft_reset(IOMEM(AM33XX_UART0_BASE));
 		am33xx_enable_uart0_pin_mux();
-		omap_uart_lowlevel_init(IOMEM(AM33XX_UART0_BASE));
+		omap_uart_lowlevel_init();
 		putc_ll('>');
 	}

arch/arm/boards/phytec-som-am335x/lowlevel.c +1 -1

index bffb3ad880..e4f3edd211 100644
@@ -172,7 +172,7 @@ static noinline void physom_board_init(void *fdt, int sdram, int module_family)
 
 	am33xx_uart_soft_reset((void *)AM33XX_UART0_BASE);
 	am33xx_enable_uart0_pin_mux();
-	omap_uart_lowlevel_init((void *)AM33XX_UART0_BASE);
+	omap_uart_lowlevel_init();
 	putc_ll('>');
 
 	am335x_barebox_entry(fdt);

arch/arm/boards/vscom-baltos/lowlevel.c +1 -1

index 7da2f92efb..2401cf20c4 100644
@@ -104,7 +104,7 @@ static noinline void baltos_sram_init(void)
 
 	am33xx_uart_soft_reset((void *)AM33XX_UART0_BASE);
 	am33xx_enable_uart0_pin_mux();
-	omap_uart_lowlevel_init((void *)AM33XX_UART0_BASE);
+	omap_uart_lowlevel_init();
 	putc_ll('>');
 
 	am335x_barebox_entry(fdt);

arch/arm/boards/wago-pfc-am35xx/lowlevel.c +1 -1

index 7da8fd0331..6a7f350dda 100644
@@ -185,7 +185,7 @@ static noinline void pfc200_board_init(void)
 
 	if (IS_ENABLED(CONFIG_DEBUG_LL)) {
 		am33xx_uart_soft_reset(IOMEM(OMAP3_UART3_BASE));
-		omap_uart_lowlevel_init(IOMEM(OMAP3_UART3_BASE));
+		omap_uart_lowlevel_init();
 		putc_ll('>');
 	}

arch/arm/mach-omap/include/mach/debug_ll.h +26 ~~-17~~

66 lines not on this page · raw message

Signature · 7 lines

2.32.0


_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Lucas Stach

2022-05-05 07:56 UTC #1 lore raw

Am Donnerstag, dem 05.05.2022 um 10:36 +0300 schrieb Alexander Shiyan:

Quoted 1 line

Signed-off-by Alexander Shiyan <[email protected]>

Is this really a change in the right direction? The way it is currently done seems to mix well with overall multi-image theme of Barebox. By using a Kconfig for that, the setting will most likely be wrong for a subset of boards in a multi-image build.

Regards,
Lucas

Quoted 166 lines

---
 arch/arm/boards/afi-gf/lowlevel.c            |  2 +-
 arch/arm/boards/beagle/lowlevel.c            |  2 +-
 arch/arm/boards/beaglebone/lowlevel.c        |  2 +-
 arch/arm/boards/myirtech-x335x/lowlevel.c    |  2 +-
 arch/arm/boards/phytec-som-am335x/lowlevel.c |  2 +-
 arch/arm/boards/vscom-baltos/lowlevel.c      |  2 +-
 arch/arm/boards/wago-pfc-am35xx/lowlevel.c   |  2 +-
 arch/arm/mach-omap/include/mach/debug_ll.h   | 43 ++++++++++++--------
 8 files changed, 33 insertions(+), 24 deletions(-)

arch/arm/boards/afi-gf/lowlevel.c +1 -1

index de40f6c5af..9e86b29b2d 100644
@@ -230,7 +230,7 @@ static noinline int gf_sram_init(void)
 
 	am33xx_uart_soft_reset((void *)AM33XX_UART2_BASE);
 	am33xx_enable_uart2_pin_mux();
-	omap_uart_lowlevel_init((void *)AM33XX_UART2_BASE);
+	omap_uart_lowlevel_init();
 	putc_ll('>');
 
 	barebox_arm_entry(0x80000000, SZ_256M, fdt);

arch/arm/boards/beagle/lowlevel.c +1 -1

index 683ab552f4..c64a2114d4 100644
@@ -196,7 +196,7 @@ static noinline int beagle_board_init(void)
 
 	mux_config();
 
-	omap_uart_lowlevel_init((void *)OMAP3_UART3_BASE);
+	omap_uart_lowlevel_init();
 
 	/* Dont reconfigure SDRAM while running in SDRAM! */
 	if (!in_sdram)

arch/arm/boards/beaglebone/lowlevel.c +1 -1

index 544e396e03..365ba64a79 100644
@@ -139,7 +139,7 @@ static noinline int beaglebone_sram_init(void)
 
 	am33xx_uart_soft_reset((void *)AM33XX_UART0_BASE);
 	am33xx_enable_uart0_pin_mux();
-	omap_uart_lowlevel_init((void *)AM33XX_UART0_BASE);
+	omap_uart_lowlevel_init();
 	putc_ll('>');
 
 	barebox_arm_entry(0x80000000, sdram_size, fdt);

arch/arm/boards/myirtech-x335x/lowlevel.c +1 -1

index 1a883da203..da47300d81 100644
@@ -98,7 +98,7 @@ ENTRY_FUNCTION(start_am33xx_myirtech_sram, bootinfo, r1, r2)
 	if (IS_ENABLED(CONFIG_DEBUG_LL)) {
 		am33xx_uart_soft_reset(IOMEM(AM33XX_UART0_BASE));
 		am33xx_enable_uart0_pin_mux();
-		omap_uart_lowlevel_init(IOMEM(AM33XX_UART0_BASE));
+		omap_uart_lowlevel_init();
 		putc_ll('>');
 	}

arch/arm/boards/phytec-som-am335x/lowlevel.c +1 -1

index bffb3ad880..e4f3edd211 100644
@@ -172,7 +172,7 @@ static noinline void physom_board_init(void *fdt, int sdram, int module_family)
 
 	am33xx_uart_soft_reset((void *)AM33XX_UART0_BASE);
 	am33xx_enable_uart0_pin_mux();
-	omap_uart_lowlevel_init((void *)AM33XX_UART0_BASE);
+	omap_uart_lowlevel_init();
 	putc_ll('>');
 
 	am335x_barebox_entry(fdt);

arch/arm/boards/vscom-baltos/lowlevel.c +1 -1

index 7da2f92efb..2401cf20c4 100644
@@ -104,7 +104,7 @@ static noinline void baltos_sram_init(void)
 
 	am33xx_uart_soft_reset((void *)AM33XX_UART0_BASE);
 	am33xx_enable_uart0_pin_mux();
-	omap_uart_lowlevel_init((void *)AM33XX_UART0_BASE);
+	omap_uart_lowlevel_init();
 	putc_ll('>');
 
 	am335x_barebox_entry(fdt);

arch/arm/boards/wago-pfc-am35xx/lowlevel.c +1 -1

index 7da8fd0331..6a7f350dda 100644
@@ -185,7 +185,7 @@ static noinline void pfc200_board_init(void)
 
 	if (IS_ENABLED(CONFIG_DEBUG_LL)) {
 		am33xx_uart_soft_reset(IOMEM(OMAP3_UART3_BASE));
-		omap_uart_lowlevel_init(IOMEM(OMAP3_UART3_BASE));
+		omap_uart_lowlevel_init();
 		putc_ll('>');
 	}

arch/arm/mach-omap/include/mach/debug_ll.h +19 ~~-16~~

index 25ddd485be..30a0d830c3 100644
@@ -22,6 +22,8 @@
 #include <mach/omap4-silicon.h>
 #include <mach/am33xx-silicon.h>
 
+#ifdef CONFIG_DEBUG_LL
+
 #define LSR_THRE	0x20	/* Xmit holding register empty */
 #define LCR_BKSE	0x80	/* Bank select enable */
 #define LSR		(5 << 2)

@@ -34,22 +36,6 @@
 #define MCR		(4 << 2)
 #define MDR		(8 << 2)
 
-static inline void omap_uart_lowlevel_init(void __iomem *base)
-{
-	writeb(0x00, base + LCR);
-	writeb(0x00, base + IER);
-	writeb(0x07, base + MDR);
-	writeb(LCR_BKSE, base + LCR);
-	writeb(26, base + DLL); /* 115200 */
-	writeb(0, base + DLM);
-	writeb(0x03, base + LCR);
-	writeb(0x03, base + MCR);
-	writeb(0x07, base + FCR);
-	writeb(0x00, base + MDR);
-}
-
-#ifdef CONFIG_DEBUG_LL
-
 #ifdef CONFIG_DEBUG_OMAP3_UART
 #define OMAP_DEBUG_SOC OMAP3
 #elif defined CONFIG_DEBUG_OMAP4_UART

@@ -63,6 +49,23 @@ static inline void omap_uart_lowlevel_init(void __iomem *base)
 #define __OMAP_UART_BASE(soc, num) soc##_UART##num##_BASE
 #define OMAP_UART_BASE(soc, num) __OMAP_UART_BASE(soc, num)
 
+static inline void omap_uart_lowlevel_init(void)
+{
+	void __iomem *base = (void *)OMAP_UART_BASE(OMAP_DEBUG_SOC,
+			CONFIG_DEBUG_OMAP_UART_PORT);
+
+	writeb(0x00, base + LCR);
+	writeb(0x00, base + IER);
+	writeb(0x07, base + MDR);
+	writeb(LCR_BKSE, base + LCR);
+	writeb(26, base + DLL); /* 115200 */
+	writeb(0, base + DLM);
+	writeb(0x03, base + LCR);
+	writeb(0x03, base + MCR);
+	writeb(0x07, base + FCR);
+	writeb(0x00, base + MDR);
+}
+
 static inline void PUTC_LL(char c)
 {
 	void __iomem *base = (void *)OMAP_UART_BASE(OMAP_DEBUG_SOC,

@@ -75,6 +78,12 @@ static inline void PUTC_LL(char c)
 	/* Wait to make sure it hits the line, in case we die too soon. */
 	while ((readb(base + LSR) & LSR_THRE) == 0);
 }

arch/arm/mach-omap/include/mach/debug_ll.h static inline void PUTC_LL(char c) · line 89

Quoted 10 lines

arch/arm/mach-omap/include/mach/debug_ll.h +7 -1

-#endif
+#else /* CONFIG_DEBUG_LL */
+
+static inline void omap_uart_lowlevel_init(void)
+{
+}
+
+#endif /* CONFIG_DEBUG_LL */
 
 #endif

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Alexander Shiyan

author

2022-05-05 08:03 UTC #2 lore raw

чт, 5 мая 2022 г. в 10:55, Lucas Stach <[email protected]>:

Quoted 6 lines

Am Donnerstag, dem 05.05.2022 um 10:36 +0300 schrieb Alexander Shiyan:
Quoted 1 line
Signed-off-by Alexander Shiyan <[email protected]>
Is this really a change in the right direction? The way it is currently done seems to mix well with overall multi-image theme of Barebox. By using a Kconfig for that, the setting will most likely be wrong for a subset of boards in a multi-image build.

Same as PUTC_LL, right?

Actually the debug code is already wrong when we create multiple images with different UARTs.

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Ahmad Fatoum

2022-05-05 08:11 UTC #3 lore raw

Quoted 8 lines · Alexander Shiyan

чт, 5 мая 2022 г. в 10:55, Lucas Stach <[email protected]>:
Quoted 6 lines
Am Donnerstag, dem 05.05.2022 um 10:36 +0300 schrieb Alexander Shiyan:
Quoted 1 line
Signed-off-by Alexander Shiyan <[email protected]>
Is this really a change in the right direction? The way it is currently done seems to mix well with overall multi-image theme of Barebox. By using a Kconfig for that, the setting will most likely be wrong for a subset of boards in a multi-image build.

Quoted 4 lines

Same as PUTC_LL, right?
Actually the debug code is already wrong when we create multiple images with different UARTs.

There is pbl_set_putc(), which can be called after relocation for use by the PBL console and is multi-image friendly.

Quoted 5 lines

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Signature · 9 lines

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Sascha Hauer

2022-05-05 08:15 UTC #4 lore raw

In reply to Alexander Shiyan

Hi Alexander,

Quoted 17 lines · Alexander Shiyan

Signed-off-by Alexander Shiyan <[email protected]>

---
 arch/arm/boards/afi-gf/lowlevel.c            |  2 +-
 arch/arm/boards/beagle/lowlevel.c            |  2 +-
 arch/arm/boards/beaglebone/lowlevel.c        |  2 +-
 arch/arm/boards/myirtech-x335x/lowlevel.c    |  2 +-
 arch/arm/boards/phytec-som-am335x/lowlevel.c |  2 +-
 arch/arm/boards/vscom-baltos/lowlevel.c      |  2 +-
 arch/arm/boards/wago-pfc-am35xx/lowlevel.c   |  2 +-
 arch/arm/mach-omap/include/mach/debug_ll.h   | 43 ++++++++++++--------
 8 files changed, 33 insertions(+), 24 deletions(-)

arch/arm/boards/afi-gf/lowlevel.c +0 -0

index de40f6c5af..9e86b29b2d 100644

arch/arm/boards/afi-gf/lowlevel.c static noinline int gf_sram_init(void) · line 233

Quoted 5 lines

arch/arm/boards/afi-gf/lowlevel.c +1 -1

@@ -230,7 +230,7 @@ static noinline int gf_sram_init(void)
 
 	am33xx_uart_soft_reset((void *)AM33XX_UART2_BASE);
 	am33xx_enable_uart2_pin_mux();
-	omap_uart_lowlevel_init((void *)AM33XX_UART2_BASE);
+	omap_uart_lowlevel_init();

I don't think removing the base address from omap_uart_lowlevel_init() is a step forward. CONFIG_DEBUG_LL is only one possible usecase for this function, another one is for regular early console with pbl_set_putc().

Sascha

Signature · 9 lines

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox

Alexander Shiyan

author

2022-05-05 09:43 UTC #5 lore raw

чт, 5 мая 2022 г. в 11:13, Sascha Hauer <[email protected]>:

Quoted 20 lines

Hi Alexander,

Quoted 18 lines · Alexander Shiyan

Signed-off-by Alexander Shiyan <[email protected]>

---
 arch/arm/boards/afi-gf/lowlevel.c            |  2 +-
 arch/arm/boards/beagle/lowlevel.c            |  2 +-
 arch/arm/boards/beaglebone/lowlevel.c        |  2 +-
 arch/arm/boards/myirtech-x335x/lowlevel.c    |  2 +-
 arch/arm/boards/phytec-som-am335x/lowlevel.c |  2 +-
 arch/arm/boards/vscom-baltos/lowlevel.c      |  2 +-
 arch/arm/boards/wago-pfc-am35xx/lowlevel.c   |  2 +-
 arch/arm/mach-omap/include/mach/debug_ll.h   | 43 ++++++++++++--------
 8 files changed, 33 insertions(+), 24 deletions(-)

arch/arm/boards/afi-gf/lowlevel.c +0 -0

index de40f6c5af..9e86b29b2d 100644
@@ -230,7 +230,7 @@ static noinline int gf_sram_init(void)

Quoted 8 lines

arch/arm/boards/afi-gf/lowlevel.c static noinline int gf_sram_init(void) · line 233
Quoted 4 lines
arch/arm/boards/afi-gf/lowlevel.c +1 -1
      am33xx_uart_soft_reset((void *)AM33XX_UART2_BASE);
      am33xx_enable_uart2_pin_mux();
-     omap_uart_lowlevel_init((void *)AM33XX_UART2_BASE);
+     omap_uart_lowlevel_init();
I don't think removing the base address from omap_uart_lowlevel_init() is a step forward. CONFIG_DEBUG_LL is only one possible usecase for this function, another one is for regular early console with pbl_set_putc().

Ok. Please take a look at the patch in the next message, is this how it should be?

_______________________________________________
barebox mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/barebox