[Qemu-devel] [PATCH 0/3] hw/arm: Enable USB OHCI/EHCI on Allwinner A10

[Qemu-devel] [PATCH 0/3] hw/arm: Enable USB OHCI/EHCI on Allwinner A10

[Philippe Mathieu-Daudé]

Hi, this series intend to salvage previous work from Guenter Roeck [1].

A similar series from Icenowy Zheng was later reviewed by Gerd Hoffmann:
https://lists.gnu.org/archive/html/qemu-arm/2017-01/msg00345.html [2].

I reviewed patches 1 and 3, Gerd already reviewed patch 2 ([2]), if he
agrees to add his R-b tag again, this series is fully reviewed.

Regards,

Phil.

[1] https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg06302.html

Guenter Roeck (3):
  arm: allwinner: Enable USB OHCI
  usb: ehci: Add support for Allwinner A10
  arm: allwinner: Enable USB EHCI

 hw/arm/allwinner-a10.c   |  9 +++++++++
 hw/usb/hcd-ehci-sysbus.c | 17 +++++++++++++++++
 hw/usb/hcd-ehci.h        |  1 +
 3 files changed, 27 insertions(+)

-- 
2.17.2

[Qemu-devel] [PATCH 1/3] arm: allwinner: Enable USB OHCI

[Philippe Mathieu-Daudé]

From: Guenter Roeck <linux@roeck-us.net>

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Intented tabs -> space]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/allwinner-a10.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
index 9fe875cdb5..0a8ac929f5 100644
--- a/hw/arm/allwinner-a10.c
+++ b/hw/arm/allwinner-a10.c
@@ -22,6 +22,8 @@
 #include "hw/sysbus.h"
 #include "hw/devices.h"
 #include "hw/arm/allwinner-a10.h"
+#include "hw/boards.h"
+#include "hw/usb/hcd-ehci.h"
 
 static void aw_a10_init(Object *obj)
 {
@@ -110,6 +112,11 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
     /* FIXME use a qdev chardev prop instead of serial_hd() */
     serial_mm_init(get_system_memory(), AW_A10_UART0_REG_BASE, 2, s->irq[1],
                    115200, serial_hd(0), DEVICE_NATIVE_ENDIAN);
+
+    if (machine_usb(current_machine)) {
+        sysbus_create_simple("sysbus-ohci", 0x01c14400, s->irq[64]);
+        sysbus_create_simple("sysbus-ohci", 0x01c1c400, s->irq[65]);
+    }
 }
 
 static void aw_a10_class_init(ObjectClass *oc, void *data)
-- 
2.17.2

[Qemu-devel] [PATCH 2/3] usb: ehci: Add support for Allwinner A10

[Philippe Mathieu-Daudé]

From: Guenter Roeck <linux@roeck-us.net>

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Gerd: You already review this code here:
https://lists.gnu.org/archive/html/qemu-arm/2017-01/msg00345.html
Can we use your R-b again?

 hw/usb/hcd-ehci-sysbus.c | 17 +++++++++++++++++
 hw/usb/hcd-ehci.h        |  1 +
 2 files changed, 18 insertions(+)

diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index 3b83beb140..87fac9de01 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -110,6 +110,22 @@ static const TypeInfo ehci_xlnx_type_info = {
     .class_init    = ehci_xlnx_class_init,
 };
 
+static void ehci_aw_a10_class_init(ObjectClass *oc, void *data)
+{
+    SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
+    DeviceClass *dc = DEVICE_CLASS(oc);
+
+    sec->capsbase = 0x0;
+    sec->opregbase = 0x10;
+    set_bit(DEVICE_CATEGORY_USB, dc->categories);
+}
+
+static const TypeInfo ehci_aw_a10_type_info = {
+    .name          = TYPE_AW_A10_EHCI,
+    .parent        = TYPE_SYS_BUS_EHCI,
+    .class_init    = ehci_aw_a10_class_init,
+};
+
 static void ehci_exynos4210_class_init(ObjectClass *oc, void *data)
 {
     SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc);
