qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH v2 1/4] pl011: reset the fifo when enabled or di


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH v2 1/4] pl011: reset the fifo when enabled or disabled
Date: Sun, 16 Mar 2014 16:16:07 +0000

On 14 March 2014 18:22, Rob Herring <address@hidden> wrote:
> From: Rob Herring <address@hidden>
>
> Intermittent issues have been seen where no serial input occurs. It
> appears the pl011 gets in a state where the rx interrupt never fires
> because the rx interrupt only asserts when crossing the fifo trigger
> level. The fifo state appears to get out of sync when the pl011 is
> re-configured. This combined with the rx timeout interrupt not being
> modeled results in no more rx interrupts.

This sounds like the bug is that we're not asserting the rx interrupt
when we should, which is not what this patch is trying to change.

Among other things, this patch doesn't actually cause a write to
this register to make us recalculate and re-send the interrupt state,
because there is no call to pl011_update() in this code path.

> Disabling the fifo is the recommended way to clear the tx fifo in the
> TRM (section 3.3.8). The behavior in this case for the rx fifo is
> undefined in the TRM, but having fifo contents to be maintained during
> configuration changes is not likely expected behavior. Reseting the
> fifo state when the fifo size is changed is the simplest solution.
>
> Signed-off-by: Rob Herring <address@hidden>
> ---
>  hw/char/pl011.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/hw/char/pl011.c b/hw/char/pl011.c
> index a8ae6f4..f0c3fa3 100644
> --- a/hw/char/pl011.c
> +++ b/hw/char/pl011.c
> @@ -162,6 +162,10 @@ static void pl011_write(void *opaque, hwaddr offset,
>          s->fbrd = value;
>          break;
>      case 11: /* UARTLCR_H */
> +        if (!((s->lcr ^ value) & 0x10)) {
> +            s->read_count = 0;
> +            s->read_pos = 0;
> +        }

Am I misreading this, or do we clear the fifo on every write
to this register where the fifo-enable bit is not changed?

>          s->lcr = value;

>          pl011_set_read_trigger(s);
>          break;
> --
> 1.8.3.2
>

I really think this patch is just covering over the actual problem
and we need to identify and fix the actual bug.

thanks
-- PMM



reply via email to

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