[Qemu-devel] [PATCH RFT 0/5] usb: Clean up and extend SysBus EHCI

[Qemu-devel] [PATCH RFT 0/5] usb: Clean up and extend SysBus EHCI

[Andreas Färber]

Hello,

Some review comments for SysBus EHCI were ignored in favor of merging into 1.3.
As requested by Gerd, this now follows up with the cleanups I had requested
from Peter C. Apart from using the new-style QOM casts it also achieves some
deduplification. It then goes on to implement the suggested more slim way to
have different capsbase and opregbase values; the alternative would be to
create a new struct SysBusEHCIInfo for declarative initialization. Given that
there are only few models for now, the imperative approach allows for more
flexibility such as inheritence when, e.g., an Exynos 5 model varies only
slightly from Exynos 4.

Appended is Liming's patch to add an EHCI device to Exynos 4 as well as a
new patch of mine to prepare a Tegra 2 EHCI device (cf. my tegra branch) to
show how this works out.

Regards,
Andreas

Changes from Liming Wang's patchset:
* Introduce abstract sysbus-ehci-usb and pci-ehci-usb types
* Move class_init / instance_init to base type as far as possible
* Use TYPE_* constant for instantiating exynos4210 EHCI
* Added my tegra2 SysBus EHCI device to the mix

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Peter A. G. Crosthwaite <peter.crosthwaite@xilinx.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Igor Mitsyanko <i.mitsyanko@samsung.com>
Cc: Liming Wang <walimisdev@gmail.com>
Cc: Vincent Palatin <vpalatin@chromium.org>

Andreas Färber (4):
  usb/ehci: Clean up SysBus and PCI EHCI split
  usb/ehci: Move capsbase and opregbase into SysBus EHCI class
  usb/ehci: Add SysBus EHCI device for Exynos4210
  usb/ehci: Add Tegra2 SysBus EHCI device

Liming Wang (1):
  exynos4210: Add EHCI support

 hw/exynos4210.c          |    7 +++++
 hw/exynos4210_gic.c      |    2 +-
 hw/usb/hcd-ehci-pci.c    |   37 ++++++++++++++++------
 hw/usb/hcd-ehci-sysbus.c |   76 ++++++++++++++++++++++++++++++++++++++++++----
 hw/usb/hcd-ehci.h        |    2 ++
 5 Dateien geändert, 108 Zeilen hinzugefügt(+), 16 Zeilen entfernt(-)

-- 
1.7.10.4

[Qemu-devel] [PATCH RFT 1/5] usb/ehci: Clean up SysBus and PCI EHCI split

[Andreas Färber]

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.

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>
Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/usb/hcd-ehci-pci.c    |   37 ++++++++++++++++++++++++++++---------
 hw/usb/hcd-ehci-sysbus.c |   20 ++++++++++++++++----
 2 Dateien geändert, 44 Zeilen hinzugefügt(+), 13 Zeilen entfernt(-)

diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index 41dbb53..bb1a197 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -19,8 +19,12 @@
 #include "hw/pci.h"
 #include "range.h"
 
+#define TYPE_PCI_EHCI "pci-ehci-usb"
+#define PCI_EHCI(obj) OBJECT_CHECK(EHCIPCIState, (obj), TYPE_PCI_EHCI)
+
 typedef struct EHCIPCIState {
     PCIDevice pcidev;
+
     EHCIState ehci;
 } EHCIPCIState;
 
@@ -33,7 +37,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 +87,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 +119,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 +128,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 +168,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..c7b68b2 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -18,8 +18,13 @@
 #include "hw/usb/hcd-ehci.h"
 #include "hw/sysbus.h"
 
+#define TYPE_SYS_BUS_EHCI "sysbus-ehci-usb"
+#define SYS_BUS_EHCI(obj) \
+    OBJECT_CHECK(EHCISysBusState, (obj), TYPE_SYS_BUS_EHCI)
+
 typedef struct EHCISysBusState {
-    SysBusDevice busdev;
+    SysBusDevice parent_obj;
+
     EHCIState ehci;
 } EHCISysBusState;
 
@@ -40,7 +45,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 +68,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);
 }
 
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH RFT 1/5] usb/ehci: Clean up SysBus and PCI EHCI split

[Igor Mitsyanko]

On 12/02/2012 06:57 AM, 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.
>
> 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>
> Cc: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
>   hw/usb/hcd-ehci-pci.c    |   37 ++++++++++++++++++++++++++++---------
>   hw/usb/hcd-ehci-sysbus.c |   20 ++++++++++++++++----
>   2 Dateien geändert, 44 Zeilen hinzugefügt(+), 13 Zeilen entfernt(-)
>
> diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
> index 41dbb53..bb1a197 100644
> --- a/hw/usb/hcd-ehci-pci.c
> +++ b/hw/usb/hcd-ehci-pci.c
> @@ -19,8 +19,12 @@
>   #include "hw/pci.h"
>   #include "range.h"
>
> +#define TYPE_PCI_EHCI "pci-ehci-usb"
> +#define PCI_EHCI(obj) OBJECT_CHECK(EHCIPCIState, (obj), TYPE_PCI_EHCI)
> +
>   typedef struct EHCIPCIState {
>       PCIDevice pcidev;
> +
>       EHCIState ehci;
>   } EHCIPCIState;
>
> @@ -33,7 +37,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 +87,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 +119,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 +128,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 +168,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..c7b68b2 100644
> --- a/hw/usb/hcd-ehci-sysbus.c
> +++ b/hw/usb/hcd-ehci-sysbus.c
> @@ -18,8 +18,13 @@
>   #include "hw/usb/hcd-ehci.h"
>   #include "hw/sysbus.h"
>
> +#define TYPE_SYS_BUS_EHCI "sysbus-ehci-usb"
> +#define SYS_BUS_EHCI(obj) \
> +    OBJECT_CHECK(EHCISysBusState, (obj), TYPE_SYS_BUS_EHCI)
> +
>   typedef struct EHCISysBusState {
> -    SysBusDevice busdev;
> +    SysBusDevice parent_obj;
> +
>       EHCIState ehci;
>   } EHCISysBusState;
>
> @@ -40,7 +45,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 +68,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);
>   }
>
>

