[PATCH] serial: a37xx: Reset whole UART when changing parent clock from TBG to XTAL

[PATCH] serial: a37xx: Reset whole UART when changing parent clock from TBG to XTAL

[Pali Rohár]

Sometimes UART stops transmitting characters after UART clock is changed
back to XTAL. In this state UART fifo is always full. Kernel during early
boot wants to print output on UART and is waiting for non-empty UART fifo.
Which leads to CPU hangup without any (debug) output on UART.

Marvell Armada 3700 Functional Specifications says that for programming
fractional divisor registers it is required to disable UART, enable
loopback mode, reset fifos, program registers, disable loopback mode,
release reset of fifos and enable UART.

But these steps do not fix above mentioned issue that UART hangup. Also
gating UART clock does not help. And even resetting UART state machines do
not help.

Experiments showed that UART fifo is unblocked after board is being reset
(during board reset UART HW transmit UART fifo even CPU is not executing
kernel/bootloader anymore).

And another experiments showed that same workaround can be achieved also
by external reset of UART HW (without need to reset board).

So do not implement any of "Marvell recommended" steps from Functional
Specifications as they do not work. And rather prior changing parent clock
back to XTAL, do external reset of UART HW. This operation also resets all
UART registers, so basically it also sets UART clock to default, which is
XTAL. It is unknown why UART hangups and enters such broken state.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/serial/serial_mvebu_a3700.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/serial/serial_mvebu_a3700.c b/drivers/serial/serial_mvebu_a3700.c
index 6bca8e4b7e2d..93e4d38d34c7 100644
--- a/drivers/serial/serial_mvebu_a3700.c
+++ b/drivers/serial/serial_mvebu_a3700.c
@@ -9,6 +9,7 @@
 #include <serial.h>
 #include <asm/io.h>
 #include <asm/arch/cpu.h>
+#include <mach/soc.h>
 
 struct mvebu_plat {
 	void __iomem *base;
@@ -213,6 +214,7 @@ static int mvebu_serial_remove(struct udevice *dev)
 	u32 new_oversampling;
 	u32 oversampling;
 	u32 d1, d2;
+	u32 nb_rst;
 
 	/*
 	 * Switch UART base clock back to XTAL because older Linux kernel
@@ -260,12 +262,22 @@ static int mvebu_serial_remove(struct udevice *dev)
 			return 0;
 	}
 
+	/* wait until TX empty */
 	while (!(readl(base + UART_STATUS_REG) & UART_STATUS_TX_EMPTY))
 		;
 
+	/* external reset of UART via North Bridge Peripheral */
+	nb_rst = readl(MVEBU_REGISTER(0x12400));
+	writel(nb_rst & ~BIT(3), MVEBU_REGISTER(0x12400));
+	writel(nb_rst | BIT(3), MVEBU_REGISTER(0x12400));
+
+	/* set baudrate and oversampling */
 	writel(new_divider, base + UART_BAUD_REG);
 	writel(new_oversampling, base + UART_POSSR_REG);
 
+	/* No Parity, 1 Stop */
+	writel(0, base + UART_CTRL_REG);
+
 	return 0;
 }
 