@@ -246,6 +262,7 @@ static void ehci_sysbus_register_types(void)
 {
     type_register_static(&ehci_type_info);
     type_register_static(&ehci_xlnx_type_info);
+    type_register_static(&ehci_aw_a10_type_info);
     type_register_static(&ehci_exynos4210_type_info);
     type_register_static(&ehci_tegra2_type_info);
     type_register_static(&ehci_ppc4xx_type_info);
diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index 0bc364b286..8d78864a82 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -342,6 +342,7 @@ typedef struct EHCIPCIState {
 
 
 #define TYPE_SYS_BUS_EHCI "sysbus-ehci-usb"
+#define TYPE_AW_A10_EHCI "aw-a10-ehci-usb"
 #define TYPE_EXYNOS4210_EHCI "exynos4210-ehci-usb"
 #define TYPE_TEGRA2_EHCI "tegra2-ehci-usb"
 #define TYPE_PPC4xx_EHCI "ppc4xx-ehci-usb"
-- 
2.17.2

Re: [Qemu-devel] [PATCH 2/3] usb: ehci: Add support for Allwinner A10

[Gerd Hoffmann]

On Mon, Oct 29, 2018 at 11:32:06PM +0100, Philippe Mathieu-Daudé wrote:
> From: Guenter Roeck <linux@roeck-us.net>
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Gerd: You already review this code here:
> https://lists.gnu.org/archive/html/qemu-arm/2017-01/msg00345.html
> Can we use your R-b again?

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

cheers,
  Gerd

[Qemu-devel] [PATCH 3/3] arm: allwinner: Enable USB EHCI

[Philippe Mathieu-Daudé]

From: Guenter Roeck <linux@roeck-us.net>

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
[PMD: Intented tabs -> space]
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/arm/allwinner-a10.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
index 0a8ac929f5..92ab66354a 100644
--- a/hw/arm/allwinner-a10.c
+++ b/hw/arm/allwinner-a10.c
@@ -116,6 +116,8 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
     if (machine_usb(current_machine)) {
         sysbus_create_simple("sysbus-ohci", 0x01c14400, s->irq[64]);
         sysbus_create_simple("sysbus-ohci", 0x01c1c400, s->irq[65]);
+        sysbus_create_simple(TYPE_AW_A10_EHCI, 0x01c14000, s->irq[39]);
+        sysbus_create_simple(TYPE_AW_A10_EHCI, 0x01c1c000, s->irq[40]);
     }
 }
 
-- 
2.17.2

Re: [Qemu-devel] [PATCH 3/3] arm: allwinner: Enable USB EHCI

[Gerd Hoffmann]

On Mon, Oct 29, 2018 at 11:32:07PM +0100, Philippe Mathieu-Daudé wrote:
> From: Guenter Roeck <linux@roeck-us.net>
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> [PMD: Intented tabs -> space]
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/arm/allwinner-a10.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
> index 0a8ac929f5..92ab66354a 100644
> --- a/hw/arm/allwinner-a10.c
> +++ b/hw/arm/allwinner-a10.c
> @@ -116,6 +116,8 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
>      if (machine_usb(current_machine)) {
>          sysbus_create_simple("sysbus-ohci", 0x01c14400, s->irq[64]);
>          sysbus_create_simple("sysbus-ohci", 0x01c1c400, s->irq[65]);
> +        sysbus_create_simple(TYPE_AW_A10_EHCI, 0x01c14000, s->irq[39]);
> +        sysbus_create_simple(TYPE_AW_A10_EHCI, 0x01c1c000, s->irq[40]);

Hmm, is this a companion controller setup?  If so, then this is a bit
more complicated  (see ehci_create_ich9_with_companions()).

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 3/3] arm: allwinner: Enable USB EHCI

[Philippe Mathieu-Daudé]

Hi Gerd,

On 30/10/18 7:38, Gerd Hoffmann wrote:
> On Mon, Oct 29, 2018 at 11:32:07PM +0100, Philippe Mathieu-Daudé wrote:
>> From: Guenter Roeck <linux@roeck-us.net>
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> [PMD: Intented tabs -> space]
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>   hw/arm/allwinner-a10.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
>> index 0a8ac929f5..92ab66354a 100644
>> --- a/hw/arm/allwinner-a10.c
>> +++ b/hw/arm/allwinner-a10.c
>> @@ -116,6 +116,8 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
>>       if (machine_usb(current_machine)) {
>>           sysbus_create_simple("sysbus-ohci", 0x01c14400, s->irq[64]);
>>           sysbus_create_simple("sysbus-ohci", 0x01c1c400, s->irq[65]);
>> +        sysbus_create_simple(TYPE_AW_A10_EHCI, 0x01c14000, s->irq[39]);
>> +        sysbus_create_simple(TYPE_AW_A10_EHCI, 0x01c1c000, s->irq[40]);
> 
> Hmm, is this a companion controller setup?  If so, then this is a bit
> more complicated  (see ehci_create_ich9_with_companions()).

I don't have enough USB knowledge. Can we have companion setup without PCI?

Per the datashit [1] p. 215 "21. USB Host Controller", each controller 
share the same PHY. There is no mention of companion setup:

   The USB host controller includes the following features:
   ..
   - Complies with EHCI Specification, Version 1.0,
   - and  the OHCI Specification, Version 1.0a.
   ..
   - Support only 1 USB Root Port shared between EHCI and OHCI.


[1] http://linux-sunxi.org/images/1/1e/Allwinner_A10_User_manual_V1.5.pdf

> 
> cheers,
>    Gerd
> 

Re: [Qemu-devel] [PATCH 3/3] arm: allwinner: Enable USB EHCI

[Gerd Hoffmann]

> > > @@ -116,6 +116,8 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
> > >       if (machine_usb(current_machine)) {
> > >           sysbus_create_simple("sysbus-ohci", 0x01c14400, s->irq[64]);
> > >           sysbus_create_simple("sysbus-ohci", 0x01c1c400, s->irq[65]);
> > > +        sysbus_create_simple(TYPE_AW_A10_EHCI, 0x01c14000, s->irq[39]);
> > > +        sysbus_create_simple(TYPE_AW_A10_EHCI, 0x01c1c000, s->irq[40]);
> > 
> > Hmm, is this a companion controller setup?  If so, then this is a bit
> > more complicated  (see ehci_create_ich9_with_companions()).
> 
> I don't have enough USB knowledge. Can we have companion setup without PCI?
> 
> Per the datashit [1] p. 215 "21. USB Host Controller", each controller share
> the same PHY. There is no mention of companion setup:
> 
>   The USB host controller includes the following features:
>   ..
>   - Complies with EHCI Specification, Version 1.0,
>   - and  the OHCI Specification, Version 1.0a.
>   ..
>   - Support only 1 USB Root Port shared between EHCI and OHCI.

Looks like a companion setup.  Short introduction:

The port is shared, and ehci controls whenever the port is assigned to
ehci or the companion (ohci in this case).  Typically the ehci driver
sets the bit depending on usb device speed, i.e. usb2 devices are
handled by ehci and usb1 devices are handled by ohci.

By default ohci owns the devices, so in case the OS has no ehci driver
ohci can be used to handle usb2 devices in usb1 compat mode.  Pretty
much moot these days.

On the qemu side you basically have to initialize ehci first (so the usb
bus exists), then set the masterbus property to the ehci usb-bus for the
ohci controller.

HTH,
  Gerd

Re: [Qemu-devel] [PATCH 3/3] arm: allwinner: Enable USB EHCI

[Guenter Roeck]

On Mon, Nov 05, 2018 at 09:16:56AM +0100, Gerd Hoffmann wrote:
> > > > @@ -116,6 +116,8 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
> > > >       if (machine_usb(current_machine)) {
> > > >           sysbus_create_simple("sysbus-ohci", 0x01c14400, s->irq[64]);
> > > >           sysbus_create_simple("sysbus-ohci", 0x01c1c400, s->irq[65]);
> > > > +        sysbus_create_simple(TYPE_AW_A10_EHCI, 0x01c14000, s->irq[39]);
> > > > +        sysbus_create_simple(TYPE_AW_A10_EHCI, 0x01c1c000, s->irq[40]);
> > > 
> > > Hmm, is this a companion controller setup?  If so, then this is a bit
> > > more complicated  (see ehci_create_ich9_with_companions()).
> > 
> > I don't have enough USB knowledge. Can we have companion setup without PCI?
> > 
> > Per the datashit [1] p. 215 "21. USB Host Controller", each controller share
> > the same PHY. There is no mention of companion setup:
> > 
> >   The USB host controller includes the following features:
> >   ..
> >   - Complies with EHCI Specification, Version 1.0,
> >   - and  the OHCI Specification, Version 1.0a.
> >   ..
> >   - Support only 1 USB Root Port shared between EHCI and OHCI.
> 
> Looks like a companion setup.  Short introduction:
> 
> The port is shared, and ehci controls whenever the port is assigned to
> ehci or the companion (ohci in this case).  Typically the ehci driver
> sets the bit depending on usb device speed, i.e. usb2 devices are
> handled by ehci and usb1 devices are handled by ohci.
> 
> By default ohci owns the devices, so in case the OS has no ehci driver
> ohci can be used to handle usb2 devices in usb1 compat mode.  Pretty
> much moot these days.
> 
> On the qemu side you basically have to initialize ehci first (so the usb
> bus exists), then set the masterbus property to the ehci usb-bus for the
> ohci controller.
> 

I tried that; in theory it looks like it works, or at least both devices
instantiate. I took the implementation from hw/ppc/sam460ex.c as guideline.

In practice, however, it doesn't work (nor does my current code). EHCI
fails to instantiate with error -110, and OHCI fails to instantiate with
error -75.

Weird, I know I tested this when I implemented it. Nevertheless,
it doesn't work. Given that, I think the series should be dropped
for now, at least until I find the time to track down the problem.

Guenter