On 12 February 2011 01:15, Dmitry Eremin-Solenikov <dbarysh...@gmail.com> wrote:
> Simplify IRQ handling to stop setting an input irq pin. As a win, also get
> correct IRQ status after save/load cycle.
Thanks, I pushed the three patches from you but see a question below.
>
> Signed-off-by: Dmitry Eremin-Solenikov <dbarysh...@gmail.com>
> ---
> hw/mst_fpga.c | 29 ++++++++++-------------------
> 1 files changed, 10 insertions(+), 19 deletions(-)
>
> diff --git a/hw/mst_fpga.c b/hw/mst_fpga.c
> index 93c6514..3c594b8 100644
> --- a/hw/mst_fpga.c
> +++ b/hw/mst_fpga.c
> @@ -46,33 +46,21 @@ typedef struct mst_irq_state{
> }mst_irq_state;
>
> static void
> -mst_fpga_update_gpio(mst_irq_state *s)
> -{
> - uint32_t level, diff;
> - int bit;
> - level = s->prev_level ^ s->intsetclr;
> -
> - for (diff = s->prev_level ^ level; diff; diff ^= 1 << bit) {
> - bit = ffs(diff) - 1;
> - qemu_set_irq(s->pins[bit], (level >> bit) & 1 );
> - }
> - s->prev_level = level;
> -}
> -
> -static void
> mst_fpga_set_irq(void *opaque, int irq, int level)
> {
> mst_irq_state *s = (mst_irq_state *)opaque;
> + uint32_t oldint = s->intsetclr;
>
> if (level)
> s->prev_level |= 1u << irq;
> else
> s->prev_level &= ~(1u << irq);
>
> - if(s->intmskena & (1u << irq)) {
> - s->intsetclr = 1u << irq;
> - qemu_set_irq(s->parent, level);
> - }
> + if ((s->intmskena & (1u << irq)) && level)
> + s->intsetclr |= 1u << irq;
> +
> + if (oldint != (s->intsetclr & s->intmskena))
> + qemu_set_irq(s->parent, s->intsetclr & s->intmskena);
Shouldn't this looks something like:
oldint = s->intsetclr & s->intmskena;
if (level)
s->intsetclr |= 1 << irq;
if (oldint != (s->intsetclr & s->intmskena))
qemu_set_irq(s->parent, s->intsetclr & s->intmskena);
I don't know this device but this is the usual outline.
> }
>
>
> @@ -146,10 +134,11 @@ mst_fpga_writeb(void *opaque, target_phys_addr_t addr,
> uint32_t value)
> break;
> case MST_INTMSKENA: /* Mask interupt */
> s->intmskena = (value & 0xFEEFF);
> - mst_fpga_update_gpio(s);
> + qemu_set_irq(s->parent, s->intsetclr & s->intmskena);
> break;
> case MST_INTSETCLR: /* clear or set interrupt */
> s->intsetclr = (value & 0xFEEFF);
> + qemu_set_irq(s->parent, s->intsetclr);
Shouldn't this also be masked?
Cheers
