Extract the VT82C686 PCI UHCI function into a new unit so
it is only build when the VT82C686 south bridge is selected.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/usb/hcd-uhci.c | 23 --------------------
hw/usb/vt82c686-uhci-pci.c | 43 ++++++++++++++++++++++++++++++++++++++
MAINTAINERS | 1 +
hw/usb/meson.build | 1 +
4 files changed, 45 insertions(+), 23 deletions(-)
create mode 100644 hw/usb/vt82c686-uhci-pci.c
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index d6338c33d86..0cb02a64321 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -1207,21 +1207,6 @@ void usb_uhci_common_realize(PCIDevice *dev, Error **errp)
pci_register_bar(&s->dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &s->io_bar);
}
-static void usb_uhci_vt82c686b_realize(PCIDevice *dev, Error **errp)
-{
- UHCIState *s = UHCI(dev);
- uint8_t *pci_conf = s->dev.config;
-
- /* USB misc control 1/2 */
- pci_set_long(pci_conf + 0x40,0x00001000);
- /* PM capability */
- pci_set_long(pci_conf + 0x80,0x00020001);
- /* USB legacy support */
- pci_set_long(pci_conf + 0xc0,0x00002000);
-
- usb_uhci_common_realize(dev, errp);
-}
-
static void usb_uhci_exit(PCIDevice *dev)
{
UHCIState *s = UHCI(dev);
@@ -1318,14 +1303,6 @@ static UHCIInfo uhci_info[] = {
.revision = 0x01,
.irq_pin = 3,
.unplug = true,
- },{
- .name = "vt82c686b-usb-uhci",
- .vendor_id = PCI_VENDOR_ID_VIA,
- .device_id = PCI_DEVICE_ID_VIA_UHCI,
- .revision = 0x01,
- .irq_pin = 3,
- .realize = usb_uhci_vt82c686b_realize,
- .unplug = true,
},{
.name = "ich9-usb-uhci1", /* 00:1d.0 */
.vendor_id = PCI_VENDOR_ID_INTEL,
diff --git a/hw/usb/vt82c686-uhci-pci.c b/hw/usb/vt82c686-uhci-pci.c
new file mode 100644
index 00000000000..b109c216033
--- /dev/null
+++ b/hw/usb/vt82c686-uhci-pci.c
@@ -0,0 +1,43 @@
+#include "qemu/osdep.h"
+#include "hcd-uhci.h"
+
+static void usb_uhci_vt82c686b_realize(PCIDevice *dev, Error **errp)
+{
+ UHCIState *s = UHCI(dev);
+ uint8_t *pci_conf = s->dev.config;
+
+ /* USB misc control 1/2 */
+ pci_set_long(pci_conf + 0x40, 0x00001000);
+ /* PM capability */
+ pci_set_long(pci_conf + 0x80, 0x00020001);
+ /* USB legacy support */
+ pci_set_long(pci_conf + 0xc0, 0x00002000);
+
+ usb_uhci_common_realize(dev, errp);
+}
+
+static UHCIInfo uhci_info[] = {
+ {
+ .name = "vt82c686b-usb-uhci",
+ .vendor_id = PCI_VENDOR_ID_VIA,
+ .device_id = PCI_DEVICE_ID_VIA_UHCI,
+ .revision = 0x01,
+ .irq_pin = 3,
+ .realize = usb_uhci_vt82c686b_realize,
+ .unplug = true,
+ }
+};
+
+static const TypeInfo vt82c686b_usb_uhci_type_info = {
+ .parent = TYPE_UHCI,
+ .name = "vt82c686b-usb-uhci",
+ .class_init = uhci_data_class_init,
+ .class_data = uhci_info,
+};
+
+static void vt82c686b_usb_uhci_register_types(void)
+{
+ type_register_static(&vt82c686b_usb_uhci_type_info);
+}
+
+type_init(vt82c686b_usb_uhci_register_types)
diff --git a/MAINTAINERS b/MAINTAINERS
index f22d83c1782..6fd55c0a40c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1173,6 +1173,7 @@ S: Odd Fixes
F: hw/mips/fuloong2e.c
F: hw/isa/vt82c686.c
F: hw/pci-host/bonito.c
+F: hw/usb/vt82c686-uhci-pci.c
F: include/hw/isa/vt82c686.h
Loongson-3 virtual platforms
diff --git a/hw/usb/meson.build b/hw/usb/meson.build
index 653192cff6f..6e3159798e9 100644
--- a/hw/usb/meson.build
+++ b/hw/usb/meson.build
@@ -32,6 +32,7 @@
softmmu_ss.add(when: 'CONFIG_TUSB6010', if_true: files('tusb6010.c'))
softmmu_ss.add(when: 'CONFIG_IMX', if_true: files('chipidea.c'))
softmmu_ss.add(when: 'CONFIG_IMX_USBPHY', if_true: files('imx-usb-phy.c'))
+softmmu_ss.add(when: 'CONFIG_VT82C686', if_true: files('vt82c686-uhci-pci.c'))
specific_ss.add(when: 'CONFIG_XLNX_VERSAL', if_true: files('xlnx-versal-usb2-ctrl-regs.c'))
specific_ss.add(when: 'CONFIG_XLNX_USB_SUBSYS', if_true: files('xlnx-usb-subsystem.c'))
--
2.26.2
On Tue, 9 Mar 2021, Philippe Mathieu-Daudé wrote:
> Extract the VT82C686 PCI UHCI function into a new unit so
> it is only build when the VT82C686 south bridge is selected.
I'm not sure it's worth separating just this one device from the other
similar usb devices when the others that are also part of south bridge
chips are left there. Maybe you could just set user_creatable = false so
it can only be created as part of the chips that contain it or just don't
bother and leave it as it is which is the least likely to break anything
that may rely on it as removing it from the device list may need to go
through deprecation.
But I don't really mind, so if others like this approach I don't want to
block the patch. I think it's unlikely anybody would use this device other
than part of fuloong2e or pegasos2 so probably it's unlikely to break
anything if it suddenly goes away from a new build.
Regards,
BALATON Zoltan
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/usb/hcd-uhci.c | 23 --------------------
> hw/usb/vt82c686-uhci-pci.c | 43 ++++++++++++++++++++++++++++++++++++++
> MAINTAINERS | 1 +
> hw/usb/meson.build | 1 +
> 4 files changed, 45 insertions(+), 23 deletions(-)
> create mode 100644 hw/usb/vt82c686-uhci-pci.c
>
> diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
> index d6338c33d86..0cb02a64321 100644
> --- a/hw/usb/hcd-uhci.c
> +++ b/hw/usb/hcd-uhci.c
> @@ -1207,21 +1207,6 @@ void usb_uhci_common_realize(PCIDevice *dev, Error **errp)
> pci_register_bar(&s->dev, 4, PCI_BASE_ADDRESS_SPACE_IO, &s->io_bar);
> }
>
> -static void usb_uhci_vt82c686b_realize(PCIDevice *dev, Error **errp)
> -{
> - UHCIState *s = UHCI(dev);
> - uint8_t *pci_conf = s->dev.config;
> -
> - /* USB misc control 1/2 */
> - pci_set_long(pci_conf + 0x40,0x00001000);
> - /* PM capability */
> - pci_set_long(pci_conf + 0x80,0x00020001);
> - /* USB legacy support */
> - pci_set_long(pci_conf + 0xc0,0x00002000);
> -
> - usb_uhci_common_realize(dev, errp);
> -}
> -
> static void usb_uhci_exit(PCIDevice *dev)
> {
> UHCIState *s = UHCI(dev);
> @@ -1318,14 +1303,6 @@ static UHCIInfo uhci_info[] = {
> .revision = 0x01,
> .irq_pin = 3,
> .unplug = true,
> - },{
> - .name = "vt82c686b-usb-uhci",
> - .vendor_id = PCI_VENDOR_ID_VIA,
> - .device_id = PCI_DEVICE_ID_VIA_UHCI,
> - .revision = 0x01,
> - .irq_pin = 3,
> - .realize = usb_uhci_vt82c686b_realize,
> - .unplug = true,
> },{
> .name = "ich9-usb-uhci1", /* 00:1d.0 */
> .vendor_id = PCI_VENDOR_ID_INTEL,
> diff --git a/hw/usb/vt82c686-uhci-pci.c b/hw/usb/vt82c686-uhci-pci.c
> new file mode 100644
> index 00000000000..b109c216033
> --- /dev/null
> +++ b/hw/usb/vt82c686-uhci-pci.c
> @@ -0,0 +1,43 @@
> +#include "qemu/osdep.h"
> +#include "hcd-uhci.h"
> +
> +static void usb_uhci_vt82c686b_realize(PCIDevice *dev, Error **errp)
> +{
> + UHCIState *s = UHCI(dev);
> + uint8_t *pci_conf = s->dev.config;
> +
> + /* USB misc control 1/2 */
> + pci_set_long(pci_conf + 0x40, 0x00001000);
> + /* PM capability */
> + pci_set_long(pci_conf + 0x80, 0x00020001);
> + /* USB legacy support */
> + pci_set_long(pci_conf + 0xc0, 0x00002000);
> +
> + usb_uhci_common_realize(dev, errp);
> +}
> +
> +static UHCIInfo uhci_info[] = {
> + {
> + .name = "vt82c686b-usb-uhci",
> + .vendor_id = PCI_VENDOR_ID_VIA,
> + .device_id = PCI_DEVICE_ID_VIA_UHCI,
> + .revision = 0x01,
> + .irq_pin = 3,
> + .realize = usb_uhci_vt82c686b_realize,
> + .unplug = true,
> + }
> +};
> +
> +static const TypeInfo vt82c686b_usb_uhci_type_info = {
> + .parent = TYPE_UHCI,
> + .name = "vt82c686b-usb-uhci",
> + .class_init = uhci_data_class_init,
> + .class_data = uhci_info,
> +};
> +
> +static void vt82c686b_usb_uhci_register_types(void)
> +{
> + type_register_static(&vt82c686b_usb_uhci_type_info);
> +}
> +
> +type_init(vt82c686b_usb_uhci_register_types)
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f22d83c1782..6fd55c0a40c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1173,6 +1173,7 @@ S: Odd Fixes
> F: hw/mips/fuloong2e.c
> F: hw/isa/vt82c686.c
> F: hw/pci-host/bonito.c
> +F: hw/usb/vt82c686-uhci-pci.c
> F: include/hw/isa/vt82c686.h
>
> Loongson-3 virtual platforms
> diff --git a/hw/usb/meson.build b/hw/usb/meson.build
> index 653192cff6f..6e3159798e9 100644
> --- a/hw/usb/meson.build
> +++ b/hw/usb/meson.build
> @@ -32,6 +32,7 @@
> softmmu_ss.add(when: 'CONFIG_TUSB6010', if_true: files('tusb6010.c'))
> softmmu_ss.add(when: 'CONFIG_IMX', if_true: files('chipidea.c'))
> softmmu_ss.add(when: 'CONFIG_IMX_USBPHY', if_true: files('imx-usb-phy.c'))
> +softmmu_ss.add(when: 'CONFIG_VT82C686', if_true: files('vt82c686-uhci-pci.c'))
> specific_ss.add(when: 'CONFIG_XLNX_VERSAL', if_true: files('xlnx-versal-usb2-ctrl-regs.c'))
> specific_ss.add(when: 'CONFIG_XLNX_USB_SUBSYS', if_true: files('xlnx-usb-subsystem.c'))
>
>
On 3/9/21 9:13 PM, BALATON Zoltan wrote: > On Tue, 9 Mar 2021, Philippe Mathieu-Daudé wrote: >> Extract the VT82C686 PCI UHCI function into a new unit so >> it is only build when the VT82C686 south bridge is selected. > > I'm not sure it's worth separating just this one device from the other > similar usb devices when the others that are also part of south bridge > chips are left there. Maybe you could just set user_creatable = false so > it can only be created as part of the chips that contain it or just > don't bother and leave it as it is which is the least likely to break > anything that may rely on it as removing it from the device list may > need to go through deprecation. > > But I don't really mind, so if others like this approach I don't want to > block the patch. I think it's unlikely anybody would use this device > other than part of fuloong2e or pegasos2 so probably it's unlikely to > break anything if it suddenly goes away from a new build. OK. As I sent this series to help you with your Pegasos2 board, do you mind sending a patch then? I suppose you mean using "#include CONFIG_DEVICES" and checking for the CONFIG_VT82C686 symbol to register the QOM type? Thanks, Phil.
On Wed, 10 Mar 2021, Philippe Mathieu-Daudé wrote: > On 3/9/21 9:13 PM, BALATON Zoltan wrote: >> On Tue, 9 Mar 2021, Philippe Mathieu-Daudé wrote: >>> Extract the VT82C686 PCI UHCI function into a new unit so >>> it is only build when the VT82C686 south bridge is selected. >> >> I'm not sure it's worth separating just this one device from the other >> similar usb devices when the others that are also part of south bridge >> chips are left there. Maybe you could just set user_creatable = false so >> it can only be created as part of the chips that contain it or just >> don't bother and leave it as it is which is the least likely to break >> anything that may rely on it as removing it from the device list may >> need to go through deprecation. >> >> But I don't really mind, so if others like this approach I don't want to >> block the patch. I think it's unlikely anybody would use this device >> other than part of fuloong2e or pegasos2 so probably it's unlikely to >> break anything if it suddenly goes away from a new build. > > OK. As I sent this series to help you with your Pegasos2 board, do you > mind sending a patch then? > > I suppose you mean using "#include CONFIG_DEVICES" and checking for the > CONFIG_VT82C686 symbol to register the QOM type? I don't see how this should help with the pegasos2 series as that's already working without this change. I really just meant I don't see this would improve much when you leave the piix and ich ones there that are the same for other south bridges so this patch makes one of these configurable but not the others, that makes them different when they are now similar. Additionally it may break something as it changes the availability of this device (although that's very unlikely to cause problems in practice). So I'd just leave this alone for now and not change it at all until all of these are cleaned up in a uniform way at some later point if that's deemed useful or desirable. Maybe I'm missing the point though. But while pegasos2 does not need this change it should also not break it so I don't really mind. Regards, BALATON Zoltan
© 2016 - 2025 Red Hat, Inc.