[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [PATCH] cadence_uart: Check if receiver timeout count
From: |
Andrew Gacek |
Subject: |
Re: [Qemu-trivial] [PATCH] cadence_uart: Check if receiver timeout counter is disabled |
Date: |
Thu, 8 Dec 2016 05:21:01 -0600 |
When register Rcvr_timeout_reg0 (R_RTOR in cadence_uart.c) is set to
0, the receiver timeout counter should be disabled. See page 1801 of
"Zynq-7000 AP SoC Technical Reference Manual". This commit adds a
such a check before setting the receive timeout interrupt.
Signed-off-by: Andrew Gacek <address@hidden>
Reviewed-by: Edgar E. Iglesias <address@hidden>
---
hw/char/cadence_uart.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index 0215d65..378630d 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -138,9 +138,11 @@ static void fifo_trigger_update(void *opaque)
{
CadenceUARTState *s = opaque;
- s->r[R_CISR] |= UART_INTR_TIMEOUT;
+ if (s->r[R_RTOR]) {
+ s->r[R_CISR] |= UART_INTR_TIMEOUT;
- uart_update_status(s);
+ uart_update_status(s);
+ }
}
static void uart_rx_reset(CadenceUARTState *s)
--
2.7.4
On Thu, Dec 8, 2016 at 4:42 AM, Edgar E. Iglesias
<address@hidden> wrote:
> On Thu, Dec 08, 2016 at 08:50:40AM +0100, Laurent Vivier wrote:
>> I CC: Xilinx Zynq Maintainers.
>>
>> Laurent
>
> Thanks
>
>>
>> On 07/12/2016 22:12, Andrew Gacek wrote:
>> > When register Rcvr_timeout_reg0 (R_RTOR in cadence_uart.c) is set to
>> > 0, the receiver timeout counter should be disabled. See page 1801 of
>> > "Zynq-7000 AP SoC Technical Reference Manual". This commit adds a
>> > such a check before setting the receive timeout interrupt.
>
> We could also try to disable the timer when rtor is zero but I think
> that exposes a bunch of corner cases that would complicate the model a bit.
> So IMO, this patch is good.
>
>
>> > Signed-off-by: Andrew Gacek <address@hidden>
>> > ---
>> > hw/char/cadence_uart.c | 4 +++-
>> > 1 file changed, 3 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
>> > index 0215d65..54194b1 100644
>> > --- a/hw/char/cadence_uart.c
>> > +++ b/hw/char/cadence_uart.c
>> > @@ -138,7 +138,9 @@ static void fifo_trigger_update(void *opaque)
>> > {
>> > CadenceUARTState *s = opaque;
>> >
>> > - s->r[R_CISR] |= UART_INTR_TIMEOUT;
>> > + if (s->r[R_RTOR]) {
>> > + s->r[R_CISR] |= UART_INTR_TIMEOUT;
>> > + }
>> >
>> > uart_update_status(s);
>
> Since you are not modifying the IRQ state when the timeout is disabled, you
> can avoid calling uart_update_status too (because it will only end up
> recomputing the same state).
>
> With that fix:
> Reviewed-by: Edgar E. Iglesias <address@hidden>
>
> Best regards,
> Edgar