Reviewed-by: Igor Mitsyanko <i.mitsyanko@samsung.com>

-- 
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: i.mitsyanko@samsung.com

[Qemu-devel] [PATCH RFT 2/5] usb/ehci: Move capsbase and opregbase into SysBus EHCI class

[Andreas Färber]

This allows specific derived models to use different values.

Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/usb/hcd-ehci-sysbus.c |   26 ++++++++++++++++++++++++--
 1 Datei geändert, 24 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)

diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index c7b68b2..38e82bb 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -21,6 +21,17 @@
 #define TYPE_SYS_BUS_EHCI "sysbus-ehci-usb"
 #define SYS_BUS_EHCI(obj) \
     OBJECT_CHECK(EHCISysBusState, (obj), TYPE_SYS_BUS_EHCI)
+#define SYS_BUS_EHCI_CLASS(class) \
+    OBJECT_CLASS_CHECK(SysBusEHCIClass, (class), TYPE_SYS_BUS_EHCI)
+#define SYS_BUS_EHCI_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(SysBusEHCIClass, (obj), TYPE_SYS_BUS_EHCI)
+
+typedef struct SysBusEHCIClass {
+    SysBusDeviceClass parent_class;
+
+    uint16_t capsbase;
+    uint16_t opregbase;
+} SysBusEHCIClass;
 
 typedef struct EHCISysBusState {
     SysBusDevice parent_obj;
@@ -46,10 +57,11 @@ static Property ehci_sysbus_properties[] = {
 static int usb_ehci_sysbus_initfn(SysBusDevice *dev)
 {
     EHCISysBusState *i = SYS_BUS_EHCI(dev);
+    SysBusEHCIClass *sec = SYS_BUS_EHCI_GET_CLASS(dev);
     EHCIState *s = &i->ehci;
 
-    s->capsbase = 0x100;
-    s->opregbase = 0x140;
+    s->capsbase = sec->capsbase;
+    s->opregbase = sec->opregbase;
     s->dma = &dma_context_memory;
 
     usb_ehci_initfn(s, DEVICE(dev));
@@ -74,11 +86,21 @@ static const TypeInfo ehci_type_info = {
     .instance_size = sizeof(EHCISysBusState),
     .abstract      = true,
     .class_init    = ehci_sysbus_class_init,
+    .class_size    = sizeof(SysBusEHCIClass),
 };
 
+static void ehci_xlnx_class_init(ObjectClass *oc, void *data)
+{
+    SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
+
+    sec->capsbase = 0x100;
+    sec->opregbase = 0x140;
+}
+
 static const TypeInfo ehci_xlnx_type_info = {
     .name          = "xlnx,ps7-usb",
     .parent        = TYPE_SYS_BUS_EHCI,
+    .class_init    = ehci_xlnx_class_init,
 };
 
 static void ehci_sysbus_register_types(void)
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH RFT 2/5] usb/ehci: Move capsbase and opregbase into SysBus EHCI class

[Igor Mitsyanko]

On 12/02/2012 06:57 AM, Andreas Färber wrote:
> This allows specific derived models to use different values.
>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
>   hw/usb/hcd-ehci-sysbus.c |   26 ++++++++++++++++++++++++--
>   1 Datei geändert, 24 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-)
>
> diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
> index c7b68b2..38e82bb 100644
> --- a/hw/usb/hcd-ehci-sysbus.c
> +++ b/hw/usb/hcd-ehci-sysbus.c
> @@ -21,6 +21,17 @@
>   #define TYPE_SYS_BUS_EHCI "sysbus-ehci-usb"
>   #define SYS_BUS_EHCI(obj) \
>       OBJECT_CHECK(EHCISysBusState, (obj), TYPE_SYS_BUS_EHCI)
> +#define SYS_BUS_EHCI_CLASS(class) \
> +    OBJECT_CLASS_CHECK(SysBusEHCIClass, (class), TYPE_SYS_BUS_EHCI)
> +#define SYS_BUS_EHCI_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(SysBusEHCIClass, (obj), TYPE_SYS_BUS_EHCI)
> +
> +typedef struct SysBusEHCIClass {
> +    SysBusDeviceClass parent_class;
> +
> +    uint16_t capsbase;
> +    uint16_t opregbase;
> +} SysBusEHCIClass;
>
>   typedef struct EHCISysBusState {
>       SysBusDevice parent_obj;
> @@ -46,10 +57,11 @@ static Property ehci_sysbus_properties[] = {
>   static int usb_ehci_sysbus_initfn(SysBusDevice *dev)
>   {
>       EHCISysBusState *i = SYS_BUS_EHCI(dev);
> +    SysBusEHCIClass *sec = SYS_BUS_EHCI_GET_CLASS(dev);
>       EHCIState *s = &i->ehci;
>
> -    s->capsbase = 0x100;
> -    s->opregbase = 0x140;
> +    s->capsbase = sec->capsbase;
> +    s->opregbase = sec->opregbase;
>       s->dma = &dma_context_memory;
>
>       usb_ehci_initfn(s, DEVICE(dev));
> @@ -74,11 +86,21 @@ static const TypeInfo ehci_type_info = {
>       .instance_size = sizeof(EHCISysBusState),
>       .abstract      = true,
>       .class_init    = ehci_sysbus_class_init,
> +    .class_size    = sizeof(SysBusEHCIClass),
>   };
>
> +static void ehci_xlnx_class_init(ObjectClass *oc, void *data)
> +{
> +    SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
> +
> +    sec->capsbase = 0x100;
> +    sec->opregbase = 0x140;
> +}
> +
>   static const TypeInfo ehci_xlnx_type_info = {
>       .name          = "xlnx,ps7-usb",
>       .parent        = TYPE_SYS_BUS_EHCI,
> +    .class_init    = ehci_xlnx_class_init,
>   };
>
>   static void ehci_sysbus_register_types(void)
>

Reviewed-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
-- 
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: i.mitsyanko@samsung.com

[Qemu-devel] [PATCH RFT 3/5] usb/ehci: Add SysBus EHCI device for Exynos4210

[Andreas Färber]

It uses a different capsbase and opregbase than the Xilinx device.

Signed-off-by: Liming Wang <walimisdev@gmail.com>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
Cc: Igor Mitsyanko <i.mitsyanko@samsung.com>
---
 hw/usb/hcd-ehci-sysbus.c |   15 +++++++++++++++
 hw/usb/hcd-ehci.h        |    2 ++
 2 Dateien geändert, 17 Zeilen hinzugefügt(+)

diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index 38e82bb..2ac61e6 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -103,10 +103,25 @@ static const TypeInfo ehci_xlnx_type_info = {
     .class_init    = ehci_xlnx_class_init,
 };
 
+static void ehci_exynos4210_class_init(ObjectClass *oc, void *data)
+{
+    SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
+
+    sec->capsbase = 0x0;
+    sec->opregbase = 0x40;
+}
+
+static const TypeInfo ehci_exynos4210_type_info = {
+    .name          = TYPE_EXYNOS4210_EHCI,
+    .parent        = TYPE_SYS_BUS_EHCI,
+    .class_init    = ehci_exynos4210_class_init,
+};
+
 static void ehci_sysbus_register_types(void)
 {
     type_register_static(&ehci_type_info);
     type_register_static(&ehci_xlnx_type_info);
+    type_register_static(&ehci_exynos4210_type_info);
 }
 
 type_init(ehci_sysbus_register_types)
diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index d8078f4..b8b6461 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -314,6 +314,8 @@ struct EHCIState {
     bool int_req_by_async;
 };
 
+#define TYPE_EXYNOS4210_EHCI "exynos4210-usb"
+
 extern const VMStateDescription vmstate_ehci;
 
 void usb_ehci_initfn(EHCIState *s, DeviceState *dev);
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH RFT 3/5] usb/ehci: Add SysBus EHCI device for Exynos4210

[Igor Mitsyanko]

On 12/02/2012 06:57 AM, Andreas Färber wrote:
> It uses a different capsbase and opregbase than the Xilinx device.
>
> Signed-off-by: Liming Wang <walimisdev@gmail.com>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> Cc: Igor Mitsyanko <i.mitsyanko@samsung.com>
> ---
>   hw/usb/hcd-ehci-sysbus.c |   15 +++++++++++++++
>   hw/usb/hcd-ehci.h        |    2 ++
>   2 Dateien geändert, 17 Zeilen hinzugefügt(+)
>
> diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
> index 38e82bb..2ac61e6 100644
> --- a/hw/usb/hcd-ehci-sysbus.c
> +++ b/hw/usb/hcd-ehci-sysbus.c
> @@ -103,10 +103,25 @@ static const TypeInfo ehci_xlnx_type_info = {
>       .class_init    = ehci_xlnx_class_init,
>   };
>
> +static void ehci_exynos4210_class_init(ObjectClass *oc, void *data)
> +{
> +    SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
> +
> +    sec->capsbase = 0x0;
> +    sec->opregbase = 0x40;
> +}

Hi, Liming, where did you get value 0x40 for opregbase? My documentation 
states that its 0x10 for Exynos4210 soc.


> +
> +static const TypeInfo ehci_exynos4210_type_info = {
> +    .name          = TYPE_EXYNOS4210_EHCI,
> +    .parent        = TYPE_SYS_BUS_EHCI,
> +    .class_init    = ehci_exynos4210_class_init,
> +};
> +
>   static void ehci_sysbus_register_types(void)
>   {
>       type_register_static(&ehci_type_info);
>       type_register_static(&ehci_xlnx_type_info);
> +    type_register_static(&ehci_exynos4210_type_info);
>   }
>
>   type_init(ehci_sysbus_register_types)
> diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
> index d8078f4..b8b6461 100644
> --- a/hw/usb/hcd-ehci.h
> +++ b/hw/usb/hcd-ehci.h
> @@ -314,6 +314,8 @@ struct EHCIState {
>       bool int_req_by_async;
>   };
>
> +#define TYPE_EXYNOS4210_EHCI "exynos4210-usb"
> +

Maybe use a more descriptive name "exynos4210-usb-ehci" here, for 
consistency with  hcd-ehci-pci.c.

But anyway, I tested it, it works fine)

Reviewed-by: Igor Mitsyanko <i.mitsyanko@samsung.com>


>   extern const VMStateDescription vmstate_ehci;
>
>   void usb_ehci_initfn(EHCIState *s, DeviceState *dev);
>


-- 
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: i.mitsyanko@samsung.com

Re: [Qemu-devel] [PATCH RFT 3/5] usb/ehci: Add SysBus EHCI device for Exynos4210

[walimis]

On Mon, Dec 03, 2012 at 10:51:49PM +0400, Igor Mitsyanko wrote:
>On 12/02/2012 06:57 AM, Andreas Färber wrote:
>>It uses a different capsbase and opregbase than the Xilinx device.
>>
>>Signed-off-by: Liming Wang <walimisdev@gmail.com>
>>Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>>Cc: Igor Mitsyanko <i.mitsyanko@samsung.com>
>>---
>>  hw/usb/hcd-ehci-sysbus.c |   15 +++++++++++++++
>>  hw/usb/hcd-ehci.h        |    2 ++
>>  2 Dateien geändert, 17 Zeilen hinzugefügt(+)
>>
>>diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
>>index 38e82bb..2ac61e6 100644
>>--- a/hw/usb/hcd-ehci-sysbus.c
>>+++ b/hw/usb/hcd-ehci-sysbus.c
>>@@ -103,10 +103,25 @@ static const TypeInfo ehci_xlnx_type_info = {
>>      .class_init    = ehci_xlnx_class_init,
>>  };
>>
>>+static void ehci_exynos4210_class_init(ObjectClass *oc, void *data)
>>+{
>>+    SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
>>+
>>+    sec->capsbase = 0x0;
>>+    sec->opregbase = 0x40;
>>+}
>
>Hi, Liming, where did you get value 0x40 for opregbase? My
>documentation states that its 0x10 for Exynos4210 soc.

Hmm, no way. Because I can't find the documentation, I assume a sane
value to work well.
Thank you to correct me for this value.

Liming Wang

>
>
>>+
>>+static const TypeInfo ehci_exynos4210_type_info = {
>>+    .name          = TYPE_EXYNOS4210_EHCI,
>>+    .parent        = TYPE_SYS_BUS_EHCI,
>>+    .class_init    = ehci_exynos4210_class_init,
>>+};
>>+
>>  static void ehci_sysbus_register_types(void)
>>  {
>>      type_register_static(&ehci_type_info);
>>      type_register_static(&ehci_xlnx_type_info);
>>+    type_register_static(&ehci_exynos4210_type_info);
>>  }
>>
>>  type_init(ehci_sysbus_register_types)
>>diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
>>index d8078f4..b8b6461 100644
>>--- a/hw/usb/hcd-ehci.h
>>+++ b/hw/usb/hcd-ehci.h
>>@@ -314,6 +314,8 @@ struct EHCIState {
>>      bool int_req_by_async;
>>  };
>>
>>+#define TYPE_EXYNOS4210_EHCI "exynos4210-usb"
>>+
>
>Maybe use a more descriptive name "exynos4210-usb-ehci" here, for
>consistency with  hcd-ehci-pci.c.
>
>But anyway, I tested it, it works fine)
>
>Reviewed-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
>
>
>>  extern const VMStateDescription vmstate_ehci;
>>
>>  void usb_ehci_initfn(EHCIState *s, DeviceState *dev);
>>
>
>
>-- 
>Mitsyanko Igor
>ASWG, Moscow R&D center, Samsung Electronics
>email: i.mitsyanko@samsung.com

Re: [Qemu-devel] [PATCH RFT 3/5] usb/ehci: Add SysBus EHCI device for Exynos4210

[walimis]

On Mon, Dec 03, 2012 at 10:51:49PM +0400, Igor Mitsyanko wrote:
>On 12/02/2012 06:57 AM, Andreas Färber wrote:
>>It uses a different capsbase and opregbase than the Xilinx device.
>>
>>Signed-off-by: Liming Wang <walimisdev@gmail.com>
>>Signed-off-by: Andreas Färber <andreas.faerber@web.de>
>>Cc: Igor Mitsyanko <i.mitsyanko@samsung.com>
>>---
>>  hw/usb/hcd-ehci-sysbus.c |   15 +++++++++++++++
>>  hw/usb/hcd-ehci.h        |    2 ++
>>  2 Dateien geändert, 17 Zeilen hinzugefügt(+)
>>
>>diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
>>index 38e82bb..2ac61e6 100644
>>--- a/hw/usb/hcd-ehci-sysbus.c
>>+++ b/hw/usb/hcd-ehci-sysbus.c
>>@@ -103,10 +103,25 @@ static const TypeInfo ehci_xlnx_type_info = {
>>      .class_init    = ehci_xlnx_class_init,
>>  };
>>
>>+static void ehci_exynos4210_class_init(ObjectClass *oc, void *data)
>>+{
>>+    SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
>>+
>>+    sec->capsbase = 0x0;
>>+    sec->opregbase = 0x40;
>>+}
>
>Hi, Liming, where did you get value 0x40 for opregbase? My
>documentation states that its 0x10 for Exynos4210 soc.

Hi Andreas,

Please update the value of opregbase to 0x10.

Thanks,
Liming Wang

>
>
>>+
>>+static const TypeInfo ehci_exynos4210_type_info = {
>>+    .name          = TYPE_EXYNOS4210_EHCI,
>>+    .parent        = TYPE_SYS_BUS_EHCI,
>>+    .class_init    = ehci_exynos4210_class_init,
>>+};
>>+
>>  static void ehci_sysbus_register_types(void)
>>  {
>>      type_register_static(&ehci_type_info);
>>      type_register_static(&ehci_xlnx_type_info);
>>+    type_register_static(&ehci_exynos4210_type_info);
>>  }
>>
>>  type_init(ehci_sysbus_register_types)
>>diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
>>index d8078f4..b8b6461 100644
>>--- a/hw/usb/hcd-ehci.h
>>+++ b/hw/usb/hcd-ehci.h
>>@@ -314,6 +314,8 @@ struct EHCIState {
>>      bool int_req_by_async;
>>  };
>>
>>+#define TYPE_EXYNOS4210_EHCI "exynos4210-usb"
>>+
>
>Maybe use a more descriptive name "exynos4210-usb-ehci" here, for
>consistency with  hcd-ehci-pci.c.
>
>But anyway, I tested it, it works fine)
>
>Reviewed-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
>
>
>>  extern const VMStateDescription vmstate_ehci;
>>
>>  void usb_ehci_initfn(EHCIState *s, DeviceState *dev);
>>
>
>
>-- 
>Mitsyanko Igor
>ASWG, Moscow R&D center, Samsung Electronics
>email: i.mitsyanko@samsung.com

