| Message ID | 1394821351-21477-5-git-send-email-robherring2@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 14 March 2014 18:22, Rob Herring <robherring2@gmail.com> wrote: > From: Rob Herring <rob.herring@linaro.org> > > 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 <rob.herring@linaro.org> > --- > 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? Also if you're updating int_level then you need to call pl011_update(). thanks -- PMM
On 16 March 2014 15:57, Peter Maydell <peter.maydell@linaro.org> wrote: > On 14 March 2014 18:22, Rob Herring <robherring2@gmail.com> wrote: >> From: Rob Herring <rob.herring@linaro.org> >> >> 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 <rob.herring@linaro.org> >> --- >> 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). 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. thanks -- PMM
On Sun, Mar 16, 2014 at 11:44 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 16 March 2014 15:57, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 14 March 2014 18:22, Rob Herring <robherring2@gmail.com> wrote: >>> From: Rob Herring <rob.herring@linaro.org> >>> >>> 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 <rob.herring@linaro.org> >>> --- >>> 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
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; + } } static void pl011_write(void *opaque, hwaddr offset,