[PATCH 0/2] hw/arm: Unconditionally map MMIO-based USB host controllers

[PATCH 0/2] hw/arm: Unconditionally map MMIO-based USB host controllers

[Philippe Mathieu-Daudé]

When a chipset contain a USB controller, we can not simply
remove it. We could disable it, but that requires more changes
this series isn't aiming for. For more context:
https://lore.kernel.org/qemu-devel/56fde49f-7dc6-4f8e-9bbf-0336a20a9ebf@roeck-us.net/

Philippe Mathieu-Daudé (2):
  hw/arm/allwinner-a10: Unconditionally map the USB Host controllers
  hw/arm/nseries: Unconditionally map the TUSB6010 USB Host controller

 hw/arm/allwinner-a10.c | 49 +++++++++++++++++-------------------------
 hw/arm/nseries.c       |  4 +---
 2 files changed, 21 insertions(+), 32 deletions(-)

-- 
2.41.0

[PATCH 1/2] hw/arm/allwinner-a10: Unconditionally map the USB Host controllers

[Philippe Mathieu-Daudé]

The USB Controllers are part of the chipset, thus are
always present and mapped in memory.

Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/allwinner-a10.c | 49 +++++++++++++++++-------------------------
 1 file changed, 20 insertions(+), 29 deletions(-)

diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
index b0ea3f7f66..0135632996 100644
--- a/hw/arm/allwinner-a10.c
+++ b/hw/arm/allwinner-a10.c
@@ -79,15 +79,10 @@ static void aw_a10_init(Object *obj)
 
     object_initialize_child(obj, "i2c0", &s->i2c0, TYPE_AW_I2C);
 
-    if (machine_usb(current_machine)) {
-        int i;
-
-        for (i = 0; i < AW_A10_NUM_USB; i++) {
-            object_initialize_child(obj, "ehci[*]", &s->ehci[i],
-                                    TYPE_PLATFORM_EHCI);
-            object_initialize_child(obj, "ohci[*]", &s->ohci[i],
-                                    TYPE_SYSBUS_OHCI);
-        }
+    for (size_t i = 0; i < AW_A10_NUM_USB; i++) {
+        object_initialize_child(obj, "ehci[*]", &s->ehci[i],
+                                TYPE_PLATFORM_EHCI);
+        object_initialize_child(obj, "ohci[*]", &s->ohci[i], TYPE_SYSBUS_OHCI);
     }
 
     object_initialize_child(obj, "mmc0", &s->mmc0, TYPE_AW_SDHOST_SUN4I);
@@ -165,28 +160,24 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
                    qdev_get_gpio_in(dev, 1),
                    115200, serial_hd(0), DEVICE_NATIVE_ENDIAN);
 
-    if (machine_usb(current_machine)) {
-        int i;
+    for (size_t i = 0; i < AW_A10_NUM_USB; i++) {
+        g_autofree char *bus = g_strdup_printf("usb-bus.%zu", i);
 
-        for (i = 0; i < AW_A10_NUM_USB; i++) {
-            g_autofree char *bus = g_strdup_printf("usb-bus.%d", i);
+        object_property_set_bool(OBJECT(&s->ehci[i]), "companion-enable",
+                                 true, &error_fatal);
+        sysbus_realize(SYS_BUS_DEVICE(&s->ehci[i]), &error_fatal);
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
+                        AW_A10_EHCI_BASE + i * 0x8000);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
+                           qdev_get_gpio_in(dev, 39 + i));
 
-            object_property_set_bool(OBJECT(&s->ehci[i]), "companion-enable",
-                                     true, &error_fatal);
-            sysbus_realize(SYS_BUS_DEVICE(&s->ehci[i]), &error_fatal);
-            sysbus_mmio_map(SYS_BUS_DEVICE(&s->ehci[i]), 0,
-                            AW_A10_EHCI_BASE + i * 0x8000);
-            sysbus_connect_irq(SYS_BUS_DEVICE(&s->ehci[i]), 0,
-                               qdev_get_gpio_in(dev, 39 + i));
-
-            object_property_set_str(OBJECT(&s->ohci[i]), "masterbus", bus,
-                                    &error_fatal);
-            sysbus_realize(SYS_BUS_DEVICE(&s->ohci[i]), &error_fatal);
-            sysbus_mmio_map(SYS_BUS_DEVICE(&s->ohci[i]), 0,
-                            AW_A10_OHCI_BASE + i * 0x8000);
-            sysbus_connect_irq(SYS_BUS_DEVICE(&s->ohci[i]), 0,
-                               qdev_get_gpio_in(dev, 64 + i));
-        }
+        object_property_set_str(OBJECT(&s->ohci[i]), "masterbus", bus,
+                                &error_fatal);
+        sysbus_realize(SYS_BUS_DEVICE(&s->ohci[i]), &error_fatal);
+        sysbus_mmio_map(SYS_BUS_DEVICE(&s->ohci[i]), 0,
+                        AW_A10_OHCI_BASE + i * 0x8000);
+        sysbus_connect_irq(SYS_BUS_DEVICE(&s->ohci[i]), 0,
+                           qdev_get_gpio_in(dev, 64 + i));
     }
 
     /* SD/MMC */
