All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niek Linnenbank <nieklinnenbank@gmail.com>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>
Cc: "Peter Maydell" <peter.maydell@linaro.org>,
	jasowang@redhat.com, "QEMU Developers" <qemu-devel@nongnu.org>,
	"Beniamino Galvani" <b.galvani@gmail.com>,
	qemu-arm <qemu-arm@nongnu.org>,
	imammedo@redhat.com, "Alex Bennée" <alex.bennee@linaro.org>
Subject: Re: [PATCH v4 04/20] hw/arm/allwinner-h3: add USB host controller
Date: Sun, 2 Feb 2020 20:33:32 +0100	[thread overview]
Message-ID: <CAPan3WqamSU7GdTQ5q-Z2KqBpZBTy3CE0p0MUW2zWOe7dKseTA@mail.gmail.com> (raw)
In-Reply-To: <851e2620-82f8-b911-7641-f1690238cc2a@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 10230 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 14495 bytes --]

  reply	other threads:[~2020-02-02 19:34 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-19  0:50 [PATCH v4 00/20] Add Allwinner H3 SoC and Orange Pi PC Machine Niek Linnenbank
2020-01-19  0:50 ` [PATCH v4 01/20] hw/arm: add Allwinner H3 System-on-Chip Niek Linnenbank
2020-01-19 18:01   ` Philippe Mathieu-Daudé
2020-02-01 19:21     ` Niek Linnenbank
2020-02-01 20:52       ` Philippe Mathieu-Daudé
2020-01-19  0:50 ` [PATCH v4 02/20] hw/arm: add Xunlong Orange Pi PC machine Niek Linnenbank
2020-01-19 18:04   ` Philippe Mathieu-Daudé
2020-02-02 22:47     ` Niek Linnenbank
2020-01-21 16:39   ` Igor Mammedov
2020-02-02 22:37     ` Niek Linnenbank
2020-01-19  0:50 ` [PATCH v4 03/20] hw/arm/allwinner-h3: add Clock Control Unit Niek Linnenbank
2020-01-19 18:34   ` Philippe Mathieu-Daudé
2020-02-01 21:15     ` Niek Linnenbank
2020-01-19  0:50 ` [PATCH v4 04/20] hw/arm/allwinner-h3: add USB host controller Niek Linnenbank
2020-01-19 18:37   ` Philippe Mathieu-Daudé
2020-01-19 18:44     ` Philippe Mathieu-Daudé
2020-02-02 19:33       ` Niek Linnenbank [this message]
2020-02-12 22:23         ` Philippe Mathieu-Daudé
2020-02-02 19:12     ` Niek Linnenbank
2020-01-19  0:50 ` [PATCH v4 05/20] hw/arm/allwinner-h3: add System Control module Niek Linnenbank
2020-01-19 18:46   ` Philippe Mathieu-Daudé
2020-01-19  0:50 ` [PATCH v4 06/20] hw/arm/allwinner: add CPU Configuration module Niek Linnenbank
2020-01-19 18:52   ` Philippe Mathieu-Daudé
2020-02-02 20:25     ` Niek Linnenbank
2020-01-19  0:50 ` [PATCH v4 07/20] hw/arm/allwinner: add Security Identifier device Niek Linnenbank
2020-01-19 18:57   ` Philippe Mathieu-Daudé
2020-02-02 20:47     ` Niek Linnenbank
2020-01-19  0:50 ` [PATCH v4 08/20] hw/arm/allwinner: add SD/MMC host controller Niek Linnenbank
2020-01-19 19:01   ` Philippe Mathieu-Daudé
2020-02-02 21:43     ` Niek Linnenbank
2020-02-07 21:09       ` Niek Linnenbank
2020-01-19  0:50 ` [PATCH v4 09/20] hw/arm/allwinner-h3: add EMAC ethernet device Niek Linnenbank
2020-01-19  0:50 ` [PATCH v4 10/20] hw/arm/allwinner-h3: add Boot ROM support Niek Linnenbank
2020-01-19 19:14   ` Philippe Mathieu-Daudé
2020-02-02 22:11     ` Niek Linnenbank
2020-01-19  0:50 ` [PATCH v4 11/20] hw/arm/allwinner-h3: add SDRAM controller device Niek Linnenbank
2020-01-19  0:50 ` [PATCH v4 12/20] hw/arm/allwinner: add RTC device support Niek Linnenbank
2020-01-19  0:50 ` [PATCH v4 13/20] tests/boot_linux_console: Add a quick test for the OrangePi PC board Niek Linnenbank
2020-01-19  0:50 ` [PATCH v4 14/20] tests/boot_linux_console: Add initrd test for the Orange Pi " Niek Linnenbank
2020-01-19  0:50 ` [PATCH v4 15/20] tests/boot_linux_console: Add a SD card test for the OrangePi " Niek Linnenbank
2020-01-19  0:50 ` [PATCH v4 16/20] tests/boot_linux_console: Add a SLOW test booting Ubuntu on OrangePi PC Niek Linnenbank
2020-01-19 22:30   ` Philippe Mathieu-Daudé
2020-02-06 21:21     ` Niek Linnenbank
2020-02-12 22:02       ` Philippe Mathieu-Daudé
2020-01-19  0:50 ` [PATCH v4 17/20] Acceptance tests: Extract _console_interaction() Niek Linnenbank
2020-01-19  0:51 ` [PATCH v4 18/20] Acceptance tests: Add interrupt_interactive_console_until_pattern() Niek Linnenbank
2020-01-19  0:51 ` [PATCH v4 19/20] tests/boot_linux_console: Test booting NetBSD via U-Boot on OrangePi PC Niek Linnenbank
2020-01-19  0:51 ` [PATCH v4 20/20] docs: add Orange Pi PC document Niek Linnenbank
2020-01-19 11:51 ` [PATCH v4 00/20] Add Allwinner H3 SoC and Orange Pi PC Machine Niek Linnenbank
2020-02-12 21:47 ` Niek Linnenbank
2020-02-12 22:12   ` Philippe Mathieu-Daudé
2020-02-17 20:27     ` Niek Linnenbank
2020-02-18  6:46       ` Philippe Mathieu-Daudé
2020-02-18 10:05         ` Peter Maydell
2020-02-18 21:50           ` Niek Linnenbank

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPan3WqamSU7GdTQ5q-Z2KqBpZBTy3CE0p0MUW2zWOe7dKseTA@mail.gmail.com \
    --to=nieklinnenbank@gmail.com \
    --cc=alex.bennee@linaro.org \
    --cc=b.galvani@gmail.com \
    --cc=imammedo@redhat.com \
    --cc=jasowang@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=philmd@redhat.com \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.