On Mon, 16 Nov 2020 at 19:58, Tadej Pečar <[email protected]> wrote: > > Previously, the RX interrupt got missed if: > - the character backend provided next character before > the RX IRQ Handler managed to clear the currently served interrupt. > - the character backend provided next character while the RX interrupt > was disabled. Enabling the interrupt did not trigger the interrupt > even if the RXFULL status bit was set. > > These bugs become apparent when the terminal emulator buffers the line > before sending it to qemu stdin (Eclipse IDE console does this). > > > diff --git a/hw/char/cmsdk-apb-uart.c b/hw/char/cmsdk-apb-uart.c > index 626b68f2ec..d76ca76e01 100644 > --- a/hw/char/cmsdk-apb-uart.c > +++ b/hw/char/cmsdk-apb-uart.c > @@ -96,19 +96,34 @@ static void uart_update_parameters(CMSDKAPBUART *s) > > static void cmsdk_apb_uart_update(CMSDKAPBUART *s) > { > - /* update outbound irqs, including handling the way the rxo and txo > - * interrupt status bits are just logical AND of the overrun bit in > - * STATE and the overrun interrupt enable bit in CTRL. > + /* > + * update outbound irqs > + * ( > + * state [rxo, txo, rxbf, txbf ] at bit [3, 2, 1, 0] > + * | intstatus [rxo, txo, rx, tx ] at bit [3, 2, 1, 0] > + * ) > + * & ctrl [rxoe, txoe, rxe, txe ] at bit [5, 4, 3, 2] > + * = masked_intstatus > + * > + * state: status register > + * intstatus: pending interrupts and is sticky (has to be cleared by sw) > + * masked_intstatus: masked (by ctrl) pending interrupts > + * > + * intstatus [rxo, txo, rx] bits are set here > + * intstatus [tx] is managed in uart_transmit > */ > - uint32_t omask = (R_INTSTATUS_RXO_MASK | R_INTSTATUS_TXO_MASK); > - s->intstatus &= ~omask; > - s->intstatus |= (s->state & (s->ctrl >> 2) & omask); > - > - qemu_set_irq(s->txint, !!(s->intstatus & R_INTSTATUS_TX_MASK)); > - qemu_set_irq(s->rxint, !!(s->intstatus & R_INTSTATUS_RX_MASK)); > - qemu_set_irq(s->txovrint, !!(s->intstatus & R_INTSTATUS_TXO_MASK)); > - qemu_set_irq(s->rxovrint, !!(s->intstatus & R_INTSTATUS_RXO_MASK)); > - qemu_set_irq(s->uartint, !!(s->intstatus)); > + s->intstatus |= s->state & > + (R_INTSTATUS_RXO_MASK | R_INTSTATUS_TXO_MASK | R_INTSTATUS_RX_MASK); > + > + uint32_t masked_intstatus = s->intstatus & (s->ctrl >> 2);
I don't think this logic is correct. It makes the values we send out on the output interrupt lines different from the values visible in the INTSTATUS register bits, and I don't think that's how the hardware behaves. thanks -- PMM