[Qemu-devel] [PATCH RFT 4/5] exynos4210: Add EHCI support

[Andreas Färber]

From: Liming Wang <walimisdev@gmail.com>

Add EHCI USB host controller to exynos4210.

Signed-off-by: Liming Wang <walimisdev@gmail.com>
[AF: Use type constant]
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/exynos4210.c     |    7 +++++++
 hw/exynos4210_gic.c |    2 +-
 2 Dateien geändert, 8 Zeilen hinzugefügt(+), 1 Zeile entfernt(-)

diff --git a/hw/exynos4210.c b/hw/exynos4210.c
index 00d4db8..35d4936 100644
--- a/hw/exynos4210.c
+++ b/hw/exynos4210.c
@@ -27,6 +27,7 @@
 #include "arm-misc.h"
 #include "loader.h"
 #include "exynos4210.h"
+#include "usb/hcd-ehci.h"
 
 #define EXYNOS4210_CHIPID_ADDR         0x10000000
 
@@ -72,6 +73,9 @@
 /* Display controllers (FIMD) */
 #define EXYNOS4210_FIMD0_BASE_ADDR          0x11C00000
 
+/* EHCI */
+#define EXYNOS4210_EHCI_BASE_ADDR           0x12580000
+
 static uint8_t chipid_and_omr[] = { 0x11, 0x02, 0x21, 0x43,
                                     0x09, 0x00, 0x00, 0x00 };
 
