From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42291) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Utaxp-00063q-Ot for qemu-devel@nongnu.org; Mon, 01 Jul 2013 06:04:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Utaxn-0003Jn-Sh for qemu-devel@nongnu.org; Mon, 01 Jul 2013 06:04:05 -0400 Received: from cantor2.suse.de ([195.135.220.15]:49946 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Utaxn-0003JK-I6 for qemu-devel@nongnu.org; Mon, 01 Jul 2013 06:04:03 -0400 Message-ID: <51D1540D.7060009@suse.de> Date: Mon, 01 Jul 2013 12:03:57 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <51CFEA74.4020600@suse.de> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 08/30] ide/ich: QOM Upcast Sweep List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: pbonzini@redhat.com, peter.crosthwaite@xilinx.com, qemu-devel@nongnu.org, "Michael S. Tsirkin" Am 01.07.2013 01:36, schrieb Alexander Graf: >=20 > On 30.06.2013, at 10:21, Andreas F=E4rber wrote: >=20 >> Am 24.06.2013 08:55, schrieb peter.crosthwaite@xilinx.com: >>> From: Peter Crosthwaite >>> >>> Define and use standard QOM cast macro. Remove usages of DO_UPCAST >>> and direct -> style upcasting. >>> >>> Signed-off-by: Peter Crosthwaite >>> --- >>> >>> 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. :) >=20 > No, please. ICH9 is a controller hub. This device really is only about = the AHCI part of it. >=20 >> >>> + >>> +#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. >=20 > 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 =3D= { >>> >>> static void pci_ich9_reset(DeviceState *dev) >>> { >>> - struct AHCIPCIState *d =3D DO_UPCAST(struct AHCIPCIState, card.q= dev, dev); >>> + struct AHCIPCIState *d =3D 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 =3D DO_UPCAST(struct AHCIPCIState, card, dev); >>> + d =3D 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 =3D DO_UPCAST(struct AHCIPCIState, card, dev); >>> + d =3D ICH_AHCI(dev); >>> >>> msi_uninit(dev); >>> ahci_uninit(&d->ahci); >>> @@ -163,7 +163,7 @@ static void ich_ahci_class_init(ObjectClass *klas= s, void *data) >>> } >>> >>> static const TypeInfo ich_ahci_info =3D { >>> - .name =3D "ich9-ahci", >>> + .name =3D TYPE_ICH_AHCI, >>> .parent =3D TYPE_PCI_DEVICE, >>> .instance_size =3D sizeof(AHCIPCIState), >>> .class_init =3D ich_ahci_class_init, >>> >> >> >> --=20 >> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany >> GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCr= nberg >=20 --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg