On 3 March 2016 at 05:14, Andrew Jeffery <and...@aj.id.au> 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. thanks -- PMM
