Message ID | e2526694ced29042c7202d54f345d6a1229dcb09.1372055322.git.peter.crosthwaite@xilinx.com |
---|---|
State | New |
Headers | show |
Am 24.06.2013 08:55, schrieb peter.crosthwaite@xilinx.com: > From: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > > Define and use standard QOM cast macro. Remove usages of DO_UPCAST > and direct -> style upcasting. > > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > --- > > hw/ide/ahci.h | 5 +++++ > hw/ide/ich.c | 10 +++++----- > 2 files changed, 10 insertions(+), 5 deletions(-) > > diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h > index 341a571..916bef0 100644 > --- a/hw/ide/ahci.h > +++ b/hw/ide/ahci.h > @@ -305,6 +305,11 @@ typedef struct AHCIPCIState { > AHCIState ahci; > } AHCIPCIState; > > +#define TYPE_ICH_AHCI "ich9-ahci" Let's be as precise as for the LSI SCSI HBA and name this ICH9. :) > + > +#define ICH_AHCI(obj) \ > + OBJECT_CHECK(AHCIPCIState, (obj), TYPE_ICH_AHCI) Wondering if this is specific to ICH(9)? Alex? Leaving it as is for now, renaming is an easy follow-up. > + > extern const VMStateDescription vmstate_ahci; > > #define VMSTATE_AHCI(_field, _state) { \ > diff --git a/hw/ide/ich.c b/hw/ide/ich.c > index 6c0c0c2..c3cbf2a 100644 > --- a/hw/ide/ich.c > +++ b/hw/ide/ich.c > @@ -92,7 +92,7 @@ static const VMStateDescription vmstate_ich9_ahci = { > > static void pci_ich9_reset(DeviceState *dev) > { > - struct AHCIPCIState *d = DO_UPCAST(struct AHCIPCIState, card.qdev, dev); > + struct AHCIPCIState *d = ICH_AHCI(dev); Let's drop the "struct" while touching the line. Thanks, applied to qom-next: https://github.com/afaerber/qemu-cpu/commits/qom-next Andreas > > ahci_reset(&d->ahci); > } > @@ -102,9 +102,9 @@ static int pci_ich9_ahci_init(PCIDevice *dev) > struct AHCIPCIState *d; > int sata_cap_offset; > uint8_t *sata_cap; > - d = DO_UPCAST(struct AHCIPCIState, card, dev); > + d = ICH_AHCI(dev); > > - ahci_init(&d->ahci, &dev->qdev, pci_get_address_space(dev), 6); > + ahci_init(&d->ahci, DEVICE(dev), pci_get_address_space(dev), 6); > > pci_config_set_prog_interface(d->card.config, AHCI_PROGMODE_MAJOR_REV_1); > > @@ -141,7 +141,7 @@ static int pci_ich9_ahci_init(PCIDevice *dev) > static void pci_ich9_uninit(PCIDevice *dev) > { > struct AHCIPCIState *d; > - d = DO_UPCAST(struct AHCIPCIState, card, dev); > + d = ICH_AHCI(dev); > > msi_uninit(dev); > ahci_uninit(&d->ahci); > @@ -163,7 +163,7 @@ static void ich_ahci_class_init(ObjectClass *klass, void *data) > } > > static const TypeInfo ich_ahci_info = { > - .name = "ich9-ahci", > + .name = TYPE_ICH_AHCI, > .parent = TYPE_PCI_DEVICE, > .instance_size = sizeof(AHCIPCIState), > .class_init = ich_ahci_class_init, >
On 30.06.2013, at 10:21, Andreas Färber wrote: > Am 24.06.2013 08:55, schrieb peter.crosthwaite@xilinx.com: >> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> >> Define and use standard QOM cast macro. Remove usages of DO_UPCAST >> and direct -> style upcasting. >> >> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> --- >> >> hw/ide/ahci.h | 5 +++++ >> hw/ide/ich.c | 10 +++++----- >> 2 files changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h >> index 341a571..916bef0 100644 >> --- a/hw/ide/ahci.h >> +++ b/hw/ide/ahci.h >> @@ -305,6 +305,11 @@ typedef struct AHCIPCIState { >> AHCIState ahci; >> } AHCIPCIState; >> >> +#define TYPE_ICH_AHCI "ich9-ahci" > > Let's be as precise as for the LSI SCSI HBA and name this ICH9. :) No, please. ICH9 is a controller hub. This device really is only about the AHCI part of it. > >> + >> +#define ICH_AHCI(obj) \ >> + OBJECT_CHECK(AHCIPCIState, (obj), TYPE_ICH_AHCI) > > Wondering if this is specific to ICH(9)? Alex? > Leaving it as is for now, renaming is an easy follow-up. This is not an ICH9 device. It's an ICH9-AHCI device. And for that the check looks sane from what I can tell. Alex > >> + >> extern const VMStateDescription vmstate_ahci; >> >> #define VMSTATE_AHCI(_field, _state) { \ >> diff --git a/hw/ide/ich.c b/hw/ide/ich.c >> index 6c0c0c2..c3cbf2a 100644 >> --- a/hw/ide/ich.c >> +++ b/hw/ide/ich.c >> @@ -92,7 +92,7 @@ static const VMStateDescription vmstate_ich9_ahci = { >> >> static void pci_ich9_reset(DeviceState *dev) >> { >> - struct AHCIPCIState *d = DO_UPCAST(struct AHCIPCIState, card.qdev, dev); >> + struct AHCIPCIState *d = ICH_AHCI(dev); > > Let's drop the "struct" while touching the line. > > Thanks, applied to qom-next: > https://github.com/afaerber/qemu-cpu/commits/qom-next > > Andreas > >> >> ahci_reset(&d->ahci); >> } >> @@ -102,9 +102,9 @@ static int pci_ich9_ahci_init(PCIDevice *dev) >> struct AHCIPCIState *d; >> int sata_cap_offset; >> uint8_t *sata_cap; >> - d = DO_UPCAST(struct AHCIPCIState, card, dev); >> + d = ICH_AHCI(dev); >> >> - ahci_init(&d->ahci, &dev->qdev, pci_get_address_space(dev), 6); >> + ahci_init(&d->ahci, DEVICE(dev), pci_get_address_space(dev), 6); >> >> pci_config_set_prog_interface(d->card.config, AHCI_PROGMODE_MAJOR_REV_1); >> >> @@ -141,7 +141,7 @@ static int pci_ich9_ahci_init(PCIDevice *dev) >> static void pci_ich9_uninit(PCIDevice *dev) >> { >> struct AHCIPCIState *d; >> - d = DO_UPCAST(struct AHCIPCIState, card, dev); >> + d = ICH_AHCI(dev); >> >> msi_uninit(dev); >> ahci_uninit(&d->ahci); >> @@ -163,7 +163,7 @@ static void ich_ahci_class_init(ObjectClass *klass, void *data) >> } >> >> static const TypeInfo ich_ahci_info = { >> - .name = "ich9-ahci", >> + .name = TYPE_ICH_AHCI, >> .parent = TYPE_PCI_DEVICE, >> .instance_size = sizeof(AHCIPCIState), >> .class_init = ich_ahci_class_init, >> > > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Am 01.07.2013 01:36, schrieb Alexander Graf: > > On 30.06.2013, at 10:21, Andreas Färber wrote: > >> Am 24.06.2013 08:55, schrieb peter.crosthwaite@xilinx.com: >>> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >>> >>> Define and use standard QOM cast macro. Remove usages of DO_UPCAST >>> and direct -> style upcasting. >>> >>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >>> --- >>> >>> hw/ide/ahci.h | 5 +++++ >>> hw/ide/ich.c | 10 +++++----- >>> 2 files changed, 10 insertions(+), 5 deletions(-) >>> >>> diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h >>> index 341a571..916bef0 100644 >>> --- a/hw/ide/ahci.h >>> +++ b/hw/ide/ahci.h >>> @@ -305,6 +305,11 @@ typedef struct AHCIPCIState { >>> AHCIState ahci; >>> } AHCIPCIState; >>> >>> +#define TYPE_ICH_AHCI "ich9-ahci" >> >> Let's be as precise as for the LSI SCSI HBA and name this ICH9. :) > > No, please. ICH9 is a controller hub. This device really is only about the AHCI part of it. > >> >>> + >>> +#define ICH_AHCI(obj) \ >>> + OBJECT_CHECK(AHCIPCIState, (obj), TYPE_ICH_AHCI) >> >> Wondering if this is specific to ICH(9)? Alex? >> Leaving it as is for now, renaming is an easy follow-up. > > This is not an ICH9 device. It's an ICH9-AHCI device. And for that the check looks sane from what I can tell. I don't see how either of your answers relate to my question? Maybe you were just tired? ;) So as you can see above, the type is called ich9-ahci, therefore I have done s/TYPE_ICH_AHCI/TYPE_ICH9_AHCI/g, and I still don't see anything wrong with it, Peter just ack'ed it. There's a link to the modified patch below to verify. What does a controller hub vs. AHCI have to do with ICH vs. ICH9? The implied question really is, how specific is AHCIPCIState to ich9-ahci (its sole user AFAICS)? Would an ich6-ahci or ich10-ahci use the same struct or not? Is it even specific to ICH? If not, we might want to name the cast macro after the struct PCI_AHCI() rather than ICH_AHCI(). Compare EHCI where we have a base struct and one derived class having its own extended state/class structs. Andreas >>> + >>> extern const VMStateDescription vmstate_ahci; >>> >>> #define VMSTATE_AHCI(_field, _state) { \ >>> diff --git a/hw/ide/ich.c b/hw/ide/ich.c >>> index 6c0c0c2..c3cbf2a 100644 >>> --- a/hw/ide/ich.c >>> +++ b/hw/ide/ich.c >>> @@ -92,7 +92,7 @@ static const VMStateDescription vmstate_ich9_ahci = { >>> >>> static void pci_ich9_reset(DeviceState *dev) >>> { >>> - struct AHCIPCIState *d = DO_UPCAST(struct AHCIPCIState, card.qdev, dev); >>> + struct AHCIPCIState *d = ICH_AHCI(dev); >> >> Let's drop the "struct" while touching the line. >> >> Thanks, applied to qom-next: >> https://github.com/afaerber/qemu-cpu/commits/qom-next >> >> Andreas >> >>> >>> ahci_reset(&d->ahci); >>> } >>> @@ -102,9 +102,9 @@ static int pci_ich9_ahci_init(PCIDevice *dev) >>> struct AHCIPCIState *d; >>> int sata_cap_offset; >>> uint8_t *sata_cap; >>> - d = DO_UPCAST(struct AHCIPCIState, card, dev); >>> + d = ICH_AHCI(dev); >>> >>> - ahci_init(&d->ahci, &dev->qdev, pci_get_address_space(dev), 6); >>> + ahci_init(&d->ahci, DEVICE(dev), pci_get_address_space(dev), 6); >>> >>> pci_config_set_prog_interface(d->card.config, AHCI_PROGMODE_MAJOR_REV_1); >>> >>> @@ -141,7 +141,7 @@ static int pci_ich9_ahci_init(PCIDevice *dev) >>> static void pci_ich9_uninit(PCIDevice *dev) >>> { >>> struct AHCIPCIState *d; >>> - d = DO_UPCAST(struct AHCIPCIState, card, dev); >>> + d = ICH_AHCI(dev); >>> >>> msi_uninit(dev); >>> ahci_uninit(&d->ahci); >>> @@ -163,7 +163,7 @@ static void ich_ahci_class_init(ObjectClass *klass, void *data) >>> } >>> >>> static const TypeInfo ich_ahci_info = { >>> - .name = "ich9-ahci", >>> + .name = TYPE_ICH_AHCI, >>> .parent = TYPE_PCI_DEVICE, >>> .instance_size = sizeof(AHCIPCIState), >>> .class_init = ich_ahci_class_init, >>> >> >> >> -- >> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany >> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >
On 01.07.2013, at 12:03, Andreas Färber wrote: > Am 01.07.2013 01:36, schrieb Alexander Graf: >> >> On 30.06.2013, at 10:21, Andreas Färber wrote: >> >>> Am 24.06.2013 08:55, schrieb peter.crosthwaite@xilinx.com: >>>> From: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >>>> >>>> Define and use standard QOM cast macro. Remove usages of DO_UPCAST >>>> and direct -> style upcasting. >>>> >>>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >>>> --- >>>> >>>> hw/ide/ahci.h | 5 +++++ >>>> hw/ide/ich.c | 10 +++++----- >>>> 2 files changed, 10 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h >>>> index 341a571..916bef0 100644 >>>> --- a/hw/ide/ahci.h >>>> +++ b/hw/ide/ahci.h >>>> @@ -305,6 +305,11 @@ typedef struct AHCIPCIState { >>>> AHCIState ahci; >>>> } AHCIPCIState; >>>> >>>> +#define TYPE_ICH_AHCI "ich9-ahci" >>> >>> Let's be as precise as for the LSI SCSI HBA and name this ICH9. :) >> >> No, please. ICH9 is a controller hub. This device really is only about the AHCI part of it. >> >>> >>>> + >>>> +#define ICH_AHCI(obj) \ >>>> + OBJECT_CHECK(AHCIPCIState, (obj), TYPE_ICH_AHCI) >>> >>> Wondering if this is specific to ICH(9)? Alex? >>> Leaving it as is for now, renaming is an easy follow-up. >> >> This is not an ICH9 device. It's an ICH9-AHCI device. And for that the check looks sane from what I can tell. > > I don't see how either of your answers relate to my question? Maybe you > were just tired? ;) Ah, sorry. I thought you wanted to just name it "ICH" instead of "ich9-ahci". > > So as you can see above, the type is called ich9-ahci, therefore I have > done s/TYPE_ICH_AHCI/TYPE_ICH9_AHCI/g, and I still don't see anything > wrong with it, Peter just ack'ed it. There's a link to the modified > patch below to verify. > What does a controller hub vs. AHCI have to do with ICH vs. ICH9? > > The implied question really is, how specific is AHCIPCIState to > ich9-ahci (its sole user AFAICS)? Would an ich6-ahci or ich10-ahci use > the same struct or not? Is it even specific to ICH? If not, we might > want to name the cast macro after the struct PCI_AHCI() rather than > ICH_AHCI(). Compare EHCI where we have a base struct and one derived > class having its own extended state/class structs. ich9-ahci is-a pci-ahci is-a ahci makes sense I would say. Different controllers can have different numbers of connectors, but apart from that they should look pretty similar. Alex
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h index 341a571..916bef0 100644 --- a/hw/ide/ahci.h +++ b/hw/ide/ahci.h @@ -305,6 +305,11 @@ typedef struct AHCIPCIState { AHCIState ahci; } AHCIPCIState; +#define TYPE_ICH_AHCI "ich9-ahci" + +#define ICH_AHCI(obj) \ + OBJECT_CHECK(AHCIPCIState, (obj), TYPE_ICH_AHCI) + extern const VMStateDescription vmstate_ahci; #define VMSTATE_AHCI(_field, _state) { \ diff --git a/hw/ide/ich.c b/hw/ide/ich.c index 6c0c0c2..c3cbf2a 100644 --- a/hw/ide/ich.c +++ b/hw/ide/ich.c @@ -92,7 +92,7 @@ static const VMStateDescription vmstate_ich9_ahci = { static void pci_ich9_reset(DeviceState *dev) { - struct AHCIPCIState *d = DO_UPCAST(struct AHCIPCIState, card.qdev, dev); + struct AHCIPCIState *d = ICH_AHCI(dev); ahci_reset(&d->ahci); } @@ -102,9 +102,9 @@ static int pci_ich9_ahci_init(PCIDevice *dev) struct AHCIPCIState *d; int sata_cap_offset; uint8_t *sata_cap; - d = DO_UPCAST(struct AHCIPCIState, card, dev); + d = ICH_AHCI(dev); - ahci_init(&d->ahci, &dev->qdev, pci_get_address_space(dev), 6); + ahci_init(&d->ahci, DEVICE(dev), pci_get_address_space(dev), 6); pci_config_set_prog_interface(d->card.config, AHCI_PROGMODE_MAJOR_REV_1); @@ -141,7 +141,7 @@ static int pci_ich9_ahci_init(PCIDevice *dev) static void pci_ich9_uninit(PCIDevice *dev) { struct AHCIPCIState *d; - d = DO_UPCAST(struct AHCIPCIState, card, dev); + d = ICH_AHCI(dev); msi_uninit(dev); ahci_uninit(&d->ahci); @@ -163,7 +163,7 @@ static void ich_ahci_class_init(ObjectClass *klass, void *data) } static const TypeInfo ich_ahci_info = { - .name = "ich9-ahci", + .name = TYPE_ICH_AHCI, .parent = TYPE_PCI_DEVICE, .instance_size = sizeof(AHCIPCIState), .class_init = ich_ahci_class_init,