diff mbox

[v1,3/8] usb/ehci: seperate out PCIisms

Message ID 6a52b259a18cf580984ab4263322b60a4c4877a0.1351157627.git.peter.crosthwaite@xilinx.com
State New
Headers show

Commit Message

Peter Crosthwaite Oct. 25, 2012, 9:47 a.m. UTC 
Seperate the PCI stuff from the EHCI components. Extracted the PCIDevice
out into a new wrapper struct to make EHCIState non-PCI-specific. Seperated
tho non PCI init component out into a seperate "common" init function.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---

 hw/usb/hcd-ehci.c |  132 ++++++++++++++++++++++++++++++----------------------
 1 files changed, 76 insertions(+), 56 deletions(-)

Comments

Gerd Hoffmann Oct. 25, 2012, 12:08 p.m. UTC | #1 
> +typedef struct EHCIItfState {
> +    PCIDevice pcidev;
> +    struct EHCIState ehci;
> +} EHCIItfState;

EHCIPCIState ?

>  static const VMStateDescription vmstate_ehci = {
>      .name        = "ehci",
> -    .version_id  = 2,
> -    .minimum_version_id  = 1,
> +    .version_id  = 3,
> +    .minimum_version_id  = 2,

Pick a new name for this ...

> -static void ehci_class_init(ObjectClass *klass, void *data)
> +static const VMStateDescription vmstate_ehci_pci = {
> +    .name        = "ehci-pci",

... and keep using "ehci" here ...

> +    .version_id  = 3,
> +    .minimum_version_id  = 2,

... then you don't need to fiddle with the versions as the vmstate wire
format doesn't change then.

cheers,
  Gerd
Peter Crosthwaite Oct. 25, 2012, 12:44 p.m. UTC | #2 
On Thu, Oct 25, 2012 at 10:08 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> +typedef struct EHCIItfState {
>> +    PCIDevice pcidev;
>> +    struct EHCIState ehci;
>> +} EHCIItfState;
>
> EHCIPCIState ?
>
>>  static const VMStateDescription vmstate_ehci = {
>>      .name        = "ehci",
>> -    .version_id  = 2,
>> -    .minimum_version_id  = 1,
>> +    .version_id  = 3,
>> +    .minimum_version_id  = 2,
>
> Pick a new name for this ...
>
>> -static void ehci_class_init(ObjectClass *klass, void *data)
>> +static const VMStateDescription vmstate_ehci_pci = {
>> +    .name        = "ehci-pci",
>
> ... and keep using "ehci" here ...
>
>> +    .version_id  = 3,
>> +    .minimum_version_id  = 2,
>
> ... then you don't need to fiddle with the versions as the vmstate wire
> format doesn't change then.
>

Does that work considering you have turned one layer of VMSD into two?
Can it equivocate machines saved with the old all-in-one vmsd with
this new one that is structured in two layers?

Regards,
Peter

> cheers,
>   Gerd
>
Gerd Hoffmann Oct. 25, 2012, 12:57 p.m. UTC | #3 
Hi,

>> ... then you don't need to fiddle with the versions as the vmstate wire
>> format doesn't change then.
>>
> 
> Does that work considering you have turned one layer of VMSD into two?
> Can it equivocate machines saved with the old all-in-one vmsd with
> this new one that is structured in two layers?

I'm pretty sure it does as only for top-level vmsd name and version are
stuffed into the wire bytestream.  For sub-vmsds only the actual fields
are written.  So moving stuff from the toplevel vmsd into another one
which then gets referenced via VMSTATE_STRUCT doesn't change the format.

cheers,
  Gerd
Peter Crosthwaite Oct. 25, 2012, 1:19 p.m. UTC | #4 
On Thu, Oct 25, 2012 at 10:57 PM, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
>>> ... then you don't need to fiddle with the versions as the vmstate wire
>>> format doesn't change then.
>>>
>>
>> Does that work considering you have turned one layer of VMSD into two?
>> Can it equivocate machines saved with the old all-in-one vmsd with
>> this new one that is structured in two layers?
>
> I'm pretty sure it does as only for top-level vmsd name and version are
> stuffed into the wire bytestream.  For sub-vmsds only the actual fields
> are written.  So moving stuff from the toplevel vmsd into another one
> which then gets referenced via VMSTATE_STRUCT doesn't change the format.

Nice,

Will refactor as you proposed v2 and keep backwards compat.

Regards,
Peter

>
> cheers,
>   Gerd
>
>
diff mbox

Patch

diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 53bc4e5..862564c 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -385,7 +385,6 @@  struct EHCIQueue {
 typedef QTAILQ_HEAD(EHCIQueueHead, EHCIQueue) EHCIQueueHead;
 
 struct EHCIState {
-    PCIDevice dev;
     USBBus bus;
     qemu_irq irq;
     MemoryRegion mem;
@@ -446,6 +445,11 @@  struct EHCIState {
     uint32_t async_stepdown;
 };
 
+typedef struct EHCIItfState {
+    PCIDevice pcidev;
+    struct EHCIState ehci;
+} EHCIItfState;
+
 #define SET_LAST_RUN_CLOCK(s) \
     (s)->last_run_ns = qemu_get_clock_ns(vm_clock);
 
@@ -2539,7 +2543,7 @@  static const MemoryRegionOps ehci_mmio_port_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
-static int usb_ehci_initfn(PCIDevice *dev);
+static int usb_ehci_pci_initfn(PCIDevice *dev);
 
 static USBPortOps ehci_port_ops = {
     .attach = ehci_attach,
@@ -2601,11 +2605,10 @@  static void usb_ehci_vm_state_change(void *opaque, int running, RunState state)
 
 static const VMStateDescription vmstate_ehci = {
     .name        = "ehci",
-    .version_id  = 2,
-    .minimum_version_id  = 1,
+    .version_id  = 3,
+    .minimum_version_id  = 2,
     .post_load   = usb_ehci_post_load,
     .fields      = (VMStateField[]) {
-        VMSTATE_PCI_DEVICE(dev, EHCIState),
         /* mmio registers */
         VMSTATE_UINT32(usbcmd, EHCIState),
         VMSTATE_UINT32(usbsts, EHCIState),
@@ -2637,31 +2640,42 @@  static const VMStateDescription vmstate_ehci = {
 };
 
 static Property ehci_properties[] = {
-    DEFINE_PROP_UINT32("maxframes", EHCIState, maxframes, 128),
-    DEFINE_PROP_UINT16("capabase", EHCIState, capabase, 0),
-    DEFINE_PROP_UINT16("opregbase", EHCIState, opregbase, 0x20),
+    DEFINE_PROP_UINT32("maxframes", EHCIItfState, ehci.maxframes, 128),
+    DEFINE_PROP_UINT16("capabase", EHCIItfState, ehci.capabase, 0),
+    DEFINE_PROP_UINT16("opregbase", EHCIItfState, ehci.opregbase, 0x20),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-static void ehci_class_init(ObjectClass *klass, void *data)
+static const VMStateDescription vmstate_ehci_pci = {
+    .name        = "ehci-pci",
+    .version_id  = 3,
+    .minimum_version_id  = 2,
+    .post_load   = usb_ehci_post_load,
+    .fields      = (VMStateField[]) {
+        VMSTATE_PCI_DEVICE(pcidev, EHCIItfState),
+        VMSTATE_STRUCT(ehci, EHCIItfState, 3, vmstate_ehci, EHCIState),
+    }
+};
+
+static void ehci_pci_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = usb_ehci_initfn;
+    k->init = usb_ehci_pci_initfn;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82801D; /* ich4 */
     k->revision = 0x10;
     k->class_id = PCI_CLASS_SERIAL_USB;
-    dc->vmsd = &vmstate_ehci;
+    dc->vmsd = &vmstate_ehci_pci;
     dc->props = ehci_properties;
 }
 
-static TypeInfo ehci_info = {
+static TypeInfo ehci_pci_info = {
     .name          = "usb-ehci",
     .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(EHCIState),
-    .class_init    = ehci_class_init,
+    .instance_size = sizeof(EHCIItfState),
+    .class_init    = ehci_pci_class_init,
 };
 
 static void ich9_ehci_class_init(ObjectClass *klass, void *data)
@@ -2669,57 +2683,27 @@  static void ich9_ehci_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = usb_ehci_initfn;
+    k->init = usb_ehci_pci_initfn;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->device_id = PCI_DEVICE_ID_INTEL_82801I_EHCI1;
     k->revision = 0x03;
     k->class_id = PCI_CLASS_SERIAL_USB;
-    dc->vmsd = &vmstate_ehci;
+    dc->vmsd = &vmstate_ehci_pci;
     dc->props = ehci_properties;
 }
 
 static TypeInfo ich9_ehci_info = {
     .name          = "ich9-usb-ehci1",
     .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(EHCIState),
+    .instance_size = sizeof(EHCIItfState),
     .class_init    = ich9_ehci_class_init,
 };
 
-static int usb_ehci_initfn(PCIDevice *dev)
+
+static void usb_ehci_initfn(EHCIState *s, DeviceState *dev)
 {
-    EHCIState *s = DO_UPCAST(EHCIState, dev, dev);
-    uint8_t *pci_conf = s->dev.config;
     int i;
 
-    pci_set_byte(&pci_conf[PCI_CLASS_PROG], 0x20);
-
-    /* capabilities pointer */
-    pci_set_byte(&pci_conf[PCI_CAPABILITY_LIST], 0x00);
-    //pci_set_byte(&pci_conf[PCI_CAPABILITY_LIST], 0x50);
-
-    pci_set_byte(&pci_conf[PCI_INTERRUPT_PIN], 4); /* interrupt pin D */
-    pci_set_byte(&pci_conf[PCI_MIN_GNT], 0);
-    pci_set_byte(&pci_conf[PCI_MAX_LAT], 0);
-
-    // pci_conf[0x50] = 0x01; // power management caps
-
-    pci_set_byte(&pci_conf[USB_SBRN], USB_RELEASE_2); // release number (2.1.4)
-    pci_set_byte(&pci_conf[0x61], 0x20);  // frame length adjustment (2.1.5)
-    pci_set_word(&pci_conf[0x62], 0x00);  // port wake up capability (2.1.6)
-
-    pci_conf[0x64] = 0x00;
-    pci_conf[0x65] = 0x00;
-    pci_conf[0x66] = 0x00;
-    pci_conf[0x67] = 0x00;
-    pci_conf[0x68] = 0x01;
-    pci_conf[0x69] = 0x00;
-    pci_conf[0x6a] = 0x00;
-    pci_conf[0x6b] = 0x00;  // USBLEGSUP
-    pci_conf[0x6c] = 0x00;
-    pci_conf[0x6d] = 0x00;
-    pci_conf[0x6e] = 0x00;
-    pci_conf[0x6f] = 0xc0;  // USBLEFCTLSTS
-
     s->caps = g_malloc0(s->opregbase);
 
     /* 2.2 host controller interface version */
@@ -2736,11 +2720,7 @@  static int usb_ehci_initfn(PCIDevice *dev)
     s->caps[0x0a] = 0x00;
     s->caps[0x0b] = 0x00;
 
-    s->irq = s->dev.irq[3];
-
-    s->dma = pci_dma_context(dev);
-
-    usb_bus_new(&s->bus, &ehci_bus_ops, &s->dev.qdev);
+    usb_bus_new(&s->bus, &ehci_bus_ops, dev);
     for(i = 0; i < NB_PORTS; i++) {
         usb_register_port(&s->bus, &s->ports[i], s, i, &ehci_port_ops,
                           USB_SPEED_MASK_HIGH);
@@ -2768,15 +2748,55 @@  static int usb_ehci_initfn(PCIDevice *dev)
     memory_region_add_subregion(&s->mem, s->opregbase, &s->mem_opreg);
     memory_region_add_subregion(&s->mem, s->opregbase + PORTSC_BEGIN,
                                 &s->mem_ports);
+}
+
+static int usb_ehci_pci_initfn(PCIDevice *dev)
+{
+    EHCIItfState *i = DO_UPCAST(EHCIItfState, pcidev, dev);
+    EHCIState *s = &i->ehci;
+    uint8_t *pci_conf = dev->config;
+
+    pci_set_byte(&pci_conf[PCI_CLASS_PROG], 0x20);
+
+    /* capabilities pointer */
+    pci_set_byte(&pci_conf[PCI_CAPABILITY_LIST], 0x00);
+    /* pci_set_byte(&pci_conf[PCI_CAPABILITY_LIST], 0x50); */
+
+    pci_set_byte(&pci_conf[PCI_INTERRUPT_PIN], 4); /* interrupt pin D */
+    pci_set_byte(&pci_conf[PCI_MIN_GNT], 0);
+    pci_set_byte(&pci_conf[PCI_MAX_LAT], 0);
+
+    /* pci_conf[0x50] = 0x01; *//* power management caps */
+
+    pci_set_byte(&pci_conf[USB_SBRN], USB_RELEASE_2); /* release # (2.1.4) */
+    pci_set_byte(&pci_conf[0x61], 0x20);  /* frame length adjustment (2.1.5) */
+    pci_set_word(&pci_conf[0x62], 0x00);  /* port wake up capability (2.1.6) */
+
+    pci_conf[0x64] = 0x00;
+    pci_conf[0x65] = 0x00;
+    pci_conf[0x66] = 0x00;
+    pci_conf[0x67] = 0x00;
+    pci_conf[0x68] = 0x01;
+    pci_conf[0x69] = 0x00;
+    pci_conf[0x6a] = 0x00;
+    pci_conf[0x6b] = 0x00;  /* USBLEGSUP */
+    pci_conf[0x6c] = 0x00;
+    pci_conf[0x6d] = 0x00;
+    pci_conf[0x6e] = 0x00;
+    pci_conf[0x6f] = 0xc0;  /* USBLEFCTLSTS */
+
+    s->irq = dev->irq[3];
+    s->dma = pci_dma_context(dev);
 
-    pci_register_bar(&s->dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mem);
+    usb_ehci_initfn(s, DEVICE(dev));
+    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mem);
 
     return 0;
 }
 
 static void ehci_register_types(void)
 {
-    type_register_static(&ehci_info);
+    type_register_static(&ehci_pci_info);
     type_register_static(&ich9_ehci_info);
 }