Re: [Qemu-devel] [Qemu-arm] [PATCH v2 2/3] hw/intc: Add (new) ASPEED AST2400 AVIC device model

[Andrew Jeffery]

On Thu, 2016-02-25 at 16:29 +0000, Peter Maydell wrote:
> > +    case 0x20: /* Interrupt Enable */
> > +        s->int_enable |= data;
> 
> Are you sure this only ORs in new 1 bits?

As in, am I sure I only want to take the newly set bits? If so, yes, as
the the following register serves to clear the field's set bits:

> 
> > +        break;
> > +    case 0x28: /* Interrupt Enable Clear */
> > +        s->int_enable &= ~data;
> > +        break;

The 'int_enable', 'int_trigger' and 'edge_status' fields all use the pa
ttern of separate set and clear registers (the remaining registers may
benefit from the extract64/deposit64 helpers, I'll think about that
further). I'll add some comments to help clear this up.

Otherwise, can you rephrase the question? At face value it seems like
you're implying that I'm doing more than ORing in the new 1 bits?

Andrew

signature.asc
Description: This is a digitally signed message part