[PATCH 09/10] usb: extract sysbus-ohci to a separate file

Paolo Bonzini posted 10 patches 9 months ago
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Richard Henderson <richard.henderson@linaro.org>, Helge Deller <deller@gmx.de>, "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Huacai Chen <chenhuacai@kernel.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, BALATON Zoltan <balaton@eik.bme.hu>, Nicholas Piggin <npiggin@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Yoshinori Sato <ysato@users.sourceforge.jp>, Magnus Damm <magnus.damm@gmail.com>, Gerd Hoffmann <kraxel@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>
[PATCH 09/10] usb: extract sysbus-ohci to a separate file
Posted by Paolo Bonzini 9 months ago
Split the sysbus version to a separate file so that it is not
included in PCI-only machines, and adjust Kconfig for machines
that do need sysbus-ohci.  The copyrights are based on the
time and employer of balrog and Paul Brook's contributions.

While adjusting the SM501 dependency, move it to the right place
instead of keeping it in the R4D machine.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/usb/hcd-ohci-sysbus.c | 91 ++++++++++++++++++++++++++++++++++++++++
 hw/usb/hcd-ohci.c        | 58 ------------------------
 hw/arm/Kconfig           | 12 +++--
 hw/display/Kconfig       |  1 +
 hw/ppc/Kconfig           |  2 +-
 hw/sh4/Kconfig           |  1 -
 hw/usb/Kconfig           |  4 ++
 hw/usb/meson.build       |  1 +
 8 files changed, 105 insertions(+), 65 deletions(-)
 create mode 100644 hw/usb/hcd-ohci-sysbus.c

