Patchwork [v2,1/4] usb/ehci: Clean up SysBus and PCI EHCI split

login
register
mail settings
Submitter Andreas Färber
Date Dec. 16, 2012, 3:49 a.m.
Message ID <1355629786-14649-2-git-send-email-andreas.faerber@web.de>
Download mbox | patch
Permalink /patch/206667/
State New
Headers show

Comments

Andreas Färber - Dec. 16, 2012, 3:49 a.m.
SysBus EHCI was introduced in a hurry before 1.3 Soft Freeze.
To use QOM casts in place of DO_UPCAST() / FROM_SYSBUS(), we need an
identifying type. Introduce generic abstract base types for PCI and
SysBus EHCI to allow multiple types to access the shared fields.

While at it, move the state structs being amended with macros to the
header file so that they can be embedded.

The VMSTATE_PCI_DEVICE() macro does not play nice with the QOM
parent_obj naming convention, so defer that cleanup.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/usb/hcd-ehci-pci.c    |   39 ++++++++++++++++++++++++---------------
 hw/usb/hcd-ehci-sysbus.c |   19 ++++++++++---------
 hw/usb/hcd-ehci.h        |   31 +++++++++++++++++++++++++++++++
 3 Dateien geändert, 65 Zeilen hinzugefügt(+), 24 Zeilen entfernt(-)
Gerd Hoffmann - Dec. 17, 2012, 1:29 p.m.
On 12/16/12 04:49, Andreas Färber wrote:
> SysBus EHCI was introduced in a hurry before 1.3 Soft Freeze.
> To use QOM casts in place of DO_UPCAST() / FROM_SYSBUS(), we need an
> identifying type. Introduce generic abstract base types for PCI and
> SysBus EHCI to allow multiple types to access the shared fields.
> 
> While at it, move the state structs being amended with macros to the
> header file so that they can be embedded.

--verbose please.

I fail to see the point.  EHCIPCIState should not be needed outside of
hcd-ehci-pci.c and I'd prefer to leave it there.  Likewise for sysbus.

cheers,
  Gerd

Patch

diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index 41dbb53..c3843f6 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -16,14 +16,8 @@ 
  */
 
 #include "hw/usb/hcd-ehci.h"
-#include "hw/pci.h"
 #include "range.h"
 
