On Sun, Jan 19, 2020 at 7:44 PM Philippe Mathieu-Daudé <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 <nieklinnenb...@gmail.com>
> >> Reviewed-by: Gerd Hoffmann <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] = 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é <phi...@redhat.com>
> > Tested-by: Philippe Mathieu-Daudé <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.
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