@@ -334,5 +338,8 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
             s->irq_table[exynos4210_get_irq(11, 2)],
             NULL);
 
+    sysbus_create_simple(TYPE_EXYNOS4210_EHCI, EXYNOS4210_EHCI_BASE_ADDR,
+            s->irq_table[exynos4210_get_irq(28, 3)]);
+
     return s;
 }
diff --git a/hw/exynos4210_gic.c b/hw/exynos4210_gic.c
index 4fea098..959de56 100644
--- a/hw/exynos4210_gic.c
+++ b/hw/exynos4210_gic.c
@@ -140,7 +140,7 @@ combiner_grp_to_gic_id[64-EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][8] = {
             EXT_GIC_ID_I2C4, EXT_GIC_ID_I2C5, EXT_GIC_ID_I2C6,
             EXT_GIC_ID_I2C7 },
     /* int combiner group 28 */
-    { EXT_GIC_ID_SPI0, EXT_GIC_ID_SPI1, EXT_GIC_ID_SPI2 },
+    { EXT_GIC_ID_SPI0, EXT_GIC_ID_SPI1, EXT_GIC_ID_SPI2 , EXT_GIC_ID_USB_HOST},
     /* int combiner group 29 */
     { EXT_GIC_ID_HSMMC0, EXT_GIC_ID_HSMMC1, EXT_GIC_ID_HSMMC2,
      EXT_GIC_ID_HSMMC3, EXT_GIC_ID_SDMMC },
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH RFT 4/5] exynos4210: Add EHCI support

