[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 at 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 at 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 at denx.de