@@ -305,7 +317,6 @@ U_BOOT_DRIVER(serial_mvebu) = {
 #ifdef CONFIG_DEBUG_MVEBU_A3700_UART
 
 #include <debug_uart.h>
-#include <mach/soc.h>
 
 static inline void _debug_uart_init(void)
 {
-- 
2.20.1

Re: [PATCH] serial: a37xx: Reset whole UART when changing parent clock from TBG to XTAL

[Stefan Roese]

On 11/15/21 13:48, Pali Rohár wrote:
> Sometimes UART stops transmitting characters after UART clock is changed
> back to XTAL. In this state UART fifo is always full. Kernel during early
> boot wants to print output on UART and is waiting for non-empty UART fifo.
> Which leads to CPU hangup without any (debug) output on UART.
> 
> Marvell Armada 3700 Functional Specifications says that for programming
> fractional divisor registers it is required to disable UART, enable
> loopback mode, reset fifos, program registers, disable loopback mode,
> release reset of fifos and enable UART.
> 
> But these steps do not fix above mentioned issue that UART hangup. Also
> gating UART clock does not help. And even resetting UART state machines do
> not help.
> 
> Experiments showed that UART fifo is unblocked after board is being reset
> (during board reset UART HW transmit UART fifo even CPU is not executing
> kernel/bootloader anymore).
> 
> And another experiments showed that same workaround can be achieved also
> by external reset of UART HW (without need to reset board).
> 
> So do not implement any of "Marvell recommended" steps from Functional
> Specifications as they do not work. And rather prior changing parent clock
> back to XTAL, do external reset of UART HW. This operation also resets all
> UART registers, so basically it also sets UART clock to default, which is
> XTAL. It is unknown why UART hangups and enters such broken state.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>   drivers/serial/serial_mvebu_a3700.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/serial/serial_mvebu_a3700.c b/drivers/serial/serial_mvebu_a3700.c
> index 6bca8e4b7e2d..93e4d38d34c7 100644
> --- a/drivers/serial/serial_mvebu_a3700.c
> +++ b/drivers/serial/serial_mvebu_a3700.c
> @@ -9,6 +9,7 @@
>   #include <serial.h>
>   #include <asm/io.h>
>   #include <asm/arch/cpu.h>
> +#include <mach/soc.h>
>   
>   struct mvebu_plat {
>   	void __iomem *base;
> @@ -213,6 +214,7 @@ static int mvebu_serial_remove(struct udevice *dev)
>   	u32 new_oversampling;
>   	u32 oversampling;
>   	u32 d1, d2;
> +	u32 nb_rst;
>   
>   	/*
>   	 * Switch UART base clock back to XTAL because older Linux kernel
> @@ -260,12 +262,22 @@ static int mvebu_serial_remove(struct udevice *dev)
>   			return 0;
>   	}
>   
> +	/* wait until TX empty */
>   	while (!(readl(base + UART_STATUS_REG) & UART_STATUS_TX_EMPTY))
>   		;
>   
> +	/* external reset of UART via North Bridge Peripheral */
> +	nb_rst = readl(MVEBU_REGISTER(0x12400));
> +	writel(nb_rst & ~BIT(3), MVEBU_REGISTER(0x12400));
> +	writel(nb_rst | BIT(3), MVEBU_REGISTER(0x12400));

I'm not that fond of this type of access to some "external" register in
this driver. But adding some reset driver might be a bit too much here.
So:

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan


> +
> +	/* set baudrate and oversampling */
>   	writel(new_divider, base + UART_BAUD_REG);
>   	writel(new_oversampling, base + UART_POSSR_REG);
>   
> +	/* No Parity, 1 Stop */
> +	writel(0, base + UART_CTRL_REG);
> +
>   	return 0;
>   }
>   
> @@ -305,7 +317,6 @@ U_BOOT_DRIVER(serial_mvebu) = {
>   #ifdef CONFIG_DEBUG_MVEBU_A3700_UART
>   
>   #include <debug_uart.h>
> -#include <mach/soc.h>
>   
>   static inline void _debug_uart_init(void)
>   {
> 

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH] serial: a37xx: Reset whole UART when changing parent clock from TBG to XTAL

[Stefan Roese]

On 11/15/21 13:48, Pali Rohár wrote:
> Sometimes UART stops transmitting characters after UART clock is changed
> back to XTAL. In this state UART fifo is always full. Kernel during early
> boot wants to print output on UART and is waiting for non-empty UART fifo.
> Which leads to CPU hangup without any (debug) output on UART.
> 
> Marvell Armada 3700 Functional Specifications says that for programming
> fractional divisor registers it is required to disable UART, enable
> loopback mode, reset fifos, program registers, disable loopback mode,
> release reset of fifos and enable UART.
> 
> But these steps do not fix above mentioned issue that UART hangup. Also
> gating UART clock does not help. And even resetting UART state machines do
> not help.
> 
> Experiments showed that UART fifo is unblocked after board is being reset
> (during board reset UART HW transmit UART fifo even CPU is not executing
> kernel/bootloader anymore).
> 
> And another experiments showed that same workaround can be achieved also
> by external reset of UART HW (without need to reset board).
> 
> So do not implement any of "Marvell recommended" steps from Functional
> Specifications as they do not work. And rather prior changing parent clock
> back to XTAL, do external reset of UART HW. This operation also resets all
> UART registers, so basically it also sets UART clock to default, which is
> XTAL. It is unknown why UART hangups and enters such broken state.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>

Applied to u-boot-marvell/master

Thanks,
Stefan

> ---
>   drivers/serial/serial_mvebu_a3700.c | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/serial/serial_mvebu_a3700.c b/drivers/serial/serial_mvebu_a3700.c
> index 6bca8e4b7e2d..93e4d38d34c7 100644
> --- a/drivers/serial/serial_mvebu_a3700.c
> +++ b/drivers/serial/serial_mvebu_a3700.c
> @@ -9,6 +9,7 @@
>   #include <serial.h>
>   #include <asm/io.h>
>   #include <asm/arch/cpu.h>
> +#include <mach/soc.h>
>   
>   struct mvebu_plat {
>   	void __iomem *base;
> @@ -213,6 +214,7 @@ static int mvebu_serial_remove(struct udevice *dev)
>   	u32 new_oversampling;
>   	u32 oversampling;
>   	u32 d1, d2;
> +	u32 nb_rst;
>   
>   	/*
>   	 * Switch UART base clock back to XTAL because older Linux kernel
> @@ -260,12 +262,22 @@ static int mvebu_serial_remove(struct udevice *dev)
>   			return 0;
>   	}
>   
> +	/* wait until TX empty */
>   	while (!(readl(base + UART_STATUS_REG) & UART_STATUS_TX_EMPTY))
>   		;
>   
> +	/* external reset of UART via North Bridge Peripheral */
> +	nb_rst = readl(MVEBU_REGISTER(0x12400));
> +	writel(nb_rst & ~BIT(3), MVEBU_REGISTER(0x12400));
> +	writel(nb_rst | BIT(3), MVEBU_REGISTER(0x12400));
> +
> +	/* set baudrate and oversampling */
>   	writel(new_divider, base + UART_BAUD_REG);
>   	writel(new_oversampling, base + UART_POSSR_REG);
>   
> +	/* No Parity, 1 Stop */
> +	writel(0, base + UART_CTRL_REG);
> +
>   	return 0;
>   }
>   
> @@ -305,7 +317,6 @@ U_BOOT_DRIVER(serial_mvebu) = {
>   #ifdef CONFIG_DEBUG_MVEBU_A3700_UART
>   
>   #include <debug_uart.h>
> -#include <mach/soc.h>
>   
>   static inline void _debug_uart_init(void)
>   {
> 

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de