From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33237) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TRXKh-0002p4-UC for qemu-devel@nongnu.org; Thu, 25 Oct 2012 19:59:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TRXKg-0006UN-Mu for qemu-devel@nongnu.org; Thu, 25 Oct 2012 19:59:27 -0400 Received: from mail-vc0-f173.google.com ([209.85.220.173]:63494) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TRXKg-0006UJ-I6 for qemu-devel@nongnu.org; Thu, 25 Oct 2012 19:59:26 -0400 Received: by mail-vc0-f173.google.com with SMTP id fl15so419067vcb.4 for ; Thu, 25 Oct 2012 16:59:26 -0700 (PDT) MIME-Version: 1.0 Sender: peter.crosthwaite@petalogix.com In-Reply-To: References: <50892CB0.6020706@redhat.com> <50893B33.5000908@redhat.com> <50894360.906@redhat.com> <5089485E.5090004@redhat.com> Date: Fri, 26 Oct 2012 09:59:25 +1000 Message-ID: From: Peter Crosthwaite Content-Type: text/plain; charset=ISO-8859-1 Subject: Re: [Qemu-devel] [PATCH v1 5/8] xilinx_zynq: add USB controllers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: vineshp@xilinx.com, Peter Maydell , john.williams@xilinx.com, qemu-devel@nongnu.org, edgar.iglesias@gmail.com On Fri, Oct 26, 2012 at 9:54 AM, Peter Crosthwaite wrote: > Copying patch inline to make some comments on it > > diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c > index b6b972f..64442a4 100644 > --- a/hw/usb/hcd-uhci.c > +++ b/hw/usb/hcd-uhci.c > @@ -88,6 +88,13 @@ enum { > typedef struct UHCIState UHCIState; > typedef struct UHCIAsync UHCIAsync; > typedef struct UHCIQueue UHCIQueue; > +typedef struct UHCIInfo UHCIInfo; > + > +struct UHCIInfo { > + uint16_t vendor_id; > + uint16_t device_id; > + uint8_t revision; > +}; > Theres nothing PCI specific here. Should this be defined somewhere in the PCI layers so PCI classes. First thing I had to do for EHCI equivalent was copy paste this s/UHCI/EHCI with no other change. > /* > * Pending async transaction. > @@ -1293,17 +1300,18 @@ static Property uhci_properties[] = { > DEFINE_PROP_END_OF_LIST(), > }; > > -static void piix3_uhci_class_init(ObjectClass *klass, void *data) > +static void uhci_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > + UHCIInfo *info = data; > > k->init = usb_uhci_common_initfn; > k->exit = usb_uhci_exit; > - k->vendor_id = PCI_VENDOR_ID_INTEL; > - k->device_id = PCI_DEVICE_ID_INTEL_82371SB_2; > - k->revision = 0x01; > - k->class_id = PCI_CLASS_SERIAL_USB; > + k->vendor_id = info->vendor_id; > + k->device_id = info->device_id; > + k->revision = info->revision; > + k->class_id = PCI_CLASS_SERIAL_USB; > dc->vmsd = &vmstate_uhci; > dc->props = uhci_properties; > } > @@ -1312,29 +1320,24 @@ static TypeInfo piix3_uhci_info = { > .name = "piix3-usb-uhci", > .parent = TYPE_PCI_DEVICE, > .instance_size = sizeof(UHCIState), > - .class_init = piix3_uhci_class_init, > + .class_init = uhci_class_init, Therese three elements (parent, instance_size, class_init) are repeated from one definition to the next. This will get tedious when we eventually come to make the same fix for some of the other devices that have large numbers of variants (pflash_cfi0x and m25p80 being some of the angrier ones). Regards, Peter > + .class_data = (UHCIInfo[]) { { > + .vendor_id = PCI_VENDOR_ID_INTEL, > + .device_id = PCI_DEVICE_ID_INTEL_82371SB_2, > + .revision = 0x01, > + } }, > }; > > -static void piix4_uhci_class_init(ObjectClass *klass, void *data) > -{ > - DeviceClass *dc = DEVICE_CLASS(klass); > - PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); > - > - k->init = usb_uhci_common_initfn; > - k->exit = usb_uhci_exit; > - k->vendor_id = PCI_VENDOR_ID_INTEL; > - k->device_id = PCI_DEVICE_ID_INTEL_82371AB_2; > - k->revision = 0x01; > - k->class_id = PCI_CLASS_SERIAL_USB; > - dc->vmsd = &vmstate_uhci; > - dc->props = uhci_properties; > -} > - > static TypeInfo piix4_uhci_info = { > .name = "piix4-usb-uhci", > .parent = TYPE_PCI_DEVICE, > .instance_size = sizeof(UHCIState), > - .class_init = piix4_uhci_class_init, > + .class_init = uhci_class_init, > + .class_data = (UHCIInfo[]) { { > + .vendor_id = PCI_VENDOR_ID_INTEL, > + .device_id = PCI_DEVICE_ID_INTEL_82371AB_2, > + .revision = 0x01, > + } }, > }; > > static void vt82c686b_uhci_class_init(ObjectClass *klass, void *data) > > On Fri, Oct 26, 2012 at 12:10 AM, Gerd Hoffmann wrote: >> Hi, >> >>> I'll go try that to simplify uhci ... >> >> Seems to work ... >> >> cheers, >> Gerd >> >>