-typedef struct EHCIPCIState {
-    PCIDevice pcidev;
-    EHCIState ehci;
-} EHCIPCIState;
-
 typedef struct EHCIPCIInfo {
     const char *name;
     uint16_t vendor_id;
@@ -33,7 +27,7 @@  typedef struct EHCIPCIInfo {
 
 static int usb_ehci_pci_initfn(PCIDevice *dev)
 {
-    EHCIPCIState *i = DO_UPCAST(EHCIPCIState, pcidev, dev);
+    EHCIPCIState *i = PCI_EHCI(dev);
     EHCIState *s = &i->ehci;
     uint8_t *pci_conf = dev->config;
 
@@ -83,7 +77,7 @@  static int usb_ehci_pci_initfn(PCIDevice *dev)
 static void usb_ehci_pci_write_config(PCIDevice *dev, uint32_t addr,
                                       uint32_t val, int l)
 {
-    EHCIPCIState *i = DO_UPCAST(EHCIPCIState, pcidev, dev);
+    EHCIPCIState *i = PCI_EHCI(dev);
     bool busmaster;
 
     pci_default_write_config(dev, addr, val, l);
@@ -115,12 +109,8 @@  static void ehci_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-    EHCIPCIInfo *i = data;
 
     k->init = usb_ehci_pci_initfn;
-    k->vendor_id = i->vendor_id;
-    k->device_id = i->device_id;
-    k->revision = i->revision;
     k->class_id = PCI_CLASS_SERIAL_USB;
     k->config_write = usb_ehci_pci_write_config;
     k->no_hotplug = 1;
@@ -128,6 +118,24 @@  static void ehci_class_init(ObjectClass *klass, void *data)
     dc->props = ehci_pci_properties;
 }
 
+static const TypeInfo ehci_pci_type_info = {
+    .name = TYPE_PCI_EHCI,
+    .parent = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(EHCIPCIState),
+    .abstract = true,
+    .class_init = ehci_class_init,
+};
+
+static void ehci_data_class_init(ObjectClass *klass, void *data)
+{
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    EHCIPCIInfo *i = data;
+
+    k->vendor_id = i->vendor_id;
+    k->device_id = i->device_id;
+    k->revision = i->revision;
+}
+
 static struct EHCIPCIInfo ehci_pci_info[] = {
     {
         .name      = "usb-ehci",
@@ -150,12 +158,13 @@  static struct EHCIPCIInfo ehci_pci_info[] = {
 static void ehci_pci_register_types(void)
 {
     TypeInfo ehci_type_info = {
-        .parent        = TYPE_PCI_DEVICE,
-        .instance_size = sizeof(EHCIPCIState),
-        .class_init    = ehci_class_init,
+        .parent        = TYPE_PCI_EHCI,
+        .class_init    = ehci_data_class_init,
     };
     int i;
 
+    type_register_static(&ehci_pci_type_info);
+
     for (i = 0; i < ARRAY_SIZE(ehci_pci_info); i++) {
         ehci_type_info.name = ehci_pci_info[i].name;
         ehci_type_info.class_data = ehci_pci_info + i;
diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index 803df92..d431193 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -16,12 +16,6 @@ 
  */
 
 #include "hw/usb/hcd-ehci.h"
-#include "hw/sysbus.h"
-
-typedef struct EHCISysBusState {
-    SysBusDevice busdev;
-    EHCIState ehci;
-} EHCISysBusState;
 
 static const VMStateDescription vmstate_ehci_sysbus = {
     .name        = "ehci-sysbus",
@@ -40,7 +34,7 @@  static Property ehci_sysbus_properties[] = {
 
 static int usb_ehci_sysbus_initfn(SysBusDevice *dev)
 {
-    EHCISysBusState *i = FROM_SYSBUS(EHCISysBusState, dev);
+    EHCISysBusState *i = SYS_BUS_EHCI(dev);
     EHCIState *s = &i->ehci;
 
     s->capsbase = 0x100;
@@ -63,15 +57,22 @@  static void ehci_sysbus_class_init(ObjectClass *klass, void *data)
     dc->props = ehci_sysbus_properties;
 }
 
-TypeInfo ehci_xlnx_type_info = {
-    .name          = "xlnx,ps7-usb",
+static const TypeInfo ehci_type_info = {
+    .name          = TYPE_SYS_BUS_EHCI,
     .parent        = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(EHCISysBusState),
+    .abstract      = true,
     .class_init    = ehci_sysbus_class_init,
 };
 
+static const TypeInfo ehci_xlnx_type_info = {
+    .name          = "xlnx,ps7-usb",
+    .parent        = TYPE_SYS_BUS_EHCI,
+};
+
 static void ehci_sysbus_register_types(void)
 {
+    type_register_static(&ehci_type_info);
     type_register_static(&ehci_xlnx_type_info);
 }
 
diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index 772870b..9825b19 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -14,6 +14,8 @@ 
  * You should have received a copy of the GNU General Public License
  * along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
+#ifndef QEMU_USB_EHCI_H
+#define QEMU_USB_EHCI_H
 
 #include "hw/hw.h"
 #include "qemu-timer.h"
@@ -22,6 +24,8 @@ 
 #include "trace.h"
 #include "dma.h"
 #include "sysemu.h"
+#include "hw/pci.h"
+#include "hw/sysbus.h"
 
 #ifndef EHCI_DEBUG
 #define EHCI_DEBUG   0
@@ -318,3 +322,30 @@  struct EHCIState {
 extern const VMStateDescription vmstate_ehci;
 
 void usb_ehci_initfn(EHCIState *s, DeviceState *dev);
+
+
+#define TYPE_PCI_EHCI "pci-ehci-usb"
+#define PCI_EHCI(obj) OBJECT_CHECK(EHCIPCIState, (obj), TYPE_PCI_EHCI)
+
+typedef struct EHCIPCIState {
+    /*< private >*/
+    PCIDevice pcidev;
+    /*< public >*/
+
+    EHCIState ehci;
+} EHCIPCIState;
+
+
+#define TYPE_SYS_BUS_EHCI "sysbus-ehci-usb"
+#define SYS_BUS_EHCI(obj) \
+    OBJECT_CHECK(EHCISysBusState, (obj), TYPE_SYS_BUS_EHCI)
+
+typedef struct EHCISysBusState {
+    /*< private >*/
+    SysBusDevice parent_obj;
+    /*< public >*/
+
+    EHCIState ehci;
+} EHCISysBusState;
+
+#endif