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

Andrew Jeffery andrew at aj.id.au
Thu Mar 3 16:14:29 AEDT 2016


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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20160303/8f8a95a1/attachment-0001.sig>


More information about the openbmc mailing list