diff mbox series

[v2] hw/arm: ast2600: Wire up EHCI controllers

Message ID 20200207174548.9087-1-linux@roeck-us.net (mailing list archive)
State New, archived
Headers show
Series [v2] hw/arm: ast2600: Wire up EHCI controllers | expand

Commit Message

Guenter Roeck Feb. 7, 2020, 5:45 p.m. UTC
Initialize EHCI controllers on AST2600 using the existing
TYPE_PLATFORM_EHCI. After this change, booting ast2600-evb
into Linux successfully instantiates a USB interface after
the necessary changes are made to its devicetree files.

ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
ehci-platform: EHCI generic platform driver
ehci-platform 1e6a3000.usb: EHCI Host Controller
ehci-platform 1e6a3000.usb: new USB bus registered, assigned bus number 1
ehci-platform 1e6a3000.usb: irq 25, io mem 0x1e6a3000
ehci-platform 1e6a3000.usb: USB 2.0 started, EHCI 1.00
usb usb1: Manufacturer: Linux 5.5.0-09825-ga0802f2d0ef5-dirty ehci_hcd
usb 1-1: new high-speed USB device number 2 using ehci-platform

Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Rebased to master (to fix context conflict)
    Added Reviewed-by: tag

 hw/arm/aspeed_ast2600.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Comments

Niek Linnenbank Feb. 7, 2020, 9:25 p.m. UTC | #1
Hi Guenter,

On Fri, Feb 7, 2020 at 6:46 PM Guenter Roeck <linux@roeck-us.net> wrote:

