qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Qemu-trivial] [PATCH] cadence_uart: Check if receiver


From: Andrew Gacek
Subject: Re: [Qemu-devel] [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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]