[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Qemu-arm] [PATCH v2 2/3] hw/intc: Add (new) ASPEED AST
Re: [Qemu-devel] [Qemu-arm] [PATCH v2 2/3] hw/intc: Add (new) ASPEED AST2400 AVIC device model
Thu, 3 Mar 2016 08:39:17 +0000
On 3 March 2016 at 05:14, Andrew Jeffery <address@hidden> wrote:
> 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?
It was just that the register name didn't imply a set-bits-only
semantic and some of the other registers looked like they were
also incorrectly not handling updates right.