diff --git a/hw/usb/hcd-ohci-sysbus.c b/hw/usb/hcd-ohci-sysbus.c
new file mode 100644
index 00000000000..4e4481232b6
--- /dev/null
+++ b/hw/usb/hcd-ohci-sysbus.c
@@ -0,0 +1,91 @@
+/*
+ * QEMU USB OHCI Emulation
+ * Copyright (c) 2006 Openedhand Ltd.
+ * Copyright (c) 2010 CodeSourcery
+ * Copyright (c) 2024 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/irq.h"
+#include "qapi/error.h"
+#include "qemu/module.h"
+#include "qemu/timer.h"
+#include "hw/usb.h"
+#include "migration/vmstate.h"
+#include "hw/sysbus.h"
+#include "hw/qdev-dma.h"
+#include "hw/qdev-properties.h"
+#include "trace.h"
+#include "hcd-ohci.h"
+
+
+static void ohci_realize_pxa(DeviceState *dev, Error **errp)
+{
+    OHCISysBusState *s = SYSBUS_OHCI(dev);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    Error *err = NULL;
+
+    usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset,
+                  s->masterbus, s->firstport,
+                  &address_space_memory, ohci_sysbus_die, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    sysbus_init_irq(sbd, &s->ohci.irq);
+    sysbus_init_mmio(sbd, &s->ohci.mem);
+}
+
+static void usb_ohci_reset_sysbus(DeviceState *dev)
+{
+    OHCISysBusState *s = SYSBUS_OHCI(dev);
+    OHCIState *ohci = &s->ohci;
+
+    ohci_hard_reset(ohci);
+}
+
+static Property ohci_sysbus_properties[] = {
+    DEFINE_PROP_STRING("masterbus", OHCISysBusState, masterbus),
+    DEFINE_PROP_UINT32("num-ports", OHCISysBusState, num_ports, 3),
+    DEFINE_PROP_UINT32("firstport", OHCISysBusState, firstport, 0),
+    DEFINE_PROP_DMAADDR("dma-offset", OHCISysBusState, dma_offset, 0),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void ohci_sysbus_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = ohci_realize_pxa;
+    set_bit(DEVICE_CATEGORY_USB, dc->categories);
+    dc->desc = "OHCI USB Controller";
+    device_class_set_props(dc, ohci_sysbus_properties);
+    dc->reset = usb_ohci_reset_sysbus;
+}
+
+static const TypeInfo ohci_sysbus_info = {
+    .name          = TYPE_SYSBUS_OHCI,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(OHCISysBusState),
+    .class_init    = ohci_sysbus_class_init,
+};
+
+static void ohci_register_types(void)
+{
+    type_register_static(&ohci_sysbus_info);
+}
+
+type_init(ohci_register_types)
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index d73b53f33c8..fc8fc91a1d1 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1955,31 +1955,6 @@ void ohci_sysbus_die(struct OHCIState *ohci)
     ohci_bus_stop(ohci);
 }
 
-static void ohci_realize_pxa(DeviceState *dev, Error **errp)
-{
-    OHCISysBusState *s = SYSBUS_OHCI(dev);
-    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
-    Error *err = NULL;
-
-    usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset,
-                  s->masterbus, s->firstport,
-                  &address_space_memory, ohci_sysbus_die, &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
-    sysbus_init_irq(sbd, &s->ohci.irq);
-    sysbus_init_mmio(sbd, &s->ohci.mem);
-}
-
-static void usb_ohci_reset_sysbus(DeviceState *dev)
-{
-    OHCISysBusState *s = SYSBUS_OHCI(dev);
-    OHCIState *ohci = &s->ohci;
-
-    ohci_hard_reset(ohci);
-}
-
 static const VMStateDescription vmstate_ohci_state_port = {
     .name = "ohci-core/port",
     .version_id = 1,
@@ -2054,36 +2029,3 @@ const VMStateDescription vmstate_ohci_state = {
         NULL
     }
 };
-
-static Property ohci_sysbus_properties[] = {
-    DEFINE_PROP_STRING("masterbus", OHCISysBusState, masterbus),
-    DEFINE_PROP_UINT32("num-ports", OHCISysBusState, num_ports, 3),
-    DEFINE_PROP_UINT32("firstport", OHCISysBusState, firstport, 0),
-    DEFINE_PROP_DMAADDR("dma-offset", OHCISysBusState, dma_offset, 0),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-static void ohci_sysbus_class_init(ObjectClass *klass, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(klass);
-
-    dc->realize = ohci_realize_pxa;
-    set_bit(DEVICE_CATEGORY_USB, dc->categories);
-    dc->desc = "OHCI USB Controller";
-    device_class_set_props(dc, ohci_sysbus_properties);
-    dc->reset = usb_ohci_reset_sysbus;
-}
-
-static const TypeInfo ohci_sysbus_info = {
-    .name          = TYPE_SYSBUS_OHCI,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(OHCISysBusState),
-    .class_init    = ohci_sysbus_class_init,
-};
-
-static void ohci_register_types(void)
-{
-    type_register_static(&ohci_sysbus_info);
-}
-
-type_init(ohci_register_types)
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 29abe1da29c..ffb9041b07f 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -185,7 +185,7 @@ config PXA2XX
     select SERIAL
     select SD
     select SSI
-    select USB_OHCI
+    select USB_OHCI_SYSBUS
     select PCMCIA
 
 config GUMSTIX
@@ -256,7 +256,7 @@ config REALVIEW
     select PL310  # cache controller
     select ARM_SBCON_I2C
     select DS1338 # I2C RTC+NVRAM
-    select USB_OHCI
+    select USB_OHCI_SYSBUS
 
 config SBSA_REF
     bool
@@ -337,7 +337,7 @@ config VERSATILE
     select PL080  # DMA controller
     select PL190  # Vector PIC
     select REALVIEW
-    select USB_OHCI
+    select USB_OHCI_SYSBUS
 
 config VEXPRESS
     bool
@@ -393,6 +393,7 @@ config ALLWINNER_A10
     select AXP2XX_PMU
     select SERIAL
     select UNIMP
+    select USB_OHCI_SYSBUS
 
 config ALLWINNER_H3
     bool
@@ -406,7 +407,7 @@ config ALLWINNER_H3
     select ARM_TIMER
     select ARM_GIC
     select UNIMP
-    select USB_OHCI
+    select USB_OHCI_SYSBUS
     select USB_EHCI_SYSBUS
     select SD
 
@@ -422,7 +423,7 @@ config ALLWINNER_R40
     select ARM_TIMER
     select ARM_GIC
     select UNIMP
-    select USB_OHCI
+    select USB_OHCI_SYSBUS
     select USB_EHCI_SYSBUS
     select SD
 
@@ -528,6 +529,7 @@ config NPCM7XX
     select SSI
     select UNIMP
     select PCA954X
+    select USB_OHCI_SYSBUS
 
 config FSL_IMX25
     bool
diff --git a/hw/display/Kconfig b/hw/display/Kconfig
index 1aafe1923d2..07acb37dc66 100644
--- a/hw/display/Kconfig
+++ b/hw/display/Kconfig
@@ -77,6 +77,7 @@ config SM501
     select I2C
     select DDC
     select SERIAL
+    select USB_OHCI_SYSBUS
 
 config TCX
     bool
diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig
index d497fa2b825..d49e7847909 100644
--- a/hw/ppc/Kconfig
+++ b/hw/ppc/Kconfig
@@ -67,7 +67,7 @@ config SAM460EX
     select SM501
     select SMBUS_EEPROM
     select USB_EHCI_SYSBUS
-    select USB_OHCI
+    select USB_OHCI_SYSBUS
     select FDT_PPC
 
 config AMIGAONE
diff --git a/hw/sh4/Kconfig b/hw/sh4/Kconfig
index ab733a3f760..e0c4ecd1a53 100644
--- a/hw/sh4/Kconfig
+++ b/hw/sh4/Kconfig
@@ -6,7 +6,6 @@ config R2D
     select I82378 if TEST_DEVICES
     select IDE_MMIO
     select PFLASH_CFI02
-    select USB_OHCI_PCI
     select PCI
     select SM501
     select SH7750
diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index 0f486764ed6..f569ed7eeaa 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -11,6 +11,10 @@ config USB_OHCI
     bool
     select USB
 
+config USB_OHCI_SYSBUS
+    bool
+    select USB_OHCI
+
 config USB_OHCI_PCI
     bool
     default y if PCI_DEVICES
diff --git a/hw/usb/meson.build b/hw/usb/meson.build
index 2c13c528785..94f0e3b034d 100644
--- a/hw/usb/meson.build
+++ b/hw/usb/meson.build
@@ -15,6 +15,7 @@ system_ss.add(when: 'CONFIG_USB', if_true: files(
 system_ss.add(when: 'CONFIG_USB_UHCI', if_true: files('hcd-uhci.c'))
 system_ss.add(when: 'CONFIG_USB_OHCI', if_true: files('hcd-ohci.c'))
 system_ss.add(when: 'CONFIG_USB_OHCI_PCI', if_true: files('hcd-ohci-pci.c'))
+system_ss.add(when: 'CONFIG_USB_OHCI_SYSBUS', if_true: files('hcd-ohci-sysbus.c'))
 system_ss.add(when: 'CONFIG_USB_EHCI', if_true: files('hcd-ehci.c'))
 system_ss.add(when: 'CONFIG_USB_EHCI_PCI', if_true: files('hcd-ehci-pci.c'))
 system_ss.add(when: 'CONFIG_USB_EHCI_SYSBUS', if_true: files('hcd-ehci.c', 'hcd-ehci-sysbus.c'))
-- 
2.43.0
Re: [PATCH 09/10] usb: extract sysbus-ohci to a separate file
Posted by Thomas Huth 9 months ago
On 23/02/2024 13.44, Paolo Bonzini wrote:
> Split the sysbus version to a separate file so that it is not
> included in PCI-only machines, and adjust Kconfig for machines
> that do need sysbus-ohci.  The copyrights are based on the
> time and employer of balrog and Paul Brook's contributions.
> 
> While adjusting the SM501 dependency, move it to the right place
> instead of keeping it in the R4D machine.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   hw/usb/hcd-ohci-sysbus.c | 91 ++++++++++++++++++++++++++++++++++++++++
>   hw/usb/hcd-ohci.c        | 58 ------------------------
>   hw/arm/Kconfig           | 12 +++--
>   hw/display/Kconfig       |  1 +
>   hw/ppc/Kconfig           |  2 +-
>   hw/sh4/Kconfig           |  1 -
>   hw/usb/Kconfig           |  4 ++
>   hw/usb/meson.build       |  1 +
>   8 files changed, 105 insertions(+), 65 deletions(-)
>   create mode 100644 hw/usb/hcd-ohci-sysbus.c
> 
> diff --git a/hw/usb/hcd-ohci-sysbus.c b/hw/usb/hcd-ohci-sysbus.c
> new file mode 100644
> index 00000000000..4e4481232b6
> --- /dev/null
> +++ b/hw/usb/hcd-ohci-sysbus.c
> @@ -0,0 +1,91 @@
> +/*
> + * QEMU USB OHCI Emulation
> + * Copyright (c) 2006 Openedhand Ltd.
> + * Copyright (c) 2010 CodeSourcery
> + * Copyright (c) 2024 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/irq.h"
> +#include "qapi/error.h"
> +#include "qemu/module.h"
> +#include "qemu/timer.h"
> +#include "hw/usb.h"
> +#include "migration/vmstate.h"
> +#include "hw/sysbus.h"
> +#include "hw/qdev-dma.h"
> +#include "hw/qdev-properties.h"
> +#include "trace.h"
> +#include "hcd-ohci.h"
> +
> +
> +static void ohci_realize_pxa(DeviceState *dev, Error **errp)

Maybe this could be renamed to ohci_sysbs_realize() now since the code is 
used in non-pxa devices nowadays, too?

Anyway:
Reviewed-by: Thomas Huth <thuth@redhat.com>
Re: [PATCH 09/10] usb: extract sysbus-ohci to a separate file
Posted by Philippe Mathieu-Daudé 9 months ago
On 23/2/24 13:44, Paolo Bonzini wrote:
> Split the sysbus version to a separate file so that it is not
> included in PCI-only machines, and adjust Kconfig for machines
> that do need sysbus-ohci.  The copyrights are based on the
> time and employer of balrog and Paul Brook's contributions.
> 
> While adjusting the SM501 dependency, move it to the right place
> instead of keeping it in the R4D machine.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   hw/usb/hcd-ohci-sysbus.c | 91 ++++++++++++++++++++++++++++++++++++++++
>   hw/usb/hcd-ohci.c        | 58 ------------------------
>   hw/arm/Kconfig           | 12 +++--
>   hw/display/Kconfig       |  1 +
>   hw/ppc/Kconfig           |  2 +-
>   hw/sh4/Kconfig           |  1 -
>   hw/usb/Kconfig           |  4 ++
>   hw/usb/meson.build       |  1 +
>   8 files changed, 105 insertions(+), 65 deletions(-)
>   create mode 100644 hw/usb/hcd-ohci-sysbus.c


> +static void ohci_realize_pxa(DeviceState *dev, Error **errp)

s/ohci_realize_pxa/ohci_sysbus_realize/

> +{
> +    OHCISysBusState *s = SYSBUS_OHCI(dev);
> +    SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +    Error *err = NULL;
> +
> +    usb_ohci_init(&s->ohci, dev, s->num_ports, s->dma_offset,
> +                  s->masterbus, s->firstport,
> +                  &address_space_memory, ohci_sysbus_die, &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    sysbus_init_irq(sbd, &s->ohci.irq);
> +    sysbus_init_mmio(sbd, &s->ohci.mem);
> +}
> +
> +static void usb_ohci_reset_sysbus(DeviceState *dev)

s/usb_ohci_reset_sysbus/ohci_sysbus/reset/.

To be converted to Resettable API.

> +{
> +    OHCISysBusState *s = SYSBUS_OHCI(dev);
> +    OHCIState *ohci = &s->ohci;
> +
> +    ohci_hard_reset(ohci);
> +}


> +static void ohci_register_types(void)
> +{
> +    type_register_static(&ohci_sysbus_info);
> +}
> +
> +type_init(ohci_register_types)

Better directly use DEFINE_TYPES() in new uses, even for
a single type.


> diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
> index 0f486764ed6..f569ed7eeaa 100644
> --- a/hw/usb/Kconfig
> +++ b/hw/usb/Kconfig
> @@ -11,6 +11,10 @@ config USB_OHCI
>       bool
>       select USB
>   
> +config USB_OHCI_SYSBUS
> +    bool
> +    select USB_OHCI

I start to think USB_OHCI_MMIO would be clearer. I can
clean that up later, so:

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

>   config USB_OHCI_PCI
>       bool
>       default y if PCI_DEVICES