Re: [Xen-devel] [PATCH v3] xen/ns16550: Fix ISR lockup on Allwinner uart

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 4 Oct 2017, Awais Masood wrote:
> This patch fixes an ISR lockup seen on Allwinner uart
> 
> On Allwinner H5, serial driver goes into an infinite loop
> when interrupts are enabled. The reason is a residual
> "busy detect" interrupt. Since the condition UART_IIR_NOINT
> will not be true unless this interrupt is cleared, the
> interrupt handler will remain locked up in this while loop.
> 
> A HW quirk fix was previously added for designware uart under
> commit:
> 50417cd978aa54930d065ac1f139f935d14af76d
> 
> It checks for a busy condition during setup and clears the
> condition by reading UART_USR register.
> 
> On Allwinner hardware, the "busy detect" condition occurs
> later because an LCR write is performed during setup 'after'
> this clear and if uart is busy, the "busy detect" condition
> will trigger again and cause the ISR lockup.
> 
> To solve this problem, the same UART_USR read operation needs
> to be performed within the interrupt handler to clear this
> condition.
> 
> Linux dw 8250 driver also handles this condition within
> interrupt handler
> http://elixir.free-electrons.com/linux/latest/source/drivers/tty/serial/8250/8250_dw.c#L233
> 
> Tested on Orange Pi PC2 (H5). This issue is seen on H3
> as well and the same fix works.
> 
> Signed-off-by: Awais Masood <awais.masood@xxxxxxxxxx>

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
> 
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> CC: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Tim Deegan <tim@xxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> 
> Changes since v2
>  - Updated comments to clarify that fix is for Allwinner hardware
>  - Removed ns16550 prefix from local function
> 
> Changes since v1
>  - Common quirk fix code moved to a helper function
>  - Patch description improved with earlier commit link
> ---
>  xen/drivers/char/ns16550.c | 38 +++++++++++++++++++++++++++++---------
>  1 file changed, 29 insertions(+), 9 deletions(-)
> 
> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index 6ab5ec3..e0f8199 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -505,6 +505,23 @@ static int ns16550_ioport_invalid(struct ns16550 *uart)
>      return ns_read_reg(uart, UART_IER) == 0xff;
>  }
>  
> +static void handle_dw_usr_busy_quirk(struct ns16550 *uart)
> +{
> +    if ( uart->dw_usr_bsy &&
> +         (ns_read_reg(uart, UART_IIR) & UART_IIR_BSY) == UART_IIR_BSY )
> +    {
> +        /* DesignWare 8250 detects if LCR is written while the UART is
> +         * busy and raises a "busy detect" interrupt. Read the UART
> +         * Status Register to clear this state.
> +         *
> +         * Allwinner/sunxi UART hardware is similar to DesignWare 8250
> +         * and also contains a "busy detect" interrupt. So this quirk
> +         * fix will also be used for Allwinner UART.
> +         */
> +        ns_read_reg(uart, UART_USR);
> +    }
> +}
> +
>  static void ns16550_interrupt(
>      int irq, void *dev_id, struct cpu_user_regs *regs)
>  {
> @@ -521,6 +538,16 @@ static void ns16550_interrupt(
>              serial_tx_interrupt(port, regs);
>          if ( lsr & UART_LSR_DR )
>              serial_rx_interrupt(port, regs);
> +
> +        /* A "busy-detect" condition is observed on Allwinner/sunxi UART
> +         * after LCR is written during setup. It needs to be cleared at
> +         * this point or UART_IIR_NOINT will never be set and this loop
> +         * will continue forever.
> +         *
> +         * This state can be cleared by calling the dw_usr_busy quirk
> +         * handler that resolves "busy-detect" for  DesignWare uart.
> +         */
> +        handle_dw_usr_busy_quirk(uart);
>      }
>  }
>  
> @@ -623,15 +650,8 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
>      /* No interrupts. */
>      ns_write_reg(uart, UART_IER, 0);
>  
> -    if ( uart->dw_usr_bsy &&
> -         (ns_read_reg(uart, UART_IIR) & UART_IIR_BSY) == UART_IIR_BSY )
> -    {
> -        /* DesignWare 8250 detects if LCR is written while the UART is
> -         * busy and raises a "busy detect" interrupt. Read the UART
> -         * Status Register to clear this state.
> -         */
> -        ns_read_reg(uart, UART_USR);
> -    }
> +    /* Handle the DesignWare 8250 'busy-detect' quirk. */
> +    handle_dw_usr_busy_quirk(uart);
>  
>      /* Line control and baud-rate generator. */
>      ns_write_reg(uart, UART_LCR, lcr | UART_LCR_DLAB);
> -- 
> 2.7.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel