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] = 0x00000000, > >> [AW_H3_SRAM_A2] = 0x00044000, > >> [AW_H3_SRAM_C] = 0x00010000, > >> + [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] = 0x01c20000, > >> [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, but I don't see that yet. Nevertheless I will surely add the fix below to the patch. Regards, Niek > 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(&s->ccu)); > >> sysbus_mmio_map(SYS_BUS_DEVICE(&s->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(&s->gic), > >> + AW_H3_GIC_SPI_EHCI0)); > >> + sysbus_create_simple(TYPE_AW_H3_EHCI, s->memmap[AW_H3_EHCI1], > >> + qdev_get_gpio_in(DEVICE(&s->gic), > >> + AW_H3_GIC_SPI_EHCI1)); > >> + sysbus_create_simple(TYPE_AW_H3_EHCI, s->memmap[AW_H3_EHCI2], > >> + qdev_get_gpio_in(DEVICE(&s->gic), > >> + AW_H3_GIC_SPI_EHCI2)); > >> + sysbus_create_simple(TYPE_AW_H3_EHCI, s->memmap[AW_H3_EHCI3], > >> + qdev_get_gpio_in(DEVICE(&s->gic), > >> + AW_H3_GIC_SPI_EHCI3)); > >> + > >> + sysbus_create_simple("sysbus-ohci", s->memmap[AW_H3_OHCI0], > >> + qdev_get_gpio_in(DEVICE(&s->gic), > >> + AW_H3_GIC_SPI_OHCI0)); > >> + sysbus_create_simple("sysbus-ohci", s->memmap[AW_H3_OHCI1], > >> + qdev_get_gpio_in(DEVICE(&s->gic), > >> + AW_H3_GIC_SPI_OHCI1)); > >> + sysbus_create_simple("sysbus-ohci", s->memmap[AW_H3_OHCI2], > >> + qdev_get_gpio_in(DEVICE(&s->gic), > >> + AW_H3_GIC_SPI_OHCI2)); > >> + sysbus_create_simple("sysbus-ohci", s->memmap[AW_H3_OHCI3], > >> + qdev_get_gpio_in(DEVICE(&s->gic), > >> + AW_H3_GIC_SPI_OHCI3)); > >> + > >> /* UART0. For future clocktree API: All UARTS are connected to > >> APB2_CLK. */ > >> serial_mm_init(get_system_memory(), s->memmap[AW_H3_UART0], 2, > >> qdev_get_gpio_in(DEVICE(&s->gic), > >> AW_H3_GIC_SPI_UART0), > >> diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c > >> index 020211fd10..174c3446ef 100644 > >> --- a/hw/usb/hcd-ehci-sysbus.c > >> +++ b/hw/usb/hcd-ehci-sysbus.c > >> @@ -145,6 +145,22 @@ static const TypeInfo ehci_exynos4210_type_info = { > >> .class_init = ehci_exynos4210_class_init, > >> }; > >> +static void ehci_aw_h3_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_h3_type_info = { > >> + .name = TYPE_AW_H3_EHCI, > >> + .parent = TYPE_SYS_BUS_EHCI, > >> + .class_init = ehci_aw_h3_class_init, > >> +}; > >> + > >> static void ehci_tegra2_class_init(ObjectClass *oc, void *data) > >> { > >> SysBusEHCIClass *sec = SYS_BUS_EHCI_CLASS(oc); > >> @@ -267,6 +283,7 @@ static void ehci_sysbus_register_types(void) > >> type_register_static(&ehci_platform_type_info); > >> type_register_static(&ehci_xlnx_type_info); > >> type_register_static(&ehci_exynos4210_type_info); > >> + type_register_static(&ehci_aw_h3_type_info); > >> type_register_static(&ehci_tegra2_type_info); > >> type_register_static(&ehci_ppc4xx_type_info); > >> type_register_static(&ehci_fusbh200_type_info); > >> > > -- Niek Linnenbank