[Igor Mitsyanko]

On 12/02/2012 06:57 AM, Andreas Färber wrote:
> From: Liming Wang <walimisdev@gmail.com>
>
> Add EHCI USB host controller to exynos4210.
>
> Signed-off-by: Liming Wang <walimisdev@gmail.com>
> [AF: Use type constant]
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---
>   hw/exynos4210.c     |    7 +++++++
>   hw/exynos4210_gic.c |    2 +-
>   2 Dateien geändert, 8 Zeilen hinzugefügt(+), 1 Zeile entfernt(-)
>
> diff --git a/hw/exynos4210.c b/hw/exynos4210.c
> index 00d4db8..35d4936 100644
> --- a/hw/exynos4210.c
> +++ b/hw/exynos4210.c
> @@ -27,6 +27,7 @@
>   #include "arm-misc.h"
>   #include "loader.h"
>   #include "exynos4210.h"
> +#include "usb/hcd-ehci.h"
>
>   #define EXYNOS4210_CHIPID_ADDR         0x10000000
>
> @@ -72,6 +73,9 @@
>   /* Display controllers (FIMD) */
>   #define EXYNOS4210_FIMD0_BASE_ADDR          0x11C00000
>
> +/* EHCI */
> +#define EXYNOS4210_EHCI_BASE_ADDR           0x12580000
> +
>   static uint8_t chipid_and_omr[] = { 0x11, 0x02, 0x21, 0x43,
>                                       0x09, 0x00, 0x00, 0x00 };
>
> @@ -334,5 +338,8 @@ Exynos4210State *exynos4210_init(MemoryRegion *system_mem,
>               s->irq_table[exynos4210_get_irq(11, 2)],
>               NULL);
>
> +    sysbus_create_simple(TYPE_EXYNOS4210_EHCI, EXYNOS4210_EHCI_BASE_ADDR,
> +            s->irq_table[exynos4210_get_irq(28, 3)]);
> +
>       return s;
>   }
> diff --git a/hw/exynos4210_gic.c b/hw/exynos4210_gic.c
> index 4fea098..959de56 100644
> --- a/hw/exynos4210_gic.c
> +++ b/hw/exynos4210_gic.c
> @@ -140,7 +140,7 @@ combiner_grp_to_gic_id[64-EXYNOS4210_MAX_EXT_COMBINER_OUT_IRQ][8] = {
>               EXT_GIC_ID_I2C4, EXT_GIC_ID_I2C5, EXT_GIC_ID_I2C6,
>               EXT_GIC_ID_I2C7 },
>       /* int combiner group 28 */
> -    { EXT_GIC_ID_SPI0, EXT_GIC_ID_SPI1, EXT_GIC_ID_SPI2 },
> +    { EXT_GIC_ID_SPI0, EXT_GIC_ID_SPI1, EXT_GIC_ID_SPI2 , EXT_GIC_ID_USB_HOST},
>       /* int combiner group 29 */
>       { EXT_GIC_ID_HSMMC0, EXT_GIC_ID_HSMMC1, EXT_GIC_ID_HSMMC2,
>        EXT_GIC_ID_HSMMC3, EXT_GIC_ID_SDMMC },
>


