From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:53852) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TSlko-0008Re-QH for qemu-devel@nongnu.org; Mon, 29 Oct 2012 05:35:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TSlkk-0000yA-Dy for qemu-devel@nongnu.org; Mon, 29 Oct 2012 05:35:30 -0400 Received: from cantor2.suse.de ([195.135.220.15]:48556 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TSlkk-0000y3-2m for qemu-devel@nongnu.org; Mon, 29 Oct 2012 05:35:26 -0400 Message-ID: <508E4DD6.2030407@suse.de> Date: Mon, 29 Oct 2012 10:35:18 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 2/8] usb/ehci: Use class_data to init PCI variations List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Crosthwaite , Gerd Hoffmann , Anthony Liguori Cc: vineshp@xilinx.com, peter.maydell@linaro.org, Jason Baron , qemu-devel@nongnu.org, john.williams@xilinx.com, edgar.iglesias@gmail.com Am 29.10.2012 02:34, schrieb Peter Crosthwaite: > Got rid of the duplication of the class init functions for the two PCI = EHCI > variants. The PCI specifics are passed in as as class_data and set by a= common > class_init function. >=20 > Premeptively defined a new Class "EHCICLass" for the upcomming addition= of new "Preemptively", "upcoming" > fields. The class_data is an instance of EHCICLass that forms a templat= e for the > class to generate. Using "EHCI[PCI]Class" to template itself seems a bit awkward, Anthony do you have any thoughts on this? The usual way would be to have a dedicated EHCIInfo struct or so. >=20 > Signed-off-by: Peter Crosthwaite > --- > Got rid of union for sharing EHCIClassDefinition - made PCI specific > Simplified literal class_data arrays in ehci_info accordingly > removed null sentinel from ehci_info and used ARRAY_SIZE for type_regsi= ter loop > bound instead >=20 > hw/usb/hcd-ehci.c | 76 ++++++++++++++++++++++++++++-----------------= ------- > 1 files changed, 41 insertions(+), 35 deletions(-) >=20 > diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c > index 6c65a73..274225b 100644 > --- a/hw/usb/hcd-ehci.c > +++ b/hw/usb/hcd-ehci.c > @@ -2641,46 +2641,49 @@ static Property ehci_properties[] =3D { > DEFINE_PROP_END_OF_LIST(), > }; > =20 > -static void ehci_class_init(ObjectClass *klass, void *data) > -{ > - DeviceClass *dc =3D DEVICE_CLASS(klass); > - PCIDeviceClass *k =3D PCI_DEVICE_CLASS(klass); > - > - k->init =3D usb_ehci_initfn; > - k->vendor_id =3D PCI_VENDOR_ID_INTEL; > - k->device_id =3D PCI_DEVICE_ID_INTEL_82801D; /* ich4 */ > - k->revision =3D 0x10; > - k->class_id =3D PCI_CLASS_SERIAL_USB; > - dc->vmsd =3D &vmstate_ehci; > - dc->props =3D ehci_properties; > -} > +typedef struct EHCIPCIClass { > + PCIDeviceClass pci; > +} EHCIPCIClass; > =20 > -static TypeInfo ehci_info =3D { > - .name =3D "usb-ehci", > - .parent =3D TYPE_PCI_DEVICE, > - .instance_size =3D sizeof(EHCIState), > - .class_init =3D ehci_class_init, > -}; > - > -static void ich9_ehci_class_init(ObjectClass *klass, void *data) > +static void ehci_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc =3D DEVICE_CLASS(klass); > - PCIDeviceClass *k =3D PCI_DEVICE_CLASS(klass); > - > - k->init =3D usb_ehci_initfn; > - k->vendor_id =3D PCI_VENDOR_ID_INTEL; > - k->device_id =3D PCI_DEVICE_ID_INTEL_82801I_EHCI1; > - k->revision =3D 0x03; > - k->class_id =3D PCI_CLASS_SERIAL_USB; > + EHCIPCIClass *k =3D (EHCIPCIClass *)klass; Please use a proper QOM cast macro: EHCI_PCI_CLASS(klass) In this case however, please keep using PCIDeviceClass rather than trying to access it through a named member. If we need to access any dedicated EHCIPCIClass fields later in the series we can add additional variables the QOM way. > + EHCIPCIClass *template =3D data; > + > + k->pci.init =3D usb_ehci_initfn; > + k->pci.vendor_id =3D template->pci.vendor_id; > + k->pci.device_id =3D template->pci.device_id; /* ich4 */ > + k->pci.revision =3D template->pci.revision; > + k->pci.class_id =3D PCI_CLASS_SERIAL_USB; > dc->vmsd =3D &vmstate_ehci; > dc->props =3D ehci_properties; > } > =20 > -static TypeInfo ich9_ehci_info =3D { > - .name =3D "ich9-usb-ehci1", > - .parent =3D TYPE_PCI_DEVICE, > - .instance_size =3D sizeof(EHCIState), > - .class_init =3D ich9_ehci_class_init, > +static TypeInfo ehci_info[] =3D { Can this still be made const despite the embedded class_data? > + { > + .name =3D "usb-ehci", > + .parent =3D TYPE_PCI_DEVICE, > + .instance_size =3D sizeof(EHCIState), > + .class_init =3D ehci_class_init, > + .class_size =3D sizeof(EHCIPCIClass), > + .class_data =3D (EHCIPCIClass[]) {{ > + .pci.vendor_id =3D PCI_VENDOR_ID_INTEL, > + .pci.device_id =3D PCI_DEVICE_ID_INTEL_82801D, > + .pci.revision =3D 0x10, > + } } > + }, { > + .name =3D "ich9-usb-ehci1", Do we have a suitable header to introduce TYPE_* constants for these while at it? That would benefit q35. Andreas > + .parent =3D TYPE_PCI_DEVICE, > + .instance_size =3D sizeof(EHCIState), > + .class_init =3D ehci_class_init, > + .class_size =3D sizeof(EHCIPCIClass), > + .class_data =3D (EHCIPCIClass[]) {{ > + .pci.vendor_id =3D PCI_VENDOR_ID_INTEL, > + .pci.device_id =3D PCI_DEVICE_ID_INTEL_82801I_EHCI1, > + .pci.revision =3D 0x03, > + } } > + }, > }; > =20 > static int usb_ehci_initfn(PCIDevice *dev) > @@ -2769,8 +2772,11 @@ static int usb_ehci_initfn(PCIDevice *dev) > =20 > static void ehci_register_types(void) > { > - type_register_static(&ehci_info); > - type_register_static(&ich9_ehci_info); > + int i; > + > + for (i =3D 0; i < ARRAY_SIZE(ehci_info); i++) { > + type_register_static(&ehci_info[i]); > + } > } > =20 > type_init(ehci_register_types) >=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