Various machine/board/soc models create UHCI device instances
with the generic QDEV API, and don't need to access USB internals.
Simplify header inclusions by moving the QOM type names into a
simple header, with no need to include other "hw/usb" headers.
Suggested-by: BALATON Zoltan <balaton@eik.bme.hu>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/usb/usb-hcd.h | 6 ++++++
hw/i386/pc_piix.c | 3 ++-
hw/i386/pc_q35.c | 13 +++++++------
hw/isa/piix4.c | 3 ++-
hw/mips/fuloong2e.c | 5 +++--
hw/usb/hcd-uhci.c | 19 ++++++++++---------
6 files changed, 30 insertions(+), 19 deletions(-)
diff --git a/include/hw/usb/usb-hcd.h b/include/hw/usb/usb-hcd.h
index 74af3a4533..c9d0a88984 100644
--- a/include/hw/usb/usb-hcd.h
+++ b/include/hw/usb/usb-hcd.h
@@ -24,4 +24,10 @@
#define TYPE_FUSBH200_EHCI "fusbh200-ehci-usb"
#define TYPE_CHIPIDEA "usb-chipidea"
+/* UHCI */
+#define TYPE_PIIX3_USB_UHCI "piix3-usb-uhci"
+#define TYPE_PIIX4_USB_UHCI "piix4-usb-uhci"
+#define TYPE_VT82C686B_USB_UHCI "vt82c686b-usb-uhci"
+#define TYPE_ICH9_USB_UHCI(n) "ich9-usb-uhci" #n
+
#endif
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 4d1de7cfab..0024c346c6 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -37,6 +37,7 @@
#include "hw/pci/pci.h"
#include "hw/pci/pci_ids.h"
#include "hw/usb/usb.h"
+#include "hw/usb/usb-hcd.h"
#include "net/net.h"
#include "hw/ide/pci.h"
#include "hw/irq.h"
@@ -275,7 +276,7 @@ static void pc_init1(MachineState *machine,
#endif
if (pcmc->pci_enabled && machine_usb(machine)) {
- pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
+ pci_create_simple(pci_bus, piix3_devfn + 2, TYPE_PIIX3_USB_UHCI);
}
if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index b985f5bea1..a80527e6ed 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -51,6 +51,7 @@
#include "hw/ide/pci.h"
#include "hw/ide/ahci.h"
#include "hw/usb/usb.h"
+#include "hw/usb/usb-hcd.h"
#include "qapi/error.h"
#include "qemu/error-report.h"
#include "sysemu/numa.h"
@@ -68,15 +69,15 @@ struct ehci_companions {
};
static const struct ehci_companions ich9_1d[] = {
- { .name = "ich9-usb-uhci1", .func = 0, .port = 0 },
- { .name = "ich9-usb-uhci2", .func = 1, .port = 2 },
- { .name = "ich9-usb-uhci3", .func = 2, .port = 4 },
+ { .name = TYPE_ICH9_USB_UHCI(1), .func = 0, .port = 0 },
+ { .name = TYPE_ICH9_USB_UHCI(2), .func = 1, .port = 2 },
+ { .name = TYPE_ICH9_USB_UHCI(3), .func = 2, .port = 4 },
};
static const struct ehci_companions ich9_1a[] = {
- { .name = "ich9-usb-uhci4", .func = 0, .port = 0 },
- { .name = "ich9-usb-uhci5", .func = 1, .port = 2 },
- { .name = "ich9-usb-uhci6", .func = 2, .port = 4 },
+ { .name = TYPE_ICH9_USB_UHCI(4), .func = 0, .port = 0 },
+ { .name = TYPE_ICH9_USB_UHCI(5), .func = 1, .port = 2 },
+ { .name = TYPE_ICH9_USB_UHCI(6), .func = 2, .port = 4 },
};
static int ehci_create_ich9_with_companions(PCIBus *bus, int slot)
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index f634bcb2d1..e11e5fae21 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -29,6 +29,7 @@
#include "hw/southbridge/piix.h"
#include "hw/pci/pci.h"
#include "hw/isa/isa.h"
+#include "hw/usb/usb-hcd.h"
#include "hw/sysbus.h"
#include "hw/intc/i8259.h"
#include "hw/dma/i8257.h"
@@ -255,7 +256,7 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus)
pci = pci_create_simple(pci_bus, devfn + 1, "piix4-ide");
pci_ide_create_devs(pci);
- pci_create_simple(pci_bus, devfn + 2, "piix4-usb-uhci");
+ pci_create_simple(pci_bus, devfn + 2, TYPE_PIIX4_USB_UHCI);
if (smbus) {
*smbus = piix4_pm_init(pci_bus, devfn + 3, 0x1100,
isa_get_irq(NULL, 9), NULL, 0, NULL);
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index 8ca31e5162..b6d33dd2cd 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -33,6 +33,7 @@
#include "hw/mips/mips.h"
#include "hw/mips/cpudevs.h"
#include "hw/pci/pci.h"
+#include "hw/usb/usb-hcd.h"
#include "qemu/log.h"
#include "hw/loader.h"
#include "hw/ide/pci.h"
@@ -258,8 +259,8 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
pci_ide_create_devs(dev);
- pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
- pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
+ pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), TYPE_VT82C686B_USB_UHCI);
+ pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), TYPE_VT82C686B_USB_UHCI);
*i2c_bus = vt82c686b_pm_init(pci_bus, PCI_DEVFN(slot, 4), 0xeee1, NULL);
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 1d4dd33b6c..da078dc3fa 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -39,6 +39,7 @@
#include "qemu/main-loop.h"
#include "qemu/module.h"
#include "usb-internal.h"
+#include "hw/usb/usb-hcd.h"
#define FRAME_TIMER_FREQ 1000
@@ -1358,21 +1359,21 @@ static void uhci_data_class_init(ObjectClass *klass, void *data)
static UHCIInfo uhci_info[] = {
{
- .name = "piix3-usb-uhci",
+ .name = TYPE_PIIX3_USB_UHCI,
.vendor_id = PCI_VENDOR_ID_INTEL,
.device_id = PCI_DEVICE_ID_INTEL_82371SB_2,
.revision = 0x01,
.irq_pin = 3,
.unplug = true,
},{
- .name = "piix4-usb-uhci",
+ .name = TYPE_PIIX4_USB_UHCI,
.vendor_id = PCI_VENDOR_ID_INTEL,
.device_id = PCI_DEVICE_ID_INTEL_82371AB_2,
.revision = 0x01,
.irq_pin = 3,
.unplug = true,
},{
- .name = "vt82c686b-usb-uhci",
+ .name = TYPE_VT82C686B_USB_UHCI,
.vendor_id = PCI_VENDOR_ID_VIA,
.device_id = PCI_DEVICE_ID_VIA_UHCI,
.revision = 0x01,
@@ -1380,42 +1381,42 @@ static UHCIInfo uhci_info[] = {
.realize = usb_uhci_vt82c686b_realize,
.unplug = true,
},{
- .name = "ich9-usb-uhci1", /* 00:1d.0 */
+ .name = TYPE_ICH9_USB_UHCI(1), /* 00:1d.0 */
.vendor_id = PCI_VENDOR_ID_INTEL,
.device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI1,
.revision = 0x03,
.irq_pin = 0,
.unplug = false,
},{
- .name = "ich9-usb-uhci2", /* 00:1d.1 */
+ .name = TYPE_ICH9_USB_UHCI(2), /* 00:1d.1 */
.vendor_id = PCI_VENDOR_ID_INTEL,
.device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI2,
.revision = 0x03,
.irq_pin = 1,
.unplug = false,
},{
- .name = "ich9-usb-uhci3", /* 00:1d.2 */
+ .name = TYPE_ICH9_USB_UHCI(3), /* 00:1d.2 */
.vendor_id = PCI_VENDOR_ID_INTEL,
.device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI3,
.revision = 0x03,
.irq_pin = 2,
.unplug = false,
},{
- .name = "ich9-usb-uhci4", /* 00:1a.0 */
+ .name = TYPE_ICH9_USB_UHCI(4), /* 00:1a.0 */
.vendor_id = PCI_VENDOR_ID_INTEL,
.device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI4,
.revision = 0x03,
.irq_pin = 0,
.unplug = false,
},{
- .name = "ich9-usb-uhci5", /* 00:1a.1 */
+ .name = TYPE_ICH9_USB_UHCI(5), /* 00:1a.1 */
.vendor_id = PCI_VENDOR_ID_INTEL,
.device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI5,
.revision = 0x03,
.irq_pin = 1,
.unplug = false,
},{
- .name = "ich9-usb-uhci6", /* 00:1a.2 */
+ .name = TYPE_ICH9_USB_UHCI(6), /* 00:1a.2 */
.vendor_id = PCI_VENDOR_ID_INTEL,
.device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI6,
.revision = 0x03,
--
2.21.3
On Sat, 4 Jul 2020, Philippe Mathieu-Daudé wrote:
> Various machine/board/soc models create UHCI device instances
> with the generic QDEV API, and don't need to access USB internals.
>
> Simplify header inclusions by moving the QOM type names into a
> simple header, with no need to include other "hw/usb" headers.
>
> Suggested-by: BALATON Zoltan <balaton@eik.bme.hu>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/usb/usb-hcd.h | 6 ++++++
> hw/i386/pc_piix.c | 3 ++-
> hw/i386/pc_q35.c | 13 +++++++------
> hw/isa/piix4.c | 3 ++-
> hw/mips/fuloong2e.c | 5 +++--
> hw/usb/hcd-uhci.c | 19 ++++++++++---------
> 6 files changed, 30 insertions(+), 19 deletions(-)
>
> diff --git a/include/hw/usb/usb-hcd.h b/include/hw/usb/usb-hcd.h
> index 74af3a4533..c9d0a88984 100644
> --- a/include/hw/usb/usb-hcd.h
> +++ b/include/hw/usb/usb-hcd.h
> @@ -24,4 +24,10 @@
> #define TYPE_FUSBH200_EHCI "fusbh200-ehci-usb"
> #define TYPE_CHIPIDEA "usb-chipidea"
>
> +/* UHCI */
> +#define TYPE_PIIX3_USB_UHCI "piix3-usb-uhci"
> +#define TYPE_PIIX4_USB_UHCI "piix4-usb-uhci"
> +#define TYPE_VT82C686B_USB_UHCI "vt82c686b-usb-uhci"
> +#define TYPE_ICH9_USB_UHCI(n) "ich9-usb-uhci" #n
What is that #n at the end? Looks like a typo. Does it break compilation?
Regards,
BALATON Zoltan
> +
> #endif
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 4d1de7cfab..0024c346c6 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -37,6 +37,7 @@
> #include "hw/pci/pci.h"
> #include "hw/pci/pci_ids.h"
> #include "hw/usb/usb.h"
> +#include "hw/usb/usb-hcd.h"
> #include "net/net.h"
> #include "hw/ide/pci.h"
> #include "hw/irq.h"
> @@ -275,7 +276,7 @@ static void pc_init1(MachineState *machine,
> #endif
>
> if (pcmc->pci_enabled && machine_usb(machine)) {
> - pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
> + pci_create_simple(pci_bus, piix3_devfn + 2, TYPE_PIIX3_USB_UHCI);
> }
>
> if (pcmc->pci_enabled && x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index b985f5bea1..a80527e6ed 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -51,6 +51,7 @@
> #include "hw/ide/pci.h"
> #include "hw/ide/ahci.h"
> #include "hw/usb/usb.h"
> +#include "hw/usb/usb-hcd.h"
> #include "qapi/error.h"
> #include "qemu/error-report.h"
> #include "sysemu/numa.h"
> @@ -68,15 +69,15 @@ struct ehci_companions {
> };
>
> static const struct ehci_companions ich9_1d[] = {
> - { .name = "ich9-usb-uhci1", .func = 0, .port = 0 },
> - { .name = "ich9-usb-uhci2", .func = 1, .port = 2 },
> - { .name = "ich9-usb-uhci3", .func = 2, .port = 4 },
> + { .name = TYPE_ICH9_USB_UHCI(1), .func = 0, .port = 0 },
> + { .name = TYPE_ICH9_USB_UHCI(2), .func = 1, .port = 2 },
> + { .name = TYPE_ICH9_USB_UHCI(3), .func = 2, .port = 4 },
> };
>
> static const struct ehci_companions ich9_1a[] = {
> - { .name = "ich9-usb-uhci4", .func = 0, .port = 0 },
> - { .name = "ich9-usb-uhci5", .func = 1, .port = 2 },
> - { .name = "ich9-usb-uhci6", .func = 2, .port = 4 },
> + { .name = TYPE_ICH9_USB_UHCI(4), .func = 0, .port = 0 },
> + { .name = TYPE_ICH9_USB_UHCI(5), .func = 1, .port = 2 },
> + { .name = TYPE_ICH9_USB_UHCI(6), .func = 2, .port = 4 },
> };
>
> static int ehci_create_ich9_with_companions(PCIBus *bus, int slot)
> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
> index f634bcb2d1..e11e5fae21 100644
> --- a/hw/isa/piix4.c
> +++ b/hw/isa/piix4.c
> @@ -29,6 +29,7 @@
> #include "hw/southbridge/piix.h"
> #include "hw/pci/pci.h"
> #include "hw/isa/isa.h"
> +#include "hw/usb/usb-hcd.h"
> #include "hw/sysbus.h"
> #include "hw/intc/i8259.h"
> #include "hw/dma/i8257.h"
> @@ -255,7 +256,7 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus **isa_bus, I2CBus **smbus)
> pci = pci_create_simple(pci_bus, devfn + 1, "piix4-ide");
> pci_ide_create_devs(pci);
>
> - pci_create_simple(pci_bus, devfn + 2, "piix4-usb-uhci");
> + pci_create_simple(pci_bus, devfn + 2, TYPE_PIIX4_USB_UHCI);
> if (smbus) {
> *smbus = piix4_pm_init(pci_bus, devfn + 3, 0x1100,
> isa_get_irq(NULL, 9), NULL, 0, NULL);
> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> index 8ca31e5162..b6d33dd2cd 100644
> --- a/hw/mips/fuloong2e.c
> +++ b/hw/mips/fuloong2e.c
> @@ -33,6 +33,7 @@
> #include "hw/mips/mips.h"
> #include "hw/mips/cpudevs.h"
> #include "hw/pci/pci.h"
> +#include "hw/usb/usb-hcd.h"
> #include "qemu/log.h"
> #include "hw/loader.h"
> #include "hw/ide/pci.h"
> @@ -258,8 +259,8 @@ static void vt82c686b_southbridge_init(PCIBus *pci_bus, int slot, qemu_irq intc,
> dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
> pci_ide_create_devs(dev);
>
> - pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), "vt82c686b-usb-uhci");
> - pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), "vt82c686b-usb-uhci");
> + pci_create_simple(pci_bus, PCI_DEVFN(slot, 2), TYPE_VT82C686B_USB_UHCI);
> + pci_create_simple(pci_bus, PCI_DEVFN(slot, 3), TYPE_VT82C686B_USB_UHCI);
>
> *i2c_bus = vt82c686b_pm_init(pci_bus, PCI_DEVFN(slot, 4), 0xeee1, NULL);
>
> diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
> index 1d4dd33b6c..da078dc3fa 100644
> --- a/hw/usb/hcd-uhci.c
> +++ b/hw/usb/hcd-uhci.c
> @@ -39,6 +39,7 @@
> #include "qemu/main-loop.h"
> #include "qemu/module.h"
> #include "usb-internal.h"
> +#include "hw/usb/usb-hcd.h"
>
> #define FRAME_TIMER_FREQ 1000
>
> @@ -1358,21 +1359,21 @@ static void uhci_data_class_init(ObjectClass *klass, void *data)
>
> static UHCIInfo uhci_info[] = {
> {
> - .name = "piix3-usb-uhci",
> + .name = TYPE_PIIX3_USB_UHCI,
> .vendor_id = PCI_VENDOR_ID_INTEL,
> .device_id = PCI_DEVICE_ID_INTEL_82371SB_2,
> .revision = 0x01,
> .irq_pin = 3,
> .unplug = true,
> },{
> - .name = "piix4-usb-uhci",
> + .name = TYPE_PIIX4_USB_UHCI,
> .vendor_id = PCI_VENDOR_ID_INTEL,
> .device_id = PCI_DEVICE_ID_INTEL_82371AB_2,
> .revision = 0x01,
> .irq_pin = 3,
> .unplug = true,
> },{
> - .name = "vt82c686b-usb-uhci",
> + .name = TYPE_VT82C686B_USB_UHCI,
> .vendor_id = PCI_VENDOR_ID_VIA,
> .device_id = PCI_DEVICE_ID_VIA_UHCI,
> .revision = 0x01,
> @@ -1380,42 +1381,42 @@ static UHCIInfo uhci_info[] = {
> .realize = usb_uhci_vt82c686b_realize,
> .unplug = true,
> },{
> - .name = "ich9-usb-uhci1", /* 00:1d.0 */
> + .name = TYPE_ICH9_USB_UHCI(1), /* 00:1d.0 */
> .vendor_id = PCI_VENDOR_ID_INTEL,
> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI1,
> .revision = 0x03,
> .irq_pin = 0,
> .unplug = false,
> },{
> - .name = "ich9-usb-uhci2", /* 00:1d.1 */
> + .name = TYPE_ICH9_USB_UHCI(2), /* 00:1d.1 */
> .vendor_id = PCI_VENDOR_ID_INTEL,
> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI2,
> .revision = 0x03,
> .irq_pin = 1,
> .unplug = false,
> },{
> - .name = "ich9-usb-uhci3", /* 00:1d.2 */
> + .name = TYPE_ICH9_USB_UHCI(3), /* 00:1d.2 */
> .vendor_id = PCI_VENDOR_ID_INTEL,
> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI3,
> .revision = 0x03,
> .irq_pin = 2,
> .unplug = false,
> },{
> - .name = "ich9-usb-uhci4", /* 00:1a.0 */
> + .name = TYPE_ICH9_USB_UHCI(4), /* 00:1a.0 */
> .vendor_id = PCI_VENDOR_ID_INTEL,
> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI4,
> .revision = 0x03,
> .irq_pin = 0,
> .unplug = false,
> },{
> - .name = "ich9-usb-uhci5", /* 00:1a.1 */
> + .name = TYPE_ICH9_USB_UHCI(5), /* 00:1a.1 */
> .vendor_id = PCI_VENDOR_ID_INTEL,
> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI5,
> .revision = 0x03,
> .irq_pin = 1,
> .unplug = false,
> },{
> - .name = "ich9-usb-uhci6", /* 00:1a.2 */
> + .name = TYPE_ICH9_USB_UHCI(6), /* 00:1a.2 */
> .vendor_id = PCI_VENDOR_ID_INTEL,
> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI6,
> .revision = 0x03,
>
On 7/4/20 7:17 PM, BALATON Zoltan wrote:
> On Sat, 4 Jul 2020, Philippe Mathieu-Daudé wrote:
>> Various machine/board/soc models create UHCI device instances
>> with the generic QDEV API, and don't need to access USB internals.
>>
>> Simplify header inclusions by moving the QOM type names into a
>> simple header, with no need to include other "hw/usb" headers.
>>
>> Suggested-by: BALATON Zoltan <balaton@eik.bme.hu>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> include/hw/usb/usb-hcd.h | 6 ++++++
>> hw/i386/pc_piix.c | 3 ++-
>> hw/i386/pc_q35.c | 13 +++++++------
>> hw/isa/piix4.c | 3 ++-
>> hw/mips/fuloong2e.c | 5 +++--
>> hw/usb/hcd-uhci.c | 19 ++++++++++---------
>> 6 files changed, 30 insertions(+), 19 deletions(-)
>>
>> diff --git a/include/hw/usb/usb-hcd.h b/include/hw/usb/usb-hcd.h
>> index 74af3a4533..c9d0a88984 100644
>> --- a/include/hw/usb/usb-hcd.h
>> +++ b/include/hw/usb/usb-hcd.h
>> @@ -24,4 +24,10 @@
>> #define TYPE_FUSBH200_EHCI "fusbh200-ehci-usb"
>> #define TYPE_CHIPIDEA "usb-chipidea"
>>
>> +/* UHCI */
>> +#define TYPE_PIIX3_USB_UHCI "piix3-usb-uhci"
>> +#define TYPE_PIIX4_USB_UHCI "piix4-usb-uhci"
>> +#define TYPE_VT82C686B_USB_UHCI "vt82c686b-usb-uhci"
>> +#define TYPE_ICH9_USB_UHCI(n) "ich9-usb-uhci" #n
>
> What is that #n at the end? Looks like a typo. Does it break compilation?
#n is a C preprocessor feature that expand the 'n' argument to a "n"
string, so:
TYPE_ICH9_USB_UHCI(1) = "ich9-usb-uhci" #1
= "ich9-usb-uhci" "1"
= "ich9-usb-uhci1"
I'm pretty sure we use that elsewhere. If not, I can add a definition
for each 1 ... 6 typenames.
>
> Regards,
> BALATON Zoltan
>
>> +
>> #endif
>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>> index 4d1de7cfab..0024c346c6 100644
>> --- a/hw/i386/pc_piix.c
>> +++ b/hw/i386/pc_piix.c
>> @@ -37,6 +37,7 @@
>> #include "hw/pci/pci.h"
>> #include "hw/pci/pci_ids.h"
>> #include "hw/usb/usb.h"
>> +#include "hw/usb/usb-hcd.h"
>> #include "net/net.h"
>> #include "hw/ide/pci.h"
>> #include "hw/irq.h"
>> @@ -275,7 +276,7 @@ static void pc_init1(MachineState *machine,
>> #endif
>>
>> if (pcmc->pci_enabled && machine_usb(machine)) {
>> - pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
>> + pci_create_simple(pci_bus, piix3_devfn + 2,
>> TYPE_PIIX3_USB_UHCI);
>> }
>>
>> if (pcmc->pci_enabled &&
>> x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>> index b985f5bea1..a80527e6ed 100644
>> --- a/hw/i386/pc_q35.c
>> +++ b/hw/i386/pc_q35.c
>> @@ -51,6 +51,7 @@
>> #include "hw/ide/pci.h"
>> #include "hw/ide/ahci.h"
>> #include "hw/usb/usb.h"
>> +#include "hw/usb/usb-hcd.h"
>> #include "qapi/error.h"
>> #include "qemu/error-report.h"
>> #include "sysemu/numa.h"
>> @@ -68,15 +69,15 @@ struct ehci_companions {
>> };
>>
>> static const struct ehci_companions ich9_1d[] = {
>> - { .name = "ich9-usb-uhci1", .func = 0, .port = 0 },
>> - { .name = "ich9-usb-uhci2", .func = 1, .port = 2 },
>> - { .name = "ich9-usb-uhci3", .func = 2, .port = 4 },
>> + { .name = TYPE_ICH9_USB_UHCI(1), .func = 0, .port = 0 },
>> + { .name = TYPE_ICH9_USB_UHCI(2), .func = 1, .port = 2 },
>> + { .name = TYPE_ICH9_USB_UHCI(3), .func = 2, .port = 4 },
>> };
>>
>> static const struct ehci_companions ich9_1a[] = {
>> - { .name = "ich9-usb-uhci4", .func = 0, .port = 0 },
>> - { .name = "ich9-usb-uhci5", .func = 1, .port = 2 },
>> - { .name = "ich9-usb-uhci6", .func = 2, .port = 4 },
>> + { .name = TYPE_ICH9_USB_UHCI(4), .func = 0, .port = 0 },
>> + { .name = TYPE_ICH9_USB_UHCI(5), .func = 1, .port = 2 },
>> + { .name = TYPE_ICH9_USB_UHCI(6), .func = 2, .port = 4 },
>> };
>>
>> static int ehci_create_ich9_with_companions(PCIBus *bus, int slot)
>> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
>> index f634bcb2d1..e11e5fae21 100644
>> --- a/hw/isa/piix4.c
>> +++ b/hw/isa/piix4.c
>> @@ -29,6 +29,7 @@
>> #include "hw/southbridge/piix.h"
>> #include "hw/pci/pci.h"
>> #include "hw/isa/isa.h"
>> +#include "hw/usb/usb-hcd.h"
>> #include "hw/sysbus.h"
>> #include "hw/intc/i8259.h"
>> #include "hw/dma/i8257.h"
>> @@ -255,7 +256,7 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
>> **isa_bus, I2CBus **smbus)
>> pci = pci_create_simple(pci_bus, devfn + 1, "piix4-ide");
>> pci_ide_create_devs(pci);
>>
>> - pci_create_simple(pci_bus, devfn + 2, "piix4-usb-uhci");
>> + pci_create_simple(pci_bus, devfn + 2, TYPE_PIIX4_USB_UHCI);
>> if (smbus) {
>> *smbus = piix4_pm_init(pci_bus, devfn + 3, 0x1100,
>> isa_get_irq(NULL, 9), NULL, 0, NULL);
>> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
>> index 8ca31e5162..b6d33dd2cd 100644
>> --- a/hw/mips/fuloong2e.c
>> +++ b/hw/mips/fuloong2e.c
>> @@ -33,6 +33,7 @@
>> #include "hw/mips/mips.h"
>> #include "hw/mips/cpudevs.h"
>> #include "hw/pci/pci.h"
>> +#include "hw/usb/usb-hcd.h"
>> #include "qemu/log.h"
>> #include "hw/loader.h"
>> #include "hw/ide/pci.h"
>> @@ -258,8 +259,8 @@ static void vt82c686b_southbridge_init(PCIBus
>> *pci_bus, int slot, qemu_irq intc,
>> dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
>> pci_ide_create_devs(dev);
>>
>> - pci_create_simple(pci_bus, PCI_DEVFN(slot, 2),
>> "vt82c686b-usb-uhci");
>> - pci_create_simple(pci_bus, PCI_DEVFN(slot, 3),
>> "vt82c686b-usb-uhci");
>> + pci_create_simple(pci_bus, PCI_DEVFN(slot, 2),
>> TYPE_VT82C686B_USB_UHCI);
>> + pci_create_simple(pci_bus, PCI_DEVFN(slot, 3),
>> TYPE_VT82C686B_USB_UHCI);
>>
>> *i2c_bus = vt82c686b_pm_init(pci_bus, PCI_DEVFN(slot, 4), 0xeee1,
>> NULL);
>>
>> diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
>> index 1d4dd33b6c..da078dc3fa 100644
>> --- a/hw/usb/hcd-uhci.c
>> +++ b/hw/usb/hcd-uhci.c
>> @@ -39,6 +39,7 @@
>> #include "qemu/main-loop.h"
>> #include "qemu/module.h"
>> #include "usb-internal.h"
>> +#include "hw/usb/usb-hcd.h"
>>
>> #define FRAME_TIMER_FREQ 1000
>>
>> @@ -1358,21 +1359,21 @@ static void uhci_data_class_init(ObjectClass
>> *klass, void *data)
>>
>> static UHCIInfo uhci_info[] = {
>> {
>> - .name = "piix3-usb-uhci",
>> + .name = TYPE_PIIX3_USB_UHCI,
>> .vendor_id = PCI_VENDOR_ID_INTEL,
>> .device_id = PCI_DEVICE_ID_INTEL_82371SB_2,
>> .revision = 0x01,
>> .irq_pin = 3,
>> .unplug = true,
>> },{
>> - .name = "piix4-usb-uhci",
>> + .name = TYPE_PIIX4_USB_UHCI,
>> .vendor_id = PCI_VENDOR_ID_INTEL,
>> .device_id = PCI_DEVICE_ID_INTEL_82371AB_2,
>> .revision = 0x01,
>> .irq_pin = 3,
>> .unplug = true,
>> },{
>> - .name = "vt82c686b-usb-uhci",
>> + .name = TYPE_VT82C686B_USB_UHCI,
>> .vendor_id = PCI_VENDOR_ID_VIA,
>> .device_id = PCI_DEVICE_ID_VIA_UHCI,
>> .revision = 0x01,
>> @@ -1380,42 +1381,42 @@ static UHCIInfo uhci_info[] = {
>> .realize = usb_uhci_vt82c686b_realize,
>> .unplug = true,
>> },{
>> - .name = "ich9-usb-uhci1", /* 00:1d.0 */
>> + .name = TYPE_ICH9_USB_UHCI(1), /* 00:1d.0 */
>> .vendor_id = PCI_VENDOR_ID_INTEL,
>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI1,
>> .revision = 0x03,
>> .irq_pin = 0,
>> .unplug = false,
>> },{
>> - .name = "ich9-usb-uhci2", /* 00:1d.1 */
>> + .name = TYPE_ICH9_USB_UHCI(2), /* 00:1d.1 */
>> .vendor_id = PCI_VENDOR_ID_INTEL,
>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI2,
>> .revision = 0x03,
>> .irq_pin = 1,
>> .unplug = false,
>> },{
>> - .name = "ich9-usb-uhci3", /* 00:1d.2 */
>> + .name = TYPE_ICH9_USB_UHCI(3), /* 00:1d.2 */
>> .vendor_id = PCI_VENDOR_ID_INTEL,
>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI3,
>> .revision = 0x03,
>> .irq_pin = 2,
>> .unplug = false,
>> },{
>> - .name = "ich9-usb-uhci4", /* 00:1a.0 */
>> + .name = TYPE_ICH9_USB_UHCI(4), /* 00:1a.0 */
>> .vendor_id = PCI_VENDOR_ID_INTEL,
>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI4,
>> .revision = 0x03,
>> .irq_pin = 0,
>> .unplug = false,
>> },{
>> - .name = "ich9-usb-uhci5", /* 00:1a.1 */
>> + .name = TYPE_ICH9_USB_UHCI(5), /* 00:1a.1 */
>> .vendor_id = PCI_VENDOR_ID_INTEL,
>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI5,
>> .revision = 0x03,
>> .irq_pin = 1,
>> .unplug = false,
>> },{
>> - .name = "ich9-usb-uhci6", /* 00:1a.2 */
>> + .name = TYPE_ICH9_USB_UHCI(6), /* 00:1a.2 */
>> .vendor_id = PCI_VENDOR_ID_INTEL,
>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI6,
>> .revision = 0x03,
>>
On Sat, 4 Jul 2020, Philippe Mathieu-Daudé wrote:
> On 7/4/20 7:17 PM, BALATON Zoltan wrote:
>> On Sat, 4 Jul 2020, Philippe Mathieu-Daudé wrote:
>>> Various machine/board/soc models create UHCI device instances
>>> with the generic QDEV API, and don't need to access USB internals.
>>>
>>> Simplify header inclusions by moving the QOM type names into a
>>> simple header, with no need to include other "hw/usb" headers.
>>>
>>> Suggested-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>> include/hw/usb/usb-hcd.h | 6 ++++++
>>> hw/i386/pc_piix.c | 3 ++-
>>> hw/i386/pc_q35.c | 13 +++++++------
>>> hw/isa/piix4.c | 3 ++-
>>> hw/mips/fuloong2e.c | 5 +++--
>>> hw/usb/hcd-uhci.c | 19 ++++++++++---------
>>> 6 files changed, 30 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/include/hw/usb/usb-hcd.h b/include/hw/usb/usb-hcd.h
>>> index 74af3a4533..c9d0a88984 100644
>>> --- a/include/hw/usb/usb-hcd.h
>>> +++ b/include/hw/usb/usb-hcd.h
>>> @@ -24,4 +24,10 @@
>>> #define TYPE_FUSBH200_EHCI "fusbh200-ehci-usb"
>>> #define TYPE_CHIPIDEA "usb-chipidea"
>>>
>>> +/* UHCI */
>>> +#define TYPE_PIIX3_USB_UHCI "piix3-usb-uhci"
>>> +#define TYPE_PIIX4_USB_UHCI "piix4-usb-uhci"
>>> +#define TYPE_VT82C686B_USB_UHCI "vt82c686b-usb-uhci"
>>> +#define TYPE_ICH9_USB_UHCI(n) "ich9-usb-uhci" #n
>>
>> What is that #n at the end? Looks like a typo. Does it break compilation?
>
> #n is a C preprocessor feature that expand the 'n' argument to a "n"
> string, so:
>
> TYPE_ICH9_USB_UHCI(1) = "ich9-usb-uhci" #1
> = "ich9-usb-uhci" "1"
> = "ich9-usb-uhci1"
>
> I'm pretty sure we use that elsewhere. If not, I can add a definition
> for each 1 ... 6 typenames.
No it's OK, no need to list all defines. I just did not notice the macro
argument that's why I was wondering where it comes from. This seems to be
used elsewhere at least here:
hw/audio/es1370.c:#define a(n) if (val & CTRL_##n) strcat (buf, " "#n)
hw/audio/es1370.c:#define a(n) if (val & SCTRL_##n) strcat (buf, " "#n)
hw/audio/es1370.c:#define b(n) if (!(val & SCTRL_##n)) strcat (buf, " "#n)
Maybe writing it without the space between "# is clearer as then it looks
more like it's part of the value.
Regards,
BALATON Zoltan
>>> +
>>> #endif
>>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>>> index 4d1de7cfab..0024c346c6 100644
>>> --- a/hw/i386/pc_piix.c
>>> +++ b/hw/i386/pc_piix.c
>>> @@ -37,6 +37,7 @@
>>> #include "hw/pci/pci.h"
>>> #include "hw/pci/pci_ids.h"
>>> #include "hw/usb/usb.h"
>>> +#include "hw/usb/usb-hcd.h"
>>> #include "net/net.h"
>>> #include "hw/ide/pci.h"
>>> #include "hw/irq.h"
>>> @@ -275,7 +276,7 @@ static void pc_init1(MachineState *machine,
>>> #endif
>>>
>>> if (pcmc->pci_enabled && machine_usb(machine)) {
>>> - pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
>>> + pci_create_simple(pci_bus, piix3_devfn + 2,
>>> TYPE_PIIX3_USB_UHCI);
>>> }
>>>
>>> if (pcmc->pci_enabled &&
>>> x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
>>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>>> index b985f5bea1..a80527e6ed 100644
>>> --- a/hw/i386/pc_q35.c
>>> +++ b/hw/i386/pc_q35.c
>>> @@ -51,6 +51,7 @@
>>> #include "hw/ide/pci.h"
>>> #include "hw/ide/ahci.h"
>>> #include "hw/usb/usb.h"
>>> +#include "hw/usb/usb-hcd.h"
>>> #include "qapi/error.h"
>>> #include "qemu/error-report.h"
>>> #include "sysemu/numa.h"
>>> @@ -68,15 +69,15 @@ struct ehci_companions {
>>> };
>>>
>>> static const struct ehci_companions ich9_1d[] = {
>>> - { .name = "ich9-usb-uhci1", .func = 0, .port = 0 },
>>> - { .name = "ich9-usb-uhci2", .func = 1, .port = 2 },
>>> - { .name = "ich9-usb-uhci3", .func = 2, .port = 4 },
>>> + { .name = TYPE_ICH9_USB_UHCI(1), .func = 0, .port = 0 },
>>> + { .name = TYPE_ICH9_USB_UHCI(2), .func = 1, .port = 2 },
>>> + { .name = TYPE_ICH9_USB_UHCI(3), .func = 2, .port = 4 },
>>> };
>>>
>>> static const struct ehci_companions ich9_1a[] = {
>>> - { .name = "ich9-usb-uhci4", .func = 0, .port = 0 },
>>> - { .name = "ich9-usb-uhci5", .func = 1, .port = 2 },
>>> - { .name = "ich9-usb-uhci6", .func = 2, .port = 4 },
>>> + { .name = TYPE_ICH9_USB_UHCI(4), .func = 0, .port = 0 },
>>> + { .name = TYPE_ICH9_USB_UHCI(5), .func = 1, .port = 2 },
>>> + { .name = TYPE_ICH9_USB_UHCI(6), .func = 2, .port = 4 },
>>> };
>>>
>>> static int ehci_create_ich9_with_companions(PCIBus *bus, int slot)
>>> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
>>> index f634bcb2d1..e11e5fae21 100644
>>> --- a/hw/isa/piix4.c
>>> +++ b/hw/isa/piix4.c
>>> @@ -29,6 +29,7 @@
>>> #include "hw/southbridge/piix.h"
>>> #include "hw/pci/pci.h"
>>> #include "hw/isa/isa.h"
>>> +#include "hw/usb/usb-hcd.h"
>>> #include "hw/sysbus.h"
>>> #include "hw/intc/i8259.h"
>>> #include "hw/dma/i8257.h"
>>> @@ -255,7 +256,7 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
>>> **isa_bus, I2CBus **smbus)
>>> pci = pci_create_simple(pci_bus, devfn + 1, "piix4-ide");
>>> pci_ide_create_devs(pci);
>>>
>>> - pci_create_simple(pci_bus, devfn + 2, "piix4-usb-uhci");
>>> + pci_create_simple(pci_bus, devfn + 2, TYPE_PIIX4_USB_UHCI);
>>> if (smbus) {
>>> *smbus = piix4_pm_init(pci_bus, devfn + 3, 0x1100,
>>> isa_get_irq(NULL, 9), NULL, 0, NULL);
>>> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
>>> index 8ca31e5162..b6d33dd2cd 100644
>>> --- a/hw/mips/fuloong2e.c
>>> +++ b/hw/mips/fuloong2e.c
>>> @@ -33,6 +33,7 @@
>>> #include "hw/mips/mips.h"
>>> #include "hw/mips/cpudevs.h"
>>> #include "hw/pci/pci.h"
>>> +#include "hw/usb/usb-hcd.h"
>>> #include "qemu/log.h"
>>> #include "hw/loader.h"
>>> #include "hw/ide/pci.h"
>>> @@ -258,8 +259,8 @@ static void vt82c686b_southbridge_init(PCIBus
>>> *pci_bus, int slot, qemu_irq intc,
>>> dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
>>> pci_ide_create_devs(dev);
>>>
>>> - pci_create_simple(pci_bus, PCI_DEVFN(slot, 2),
>>> "vt82c686b-usb-uhci");
>>> - pci_create_simple(pci_bus, PCI_DEVFN(slot, 3),
>>> "vt82c686b-usb-uhci");
>>> + pci_create_simple(pci_bus, PCI_DEVFN(slot, 2),
>>> TYPE_VT82C686B_USB_UHCI);
>>> + pci_create_simple(pci_bus, PCI_DEVFN(slot, 3),
>>> TYPE_VT82C686B_USB_UHCI);
>>>
>>> *i2c_bus = vt82c686b_pm_init(pci_bus, PCI_DEVFN(slot, 4), 0xeee1,
>>> NULL);
>>>
>>> diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
>>> index 1d4dd33b6c..da078dc3fa 100644
>>> --- a/hw/usb/hcd-uhci.c
>>> +++ b/hw/usb/hcd-uhci.c
>>> @@ -39,6 +39,7 @@
>>> #include "qemu/main-loop.h"
>>> #include "qemu/module.h"
>>> #include "usb-internal.h"
>>> +#include "hw/usb/usb-hcd.h"
>>>
>>> #define FRAME_TIMER_FREQ 1000
>>>
>>> @@ -1358,21 +1359,21 @@ static void uhci_data_class_init(ObjectClass
>>> *klass, void *data)
>>>
>>> static UHCIInfo uhci_info[] = {
>>> {
>>> - .name = "piix3-usb-uhci",
>>> + .name = TYPE_PIIX3_USB_UHCI,
>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>> .device_id = PCI_DEVICE_ID_INTEL_82371SB_2,
>>> .revision = 0x01,
>>> .irq_pin = 3,
>>> .unplug = true,
>>> },{
>>> - .name = "piix4-usb-uhci",
>>> + .name = TYPE_PIIX4_USB_UHCI,
>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>> .device_id = PCI_DEVICE_ID_INTEL_82371AB_2,
>>> .revision = 0x01,
>>> .irq_pin = 3,
>>> .unplug = true,
>>> },{
>>> - .name = "vt82c686b-usb-uhci",
>>> + .name = TYPE_VT82C686B_USB_UHCI,
>>> .vendor_id = PCI_VENDOR_ID_VIA,
>>> .device_id = PCI_DEVICE_ID_VIA_UHCI,
>>> .revision = 0x01,
>>> @@ -1380,42 +1381,42 @@ static UHCIInfo uhci_info[] = {
>>> .realize = usb_uhci_vt82c686b_realize,
>>> .unplug = true,
>>> },{
>>> - .name = "ich9-usb-uhci1", /* 00:1d.0 */
>>> + .name = TYPE_ICH9_USB_UHCI(1), /* 00:1d.0 */
>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI1,
>>> .revision = 0x03,
>>> .irq_pin = 0,
>>> .unplug = false,
>>> },{
>>> - .name = "ich9-usb-uhci2", /* 00:1d.1 */
>>> + .name = TYPE_ICH9_USB_UHCI(2), /* 00:1d.1 */
>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI2,
>>> .revision = 0x03,
>>> .irq_pin = 1,
>>> .unplug = false,
>>> },{
>>> - .name = "ich9-usb-uhci3", /* 00:1d.2 */
>>> + .name = TYPE_ICH9_USB_UHCI(3), /* 00:1d.2 */
>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI3,
>>> .revision = 0x03,
>>> .irq_pin = 2,
>>> .unplug = false,
>>> },{
>>> - .name = "ich9-usb-uhci4", /* 00:1a.0 */
>>> + .name = TYPE_ICH9_USB_UHCI(4), /* 00:1a.0 */
>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI4,
>>> .revision = 0x03,
>>> .irq_pin = 0,
>>> .unplug = false,
>>> },{
>>> - .name = "ich9-usb-uhci5", /* 00:1a.1 */
>>> + .name = TYPE_ICH9_USB_UHCI(5), /* 00:1a.1 */
>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI5,
>>> .revision = 0x03,
>>> .irq_pin = 1,
>>> .unplug = false,
>>> },{
>>> - .name = "ich9-usb-uhci6", /* 00:1a.2 */
>>> + .name = TYPE_ICH9_USB_UHCI(6), /* 00:1a.2 */
>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI6,
>>> .revision = 0x03,
>>>
>
>
On 7/4/20 9:44 PM, BALATON Zoltan wrote:
> On Sat, 4 Jul 2020, Philippe Mathieu-Daudé wrote:
>> On 7/4/20 7:17 PM, BALATON Zoltan wrote:
>>> On Sat, 4 Jul 2020, Philippe Mathieu-Daudé wrote:
>>>> Various machine/board/soc models create UHCI device instances
>>>> with the generic QDEV API, and don't need to access USB internals.
>>>>
>>>> Simplify header inclusions by moving the QOM type names into a
>>>> simple header, with no need to include other "hw/usb" headers.
>>>>
>>>> Suggested-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> ---
>>>> include/hw/usb/usb-hcd.h | 6 ++++++
>>>> hw/i386/pc_piix.c | 3 ++-
>>>> hw/i386/pc_q35.c | 13 +++++++------
>>>> hw/isa/piix4.c | 3 ++-
>>>> hw/mips/fuloong2e.c | 5 +++--
>>>> hw/usb/hcd-uhci.c | 19 ++++++++++---------
>>>> 6 files changed, 30 insertions(+), 19 deletions(-)
>>>>
>>>> diff --git a/include/hw/usb/usb-hcd.h b/include/hw/usb/usb-hcd.h
>>>> index 74af3a4533..c9d0a88984 100644
>>>> --- a/include/hw/usb/usb-hcd.h
>>>> +++ b/include/hw/usb/usb-hcd.h
>>>> @@ -24,4 +24,10 @@
>>>> #define TYPE_FUSBH200_EHCI "fusbh200-ehci-usb"
>>>> #define TYPE_CHIPIDEA "usb-chipidea"
>>>>
>>>> +/* UHCI */
>>>> +#define TYPE_PIIX3_USB_UHCI "piix3-usb-uhci"
>>>> +#define TYPE_PIIX4_USB_UHCI "piix4-usb-uhci"
>>>> +#define TYPE_VT82C686B_USB_UHCI "vt82c686b-usb-uhci"
>>>> +#define TYPE_ICH9_USB_UHCI(n) "ich9-usb-uhci" #n
>>>
>>> What is that #n at the end? Looks like a typo. Does it break
>>> compilation?
>>
>> #n is a C preprocessor feature that expand the 'n' argument to a "n"
>> string, so:
>>
>> TYPE_ICH9_USB_UHCI(1) = "ich9-usb-uhci" #1
>> = "ich9-usb-uhci" "1"
>> = "ich9-usb-uhci1"
>>
>> I'm pretty sure we use that elsewhere. If not, I can add a definition
>> for each 1 ... 6 typenames.
>
> No it's OK, no need to list all defines. I just did not notice the macro
> argument that's why I was wondering where it comes from. This seems to
> be used elsewhere at least here:
>
> hw/audio/es1370.c:#define a(n) if (val & CTRL_##n) strcat (buf, " "#n)
> hw/audio/es1370.c:#define a(n) if (val & SCTRL_##n) strcat (buf, " "#n)
> hw/audio/es1370.c:#define b(n) if (!(val & SCTRL_##n)) strcat (buf, " "#n)
>
> Maybe writing it without the space between "# is clearer as then it
> looks more like it's part of the value.
Ah clever indeed. Thanks!
>
> Regards,
> BALATON Zoltan
>
>>>> +
>>>> #endif
>>>> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
>>>> index 4d1de7cfab..0024c346c6 100644
>>>> --- a/hw/i386/pc_piix.c
>>>> +++ b/hw/i386/pc_piix.c
>>>> @@ -37,6 +37,7 @@
>>>> #include "hw/pci/pci.h"
>>>> #include "hw/pci/pci_ids.h"
>>>> #include "hw/usb/usb.h"
>>>> +#include "hw/usb/usb-hcd.h"
>>>> #include "net/net.h"
>>>> #include "hw/ide/pci.h"
>>>> #include "hw/irq.h"
>>>> @@ -275,7 +276,7 @@ static void pc_init1(MachineState *machine,
>>>> #endif
>>>>
>>>> if (pcmc->pci_enabled && machine_usb(machine)) {
>>>> - pci_create_simple(pci_bus, piix3_devfn + 2, "piix3-usb-uhci");
>>>> + pci_create_simple(pci_bus, piix3_devfn + 2,
>>>> TYPE_PIIX3_USB_UHCI);
>>>> }
>>>>
>>>> if (pcmc->pci_enabled &&
>>>> x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
>>>> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
>>>> index b985f5bea1..a80527e6ed 100644
>>>> --- a/hw/i386/pc_q35.c
>>>> +++ b/hw/i386/pc_q35.c
>>>> @@ -51,6 +51,7 @@
>>>> #include "hw/ide/pci.h"
>>>> #include "hw/ide/ahci.h"
>>>> #include "hw/usb/usb.h"
>>>> +#include "hw/usb/usb-hcd.h"
>>>> #include "qapi/error.h"
>>>> #include "qemu/error-report.h"
>>>> #include "sysemu/numa.h"
>>>> @@ -68,15 +69,15 @@ struct ehci_companions {
>>>> };
>>>>
>>>> static const struct ehci_companions ich9_1d[] = {
>>>> - { .name = "ich9-usb-uhci1", .func = 0, .port = 0 },
>>>> - { .name = "ich9-usb-uhci2", .func = 1, .port = 2 },
>>>> - { .name = "ich9-usb-uhci3", .func = 2, .port = 4 },
>>>> + { .name = TYPE_ICH9_USB_UHCI(1), .func = 0, .port = 0 },
>>>> + { .name = TYPE_ICH9_USB_UHCI(2), .func = 1, .port = 2 },
>>>> + { .name = TYPE_ICH9_USB_UHCI(3), .func = 2, .port = 4 },
>>>> };
>>>>
>>>> static const struct ehci_companions ich9_1a[] = {
>>>> - { .name = "ich9-usb-uhci4", .func = 0, .port = 0 },
>>>> - { .name = "ich9-usb-uhci5", .func = 1, .port = 2 },
>>>> - { .name = "ich9-usb-uhci6", .func = 2, .port = 4 },
>>>> + { .name = TYPE_ICH9_USB_UHCI(4), .func = 0, .port = 0 },
>>>> + { .name = TYPE_ICH9_USB_UHCI(5), .func = 1, .port = 2 },
>>>> + { .name = TYPE_ICH9_USB_UHCI(6), .func = 2, .port = 4 },
>>>> };
>>>>
>>>> static int ehci_create_ich9_with_companions(PCIBus *bus, int slot)
>>>> diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
>>>> index f634bcb2d1..e11e5fae21 100644
>>>> --- a/hw/isa/piix4.c
>>>> +++ b/hw/isa/piix4.c
>>>> @@ -29,6 +29,7 @@
>>>> #include "hw/southbridge/piix.h"
>>>> #include "hw/pci/pci.h"
>>>> #include "hw/isa/isa.h"
>>>> +#include "hw/usb/usb-hcd.h"
>>>> #include "hw/sysbus.h"
>>>> #include "hw/intc/i8259.h"
>>>> #include "hw/dma/i8257.h"
>>>> @@ -255,7 +256,7 @@ DeviceState *piix4_create(PCIBus *pci_bus, ISABus
>>>> **isa_bus, I2CBus **smbus)
>>>> pci = pci_create_simple(pci_bus, devfn + 1, "piix4-ide");
>>>> pci_ide_create_devs(pci);
>>>>
>>>> - pci_create_simple(pci_bus, devfn + 2, "piix4-usb-uhci");
>>>> + pci_create_simple(pci_bus, devfn + 2, TYPE_PIIX4_USB_UHCI);
>>>> if (smbus) {
>>>> *smbus = piix4_pm_init(pci_bus, devfn + 3, 0x1100,
>>>> isa_get_irq(NULL, 9), NULL, 0, NULL);
>>>> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
>>>> index 8ca31e5162..b6d33dd2cd 100644
>>>> --- a/hw/mips/fuloong2e.c
>>>> +++ b/hw/mips/fuloong2e.c
>>>> @@ -33,6 +33,7 @@
>>>> #include "hw/mips/mips.h"
>>>> #include "hw/mips/cpudevs.h"
>>>> #include "hw/pci/pci.h"
>>>> +#include "hw/usb/usb-hcd.h"
>>>> #include "qemu/log.h"
>>>> #include "hw/loader.h"
>>>> #include "hw/ide/pci.h"
>>>> @@ -258,8 +259,8 @@ static void vt82c686b_southbridge_init(PCIBus
>>>> *pci_bus, int slot, qemu_irq intc,
>>>> dev = pci_create_simple(pci_bus, PCI_DEVFN(slot, 1), "via-ide");
>>>> pci_ide_create_devs(dev);
>>>>
>>>> - pci_create_simple(pci_bus, PCI_DEVFN(slot, 2),
>>>> "vt82c686b-usb-uhci");
>>>> - pci_create_simple(pci_bus, PCI_DEVFN(slot, 3),
>>>> "vt82c686b-usb-uhci");
>>>> + pci_create_simple(pci_bus, PCI_DEVFN(slot, 2),
>>>> TYPE_VT82C686B_USB_UHCI);
>>>> + pci_create_simple(pci_bus, PCI_DEVFN(slot, 3),
>>>> TYPE_VT82C686B_USB_UHCI);
>>>>
>>>> *i2c_bus = vt82c686b_pm_init(pci_bus, PCI_DEVFN(slot, 4), 0xeee1,
>>>> NULL);
>>>>
>>>> diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
>>>> index 1d4dd33b6c..da078dc3fa 100644
>>>> --- a/hw/usb/hcd-uhci.c
>>>> +++ b/hw/usb/hcd-uhci.c
>>>> @@ -39,6 +39,7 @@
>>>> #include "qemu/main-loop.h"
>>>> #include "qemu/module.h"
>>>> #include "usb-internal.h"
>>>> +#include "hw/usb/usb-hcd.h"
>>>>
>>>> #define FRAME_TIMER_FREQ 1000
>>>>
>>>> @@ -1358,21 +1359,21 @@ static void uhci_data_class_init(ObjectClass
>>>> *klass, void *data)
>>>>
>>>> static UHCIInfo uhci_info[] = {
>>>> {
>>>> - .name = "piix3-usb-uhci",
>>>> + .name = TYPE_PIIX3_USB_UHCI,
>>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>>> .device_id = PCI_DEVICE_ID_INTEL_82371SB_2,
>>>> .revision = 0x01,
>>>> .irq_pin = 3,
>>>> .unplug = true,
>>>> },{
>>>> - .name = "piix4-usb-uhci",
>>>> + .name = TYPE_PIIX4_USB_UHCI,
>>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>>> .device_id = PCI_DEVICE_ID_INTEL_82371AB_2,
>>>> .revision = 0x01,
>>>> .irq_pin = 3,
>>>> .unplug = true,
>>>> },{
>>>> - .name = "vt82c686b-usb-uhci",
>>>> + .name = TYPE_VT82C686B_USB_UHCI,
>>>> .vendor_id = PCI_VENDOR_ID_VIA,
>>>> .device_id = PCI_DEVICE_ID_VIA_UHCI,
>>>> .revision = 0x01,
>>>> @@ -1380,42 +1381,42 @@ static UHCIInfo uhci_info[] = {
>>>> .realize = usb_uhci_vt82c686b_realize,
>>>> .unplug = true,
>>>> },{
>>>> - .name = "ich9-usb-uhci1", /* 00:1d.0 */
>>>> + .name = TYPE_ICH9_USB_UHCI(1), /* 00:1d.0 */
>>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI1,
>>>> .revision = 0x03,
>>>> .irq_pin = 0,
>>>> .unplug = false,
>>>> },{
>>>> - .name = "ich9-usb-uhci2", /* 00:1d.1 */
>>>> + .name = TYPE_ICH9_USB_UHCI(2), /* 00:1d.1 */
>>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI2,
>>>> .revision = 0x03,
>>>> .irq_pin = 1,
>>>> .unplug = false,
>>>> },{
>>>> - .name = "ich9-usb-uhci3", /* 00:1d.2 */
>>>> + .name = TYPE_ICH9_USB_UHCI(3), /* 00:1d.2 */
>>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI3,
>>>> .revision = 0x03,
>>>> .irq_pin = 2,
>>>> .unplug = false,
>>>> },{
>>>> - .name = "ich9-usb-uhci4", /* 00:1a.0 */
>>>> + .name = TYPE_ICH9_USB_UHCI(4), /* 00:1a.0 */
>>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI4,
>>>> .revision = 0x03,
>>>> .irq_pin = 0,
>>>> .unplug = false,
>>>> },{
>>>> - .name = "ich9-usb-uhci5", /* 00:1a.1 */
>>>> + .name = TYPE_ICH9_USB_UHCI(5), /* 00:1a.1 */
>>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI5,
>>>> .revision = 0x03,
>>>> .irq_pin = 1,
>>>> .unplug = false,
>>>> },{
>>>> - .name = "ich9-usb-uhci6", /* 00:1a.2 */
>>>> + .name = TYPE_ICH9_USB_UHCI(6), /* 00:1a.2 */
>>>> .vendor_id = PCI_VENDOR_ID_INTEL,
>>>> .device_id = PCI_DEVICE_ID_INTEL_82801I_UHCI6,
>>>> .revision = 0x03,
>>>>
>>
>>
On 04/07/20 21:44, BALATON Zoltan wrote: > > No it's OK, no need to list all defines. I just did not notice the macro > argument that's why I was wondering where it comes from. This seems to > be used elsewhere at least here: > > hw/audio/es1370.c:#define a(n) if (val & CTRL_##n) strcat (buf, " "#n) > hw/audio/es1370.c:#define a(n) if (val & SCTRL_##n) strcat (buf, " "#n) > hw/audio/es1370.c:#define b(n) if (!(val & SCTRL_##n)) strcat (buf, " "#n) > > Maybe writing it without the space between "# is clearer as then it > looks more like it's part of the value. I think keeping the space is better. The reason is that CTRL_##n pastes together CTRL_ and n, but " "#n is different: 1) First, it turns n into a string, for example "1" 2) Then, just because there is another string just before, the two are concatenated, as in "CTRL_" "1". Paolo
© 2016 - 2026 Red Hat, Inc.