| Message ID | 1507117471-12389-1-git-send-email-awais.masood@vadion.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 04.10.17 at 13:44, <awais.masood@vadion.com> 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@vadion.com> Acked-by: Jan Beulich <jbeulich@suse.com>
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@vadion.com> Acked-by: Stefano Stabellini <sstabellini@kernel.org> > --- > > CC: Andrew Cooper <andrew.cooper3@citrix.com> > CC: George Dunlap <George.Dunlap@eu.citrix.com> > CC: Ian Jackson <ian.jackson@eu.citrix.com> > CC: Jan Beulich <jbeulich@suse.com> > CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Tim Deegan <tim@xen.org> > CC: Wei Liu <wei.liu2@citrix.com> > > 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 >
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);
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@vadion.com> --- CC: Andrew Cooper <andrew.cooper3@citrix.com> CC: George Dunlap <George.Dunlap@eu.citrix.com> CC: Ian Jackson <ian.jackson@eu.citrix.com> CC: Jan Beulich <jbeulich@suse.com> CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Tim Deegan <tim@xen.org> CC: Wei Liu <wei.liu2@citrix.com> 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(-)