| Message ID | 20180315191141.6789-1-andrew.smirnov@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] char: i.MX: Simplify imx_update() | expand |
On 15 March 2018 at 19:11, Andrey Smirnov <andrew.smirnov@gmail.com> wrote: > Code of imx_update() is slightly confusing since the "flags" variable > doesn't really corespond to anything in real hardware and server as a > kitchensink accumulating events normally reported via USR1 and USR2 > registers. > > Change the code to explicitly evaluate state of interrupts reported > via USR1 and USR2 against corresponding masking bits and use the to > detemine if IRQ line should be asserted or not. > > NOTE: Check for UTS1_TXEMPTY being set has been dropped for two > reasons: > > 1. Emulation code implements a single character FIFO, so this flag > will always be set since characters are trasmitted as a part of > the code emulating "push" into the FIFO > > 2. imx_update() is really just a function doing ORing and maksing > of reported events, so checking for UTS1_TXEMPTY should happen, > if it's ever really needed should probably happen outside of > it. > > Cc: qemu-devel@nongnu.org > Cc: qemu-arm@nongnu.org > Cc: Bill Paul <wpaul@windriver.com> > Cc: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> > --- > hw/char/imx_serial.c | 24 ++++++++++++++++-------- Thanks; I've applied this patch and patch 2 to target-arm.next. As bugfixes they'll go into 2.12. PS: if you could provide cover letters for patchsets that have more than one patch in them that would help me in finding and processing them. thanks -- PMM
diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c index 70405ccf8b..d1e8586280 100644 --- a/hw/char/imx_serial.c +++ b/hw/char/imx_serial.c @@ -56,16 +56,24 @@ static const VMStateDescription vmstate_imx_serial = { static void imx_update(IMXSerialState *s) { - uint32_t flags; + uint32_t usr1; + uint32_t usr2; + uint32_t mask; - flags = (s->usr1 & s->ucr1) & (USR1_TRDY|USR1_RRDY); - if (s->ucr1 & UCR1_TXMPTYEN) { - flags |= (s->uts1 & UTS1_TXEMPTY); - } else { - flags &= ~USR1_TRDY; - } + /* + * Lucky for us TRDY and RRDY has the same offset in both USR1 and + * UCR1, so we can get away with something as simple as the + * following: + */ + usr1 = s->usr1 & s->ucr1 & (USR1_TRDY | USR1_RRDY); + /* + * Bits that we want in USR2 are not as conveniently laid out, + * unfortunately. + */ + mask = (s->ucr1 & UCR1_TXMPTYEN) ? USR2_TXFE : 0; + usr2 = s->usr2 & mask; - qemu_set_irq(s->irq, !!flags); + qemu_set_irq(s->irq, usr1 || usr2); } static void imx_serial_reset(IMXSerialState *s)
Code of imx_update() is slightly confusing since the "flags" variable doesn't really corespond to anything in real hardware and server as a kitchensink accumulating events normally reported via USR1 and USR2 registers. Change the code to explicitly evaluate state of interrupts reported via USR1 and USR2 against corresponding masking bits and use the to detemine if IRQ line should be asserted or not. NOTE: Check for UTS1_TXEMPTY being set has been dropped for two reasons: 1. Emulation code implements a single character FIFO, so this flag will always be set since characters are trasmitted as a part of the code emulating "push" into the FIFO 2. imx_update() is really just a function doing ORing and maksing of reported events, so checking for UTS1_TXEMPTY should happen, if it's ever really needed should probably happen outside of it. Cc: qemu-devel@nongnu.org Cc: qemu-arm@nongnu.org Cc: Bill Paul <wpaul@windriver.com> Cc: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com> --- hw/char/imx_serial.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-)