Re: [Qemu-arm] [PATCH v4 8/8] raspi: add raspberry pi 2 machine

[Andrew Baumann]

> From: Peter Crosthwaite [mailto:address@hidden
> Sent: Friday, 29 January 2016 14:23
> 
> On Fri, Jan 29, 2016 at 1:50 PM, Andrew Baumann
> <address@hidden> wrote:
> > Hi Peter,
> >
> > Thanks for all the reviews. I should have a respun version on the list 
> > shortly.
> There's one minor change to this last patch:
> >
> >> From: Peter Crosthwaite [mailto:address@hidden
> >> Sent: Thursday, 28 January 2016 23:31
> >> > On Fri, Jan 15, 2016 at 3:58 PM, Andrew Baumann
> <address@hidden> wrote:
> >  [...]
> >> > +typedef struct RaspiState {
> >>
> >> A quick google search, I see the camel case form for rpi is usually
> >> "RasPi". Should we follow?
> >
> > Ok.
> >
> >> > +    union {
> >>
> >> union not needed.
> >
> > I know it's not needed now, but it will be as soon as we add pi1, which I
> hope to address in the next patch series. It will make that diff cleaner if we
> keep this here now, so I'm going to leave it as-is. I hope that's ok with you.
> >
> 
> It sounds like you are implementing an inheritance outside QOM. I'm
> not sure about this, can we just drop the union and figure it out on
> the next series?

It's nothing nearly that clever/complicated. See 
https://github.com/0xabu/qemu/blob/raspi/hw/arm/raspi.c#L116

    switch (version) {
    case 1:
        object_initialize(&s->soc.pi1, sizeof(s->soc.pi1), TYPE_BCM2835);
        break;
    case 2:
        object_initialize(&s->soc.pi2, sizeof(s->soc.pi2), TYPE_BCM2836);
        break;
    }

And then later, we always refer simply to OBJECT(&s->soc). I suppose I could 
use a generic Object pointer instead, and allocate the soc-specific type 
elsewhere, but it seemed simpler with the union.

Andrew