[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2040575a-2378-ef7a-8a23-b2458d3103f6@kernel.org>
Date: Thu, 20 Oct 2022 07:56:04 +0200
From: Jiri Slaby <jirislaby@...nel.org>
To: Sherry Sun <sherry.sun@....com>, gregkh@...uxfoundation.org,
lukas@...ner.de, ilpo.jarvinen@...ux.intel.com
Cc: linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-imx@....com
Subject: Re: [PATCH V2] tty: serial: fsl_lpuart: don't break the on-going
transfer when global reset
On 19. 10. 22, 13:07, Sherry Sun wrote:
> lpuart_global_reset() shouldn't break the on-going transmit engin, need
"engine"
> to recover the on-going data transfer after reset.
>
> This can help earlycon here, since commit 60f361722ad2 ("serial:
> fsl_lpuart: Reset prior to registration") moved lpuart_global_reset()
> before uart_add_one_port(), earlycon is writing during global reset,
> as global reset will disable the TX and clear the baud rate register,
> which caused the earlycon cannot work any more after reset, needs to
> restore the baud rate and re-enable the transmitter to recover the
> earlycon write.
>
> Fixes: bd5305dcabbc ("tty: serial: fsl_lpuart: do software reset for imx7ulp and imx8qxp")
> Signed-off-by: Sherry Sun <sherry.sun@....com>
> ---
> Changes in V2:
> 1. The while loop may never exit as the stat and sfifo are not re-read inside
> the loop, fix that.
> ---
> drivers/tty/serial/fsl_lpuart.c | 22 +++++++++++++++++++---
> 1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
> index 67fa113f77d4..9a0781395b1f 100644
> --- a/drivers/tty/serial/fsl_lpuart.c
> +++ b/drivers/tty/serial/fsl_lpuart.c
> @@ -408,11 +408,9 @@ static int lpuart_global_reset(struct lpuart_port *sport)
> {
> struct uart_port *port = &sport->port;
> void __iomem *global_addr;
> + unsigned long tx_enable, bd;
> int ret;
>
> - if (uart_console(port))
> - return 0;
> -
> ret = clk_prepare_enable(sport->ipg_clk);
> if (ret) {
> dev_err(sport->port.dev, "failed to enable uart ipg clk: %d\n", ret);
> @@ -420,11 +418,29 @@ static int lpuart_global_reset(struct lpuart_port *sport)
> }
>
> if (is_imx7ulp_lpuart(sport) || is_imx8qxp_lpuart(sport)) {
> + /*
> + * If the transmitter is used by earlycon, wait transmit engin complete
"wait for transmit engine to complete"
> + * and then reset
"." (add a period)
> + */
> + tx_enable = lpuart32_read(port, UARTCTRL) & UARTCTRL_TE;
> + if (tx_enable) {
> + bd = lpuart32_read(&sport->port, UARTBAUD);
> + while (!(lpuart32_read(port, UARTSTAT) & UARTSTAT_TC &&
> + lpuart32_read(port, UARTFIFO) & UARTFIFO_TXEMPT))
> + cpu_relax();
If the HW is stuck, you make the kernel stuck too. What about
read_poll_timeout_atomic(). Or at least a custom timeout.
> + }
> +
> global_addr = port->membase + UART_GLOBAL - IMX_REG_OFF;
> writel(UART_GLOBAL_RST, global_addr);
> usleep_range(GLOBAL_RST_MIN_US, GLOBAL_RST_MAX_US);
> writel(0, global_addr);
> usleep_range(GLOBAL_RST_MIN_US, GLOBAL_RST_MAX_US);
> +
> + /* Recover the transmitter for earlycon */
> + if (tx_enable) {
> + lpuart32_write(port, bd, UARTBAUD);
> + lpuart32_write(port, UARTCTRL_TE, UARTCTRL);
I don't know the HW. Is it enough to write TE here? IOW shouldn't we
preserve the whole register content from the above read here?
thanks,
--
js
suse labs
Powered by blists - more mailing lists