Re: [Qemu-devel] [PATCH v2 4/4] pl011: re-evaluate rx interrupt when fifo trigger changes

[Rob Herring]

On Sun, Mar 16, 2014 at 11:44 AM, Peter Maydell
<address@hidden> wrote:
> On 16 March 2014 15:57, Peter Maydell <address@hidden> wrote:
>> On 14 March 2014 18:22, Rob Herring <address@hidden> wrote:
>>> From: Rob Herring <address@hidden>
>>>
>>> When setting the fifo trigger level, the rx interrupt needs to be asserted
>>> if the current fifo level matches. This is more for correctness as the
>>> level is currently never changed.
>>>
>>> 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 5e664f4..3903933 100644
>>> --- a/hw/char/pl011.c
>>> +++ b/hw/char/pl011.c
>>> @@ -131,6 +131,10 @@ static void pl011_set_read_trigger(PL011State *s)
>>>      else
>>>  #endif
>>>          s->read_trigger = 1;
>>> +
>>> +    if (s->read_count == s->read_trigger) {
>>> +        s->int_level |= PL011_INT_RX;
>>> +    }
>>
>>>=, surely?
>
> On closer inspection of the TRM it's not quite that simple (section 2.8.2).

Humm, a section not in r1p4 rev I was looking at, but in r1p5.

> We need to assert the interrupt only if the change in the read_trigger
> means the FIFO is now above the new threshold but wasn't above
> the old one, and only if the FIFOs are enabled -- if the FIFO is disabled
> the threshold doesn't figure into the interrupt level.

I'm not even sure that is correct. I read it as the interrupt would
only assert when receiving a character that causes the fifo to be
equal to the new trigger level. I think changing the fifo trigger
level on the fly is just asking for trouble. It is all just
speculation and I don't have hardware to test actual behavior.

Rob