Reviewed-by: Igor Mitsyanko <i.mitsyanko@samsung.com>

-- 
Mitsyanko Igor
ASWG, Moscow R&D center, Samsung Electronics
email: i.mitsyanko@samsung.com

[Qemu-devel] [PATCH RFT 5/5] usb/ehci: Add Tegra2 SysBus EHCI device

[Andreas Färber]

This prepares an EHCI device for the Nvidia Tegra 2 SoC family.
Values based on patch by Vincent Palatin and verified against TRM v01p.

Cc: Vincent Palatin <vpalatin@chromium.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/usb/hcd-ehci-sysbus.c |   15 +++++++++++++++
 1 Datei geändert, 15 Zeilen hinzugefügt(+)

diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index 2ac61e6..b062d6d 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -117,11 +117,26 @@ static const TypeInfo ehci_exynos4210_type_info = {
     .class_init    = ehci_exynos4210_class_init,
 };
 
+static void ehci_tegra2_class_init(ObjectClass *oc, void *data)
+{
+    SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
+
+    sec->capsbase = 0x100;
+    sec->opregbase = 0x140;
+}
+
+static const TypeInfo ehci_tegra2_type_info = {
+    .name          = "tegra2-ehci",
+    .parent        = TYPE_SYS_BUS_EHCI,
+    .class_init    = ehci_tegra2_class_init,
+};
+
 static void ehci_sysbus_register_types(void)
 {
     type_register_static(&ehci_type_info);
     type_register_static(&ehci_xlnx_type_info);
     type_register_static(&ehci_exynos4210_type_info);
+    type_register_static(&ehci_tegra2_type_info);
 }
 
 type_init(ehci_sysbus_register_types)
-- 
1.7.10.4

Re: [Qemu-devel] [PATCH RFT 0/5] usb: Clean up and extend SysBus EHCI

[walimis]

On Sun, Dec 02, 2012 at 03:57:17AM +0100, Andreas Färber wrote:
>Hello,
>
>Some review comments for SysBus EHCI were ignored in favor of merging into 1.3.
>As requested by Gerd, this now follows up with the cleanups I had requested
>from Peter C. Apart from using the new-style QOM casts it also achieves some
>deduplification. It then goes on to implement the suggested more slim way to
>have different capsbase and opregbase values; the alternative would be to
>create a new struct SysBusEHCIInfo for declarative initialization. Given that
>there are only few models for now, the imperative approach allows for more
>flexibility such as inheritence when, e.g., an Exynos 5 model varies only
>slightly from Exynos 4.
>
>Appended is Liming's patch to add an EHCI device to Exynos 4 as well as a
>new patch of mine to prepare a Tegra 2 EHCI device (cf. my tegra branch) to

BTW, where can I find your tegra branch?

>show how this works out.
>
>Regards,
>Andreas
>
>Changes from Liming Wang's patchset:
>* Introduce abstract sysbus-ehci-usb and pci-ehci-usb types

It's good to implement these two abstract types, so that it's easy
to add sysbus-ehci-usb sub types. 

>* Move class_init / instance_init to base type as far as possible
>* Use TYPE_* constant for instantiating exynos4210 EHCI
>* Added my tegra2 SysBus EHCI device to the mix
>
>Cc: Gerd Hoffmann <kraxel@redhat.com>
>Cc: Peter A. G. Crosthwaite <peter.crosthwaite@xilinx.com>
>Cc: Peter Maydell <peter.maydell@linaro.org>
>Cc: Igor Mitsyanko <i.mitsyanko@samsung.com>
>Cc: Liming Wang <walimisdev@gmail.com>
>Cc: Vincent Palatin <vpalatin@chromium.org>
>
>Andreas Färber (4):
>  usb/ehci: Clean up SysBus and PCI EHCI split
>  usb/ehci: Move capsbase and opregbase into SysBus EHCI class
>  usb/ehci: Add SysBus EHCI device for Exynos4210
>  usb/ehci: Add Tegra2 SysBus EHCI device
>
>Liming Wang (1):
>  exynos4210: Add EHCI support

