diff mbox series

[3/3] arm: allwinner: Enable USB EHCI

Message ID 20181029223207.31878-4-philmd@redhat.com
State New
Headers show
Series hw/arm: Enable USB OHCI/EHCI on Allwinner A10 | expand

Commit Message

Philippe Mathieu-Daudé Oct. 29, 2018, 10:32 p.m. UTC 
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(+)

Comments

Gerd Hoffmann Oct. 30, 2018, 6:38 a.m. UTC | #1 
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
Philippe Mathieu-Daudé Oct. 30, 2018, 10:39 p.m. UTC | #2 
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
>
Gerd Hoffmann Nov. 5, 2018, 8:16 a.m. UTC | #3 
> > > @@ -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
Guenter Roeck Nov. 7, 2018, 6:38 p.m. UTC | #4 
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
diff mbox series

Patch

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]);
     }
 }