> Initialize EHCI controllers on AST2600 using the existing
> TYPE_PLATFORM_EHCI. After this change, booting ast2600-evb
> into Linux successfully instantiates a USB interface after
> the necessary changes are made to its devicetree files.
>
> ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
> ehci-platform: EHCI generic platform driver
> ehci-platform 1e6a3000.usb: EHCI Host Controller
> ehci-platform 1e6a3000.usb: new USB bus registered, assigned bus number 1
> ehci-platform 1e6a3000.usb: irq 25, io mem 0x1e6a3000
> ehci-platform 1e6a3000.usb: USB 2.0 started, EHCI 1.00
> usb usb1: Manufacturer: Linux 5.5.0-09825-ga0802f2d0ef5-dirty ehci_hcd
> usb 1-1: new high-speed USB device number 2 using ehci-platform
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: Rebased to master (to fix context conflict)
>     Added Reviewed-by: tag
>
>  hw/arm/aspeed_ast2600.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> index 90cf1c755d..446b44d31c 100644
> --- a/hw/arm/aspeed_ast2600.c
> +++ b/hw/arm/aspeed_ast2600.c
> @@ -31,6 +31,8 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
>      [ASPEED_FMC]       = 0x1E620000,
>      [ASPEED_SPI1]      = 0x1E630000,
>      [ASPEED_SPI2]      = 0x1E641000,
> +    [ASPEED_EHCI1]     = 0x1E6A1000,
> +    [ASPEED_EHCI2]     = 0x1E6A3000,
>      [ASPEED_MII1]      = 0x1E650000,
>      [ASPEED_MII2]      = 0x1E650008,
>      [ASPEED_MII3]      = 0x1E650010,
> @@ -79,6 +81,8 @@ static const int aspeed_soc_ast2600_irqmap[] = {
>      [ASPEED_ADC]       = 78,
>      [ASPEED_XDMA]      = 6,
>      [ASPEED_SDHCI]     = 43,
> +    [ASPEED_EHCI1]     = 5,
> +    [ASPEED_EHCI2]     = 9,
>      [ASPEED_EMMC]      = 15,
>      [ASPEED_GPIO]      = 40,
>      [ASPEED_GPIO_1_8V] = 11,
> @@ -166,6 +170,11 @@ static void aspeed_soc_ast2600_init(Object *obj)
>                                sizeof(s->spi[i]), typename);
>      }
>
> +    for (i = 0; i < sc->ehcis_num; i++) {
> +        sysbus_init_child_obj(obj, "ehci[*]", OBJECT(&s->ehci[i]),
> +                              sizeof(s->ehci[i]), TYPE_PLATFORM_EHCI);
> +    }
> +
>      snprintf(typename, sizeof(typename), "aspeed.sdmc-%s", socname);
>      sysbus_init_child_obj(obj, "sdmc", OBJECT(&s->sdmc), sizeof(s->sdmc),
>                            typename);
> @@ -416,6 +425,19 @@ static void aspeed_soc_ast2600_realize(DeviceState
> *dev, Error **errp)
>                          s->spi[i].ctrl->flash_window_base);
>      }
>
> +    /* EHCI */
> +    for (i = 0; i < sc->ehcis_num; i++) {
> +        object_property_set_bool(OBJECT(&s->ehci[i]), true, "realized",
> &err);
> +        if (err) {
> +            error_propagate(errp, err);
> +            return;
> +        }
>

Would it make sense to directly use error_fatal in the call to
object_property_set_bool?
That way you can avoid the additional code for propagating the error.


> +        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
> +                        sc->memmap[ASPEED_EHCI1 + i]);
> +        sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
> +                           aspeed_soc_get_irq(s, ASPEED_EHCI1 + i));
> +    }
> +
>      /* SDMC - SDRAM Memory Controller */
>      object_property_set_bool(OBJECT(&s->sdmc), true, "realized", &err);
>      if (err) {
> @@ -534,6 +556,7 @@ static void aspeed_soc_ast2600_class_init(ObjectClass
> *oc, void *data)
>      sc->silicon_rev  = AST2600_A0_SILICON_REV;
>      sc->sram_size    = 0x10000;
>      sc->spis_num     = 2;
> +    sc->ehcis_num    = 2;
>

Since this field is only set once here, does it need to be part of the
class state?


>      sc->wdts_num     = 4;
>      sc->macs_num     = 4;
>      sc->irqmap       = aspeed_soc_ast2600_irqmap;
> --
> 2.17.1
>
>
>
Rest looks good to me.

Regards,
Niek
Guenter Roeck Feb. 7, 2020, 9:34 p.m. UTC | #2
On Fri, Feb 07, 2020 at 10:25:01PM +0100, Niek Linnenbank wrote:
> Hi Guenter,
> 
> On Fri, Feb 7, 2020 at 6:46 PM Guenter Roeck <linux@roeck-us.net> wrote:
> 
> > Initialize EHCI controllers on AST2600 using the existing
> > TYPE_PLATFORM_EHCI. After this change, booting ast2600-evb
> > into Linux successfully instantiates a USB interface after
> > the necessary changes are made to its devicetree files.
> >
> > ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
> > ehci-platform: EHCI generic platform driver
> > ehci-platform 1e6a3000.usb: EHCI Host Controller
> > ehci-platform 1e6a3000.usb: new USB bus registered, assigned bus number 1
> > ehci-platform 1e6a3000.usb: irq 25, io mem 0x1e6a3000
> > ehci-platform 1e6a3000.usb: USB 2.0 started, EHCI 1.00
> > usb usb1: Manufacturer: Linux 5.5.0-09825-ga0802f2d0ef5-dirty ehci_hcd
> > usb 1-1: new high-speed USB device number 2 using ehci-platform
> >
> > Reviewed-by: Cédric Le Goater <clg@kaod.org>
> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > ---
> > v2: Rebased to master (to fix context conflict)
> >     Added Reviewed-by: tag
> >
> >  hw/arm/aspeed_ast2600.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> >
> > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> > index 90cf1c755d..446b44d31c 100644
> > --- a/hw/arm/aspeed_ast2600.c
> > +++ b/hw/arm/aspeed_ast2600.c
> > @@ -31,6 +31,8 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
> >      [ASPEED_FMC]       = 0x1E620000,
> >      [ASPEED_SPI1]      = 0x1E630000,
> >      [ASPEED_SPI2]      = 0x1E641000,
> > +    [ASPEED_EHCI1]     = 0x1E6A1000,
> > +    [ASPEED_EHCI2]     = 0x1E6A3000,
> >      [ASPEED_MII1]      = 0x1E650000,
> >      [ASPEED_MII2]      = 0x1E650008,
> >      [ASPEED_MII3]      = 0x1E650010,
> > @@ -79,6 +81,8 @@ static const int aspeed_soc_ast2600_irqmap[] = {
> >      [ASPEED_ADC]       = 78,
> >      [ASPEED_XDMA]      = 6,
> >      [ASPEED_SDHCI]     = 43,
> > +    [ASPEED_EHCI1]     = 5,
> > +    [ASPEED_EHCI2]     = 9,
> >      [ASPEED_EMMC]      = 15,
> >      [ASPEED_GPIO]      = 40,
> >      [ASPEED_GPIO_1_8V] = 11,
> > @@ -166,6 +170,11 @@ static void aspeed_soc_ast2600_init(Object *obj)
> >                                sizeof(s->spi[i]), typename);
> >      }
> >
> > +    for (i = 0; i < sc->ehcis_num; i++) {
> > +        sysbus_init_child_obj(obj, "ehci[*]", OBJECT(&s->ehci[i]),
> > +                              sizeof(s->ehci[i]), TYPE_PLATFORM_EHCI);
> > +    }
> > +
> >      snprintf(typename, sizeof(typename), "aspeed.sdmc-%s", socname);
> >      sysbus_init_child_obj(obj, "sdmc", OBJECT(&s->sdmc), sizeof(s->sdmc),
> >                            typename);
> > @@ -416,6 +425,19 @@ static void aspeed_soc_ast2600_realize(DeviceState
> > *dev, Error **errp)
> >                          s->spi[i].ctrl->flash_window_base);
> >      }
> >
> > +    /* EHCI */
> > +    for (i = 0; i < sc->ehcis_num; i++) {
> > +        object_property_set_bool(OBJECT(&s->ehci[i]), true, "realized",
> > &err);
> > +        if (err) {
> > +            error_propagate(errp, err);
> > +            return;
> > +        }
> >
> 
> Would it make sense to directly use error_fatal in the call to
> object_property_set_bool?
> That way you can avoid the additional code for propagating the error.
> 

The code matches the pattern used in the rest of the function.
Given that, I would be hesitant to change it for this one case.

> 
> > +        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
> > +                        sc->memmap[ASPEED_EHCI1 + i]);
> > +        sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
> > +                           aspeed_soc_get_irq(s, ASPEED_EHCI1 + i));
> > +    }
> > +
> >      /* SDMC - SDRAM Memory Controller */
> >      object_property_set_bool(OBJECT(&s->sdmc), true, "realized", &err);
> >      if (err) {
> > @@ -534,6 +556,7 @@ static void aspeed_soc_ast2600_class_init(ObjectClass
> > *oc, void *data)
> >      sc->silicon_rev  = AST2600_A0_SILICON_REV;
> >      sc->sram_size    = 0x10000;
> >      sc->spis_num     = 2;
> > +    sc->ehcis_num    = 2;
> >
> 
> Since this field is only set once here, does it need to be part of the
> class state?
> 

The same applies to spis_num, wdts_num, and macs_num.
AspeedSoCClass is defined for all ast2X00 SoCs, and
the same mechanism is used for all of them. I don't see
the benefit of deviating from a common mechanism.

Guenter
no-reply@patchew.org Feb. 7, 2020, 10:04 p.m. UTC | #3
Patchew URL: https://patchew.org/QEMU/20200207174548.9087-1-linux@roeck-us.net/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      aarch64-softmmu/target/arm/translate.o
  CC      aarch64-softmmu/trace/generated-helpers.o
  CC      aarch64-softmmu/target/arm/translate-sve.o
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:34:6: error: 'ASPEED_EHCI1' undeclared here (not in a function); did you mean 'ASPEED_MII1'?
     [ASPEED_EHCI1]     = 0x1E6A1000,
      ^~~~~~~~~~~~
      ASPEED_MII1
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:34:6: error: array index in initializer not of integer type
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:34:6: note: (near initialization for 'aspeed_soc_ast2600_memmap')
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:35:6: error: 'ASPEED_EHCI2' undeclared here (not in a function); did you mean 'ASPEED_MII2'?
     [ASPEED_EHCI2]     = 0x1E6A3000,
      ^~~~~~~~~~~~
      ASPEED_MII2
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:35:6: error: array index in initializer not of integer type
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:35:6: note: (near initialization for 'aspeed_soc_ast2600_memmap')
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:84:6: error: array index in initializer not of integer type
     [ASPEED_EHCI1]     = 5,
      ^~~~~~~~~~~~
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:84:6: note: (near initialization for 'aspeed_soc_ast2600_irqmap')
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:85:6: error: array index in initializer not of integer type
     [ASPEED_EHCI2]     = 9,
      ^~~~~~~~~~~~
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:85:6: note: (near initialization for 'aspeed_soc_ast2600_irqmap')
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c: In function 'aspeed_soc_ast2600_init':
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:173:25: error: 'AspeedSoCClass' {aka 'struct AspeedSoCClass'} has no member named 'ehcis_num'; did you mean 'spis_num'?
     for (i = 0; i < sc->ehcis_num; i++) {
                         ^~~~~~~~~
                         spis_num
---
                 from /tmp/qemu-test/src/target/arm/cpu-qom.h:23,
                 from /tmp/qemu-test/src/target/arm/cpu.h:25,
                 from /tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:12:
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:174:58: error: 'AspeedSoCState' {aka 'struct AspeedSoCState'} has no member named 'ehci'; did you mean 'sdhci'?
         sysbus_init_child_obj(obj, "ehci[*]", OBJECT(&s->ehci[i]),
                                                          ^~~~
/tmp/qemu-test/src/include/qom/object.h:511:17: note: in definition of macro 'OBJECT'
     ((Object *)(obj))
                 ^~~
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:175:41: error: 'AspeedSoCState' {aka 'struct AspeedSoCState'} has no member named 'ehci'; did you mean 'sdhci'?
                               sizeof(s->ehci[i]), TYPE_PLATFORM_EHCI);
                                         ^~~~
                                         sdhci
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:175:51: error: 'TYPE_PLATFORM_EHCI' undeclared (first use in this function); did you mean 'POWER_PLATFORM_ROLE'?
                               sizeof(s->ehci[i]), TYPE_PLATFORM_EHCI);
                                                   ^~~~~~~~~~~~~~~~~~
                                                   POWER_PLATFORM_ROLE
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:175:51: note: each undeclared identifier is reported only once for each function it appears in
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c: In function 'aspeed_soc_ast2600_realize':
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:429:25: error: 'AspeedSoCClass' {aka 'struct AspeedSoCClass'} has no member named 'ehcis_num'; did you mean 'spis_num'?
     for (i = 0; i < sc->ehcis_num; i++) {
                         ^~~~~~~~~
                         spis_num
---
                 from /tmp/qemu-test/src/target/arm/cpu-qom.h:23,
                 from /tmp/qemu-test/src/target/arm/cpu.h:25,
                 from /tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:12:
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:430:45: error: 'AspeedSoCState' {aka 'struct AspeedSoCState'} has no member named 'ehci'; did you mean 'sdhci'?
         object_property_set_bool(OBJECT(&s->ehci[i]), true, "realized", &err);
                                             ^~~~
/tmp/qemu-test/src/include/qom/object.h:511:17: note: in definition of macro 'OBJECT'
     ((Object *)(obj))
                 ^~~
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:435:44: error: 'AspeedSoCState' {aka 'struct AspeedSoCState'} has no member named 'ehci'; did you mean 'sdhci'?
         sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
                                            ^~~~
/tmp/qemu-test/src/include/qom/object.h:511:17: note: in definition of macro 'OBJECT'
---
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:435:25: note: in expansion of macro 'SYS_BUS_DEVICE'
         sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
                         ^~~~~~~~~~~~~~
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:437:47: error: 'AspeedSoCState' {aka 'struct AspeedSoCState'} has no member named 'ehci'; did you mean 'sdhci'?
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
                                               ^~~~
/tmp/qemu-test/src/include/qom/object.h:511:17: note: in definition of macro 'OBJECT'
---
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
                            ^~~~~~~~~~~~~~
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c: In function 'aspeed_soc_ast2600_class_init':
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:559:9: error: 'AspeedSoCClass' {aka 'struct AspeedSoCClass'} has no member named 'ehcis_num'; did you mean 'spis_num'?
     sc->ehcis_num    = 2;
         ^~~~~~~~~
         spis_num
make[1]: *** [/tmp/qemu-test/src/rules.mak:69: hw/arm/aspeed_ast2600.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:494: aarch64-softmmu/all] Error 2
make: *** Waiting for unfinished jobs....
  GEN     x86_64-softmmu/qemu-system-x86_64.exe
Traceback (most recent call last):
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=ecf78ad234a24d8382d2e78d714f0a83', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-j7axnp83/src/docker-src.2020-02-07-17.01.31.18417:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=ecf78ad234a24d8382d2e78d714f0a83
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-j7axnp83/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    2m38.355s
user    0m7.687s


The full log is available at
http://patchew.org/logs/20200207174548.9087-1-linux@roeck-us.net/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Niek Linnenbank Feb. 7, 2020, 10:15 p.m. UTC | #4
On Fri, Feb 7, 2020, 22:34 Guenter Roeck <linux@roeck-us.net> wrote:

> On Fri, Feb 07, 2020 at 10:25:01PM +0100, Niek Linnenbank wrote:
> > Hi Guenter,
> >
> > On Fri, Feb 7, 2020 at 6:46 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > > Initialize EHCI controllers on AST2600 using the existing
> > > TYPE_PLATFORM_EHCI. After this change, booting ast2600-evb
> > > into Linux successfully instantiates a USB interface after
> > > the necessary changes are made to its devicetree files.
> > >
> > > ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
> > > ehci-platform: EHCI generic platform driver
> > > ehci-platform 1e6a3000.usb: EHCI Host Controller
> > > ehci-platform 1e6a3000.usb: new USB bus registered, assigned bus
> number 1
> > > ehci-platform 1e6a3000.usb: irq 25, io mem 0x1e6a3000
> > > ehci-platform 1e6a3000.usb: USB 2.0 started, EHCI 1.00
> > > usb usb1: Manufacturer: Linux 5.5.0-09825-ga0802f2d0ef5-dirty ehci_hcd
> > > usb 1-1: new high-speed USB device number 2 using ehci-platform
> > >
> > > Reviewed-by: Cédric Le Goater <clg@kaod.org>
> > > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> > > ---
> > > v2: Rebased to master (to fix context conflict)
> > >     Added Reviewed-by: tag
> > >
> > >  hw/arm/aspeed_ast2600.c | 23 +++++++++++++++++++++++
> > >  1 file changed, 23 insertions(+)
> > >
> > > diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
> > > index 90cf1c755d..446b44d31c 100644
> > > --- a/hw/arm/aspeed_ast2600.c
> > > +++ b/hw/arm/aspeed_ast2600.c
> > > @@ -31,6 +31,8 @@ static const hwaddr aspeed_soc_ast2600_memmap[] = {
> > >      [ASPEED_FMC]       = 0x1E620000,
> > >      [ASPEED_SPI1]      = 0x1E630000,
> > >      [ASPEED_SPI2]      = 0x1E641000,
> > > +    [ASPEED_EHCI1]     = 0x1E6A1000,
> > > +    [ASPEED_EHCI2]     = 0x1E6A3000,
> > >      [ASPEED_MII1]      = 0x1E650000,
> > >      [ASPEED_MII2]      = 0x1E650008,
> > >      [ASPEED_MII3]      = 0x1E650010,
> > > @@ -79,6 +81,8 @@ static const int aspeed_soc_ast2600_irqmap[] = {
> > >      [ASPEED_ADC]       = 78,
> > >      [ASPEED_XDMA]      = 6,
> > >      [ASPEED_SDHCI]     = 43,
> > > +    [ASPEED_EHCI1]     = 5,
> > > +    [ASPEED_EHCI2]     = 9,
> > >      [ASPEED_EMMC]      = 15,
> > >      [ASPEED_GPIO]      = 40,
> > >      [ASPEED_GPIO_1_8V] = 11,
> > > @@ -166,6 +170,11 @@ static void aspeed_soc_ast2600_init(Object *obj)
> > >                                sizeof(s->spi[i]), typename);
> > >      }
> > >
> > > +    for (i = 0; i < sc->ehcis_num; i++) {
> > > +        sysbus_init_child_obj(obj, "ehci[*]", OBJECT(&s->ehci[i]),
> > > +                              sizeof(s->ehci[i]), TYPE_PLATFORM_EHCI);
> > > +    }
> > > +
> > >      snprintf(typename, sizeof(typename), "aspeed.sdmc-%s", socname);
> > >      sysbus_init_child_obj(obj, "sdmc", OBJECT(&s->sdmc),
> sizeof(s->sdmc),
> > >                            typename);
> > > @@ -416,6 +425,19 @@ static void aspeed_soc_ast2600_realize(DeviceState
> > > *dev, Error **errp)
> > >                          s->spi[i].ctrl->flash_window_base);
> > >      }
> > >
> > > +    /* EHCI */
> > > +    for (i = 0; i < sc->ehcis_num; i++) {
> > > +        object_property_set_bool(OBJECT(&s->ehci[i]), true,
> "realized",
> > > &err);
> > > +        if (err) {
> > > +            error_propagate(errp, err);
> > > +            return;
> > > +        }
> > >
> >
> > Would it make sense to directly use error_fatal in the call to
> > object_property_set_bool?
> > That way you can avoid the additional code for propagating the error.
> >
>
> The code matches the pattern used in the rest of the function.
> Given that, I would be hesitant to change it for this one case.
>

I see. There are some uses of error_fatal already in the function, but
improving that might be something for another patch I guess.

>
> >
> > > +        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
> > > +                        sc->memmap[ASPEED_EHCI1 + i]);
> > > +        sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
> > > +                           aspeed_soc_get_irq(s, ASPEED_EHCI1 + i));
> > > +    }
> > > +
> > >      /* SDMC - SDRAM Memory Controller */
> > >      object_property_set_bool(OBJECT(&s->sdmc), true, "realized",
> &err);
> > >      if (err) {
> > > @@ -534,6 +556,7 @@ static void
> aspeed_soc_ast2600_class_init(ObjectClass
> > > *oc, void *data)
> > >      sc->silicon_rev  = AST2600_A0_SILICON_REV;
> > >      sc->sram_size    = 0x10000;
> > >      sc->spis_num     = 2;
> > > +    sc->ehcis_num    = 2;
> > >
> >
> > Since this field is only set once here, does it need to be part of the
> > class state?
> >
>
> The same applies to spis_num, wdts_num, and macs_num.
> AspeedSoCClass is defined for all ast2X00 SoCs, and
> the same mechanism is used for all of them. I don't see
> the benefit of deviating from a common mechanism.
>

Ignore my comment here, i did not see this patch was part of previous work
done for the other AST socs as well.

Reviewed-by: Niek Linnenbank <nieklinnenbank@gmail.com>

>
> Guenter
>
Guenter Roeck Feb. 7, 2020, 10:48 p.m. UTC | #5
On Fri, Feb 07, 2020 at 02:04:09PM -0800, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20200207174548.9087-1-linux@roeck-us.net/
> 
> 
> 
> Hi,
> 
> This series failed the docker-mingw@fedora build test. Please find the testing commands and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.
> 
I forgot to mention that the patch depends on the similar
patch for ast2400/ast2500. Sorry for that. Not sure though how
to tell that to the test build system.

Guenter
no-reply@patchew.org Feb. 7, 2020, 11:23 p.m. UTC | #6
Patchew URL: https://patchew.org/QEMU/20200207174548.9087-1-linux@roeck-us.net/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      aarch64-softmmu/target/arm/translate.o
  CC      aarch64-softmmu/trace/generated-helpers.o
  CC      aarch64-softmmu/target/arm/translate-sve.o
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:34:6: error: 'ASPEED_EHCI1' undeclared here (not in a function); did you mean 'ASPEED_MII1'?
     [ASPEED_EHCI1]     = 0x1E6A1000,
      ^~~~~~~~~~~~
      ASPEED_MII1
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:34:6: error: array index in initializer not of integer type
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:34:6: note: (near initialization for 'aspeed_soc_ast2600_memmap')
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:35:6: error: 'ASPEED_EHCI2' undeclared here (not in a function); did you mean 'ASPEED_MII2'?
     [ASPEED_EHCI2]     = 0x1E6A3000,
      ^~~~~~~~~~~~
      ASPEED_MII2
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:35:6: error: array index in initializer not of integer type
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:35:6: note: (near initialization for 'aspeed_soc_ast2600_memmap')
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:84:6: error: array index in initializer not of integer type
     [ASPEED_EHCI1]     = 5,
      ^~~~~~~~~~~~
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:84:6: note: (near initialization for 'aspeed_soc_ast2600_irqmap')
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:85:6: error: array index in initializer not of integer type
     [ASPEED_EHCI2]     = 9,
      ^~~~~~~~~~~~
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:85:6: note: (near initialization for 'aspeed_soc_ast2600_irqmap')
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c: In function 'aspeed_soc_ast2600_init':
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:173:25: error: 'AspeedSoCClass' {aka 'struct AspeedSoCClass'} has no member named 'ehcis_num'; did you mean 'spis_num'?
     for (i = 0; i < sc->ehcis_num; i++) {
                         ^~~~~~~~~
                         spis_num
---
                 from /tmp/qemu-test/src/target/arm/cpu-qom.h:23,
                 from /tmp/qemu-test/src/target/arm/cpu.h:25,
                 from /tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:12:
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:174:58: error: 'AspeedSoCState' {aka 'struct AspeedSoCState'} has no member named 'ehci'; did you mean 'sdhci'?
         sysbus_init_child_obj(obj, "ehci[*]", OBJECT(&s->ehci[i]),
                                                          ^~~~
/tmp/qemu-test/src/include/qom/object.h:511:17: note: in definition of macro 'OBJECT'
     ((Object *)(obj))
                 ^~~
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:175:41: error: 'AspeedSoCState' {aka 'struct AspeedSoCState'} has no member named 'ehci'; did you mean 'sdhci'?
                               sizeof(s->ehci[i]), TYPE_PLATFORM_EHCI);
                                         ^~~~
                                         sdhci
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:175:51: error: 'TYPE_PLATFORM_EHCI' undeclared (first use in this function); did you mean 'POWER_PLATFORM_ROLE'?
                               sizeof(s->ehci[i]), TYPE_PLATFORM_EHCI);
                                                   ^~~~~~~~~~~~~~~~~~
                                                   POWER_PLATFORM_ROLE
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:175:51: note: each undeclared identifier is reported only once for each function it appears in
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c: In function 'aspeed_soc_ast2600_realize':
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:429:25: error: 'AspeedSoCClass' {aka 'struct AspeedSoCClass'} has no member named 'ehcis_num'; did you mean 'spis_num'?
     for (i = 0; i < sc->ehcis_num; i++) {
                         ^~~~~~~~~
                         spis_num
---
                 from /tmp/qemu-test/src/target/arm/cpu-qom.h:23,
                 from /tmp/qemu-test/src/target/arm/cpu.h:25,
                 from /tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:12:
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:430:45: error: 'AspeedSoCState' {aka 'struct AspeedSoCState'} has no member named 'ehci'; did you mean 'sdhci'?
         object_property_set_bool(OBJECT(&s->ehci[i]), true, "realized", &err);
                                             ^~~~
/tmp/qemu-test/src/include/qom/object.h:511:17: note: in definition of macro 'OBJECT'
     ((Object *)(obj))
                 ^~~
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:435:44: error: 'AspeedSoCState' {aka 'struct AspeedSoCState'} has no member named 'ehci'; did you mean 'sdhci'?
         sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
                                            ^~~~
/tmp/qemu-test/src/include/qom/object.h:511:17: note: in definition of macro 'OBJECT'
---
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:435:25: note: in expansion of macro 'SYS_BUS_DEVICE'
         sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
                         ^~~~~~~~~~~~~~
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:437:47: error: 'AspeedSoCState' {aka 'struct AspeedSoCState'} has no member named 'ehci'; did you mean 'sdhci'?
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
                                               ^~~~
/tmp/qemu-test/src/include/qom/object.h:511:17: note: in definition of macro 'OBJECT'
---
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
                            ^~~~~~~~~~~~~~
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c: In function 'aspeed_soc_ast2600_class_init':
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:559:9: error: 'AspeedSoCClass' {aka 'struct AspeedSoCClass'} has no member named 'ehcis_num'; did you mean 'spis_num'?
     sc->ehcis_num    = 2;
         ^~~~~~~~~
         spis_num
make[1]: *** [/tmp/qemu-test/src/rules.mak:69: hw/arm/aspeed_ast2600.o] Error 1
make: *** [Makefile:494: aarch64-softmmu/all] Error 2
make: *** Waiting for unfinished jobs....
  GEN     x86_64-softmmu/qemu-system-x86_64.exe
Traceback (most recent call last):
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=4c7c768aa9cf4be2b7c4daf3d6cda43d', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-z1p6zz25/src/docker-src.2020-02-07-18.20.08.4085:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=4c7c768aa9cf4be2b7c4daf3d6cda43d
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-z1p6zz25/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    2m52.512s
user    0m8.483s


The full log is available at
http://patchew.org/logs/20200207174548.9087-1-linux@roeck-us.net/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
no-reply@patchew.org Feb. 7, 2020, 11:25 p.m. UTC | #7
Patchew URL: https://patchew.org/QEMU/20200207174548.9087-1-linux@roeck-us.net/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      aarch64-softmmu/gdbstub-xml.o
  CC      aarch64-softmmu/target/arm/translate.o
  CC      aarch64-softmmu/trace/generated-helpers.o
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:34:6: error: 'ASPEED_EHCI1' undeclared here (not in a function)
     [ASPEED_EHCI1]     = 0x1E6A1000,
      ^
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:34:5: error: array index in initializer not of integer type
     [ASPEED_EHCI1]     = 0x1E6A1000,
     ^
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:34:5: error: (near initialization for 'aspeed_soc_ast2600_memmap')
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:35:6: error: 'ASPEED_EHCI2' undeclared here (not in a function)
     [ASPEED_EHCI2]     = 0x1E6A3000,
      ^
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:35:5: error: array index in initializer not of integer type
     [ASPEED_EHCI2]     = 0x1E6A3000,
     ^
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:35:5: error: (near initialization for 'aspeed_soc_ast2600_memmap')
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:84:5: error: array index in initializer not of integer type
     [ASPEED_EHCI1]     = 5,
     ^
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:84:5: error: (near initialization for 'aspeed_soc_ast2600_irqmap')
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:85:5: error: array index in initializer not of integer type
     [ASPEED_EHCI2]     = 9,
     ^
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:85:5: error: (near initialization for 'aspeed_soc_ast2600_irqmap')
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c: In function 'aspeed_soc_ast2600_init':
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:173:23: error: 'AspeedSoCClass' has no member named 'ehcis_num'
     for (i = 0; i < sc->ehcis_num; i++) {
                       ^
In file included from /tmp/qemu-test/src/include/hw/qdev-core.h:6:0,
---
                 from /tmp/qemu-test/src/target/arm/cpu-qom.h:23,
                 from /tmp/qemu-test/src/target/arm/cpu.h:25,
                 from /tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:12:
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:174:56: error: 'AspeedSoCState' has no member named 'ehci'
         sysbus_init_child_obj(obj, "ehci[*]", OBJECT(&s->ehci[i]),
                                                        ^
/tmp/qemu-test/src/include/qom/object.h:511:17: note: in definition of macro 'OBJECT'
     ((Object *)(obj))
                 ^
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:175:39: error: 'AspeedSoCState' has no member named 'ehci'
                               sizeof(s->ehci[i]), TYPE_PLATFORM_EHCI);
                                       ^
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:175:51: error: 'TYPE_PLATFORM_EHCI' undeclared (first use in this function)
                               sizeof(s->ehci[i]), TYPE_PLATFORM_EHCI);
                                                   ^
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:175:51: note: each undeclared identifier is reported only once for each function it appears in
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c: In function 'aspeed_soc_ast2600_realize':
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:429:23: error: 'AspeedSoCClass' has no member named 'ehcis_num'
     for (i = 0; i < sc->ehcis_num; i++) {
                       ^
In file included from /tmp/qemu-test/src/include/hw/qdev-core.h:6:0,
---
                 from /tmp/qemu-test/src/target/arm/cpu-qom.h:23,
                 from /tmp/qemu-test/src/target/arm/cpu.h:25,
                 from /tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:12:
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:430:43: error: 'AspeedSoCState' has no member named 'ehci'
         object_property_set_bool(OBJECT(&s->ehci[i]), true, "realized", &err);
                                           ^
/tmp/qemu-test/src/include/qom/object.h:511:17: note: in definition of macro 'OBJECT'
     ((Object *)(obj))
                 ^
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:435:42: error: 'AspeedSoCState' has no member named 'ehci'
         sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
                                          ^
/tmp/qemu-test/src/include/qom/object.h:511:17: note: in definition of macro 'OBJECT'
---
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:435:25: note: in expansion of macro 'SYS_BUS_DEVICE'
         sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
                         ^
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:437:45: error: 'AspeedSoCState' has no member named 'ehci'
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
                                             ^
/tmp/qemu-test/src/include/qom/object.h:511:17: note: in definition of macro 'OBJECT'
---
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
                            ^
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c: In function 'aspeed_soc_ast2600_class_init':
/tmp/qemu-test/src/hw/arm/aspeed_ast2600.c:559:7: error: 'AspeedSoCClass' has no member named 'ehcis_num'
     sc->ehcis_num    = 2;
       ^
make[1]: *** [hw/arm/aspeed_ast2600.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [aarch64-softmmu/all] Error 2
make: *** Waiting for unfinished jobs....
rm tests/qemu-iotests/socket_scm_helper.o
Traceback (most recent call last):
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=b1402087b0b84f6bbe5317e1ecabbb18', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-y9_m1s2q/src/docker-src.2020-02-07-18.23.40.10730:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=b1402087b0b84f6bbe5317e1ecabbb18
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-y9_m1s2q/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    2m7.587s
user    0m8.650s


The full log is available at
http://patchew.org/logs/20200207174548.9087-1-linux@roeck-us.net/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Philippe Mathieu-Daudé Feb. 11, 2020, 8:12 a.m. UTC | #8
On 2/7/20 11:48 PM, Guenter Roeck wrote:
> On Fri, Feb 07, 2020 at 02:04:09PM -0800, no-reply@patchew.org wrote:
>> Patchew URL: https://patchew.org/QEMU/20200207174548.9087-1-linux@roeck-us.net/
>>
>>
>>
>> Hi,
>>
>> This series failed the docker-mingw@fedora build test. Please find the testing commands and
>> their output below. If you have Docker installed, you can probably reproduce it
>> locally.
>>
> I forgot to mention that the patch depends on the similar
> patch for ast2400/ast2500. Sorry for that. Not sure though how
> to tell that to the test build system.

You mean the "Aspeed: machine extensions and fixes" series?

Use the 'based-on' tag with the series cover message-id in your cover/patch:

Based-on: <20190904070506.1052-1-clg@kaod.org>
Peter Maydell Feb. 11, 2020, 7:13 p.m. UTC | #9
On Tue, 11 Feb 2020 at 08:12, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 2/7/20 11:48 PM, Guenter Roeck wrote:
> > On Fri, Feb 07, 2020 at 02:04:09PM -0800, no-reply@patchew.org wrote:
> >> Patchew URL: https://patchew.org/QEMU/20200207174548.9087-1-linux@roeck-us.net/
> >>
> >>
> >>
> >> Hi,
> >>
> >> This series failed the docker-mingw@fedora build test. Please find the testing commands and
> >> their output below. If you have Docker installed, you can probably reproduce it
> >> locally.
> >>
> > I forgot to mention that the patch depends on the similar
> > patch for ast2400/ast2500. Sorry for that. Not sure though how
> > to tell that to the test build system.
>
> You mean the "Aspeed: machine extensions and fixes" series?

Seems unlikely given that series is from 2019 and went into
master last year... Probably Guenter means
"ast2400/ast2500: Wire up EHCI controllers"
https://patchew.org/QEMU/20200206183437.3979-1-linux@roeck-us.net/

(which I had dropped from my to-review queue because I'd
misunderstood Cedric's review and was expecting to see
a v2 of that which covered all of ast2400/2500/2600 --
I'll put it back on my list to queue before this one)

thanks
-- PMM
Guenter Roeck Feb. 11, 2020, 7:33 p.m. UTC | #10
On Tue, Feb 11, 2020 at 07:13:48PM +0000, Peter Maydell wrote:
> On Tue, 11 Feb 2020 at 08:12, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >
> > On 2/7/20 11:48 PM, Guenter Roeck wrote:
> > > On Fri, Feb 07, 2020 at 02:04:09PM -0800, no-reply@patchew.org wrote:
> > >> Patchew URL: https://patchew.org/QEMU/20200207174548.9087-1-linux@roeck-us.net/
> > >>
> > >>
> > >>
> > >> Hi,
> > >>
> > >> This series failed the docker-mingw@fedora build test. Please find the testing commands and
> > >> their output below. If you have Docker installed, you can probably reproduce it
> > >> locally.
> > >>
> > > I forgot to mention that the patch depends on the similar
> > > patch for ast2400/ast2500. Sorry for that. Not sure though how
> > > to tell that to the test build system.
> >
> > You mean the "Aspeed: machine extensions and fixes" series?
> 
> Seems unlikely given that series is from 2019 and went into
> master last year... Probably Guenter means
> "ast2400/ast2500: Wire up EHCI controllers"
> https://patchew.org/QEMU/20200206183437.3979-1-linux@roeck-us.net/
> 
> (which I had dropped from my to-review queue because I'd
> misunderstood Cedric's review and was expecting to see
> a v2 of that which covered all of ast2400/2500/2600 --
> I'll put it back on my list to queue before this one)
> 
Yes, that is correct; the patch for ast2600 depends on the patch
for ast2400/ast2500. Please let me know if I need to resend both.

Thanks,
Guenter
Peter Maydell Feb. 13, 2020, 11:58 a.m. UTC | #11
On Fri, 7 Feb 2020 at 17:45, Guenter Roeck <linux@roeck-us.net> wrote:
>
> Initialize EHCI controllers on AST2600 using the existing
> TYPE_PLATFORM_EHCI. After this change, booting ast2600-evb
> into Linux successfully instantiates a USB interface after
> the necessary changes are made to its devicetree files.
>
> ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver
> ehci-platform: EHCI generic platform driver
> ehci-platform 1e6a3000.usb: EHCI Host Controller
> ehci-platform 1e6a3000.usb: new USB bus registered, assigned bus number 1
> ehci-platform 1e6a3000.usb: irq 25, io mem 0x1e6a3000
> ehci-platform 1e6a3000.usb: USB 2.0 started, EHCI 1.00
> usb usb1: Manufacturer: Linux 5.5.0-09825-ga0802f2d0ef5-dirty ehci_hcd
> usb 1-1: new high-speed USB device number 2 using ehci-platform
>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>



Applied to target-arm.next, thanks.

-- PMM
diff mbox series

Patch

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 90cf1c755d..446b44d31c 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -31,6 +31,8 @@  static const hwaddr aspeed_soc_ast2600_memmap[] = {
     [ASPEED_FMC]       = 0x1E620000,
     [ASPEED_SPI1]      = 0x1E630000,
     [ASPEED_SPI2]      = 0x1E641000,
+    [ASPEED_EHCI1]     = 0x1E6A1000,
+    [ASPEED_EHCI2]     = 0x1E6A3000,
     [ASPEED_MII1]      = 0x1E650000,
     [ASPEED_MII2]      = 0x1E650008,
     [ASPEED_MII3]      = 0x1E650010,
@@ -79,6 +81,8 @@  static const int aspeed_soc_ast2600_irqmap[] = {
     [ASPEED_ADC]       = 78,
     [ASPEED_XDMA]      = 6,
     [ASPEED_SDHCI]     = 43,
+    [ASPEED_EHCI1]     = 5,
+    [ASPEED_EHCI2]     = 9,
     [ASPEED_EMMC]      = 15,
     [ASPEED_GPIO]      = 40,
     [ASPEED_GPIO_1_8V] = 11,
@@ -166,6 +170,11 @@  static void aspeed_soc_ast2600_init(Object *obj)
                               sizeof(s->spi[i]), typename);
     }
 
+    for (i = 0; i < sc->ehcis_num; i++) {
+        sysbus_init_child_obj(obj, "ehci[*]", OBJECT(&s->ehci[i]),
+                              sizeof(s->ehci[i]), TYPE_PLATFORM_EHCI);
+    }
+
     snprintf(typename, sizeof(typename), "aspeed.sdmc-%s", socname);
     sysbus_init_child_obj(obj, "sdmc", OBJECT(&s->sdmc), sizeof(s->sdmc),
                           typename);
@@ -416,6 +425,19 @@  static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
                         s->spi[i].ctrl->flash_window_base);
     }
 
+    /* EHCI */
+    for (i = 0; i < sc->ehcis_num; i++) {
+        object_property_set_bool(OBJECT(&s->ehci[i]), true, "realized", &err);
+        if (err) {
+            error_propagate(errp, err);
+            return;
+        }
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
+                        sc->memmap[ASPEED_EHCI1 + i]);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
+                           aspeed_soc_get_irq(s, ASPEED_EHCI1 + i));
+    }
+
     /* SDMC - SDRAM Memory Controller */
     object_property_set_bool(OBJECT(&s->sdmc), true, "realized", &err);
     if (err) {
@@ -534,6 +556,7 @@  static void aspeed_soc_ast2600_class_init(ObjectClass *oc, void *data)
     sc->silicon_rev  = AST2600_A0_SILICON_REV;
     sc->sram_size    = 0x10000;
     sc->spis_num     = 2;
+    sc->ehcis_num    = 2;
     sc->wdts_num     = 4;
     sc->macs_num     = 4;
     sc->irqmap       = aspeed_soc_ast2600_irqmap;