The test is OK for exynos4210 ehci.

Regards,
Liming Wang

>
> hw/exynos4210.c          |    7 +++++
> hw/exynos4210_gic.c      |    2 +-
> hw/usb/hcd-ehci-pci.c    |   37 ++++++++++++++++------
> hw/usb/hcd-ehci-sysbus.c |   76 ++++++++++++++++++++++++++++++++++++++++++----
> hw/usb/hcd-ehci.h        |    2 ++
> 5 Dateien geändert, 108 Zeilen hinzugefügt(+), 16 Zeilen entfernt(-)
>
>-- 
>1.7.10.4
>

Re: [Qemu-devel] [PATCH RFT 0/5] usb: Clean up and extend SysBus EHCI

[Andreas Färber]

Am 02.12.2012 11:34, schrieb walimis:
> On Sun, Dec 02, 2012 at 03:57:17AM +0100, Andreas Färber wrote:
>> Appended is Liming's patch to add an EHCI device to Exynos 4 as well as a
>> new patch of mine to prepare a Tegra 2 EHCI device (cf. my tegra branch) to
> 
> BTW, where can I find your tegra branch?

Sorry, it's here, just rebased and hinting at how it's gonna use EHCI:
http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/tegra
git://repo.or.cz/qemu/afaerber.git tegra

Gerd: In order for me to use this with the new-style Tegra2 model we
will need to further move EHCISysBusState and the accompanying macros
that this series adds into the hcd-ehci.h header so that it can be
embedded into the SoC object. So if you're okay with my approach we'd
need a v2.
Also a question: Vincent's patch has a comment "multiple EHCI
controllers support not ready". Is this still a known issue or resolved
by now? Tegra2 would want to instantiate three SysBus EHCI controllers,
with offsets 0x4000 and 0x1000 respectively.

>> Liming Wang (1):
>>  exynos4210: Add EHCI support
> 
> The test is OK for exynos4210 ehci.

Thanks! Can I drop my [AF: ...] change log if this gets resent?

Regards,
Andreas

Re: [Qemu-devel] [PATCH RFT 0/5] usb: Clean up and extend SysBus EHCI

[walimis]

On Sun, Dec 02, 2012 at 05:27:16PM +0100, Andreas F鋜ber wrote:
>Am 02.12.2012 11:34, schrieb walimis:
>> On Sun, Dec 02, 2012 at 03:57:17AM +0100, Andreas F鋜ber wrote:
>>> Appended is Liming's patch to add an EHCI device to Exynos 4 as well as a
>>> new patch of mine to prepare a Tegra 2 EHCI device (cf. my tegra branch) to
>> 
>> BTW, where can I find your tegra branch?
>
>Sorry, it's here, just rebased and hinting at how it's gonna use EHCI:
>http://repo.or.cz/w/qemu/afaerber.git/shortlog/refs/heads/tegra
>git://repo.or.cz/qemu/afaerber.git tegra

Thanks.

>
>Gerd: In order for me to use this with the new-style Tegra2 model we
>will need to further move EHCISysBusState and the accompanying macros
>that this series adds into the hcd-ehci.h header so that it can be
>embedded into the SoC object. So if you're okay with my approach we'd
>need a v2.
>Also a question: Vincent's patch has a comment "multiple EHCI
>controllers support not ready". Is this still a known issue or resolved
>by now? Tegra2 would want to instantiate three SysBus EHCI controllers,
>with offsets 0x4000 and 0x1000 respectively.
>
>>> Liming Wang (1):
>>>  exynos4210: Add EHCI support
>> 
>> The test is OK for exynos4210 ehci.
>
>Thanks! Can I drop my [AF: ...] change log if this gets resent?

OK, no problem.

Liming Wang
>
>Regards,
>Andreas
>

Re: [Qemu-devel] [PATCH RFT 0/5] usb: Clean up and extend SysBus EHCI

[Gerd Hoffmann]

  Hi,

> Gerd: In order for me to use this with the new-style Tegra2 model we
> will need to further move EHCISysBusState and the accompanying macros
> that this series adds into the hcd-ehci.h header so that it can be
> embedded into the SoC object. So if you're okay with my approach we'd
> need a v2.

Looks good to me.  Guess we want something simliar for UHCI ...

> Also a question: Vincent's patch has a comment "multiple EHCI
> controllers support not ready". Is this still a known issue or resolved
> by now?

I'm not aware of any.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH RFT 0/5] usb: Clean up and extend SysBus EHCI

[walimis]

On Mon, Dec 03, 2012 at 07:59:55AM +0100, Gerd Hoffmann wrote:
>  Hi,
>
>> Gerd: In order for me to use this with the new-style Tegra2 model we
>> will need to further move EHCISysBusState and the accompanying macros
>> that this series adds into the hcd-ehci.h header so that it can be
>> embedded into the SoC object. So if you're okay with my approach we'd
>> need a v2.
>
>Looks good to me.  Guess we want something simliar for UHCI ...
>
>> Also a question: Vincent's patch has a comment "multiple EHCI
>> controllers support not ready". Is this still a known issue or resolved
>> by now?
>
>I'm not aware of any.
Hi Andreas,

I'm aware of it today.
For example, xilinx_zynq has two EHCI controllers. If I specify a usb 
device of type "usb-storage", the question is: which EHCI controller does the 
usb device attach to? The answer is dependent.

