Hi Peter,
> From: Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com]
> Sent: Thursday, 28 January 2016 20:38
>
> On Fri, Jan 15, 2016 at 3:58 PM, Andrew Baumann
> <andrew.baum...@microsoft.com> wrote:
> > This module is specific to the bcm2836 (Pi2). It implements the top
> > level interrupt controller, and mailboxes used for inter-processor
> > synchronisation.
[...]
> > + for (i = 0; i < BCM2836_NCORES; i++) {
> > + /* handle local timer interrupts for this core */
> > + if (s->timerirqs[i]) {
> > + assert(s->timerirqs[i] < (1 << IRQ_MAILBOX0)); /* sanity check
> > */
> > + for (j = 0; j < IRQ_MAILBOX0; j++) {
>
> I think <= IRQ_CNTVIRQ is cleaner, as it keeps "MAILBOX" out of the timer
> code.
Ok.
[...]
> > +typedef struct BCM2836ControlState {
> > + /*< private >*/
> > + SysBusDevice busdev;
> > + /*< public >*/
> > + MemoryRegion iomem;
> > +
>
> > + /* interrupt status registers (not directly visible to user) */
> > + bool gpu_irq, gpu_fiq;
> > + uint8_t timerirqs[BCM2836_NCORES];
> > +
>
> This ...
>
> > + /* mailboxes */
> > + uint32_t mailboxes[BCM2836_NCORES * BCM2836_MBPERCORE];
> > +
> > + /* interrupt routing/control registers */
> > + uint8_t route_gpu_irq, route_gpu_fiq;
> > + uint32_t timercontrol[BCM2836_NCORES];
> > + uint32_t mailboxcontrol[BCM2836_NCORES];
> > +
>
> > + /* interrupt source registers, post-routing (visible) */
> > + uint32_t irqsrc[BCM2836_NCORES];
> > + uint32_t fiqsrc[BCM2836_NCORES];
> > +
>
> And this are absent from the VMSD, but after some thought they don't
> need to be as they are pure functions of the input pin state that is
> always refreshable from other state no? I would these together with a
> brief comment as to the above, and keep the migratable state (genuine
> device state) all together.
Yes, that was exactly the intention. I'll comment/revise as you suggest.
> Reviewed-by: Peter Crosthwaite <crosthwaite.pe...@gmail.com>
Thanks for the review,
Andrew
