Re: [PATCH v4 04/20] hw/arm/allwinner-h3: add USB host controller

2020-02-12 Thread Philippe Mathieu-Daudé

On 2/2/20 8:33 PM, Niek Linnenbank wrote:
On Sun, Jan 19, 2020 at 7:44 PM Philippe Mathieu-Daudé 
mailto:phi...@redhat.com>> wrote:


On 1/19/20 7:37 PM, Philippe Mathieu-Daudé wrote:
 > On 1/19/20 1:50 AM, Niek Linnenbank wrote:
 >> The Allwinner H3 System on Chip contains multiple USB 2.0 bus
 >> connections which provide software access using the Enhanced
 >> Host Controller Interface (EHCI) and Open Host Controller
 >> Interface (OHCI) interfaces. This commit adds support for
 >> both interfaces in the Allwinner H3 System on Chip.
 >>
 >> Signed-off-by: Niek Linnenbank mailto:nieklinnenb...@gmail.com>>
 >> Reviewed-by: Gerd Hoffmann mailto:kra...@redhat.com>>
 >> ---
 >>   hw/usb/hcd-ehci.h |  1 +
 >>   include/hw/arm/allwinner-h3.h |  8 ++
 >>   hw/arm/allwinner-h3.c | 52
---
 >>   hw/usb/hcd-ehci-sysbus.c  | 17 
 >>   4 files changed, 74 insertions(+), 4 deletions(-)
 >>
 >> diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
 >> index 0298238f0b..edb59311c4 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_PLATFORM_EHCI "platform-ehci-usb"
 >>   #define TYPE_EXYNOS4210_EHCI "exynos4210-ehci-usb"
 >> +#define TYPE_AW_H3_EHCI "aw-h3-ehci-usb"
 >>   #define TYPE_TEGRA2_EHCI "tegra2-ehci-usb"
 >>   #define TYPE_PPC4xx_EHCI "ppc4xx-ehci-usb"
 >>   #define TYPE_FUSBH200_EHCI "fusbh200-ehci-usb"
 >> diff --git a/include/hw/arm/allwinner-h3.h
 >> b/include/hw/arm/allwinner-h3.h
 >> index abdc20871a..4f4dcbcd17 100644
 >> --- a/include/hw/arm/allwinner-h3.h
 >> +++ b/include/hw/arm/allwinner-h3.h
 >> @@ -56,6 +56,14 @@ enum {
 >>   AW_H3_SRAM_A1,
 >>   AW_H3_SRAM_A2,
 >>   AW_H3_SRAM_C,
 >> +    AW_H3_EHCI0,
 >> +    AW_H3_OHCI0,
 >> +    AW_H3_EHCI1,
 >> +    AW_H3_OHCI1,
 >> +    AW_H3_EHCI2,
 >> +    AW_H3_OHCI2,
 >> +    AW_H3_EHCI3,
 >> +    AW_H3_OHCI3,
 >>   AW_H3_CCU,
 >>   AW_H3_PIT,
 >>   AW_H3_UART0,
 >> diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
 >> index 8df8e3e05e..f360625ee9 100644
 >> --- a/hw/arm/allwinner-h3.c
 >> +++ b/hw/arm/allwinner-h3.c
 >> @@ -28,6 +28,7 @@
 >>   #include "hw/sysbus.h"
 >>   #include "hw/char/serial.h"
 >>   #include "hw/misc/unimp.h"
 >> +#include "hw/usb/hcd-ehci.h"
 >>   #include "sysemu/sysemu.h"
 >>   #include "hw/arm/allwinner-h3.h"
 >> @@ -36,6 +37,14 @@ const hwaddr allwinner_h3_memmap[] = {
 >>   [AW_H3_SRAM_A1]    = 0x,
 >>   [AW_H3_SRAM_A2]    = 0x00044000,
 >>   [AW_H3_SRAM_C] = 0x0001,
 >> +    [AW_H3_EHCI0]  = 0x01c1a000,
 >> +    [AW_H3_OHCI0]  = 0x01c1a400,
 >> +    [AW_H3_EHCI1]  = 0x01c1b000,
 >> +    [AW_H3_OHCI1]  = 0x01c1b400,
 >> +    [AW_H3_EHCI2]  = 0x01c1c000,
 >> +    [AW_H3_OHCI2]  = 0x01c1c400,
 >> +    [AW_H3_EHCI3]  = 0x01c1d000,
 >> +    [AW_H3_OHCI3]  = 0x01c1d400,
 >>   [AW_H3_CCU]    = 0x01c2,
 >>   [AW_H3_PIT]    = 0x01c20c00,
 >>   [AW_H3_UART0]  = 0x01c28000,
 >> @@ -73,10 +82,10 @@ struct AwH3Unimplemented {
 >>   { "msgbox",    0x01c17000, 4 * KiB },
 >>   { "spinlock",  0x01c18000, 4 * KiB },
 >>   { "usb0-otg",  0x01c19000, 4 * KiB },
 >> -    { "usb0",  0x01c1a000, 4 * KiB },
 >> -    { "usb1",  0x01c1b000, 4 * KiB },
 >> -    { "usb2",  0x01c1c000, 4 * KiB },
 >> -    { "usb3",  0x01c1d000, 4 * KiB },
 >> +    { "usb0-phy",  0x01c1a000, 4 * KiB },
 >> +    { "usb1-phy",  0x01c1b000, 4 * KiB },
 >> +    { "usb2-phy",  0x01c1c000, 4 * KiB },
 >> +    { "usb3-phy",  0x01c1d000, 4 * KiB },
 >
 > As in v3 comment, this can be done in patch #1.
 >
 > Reviewed-by: Philippe Mathieu-Daudé mailto:phi...@redhat.com>>
 > Tested-by: Philippe Mathieu-Daudé mailto:phi...@redhat.com>>

Err, this patch is incomplete, when using ./configure
--without-default-devices:

$ qemu-system-arm -M orangepi-pc
qemu-system-arm: invalid accelerator kvm
qemu-system-arm: falling back to tcg
qemu-system-arm: Unknown device 'aw-h3-ehci-usb' for default sysbus
qemu-system-arm: Unknown device 'sysbus-ohci' for default sysbus
Aborted (core dumped)


Thanks for pointing this out, I was not aware at all that the 
--without-default-devices option existed.

It's not in the configure --help message also.


Indeed. This is https://bugs.launchpad.net/qemu/+bug/1836537


I tried to re-produce the error by running:
$ ./configure --target-list=arm-softmmu --without-default-devices; make -j5
$ 

Re: [PATCH v4 04/20] hw/arm/allwinner-h3: add USB host controller

2020-02-02 Thread Niek Linnenbank
On Sun, Jan 19, 2020 at 7:44 PM Philippe Mathieu-Daudé 
wrote:

> On 1/19/20 7:37 PM, Philippe Mathieu-Daudé wrote:
> > On 1/19/20 1:50 AM, Niek Linnenbank wrote:
> >> The Allwinner H3 System on Chip contains multiple USB 2.0 bus
> >> connections which provide software access using the Enhanced
> >> Host Controller Interface (EHCI) and Open Host Controller
> >> Interface (OHCI) interfaces. This commit adds support for
> >> both interfaces in the Allwinner H3 System on Chip.
> >>
> >> Signed-off-by: Niek Linnenbank 
> >> Reviewed-by: Gerd Hoffmann 
> >> ---
> >>   hw/usb/hcd-ehci.h |  1 +
> >>   include/hw/arm/allwinner-h3.h |  8 ++
> >>   hw/arm/allwinner-h3.c | 52 ---
> >>   hw/usb/hcd-ehci-sysbus.c  | 17 
> >>   4 files changed, 74 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
> >> index 0298238f0b..edb59311c4 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_PLATFORM_EHCI "platform-ehci-usb"
> >>   #define TYPE_EXYNOS4210_EHCI "exynos4210-ehci-usb"
> >> +#define TYPE_AW_H3_EHCI "aw-h3-ehci-usb"
> >>   #define TYPE_TEGRA2_EHCI "tegra2-ehci-usb"
> >>   #define TYPE_PPC4xx_EHCI "ppc4xx-ehci-usb"
> >>   #define TYPE_FUSBH200_EHCI "fusbh200-ehci-usb"
> >> diff --git a/include/hw/arm/allwinner-h3.h
> >> b/include/hw/arm/allwinner-h3.h
> >> index abdc20871a..4f4dcbcd17 100644
> >> --- a/include/hw/arm/allwinner-h3.h
> >> +++ b/include/hw/arm/allwinner-h3.h
> >> @@ -56,6 +56,14 @@ enum {
> >>   AW_H3_SRAM_A1,
> >>   AW_H3_SRAM_A2,
> >>   AW_H3_SRAM_C,
> >> +AW_H3_EHCI0,
> >> +AW_H3_OHCI0,
> >> +AW_H3_EHCI1,
> >> +AW_H3_OHCI1,
> >> +AW_H3_EHCI2,
> >> +AW_H3_OHCI2,
> >> +AW_H3_EHCI3,
> >> +AW_H3_OHCI3,
> >>   AW_H3_CCU,
> >>   AW_H3_PIT,
> >>   AW_H3_UART0,
> >> diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
> >> index 8df8e3e05e..f360625ee9 100644
> >> --- a/hw/arm/allwinner-h3.c
> >> +++ b/hw/arm/allwinner-h3.c
> >> @@ -28,6 +28,7 @@
> >>   #include "hw/sysbus.h"
> >>   #include "hw/char/serial.h"
> >>   #include "hw/misc/unimp.h"
> >> +#include "hw/usb/hcd-ehci.h"
> >>   #include "sysemu/sysemu.h"
> >>   #include "hw/arm/allwinner-h3.h"
> >> @@ -36,6 +37,14 @@ const hwaddr allwinner_h3_memmap[] = {
> >>   [AW_H3_SRAM_A1]= 0x,
> >>   [AW_H3_SRAM_A2]= 0x00044000,
> >>   [AW_H3_SRAM_C] = 0x0001,
> >> +[AW_H3_EHCI0]  = 0x01c1a000,
> >> +[AW_H3_OHCI0]  = 0x01c1a400,
> >> +[AW_H3_EHCI1]  = 0x01c1b000,
> >> +[AW_H3_OHCI1]  = 0x01c1b400,
> >> +[AW_H3_EHCI2]  = 0x01c1c000,
> >> +[AW_H3_OHCI2]  = 0x01c1c400,
> >> +[AW_H3_EHCI3]  = 0x01c1d000,
> >> +[AW_H3_OHCI3]  = 0x01c1d400,
> >>   [AW_H3_CCU]= 0x01c2,
> >>   [AW_H3_PIT]= 0x01c20c00,
> >>   [AW_H3_UART0]  = 0x01c28000,
> >> @@ -73,10 +82,10 @@ struct AwH3Unimplemented {
> >>   { "msgbox",0x01c17000, 4 * KiB },
> >>   { "spinlock",  0x01c18000, 4 * KiB },
> >>   { "usb0-otg",  0x01c19000, 4 * KiB },
> >> -{ "usb0",  0x01c1a000, 4 * KiB },
> >> -{ "usb1",  0x01c1b000, 4 * KiB },
> >> -{ "usb2",  0x01c1c000, 4 * KiB },
> >> -{ "usb3",  0x01c1d000, 4 * KiB },
> >> +{ "usb0-phy",  0x01c1a000, 4 * KiB },
> >> +{ "usb1-phy",  0x01c1b000, 4 * KiB },
> >> +{ "usb2-phy",  0x01c1c000, 4 * KiB },
> >> +{ "usb3-phy",  0x01c1d000, 4 * KiB },
> >
> > As in v3 comment, this can be done in patch #1.
> >
> > Reviewed-by: Philippe Mathieu-Daudé 
> > Tested-by: Philippe Mathieu-Daudé 
>
> Err, this patch is incomplete, when using ./configure
> --without-default-devices:
>
> $ qemu-system-arm -M orangepi-pc
> qemu-system-arm: invalid accelerator kvm
> qemu-system-arm: falling back to tcg
> qemu-system-arm: Unknown device 'aw-h3-ehci-usb' for default sysbus
> qemu-system-arm: Unknown device 'sysbus-ohci' for default sysbus
> Aborted (core dumped)
>

Thanks for pointing this out, I was not aware at all that the
--without-default-devices option existed.
It's not in the configure --help message also.

I tried to re-produce the error by running:
$ ./configure --target-list=arm-softmmu --without-default-devices; make -j5
$ ./arm-softmmu/qemu-system-arm -M orangepi-pc

On my laptop it didn't give the error, I think because somehow the build
system did select
the USB config items (even tho they were missing for the ALLWINNER_H3 item
in hw/arm/Kconfig):

$ grep USB arm-softmmu/config-devices.mak
CONFIG_TUSB6010=y
CONFIG_USB=y
CONFIG_USB_EHCI=y
CONFIG_USB_EHCI_SYSBUS=y
CONFIG_USB_MUSB=y
CONFIG_USB_OHCI=y

Is there any other option you used in addition to --without-default-devices
to trigger the error?
I also searched for something in configure to select/filter on machines to
build, 

Re: [PATCH v4 04/20] hw/arm/allwinner-h3: add USB host controller

2020-02-02 Thread Niek Linnenbank
On Sun, Jan 19, 2020 at 7:37 PM Philippe Mathieu-Daudé 
wrote:

> On 1/19/20 1:50 AM, Niek Linnenbank wrote:
> > The Allwinner H3 System on Chip contains multiple USB 2.0 bus
> > connections which provide software access using the Enhanced
> > Host Controller Interface (EHCI) and Open Host Controller
> > Interface (OHCI) interfaces. This commit adds support for
> > both interfaces in the Allwinner H3 System on Chip.
> >
> > Signed-off-by: Niek Linnenbank 
> > Reviewed-by: Gerd Hoffmann 
> > ---
> >   hw/usb/hcd-ehci.h |  1 +
> >   include/hw/arm/allwinner-h3.h |  8 ++
> >   hw/arm/allwinner-h3.c | 52 ---
> >   hw/usb/hcd-ehci-sysbus.c  | 17 
> >   4 files changed, 74 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
> > index 0298238f0b..edb59311c4 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_PLATFORM_EHCI "platform-ehci-usb"
> >   #define TYPE_EXYNOS4210_EHCI "exynos4210-ehci-usb"
> > +#define TYPE_AW_H3_EHCI "aw-h3-ehci-usb"
> >   #define TYPE_TEGRA2_EHCI "tegra2-ehci-usb"
> >   #define TYPE_PPC4xx_EHCI "ppc4xx-ehci-usb"
> >   #define TYPE_FUSBH200_EHCI "fusbh200-ehci-usb"
> > diff --git a/include/hw/arm/allwinner-h3.h
> b/include/hw/arm/allwinner-h3.h
> > index abdc20871a..4f4dcbcd17 100644
> > --- a/include/hw/arm/allwinner-h3.h
> > +++ b/include/hw/arm/allwinner-h3.h
> > @@ -56,6 +56,14 @@ enum {
> >   AW_H3_SRAM_A1,
> >   AW_H3_SRAM_A2,
> >   AW_H3_SRAM_C,
> > +AW_H3_EHCI0,
> > +AW_H3_OHCI0,
> > +AW_H3_EHCI1,
> > +AW_H3_OHCI1,
> > +AW_H3_EHCI2,
> > +AW_H3_OHCI2,
> > +AW_H3_EHCI3,
> > +AW_H3_OHCI3,
> >   AW_H3_CCU,
> >   AW_H3_PIT,
> >   AW_H3_UART0,
> > diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
> > index 8df8e3e05e..f360625ee9 100644
> > --- a/hw/arm/allwinner-h3.c
> > +++ b/hw/arm/allwinner-h3.c
> > @@ -28,6 +28,7 @@
> >   #include "hw/sysbus.h"
> >   #include "hw/char/serial.h"
> >   #include "hw/misc/unimp.h"
> > +#include "hw/usb/hcd-ehci.h"
> >   #include "sysemu/sysemu.h"
> >   #include "hw/arm/allwinner-h3.h"
> >
> > @@ -36,6 +37,14 @@ const hwaddr allwinner_h3_memmap[] = {
> >   [AW_H3_SRAM_A1]= 0x,
> >   [AW_H3_SRAM_A2]= 0x00044000,
> >   [AW_H3_SRAM_C] = 0x0001,
> > +[AW_H3_EHCI0]  = 0x01c1a000,
> > +[AW_H3_OHCI0]  = 0x01c1a400,
> > +[AW_H3_EHCI1]  = 0x01c1b000,
> > +[AW_H3_OHCI1]  = 0x01c1b400,
> > +[AW_H3_EHCI2]  = 0x01c1c000,
> > +[AW_H3_OHCI2]  = 0x01c1c400,
> > +[AW_H3_EHCI3]  = 0x01c1d000,
> > +[AW_H3_OHCI3]  = 0x01c1d400,
> >   [AW_H3_CCU]= 0x01c2,
> >   [AW_H3_PIT]= 0x01c20c00,
> >   [AW_H3_UART0]  = 0x01c28000,
> > @@ -73,10 +82,10 @@ struct AwH3Unimplemented {
> >   { "msgbox",0x01c17000, 4 * KiB },
> >   { "spinlock",  0x01c18000, 4 * KiB },
> >   { "usb0-otg",  0x01c19000, 4 * KiB },
> > -{ "usb0",  0x01c1a000, 4 * KiB },
> > -{ "usb1",  0x01c1b000, 4 * KiB },
> > -{ "usb2",  0x01c1c000, 4 * KiB },
> > -{ "usb3",  0x01c1d000, 4 * KiB },
> > +{ "usb0-phy",  0x01c1a000, 4 * KiB },
> > +{ "usb1-phy",  0x01c1b000, 4 * KiB },
> > +{ "usb2-phy",  0x01c1c000, 4 * KiB },
> > +{ "usb3-phy",  0x01c1d000, 4 * KiB },
>
> As in v3 comment, this can be done in patch #1.
>

OK, I'll rename them in patch 1, so it won't show up here.


>
> Reviewed-by: Philippe Mathieu-Daudé 
> Tested-by: Philippe Mathieu-Daudé 
>

Thanks for reviewing and testing Philippe!
Regards,
Niek


>
> >   { "smc",   0x01c1e000, 4 * KiB },
> >   { "pio",   0x01c20800, 1 * KiB },
> >   { "owa",   0x01c21000, 1 * KiB },
> > @@ -144,6 +153,14 @@ enum {
> >   AW_H3_GIC_SPI_UART3 =  3,
> >   AW_H3_GIC_SPI_TIMER0= 18,
> >   AW_H3_GIC_SPI_TIMER1= 19,
> > +AW_H3_GIC_SPI_EHCI0 = 72,
> > +AW_H3_GIC_SPI_OHCI0 = 73,
> > +AW_H3_GIC_SPI_EHCI1 = 74,
> > +AW_H3_GIC_SPI_OHCI1 = 75,
> > +AW_H3_GIC_SPI_EHCI2 = 76,
> > +AW_H3_GIC_SPI_OHCI2 = 77,
> > +AW_H3_GIC_SPI_EHCI3 = 78,
> > +AW_H3_GIC_SPI_OHCI3 = 79,
> >   };
> >
> >   /* Allwinner H3 general constants */
> > @@ -284,6 +301,33 @@ static void allwinner_h3_realize(DeviceState *dev,
> Error **errp)
> >   qdev_init_nofail(DEVICE(>ccu));
> >   sysbus_mmio_map(SYS_BUS_DEVICE(>ccu), 0, s->memmap[AW_H3_CCU]);
> >
> > +/* Universal Serial Bus */
> > +sysbus_create_simple(TYPE_AW_H3_EHCI, s->memmap[AW_H3_EHCI0],
> > + qdev_get_gpio_in(DEVICE(>gic),
> > +  AW_H3_GIC_SPI_EHCI0));
> > +sysbus_create_simple(TYPE_AW_H3_EHCI, s->memmap[AW_H3_EHCI1],
> > + 

Re: [PATCH v4 04/20] hw/arm/allwinner-h3: add USB host controller

2020-01-19 Thread Philippe Mathieu-Daudé

On 1/19/20 7:37 PM, Philippe Mathieu-Daudé wrote:

On 1/19/20 1:50 AM, Niek Linnenbank wrote:

The Allwinner H3 System on Chip contains multiple USB 2.0 bus
connections which provide software access using the Enhanced
Host Controller Interface (EHCI) and Open Host Controller
Interface (OHCI) interfaces. This commit adds support for
both interfaces in the Allwinner H3 System on Chip.

Signed-off-by: Niek Linnenbank 
Reviewed-by: Gerd Hoffmann 
---
  hw/usb/hcd-ehci.h |  1 +
  include/hw/arm/allwinner-h3.h |  8 ++
  hw/arm/allwinner-h3.c | 52 ---
  hw/usb/hcd-ehci-sysbus.c  | 17 
  4 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index 0298238f0b..edb59311c4 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_PLATFORM_EHCI "platform-ehci-usb"
  #define TYPE_EXYNOS4210_EHCI "exynos4210-ehci-usb"
+#define TYPE_AW_H3_EHCI "aw-h3-ehci-usb"
  #define TYPE_TEGRA2_EHCI "tegra2-ehci-usb"
  #define TYPE_PPC4xx_EHCI "ppc4xx-ehci-usb"
  #define TYPE_FUSBH200_EHCI "fusbh200-ehci-usb"
diff --git a/include/hw/arm/allwinner-h3.h 
b/include/hw/arm/allwinner-h3.h

index abdc20871a..4f4dcbcd17 100644
--- a/include/hw/arm/allwinner-h3.h
+++ b/include/hw/arm/allwinner-h3.h
@@ -56,6 +56,14 @@ enum {
  AW_H3_SRAM_A1,
  AW_H3_SRAM_A2,
  AW_H3_SRAM_C,
+    AW_H3_EHCI0,
+    AW_H3_OHCI0,
+    AW_H3_EHCI1,
+    AW_H3_OHCI1,
+    AW_H3_EHCI2,
+    AW_H3_OHCI2,
+    AW_H3_EHCI3,
+    AW_H3_OHCI3,
  AW_H3_CCU,
  AW_H3_PIT,
  AW_H3_UART0,
diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
index 8df8e3e05e..f360625ee9 100644
--- a/hw/arm/allwinner-h3.c
+++ b/hw/arm/allwinner-h3.c
@@ -28,6 +28,7 @@
  #include "hw/sysbus.h"
  #include "hw/char/serial.h"
  #include "hw/misc/unimp.h"
+#include "hw/usb/hcd-ehci.h"
  #include "sysemu/sysemu.h"
  #include "hw/arm/allwinner-h3.h"
@@ -36,6 +37,14 @@ const hwaddr allwinner_h3_memmap[] = {
  [AW_H3_SRAM_A1]    = 0x,
  [AW_H3_SRAM_A2]    = 0x00044000,
  [AW_H3_SRAM_C] = 0x0001,
+    [AW_H3_EHCI0]  = 0x01c1a000,
+    [AW_H3_OHCI0]  = 0x01c1a400,
+    [AW_H3_EHCI1]  = 0x01c1b000,
+    [AW_H3_OHCI1]  = 0x01c1b400,
+    [AW_H3_EHCI2]  = 0x01c1c000,
+    [AW_H3_OHCI2]  = 0x01c1c400,
+    [AW_H3_EHCI3]  = 0x01c1d000,
+    [AW_H3_OHCI3]  = 0x01c1d400,
  [AW_H3_CCU]    = 0x01c2,
  [AW_H3_PIT]    = 0x01c20c00,
  [AW_H3_UART0]  = 0x01c28000,
@@ -73,10 +82,10 @@ struct AwH3Unimplemented {
  { "msgbox",    0x01c17000, 4 * KiB },
  { "spinlock",  0x01c18000, 4 * KiB },
  { "usb0-otg",  0x01c19000, 4 * KiB },
-    { "usb0",  0x01c1a000, 4 * KiB },
-    { "usb1",  0x01c1b000, 4 * KiB },
-    { "usb2",  0x01c1c000, 4 * KiB },
-    { "usb3",  0x01c1d000, 4 * KiB },
+    { "usb0-phy",  0x01c1a000, 4 * KiB },
+    { "usb1-phy",  0x01c1b000, 4 * KiB },
+    { "usb2-phy",  0x01c1c000, 4 * KiB },
+    { "usb3-phy",  0x01c1d000, 4 * KiB },


As in v3 comment, this can be done in patch #1.

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 


Err, this patch is incomplete, when using ./configure 
--without-default-devices:


$ qemu-system-arm -M orangepi-pc
qemu-system-arm: invalid accelerator kvm
qemu-system-arm: falling back to tcg
qemu-system-arm: Unknown device 'aw-h3-ehci-usb' for default sysbus
qemu-system-arm: Unknown device 'sysbus-ohci' for default sysbus
Aborted (core dumped)

You need to amend:

-- >8 --
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index bb75c1de17..57b29cc522 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -302,6 +302,8 @@ config ALLWINNER_H3
 select ARM_TIMER
 select ARM_GIC
 select UNIMP
+select USB_OHCI
+select USB_EHCI_SYSBUS

 config RASPI
 bool
---

R/T-b tags can stay with this amended.


  { "smc",   0x01c1e000, 4 * KiB },
  { "pio",   0x01c20800, 1 * KiB },
  { "owa",   0x01c21000, 1 * KiB },
@@ -144,6 +153,14 @@ enum {
  AW_H3_GIC_SPI_UART3 =  3,
  AW_H3_GIC_SPI_TIMER0    = 18,
  AW_H3_GIC_SPI_TIMER1    = 19,
+    AW_H3_GIC_SPI_EHCI0 = 72,
+    AW_H3_GIC_SPI_OHCI0 = 73,
+    AW_H3_GIC_SPI_EHCI1 = 74,
+    AW_H3_GIC_SPI_OHCI1 = 75,
+    AW_H3_GIC_SPI_EHCI2 = 76,
+    AW_H3_GIC_SPI_OHCI2 = 77,
+    AW_H3_GIC_SPI_EHCI3 = 78,
+    AW_H3_GIC_SPI_OHCI3 = 79,
  };
  /* Allwinner H3 general constants */
@@ -284,6 +301,33 @@ static void allwinner_h3_realize(DeviceState 
*dev, Error **errp)

  qdev_init_nofail(DEVICE(>ccu));
  sysbus_mmio_map(SYS_BUS_DEVICE(>ccu), 0, s->memmap[AW_H3_CCU]);
+    /* Universal Serial Bus */
+    sysbus_create_simple(TYPE_AW_H3_EHCI, s->memmap[AW_H3_EHCI0],
+ qdev_get_gpio_in(DEVICE(>gic),
+  

Re: [PATCH v4 04/20] hw/arm/allwinner-h3: add USB host controller

2020-01-19 Thread Philippe Mathieu-Daudé

On 1/19/20 1:50 AM, Niek Linnenbank wrote:

The Allwinner H3 System on Chip contains multiple USB 2.0 bus
connections which provide software access using the Enhanced
Host Controller Interface (EHCI) and Open Host Controller
Interface (OHCI) interfaces. This commit adds support for
both interfaces in the Allwinner H3 System on Chip.

Signed-off-by: Niek Linnenbank 
Reviewed-by: Gerd Hoffmann 
---
  hw/usb/hcd-ehci.h |  1 +
  include/hw/arm/allwinner-h3.h |  8 ++
  hw/arm/allwinner-h3.c | 52 ---
  hw/usb/hcd-ehci-sysbus.c  | 17 
  4 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index 0298238f0b..edb59311c4 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_PLATFORM_EHCI "platform-ehci-usb"
  #define TYPE_EXYNOS4210_EHCI "exynos4210-ehci-usb"
+#define TYPE_AW_H3_EHCI "aw-h3-ehci-usb"
  #define TYPE_TEGRA2_EHCI "tegra2-ehci-usb"
  #define TYPE_PPC4xx_EHCI "ppc4xx-ehci-usb"
  #define TYPE_FUSBH200_EHCI "fusbh200-ehci-usb"
diff --git a/include/hw/arm/allwinner-h3.h b/include/hw/arm/allwinner-h3.h
index abdc20871a..4f4dcbcd17 100644
--- a/include/hw/arm/allwinner-h3.h
+++ b/include/hw/arm/allwinner-h3.h
@@ -56,6 +56,14 @@ enum {
  AW_H3_SRAM_A1,
  AW_H3_SRAM_A2,
  AW_H3_SRAM_C,
+AW_H3_EHCI0,
+AW_H3_OHCI0,
+AW_H3_EHCI1,
+AW_H3_OHCI1,
+AW_H3_EHCI2,
+AW_H3_OHCI2,
+AW_H3_EHCI3,
+AW_H3_OHCI3,
  AW_H3_CCU,
  AW_H3_PIT,
  AW_H3_UART0,
diff --git a/hw/arm/allwinner-h3.c b/hw/arm/allwinner-h3.c
index 8df8e3e05e..f360625ee9 100644
--- a/hw/arm/allwinner-h3.c
+++ b/hw/arm/allwinner-h3.c
@@ -28,6 +28,7 @@
  #include "hw/sysbus.h"
  #include "hw/char/serial.h"
  #include "hw/misc/unimp.h"
+#include "hw/usb/hcd-ehci.h"
  #include "sysemu/sysemu.h"
  #include "hw/arm/allwinner-h3.h"
  
@@ -36,6 +37,14 @@ const hwaddr allwinner_h3_memmap[] = {

  [AW_H3_SRAM_A1]= 0x,
  [AW_H3_SRAM_A2]= 0x00044000,
  [AW_H3_SRAM_C] = 0x0001,
+[AW_H3_EHCI0]  = 0x01c1a000,
+[AW_H3_OHCI0]  = 0x01c1a400,
+[AW_H3_EHCI1]  = 0x01c1b000,
+[AW_H3_OHCI1]  = 0x01c1b400,
+[AW_H3_EHCI2]  = 0x01c1c000,
+[AW_H3_OHCI2]  = 0x01c1c400,
+[AW_H3_EHCI3]  = 0x01c1d000,
+[AW_H3_OHCI3]  = 0x01c1d400,
  [AW_H3_CCU]= 0x01c2,
  [AW_H3_PIT]= 0x01c20c00,
  [AW_H3_UART0]  = 0x01c28000,
@@ -73,10 +82,10 @@ struct AwH3Unimplemented {
  { "msgbox",0x01c17000, 4 * KiB },
  { "spinlock",  0x01c18000, 4 * KiB },
  { "usb0-otg",  0x01c19000, 4 * KiB },
-{ "usb0",  0x01c1a000, 4 * KiB },
-{ "usb1",  0x01c1b000, 4 * KiB },
-{ "usb2",  0x01c1c000, 4 * KiB },
-{ "usb3",  0x01c1d000, 4 * KiB },
+{ "usb0-phy",  0x01c1a000, 4 * KiB },
+{ "usb1-phy",  0x01c1b000, 4 * KiB },
+{ "usb2-phy",  0x01c1c000, 4 * KiB },
+{ "usb3-phy",  0x01c1d000, 4 * KiB },


As in v3 comment, this can be done in patch #1.

Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 


  { "smc",   0x01c1e000, 4 * KiB },
  { "pio",   0x01c20800, 1 * KiB },
  { "owa",   0x01c21000, 1 * KiB },
@@ -144,6 +153,14 @@ enum {
  AW_H3_GIC_SPI_UART3 =  3,
  AW_H3_GIC_SPI_TIMER0= 18,
  AW_H3_GIC_SPI_TIMER1= 19,
+AW_H3_GIC_SPI_EHCI0 = 72,
+AW_H3_GIC_SPI_OHCI0 = 73,
+AW_H3_GIC_SPI_EHCI1 = 74,
+AW_H3_GIC_SPI_OHCI1 = 75,
+AW_H3_GIC_SPI_EHCI2 = 76,
+AW_H3_GIC_SPI_OHCI2 = 77,
+AW_H3_GIC_SPI_EHCI3 = 78,
+AW_H3_GIC_SPI_OHCI3 = 79,
  };
  
  /* Allwinner H3 general constants */

@@ -284,6 +301,33 @@ static void allwinner_h3_realize(DeviceState *dev, Error 
**errp)
  qdev_init_nofail(DEVICE(>ccu));
  sysbus_mmio_map(SYS_BUS_DEVICE(>ccu), 0, s->memmap[AW_H3_CCU]);
  
+/* Universal Serial Bus */

+sysbus_create_simple(TYPE_AW_H3_EHCI, s->memmap[AW_H3_EHCI0],
+ qdev_get_gpio_in(DEVICE(>gic),
+  AW_H3_GIC_SPI_EHCI0));
+sysbus_create_simple(TYPE_AW_H3_EHCI, s->memmap[AW_H3_EHCI1],
+ qdev_get_gpio_in(DEVICE(>gic),
+  AW_H3_GIC_SPI_EHCI1));
+sysbus_create_simple(TYPE_AW_H3_EHCI, s->memmap[AW_H3_EHCI2],
+ qdev_get_gpio_in(DEVICE(>gic),
+  AW_H3_GIC_SPI_EHCI2));
+sysbus_create_simple(TYPE_AW_H3_EHCI, s->memmap[AW_H3_EHCI3],
+ qdev_get_gpio_in(DEVICE(>gic),
+  AW_H3_GIC_SPI_EHCI3));
+
+sysbus_create_simple("sysbus-ohci", s->memmap[AW_H3_OHCI0],
+ qdev_get_gpio_in(DEVICE(>gic),
+