qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Niek Linnenbank
Subject: Re: [PATCH v4 04/20] hw/arm/allwinner-h3: add USB host controller
Date: Sun, 2 Feb 2020 20:33:32 +0100



On Sun, Jan 19, 2020 at 7:44 PM Philippe Mathieu-Daudé <address@hidden> 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 <address@hidden>
>> Reviewed-by: Gerd Hoffmann <address@hidden>
>> ---
>>   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é <address@hidden>
> Tested-by: Philippe Mathieu-Daudé <address@hidden>

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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]