1. If I use "usbdevice" and pass the host usb device, such as:

$ qemu -usb  -usbdevice host:xxxx:xxxx

The device will attach to the first EHCI controller.

2. If I use "device" and "usb-storage", such as:

$ qemu -usb -device usb-storage,drive=ud -drive id=ud,file=usbdisk.img,if=none

The device will attach to the second EHCI controller.

Qemu doesn't provide some properties, such as "bus", to specify which EHCI controller the
usb device attach to.  If we use "device" and "usb-storage", we never attach the usb
device to the specified EHCI controller.

Liming Wang

>
>cheers,
>  Gerd

Re: [Qemu-devel] [PATCH RFT 0/5] usb: Clean up and extend SysBus EHCI

[Peter Maydell]

On 3 December 2012 11:58, walimis <walimisdev@gmail.com> wrote:
> For example, xilinx_zynq has two EHCI controllers. If I specify a usb
> device of type "usb-storage", the question is: which EHCI controller does the
> usb device attach to? The answer is dependent.
>
> 1. If I use "usbdevice" and pass the host usb device, such as:
>
> $ qemu -usb  -usbdevice host:xxxx:xxxx
>
> The device will attach to the first EHCI controller.
>
> 2. If I use "device" and "usb-storage", such as:
>
> $ qemu -usb -device usb-storage,drive=ud -drive id=ud,file=usbdisk.img,if=none
>
> The device will attach to the second EHCI controller.

This is a long standing bug which is caused by the legacy
-usbdevice search looking through the list of usb buses
in one direction, and the generic -device code looking
through it in the other direction:

http://lists.gnu.org/archive/html/qemu-devel/2011-06/msg00926.html

> Qemu doesn't provide some properties, such as "bus", to specify which EHCI controller the
> usb device attach to.  If we use "device" and "usb-storage", we never attach the usb
> device to the specified EHCI controller.

-device certainly ought to let you specify a bus= property;
you want to be able to specify the USB controller regardless
of whether we sort out the ordering mess. If that doesn't
work then we have a different bug.

-- PMM

Re: [Qemu-devel] [PATCH RFT 0/5] usb: Clean up and extend SysBus EHCI

[walimis]

On Mon, Dec 03, 2012 at 12:10:02PM +0000, Peter Maydell wrote:
>On 3 December 2012 11:58, walimis <walimisdev@gmail.com> wrote:
>> For example, xilinx_zynq has two EHCI controllers. If I specify a usb
>> device of type "usb-storage", the question is: which EHCI controller does the
>> usb device attach to? The answer is dependent.
>>
>> 1. If I use "usbdevice" and pass the host usb device, such as:
>>
>> $ qemu -usb  -usbdevice host:xxxx:xxxx
>>
>> The device will attach to the first EHCI controller.
>>
>> 2. If I use "device" and "usb-storage", such as:
>>
>> $ qemu -usb -device usb-storage,drive=ud -drive id=ud,file=usbdisk.img,if=none
>>
>> The device will attach to the second EHCI controller.
>
>This is a long standing bug which is caused by the legacy
>-usbdevice search looking through the list of usb buses
>in one direction, and the generic -device code looking
>through it in the other direction:
>
>http://lists.gnu.org/archive/html/qemu-devel/2011-06/msg00926.html
>
>> Qemu doesn't provide some properties, such as "bus", to specify which EHCI controller the
>> usb device attach to.  If we use "device" and "usb-storage", we never attach the usb
>> device to the specified EHCI controller.
>
>-device certainly ought to let you specify a bus= property;

Sorry, I don't know the bus= property. 
But why both EHCI controller are named "usb-bus.0"? How to distinguish the
different usb usb?

Liming Wang

>you want to be able to specify the USB controller regardless
>of whether we sort out the ordering mess. If that doesn't
>work then we have a different bug.
>
>-- PMM

Re: [Qemu-devel] [PATCH RFT 0/5] usb: Clean up and extend SysBus EHCI

[Gerd Hoffmann]

  Hi,

>>> Qemu doesn't provide some properties, such as "bus", to specify which EHCI controller the
>>> usb device attach to.  If we use "device" and "usb-storage", we never attach the usb
>>> device to the specified EHCI controller.
>>
>> -device certainly ought to let you specify a bus= property;
> 
> Sorry, I don't know the bus= property. 
> But why both EHCI controller are named "usb-bus.0"? How to distinguish the
> different usb usb?

That is the default name.  For usb controllers added via -device the bus
name is derived from the device name, i.e. '-device usb-ehci,id=ehci"
gives you a bus named "ehci.0".

For builtin devices that doesn't work though as you don't have to add
them manually.  Guess they should be explicitly named then by zynq_init().

cheers,
  Gerd

Re: [Qemu-devel] [PATCH RFT 0/5] usb: Clean up and extend SysBus EHCI

[Peter Maydell]

On 3 December 2012 13:37, Gerd Hoffmann <kraxel@redhat.com> wrote:
>> But why both EHCI controller are named "usb-bus.0"? How to distinguish the
>> different usb usb?
>
> That is the default name.  For usb controllers added via -device the bus
> name is derived from the device name, i.e. '-device usb-ehci,id=ehci"
> gives you a bus named "ehci.0".
>
> For builtin devices that doesn't work though as you don't have to add
> them manually.  Guess they should be explicitly named then by zynq_init().

It would probably be nice if the qdev/qom layer explicitly failed
attempts to create a bus with a name matching one we already have;
then this kind of thing would be noticed earlier...

-- PMM