-- 
2.41.0

Re: [PATCH 1/2] hw/arm/allwinner-a10: Unconditionally map the USB Host controllers

[Guenter Roeck]

On Mon, Jan 15, 2024 at 05:56:14PM +0100, Philippe Mathieu-Daudé wrote:
> The USB Controllers are part of the chipset, thus are
> always present and mapped in memory.
> 
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Tested-by: Guenter Roeck <linux@roeck-us.net>

[PATCH 2/2] hw/arm/nseries: Unconditionally map the TUSB6010 USB Host controller

[Philippe Mathieu-Daudé]

The TUSB6010 USB controller is solderer on the N800 and N810
tablets, thus is always present.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/nseries.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index 35aff46b4b..35deb74f65 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -1353,9 +1353,7 @@ static void n8x0_init(MachineState *machine,
     n8x0_spi_setup(s);
     n8x0_dss_setup(s);
     n8x0_cbus_setup(s);
-    if (machine_usb(machine)) {
-        n8x0_usb_setup(s);
-    }
+    n8x0_usb_setup(s);
 
     if (machine->kernel_filename) {
         /* Or at the linux loader.  */
-- 
2.41.0

Re: [PATCH 0/2] hw/arm: Unconditionally map MMIO-based USB host controllers

[Peter Maydell]

On Mon, 15 Jan 2024 at 16:56, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> When a chipset contain a USB controller, we can not simply
> remove it. We could disable it, but that requires more changes
> this series isn't aiming for. For more context:
> https://lore.kernel.org/qemu-devel/56fde49f-7dc6-4f8e-9bbf-0336a20a9ebf@roeck-us.net/
>
> Philippe Mathieu-Daudé (2):
>   hw/arm/allwinner-a10: Unconditionally map the USB Host controllers
>   hw/arm/nseries: Unconditionally map the TUSB6010 USB Host controller

Yes, I think this makes sense. It is technically a migration
compat break for the affected machines if the user was
passing the '-usb none' option, but I think we're OK with
that; worth mentioning in the commit messages I guess.

-- PMM

Re: [PATCH 0/2] hw/arm: Unconditionally map MMIO-based USB host controllers

[Peter Maydell]

On Mon, 15 Jan 2024 at 16:56, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> When a chipset contain a USB controller, we can not simply
> remove it. We could disable it, but that requires more changes
> this series isn't aiming for. For more context:
> https://lore.kernel.org/qemu-devel/56fde49f-7dc6-4f8e-9bbf-0336a20a9ebf@roeck-us.net/
>
> Philippe Mathieu-Daudé (2):
>   hw/arm/allwinner-a10: Unconditionally map the USB Host controllers
>   hw/arm/nseries: Unconditionally map the TUSB6010 USB Host controller
>



Applied to target-arm.next, thanks.

-- PMM

Re: [PATCH 0/2] hw/arm: Unconditionally map MMIO-based USB host controllers

[Philippe Mathieu-Daudé]

On 19/1/24 17:51, Peter Maydell wrote:
> On Mon, 15 Jan 2024 at 16:56, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> When a chipset contain a USB controller, we can not simply
>> remove it. We could disable it, but that requires more changes
>> this series isn't aiming for. For more context:
>> https://lore.kernel.org/qemu-devel/56fde49f-7dc6-4f8e-9bbf-0336a20a9ebf@roeck-us.net/
>>
>> Philippe Mathieu-Daudé (2):
>>    hw/arm/allwinner-a10: Unconditionally map the USB Host controllers
>>    hw/arm/nseries: Unconditionally map the TUSB6010 USB Host controller
>>
> 
> 
> 
> Applied to target-arm.next, thanks.

Thanks! I posted a v2 with the requested changes (mention
migration compat break):
https://lore.kernel.org/qemu-devel/20240119215106.45776-1-philmd@linaro.org/