On 19/2/24 19:24, BALATON Zoltan wrote:
On Mon, 19 Feb 2024, BALATON Zoltan wrote:
On Mon, 19 Feb 2024, Philippe Mathieu-Daudé wrote:
Expose TYPE_ICH_DMI_PCI_BRIDGE to the new
"hw/pci-bridge/ich_dmi_pci.h" header.
Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
MAINTAINERS | 1 +
include/hw/pci-bridge/ich_dmi_pci.h | 20 ++++++++++++++++++++
include/hw/southbridge/ich9.h | 2 --
hw/pci-bridge/i82801b11.c | 11 ++++-------
4 files changed, 25 insertions(+), 9 deletions(-)
create mode 100644 include/hw/pci-bridge/ich_dmi_pci.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 1b210c5cc1..50507c3dd6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2609,6 +2609,7 @@ F: hw/acpi/ich9*.c
F: hw/i2c/smbus_ich9.c
F: hw/isa/lpc_ich9.c
F: include/hw/acpi/ich9*.h
+F: include/hw/pci-bridge/ich_dmi_pci.h
F: include/hw/southbridge/ich9.h
PIIX4 South Bridge (i82371AB)
diff --git a/include/hw/pci-bridge/ich_dmi_pci.h
b/include/hw/pci-bridge/ich_dmi_pci.h
new file mode 100644
index 0000000000..7623b32b8e
--- /dev/null
+++ b/include/hw/pci-bridge/ich_dmi_pci.h
@@ -0,0 +1,20 @@
+/*
+ * QEMU ICH4 i82801b11 dmi-to-pci Bridge Emulation
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef HW_PCI_BRIDGE_ICH_D2P_H
+#define HW_PCI_BRIDGE_ICH_D2P_H
+
+#include "qom/object.h"
+#include "hw/pci/pci_bridge.h"
+
+#define TYPE_ICH_DMI_PCI_BRIDGE "i82801b11-bridge"
+OBJECT_DECLARE_SIMPLE_TYPE(I82801b11Bridge, ICH_DMI_PCI_BRIDGE)
+
+struct I82801b11Bridge {
+ PCIBridge parent_obj;
+};
If this class has no fields of its own why does it need its own state
struct defined? You could just set .instance_size = sizeof(PCIBridge)
in the TypeInfo i82801b11_bridge_info below and delete this struct
completely as it's not even used anywhere. One less needless QOM
complication :-) For an example see the empty via-mc97 device in
hw/audio/via-ac97.c.
Then you can put the OBJECT_DECLARE_SIMPLE_TYPE in
hw/pci-bridge/i82801b11.c where this object is defined and the #define
TYPE_ICH_DMI_PCI_BRIDGE in
You don't even need OBJECT_DECLARE_SIMPLE_TYPE if there's no state
struct. But on second look what is this object at all? It's never
instantiated anywhere. Is it used somewhere?
Here my view is we should always define QOM type names in headers
and use them, in particular in the TypeInfo registration. To unify
style and copy/pasting, better use the QOM DECLARE_TYPE macros.
I envision that might help moving toward DSL and have HW modelling
checks done externally, before starting QEMU. But then this is my
view and I dunno about when we'll get that DSL in so I'm OK to
revisit this patch.
Regards,
BALATON Zoltan
hw/southbridge/ich9.h and then you don't need this header at all so
you don't end up with:
4 files changed, 25 insertions(+), 9 deletions(-)
but really simplifying it.
Regards,
BALATON Zoltan
+
+#endif
diff --git a/include/hw/southbridge/ich9.h
b/include/hw/southbridge/ich9.h
index bee522a4cf..b2abf483e0 100644
--- a/include/hw/southbridge/ich9.h
+++ b/include/hw/southbridge/ich9.h
@@ -114,8 +114,6 @@ struct ICH9LPCState {
#define ICH9_D2P_SECONDARY_DEFAULT (256 - 8)
-#define ICH9_D2P_A2_REVISION 0x92
-
/* D31:F0 LPC Processor Interface */
#define ICH9_RST_CNT_IOPORT 0xCF9
diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
index c140919cbc..dd17e35b0a 100644
--- a/hw/pci-bridge/i82801b11.c
+++ b/hw/pci-bridge/i82801b11.c
@@ -45,7 +45,7 @@
#include "hw/pci/pci_bridge.h"
#include "migration/vmstate.h"
#include "qemu/module.h"
-#include "hw/southbridge/ich9.h"
+#include "hw/pci-bridge/ich_dmi_pci.h"
/*****************************************************************************/
/* ICH9 DMI-to-PCI bridge */
@@ -53,11 +53,8 @@
#define I82801ba_SSVID_SVID 0
#define I82801ba_SSVID_SSID 0
-typedef struct I82801b11Bridge {
- /*< private >*/
- PCIBridge parent_obj;
- /*< public >*/
-} I82801b11Bridge;
+
+#define ICH9_D2P_A2_REVISION 0x92
static void i82801b11_bridge_realize(PCIDevice *d, Error **errp)
{
@@ -103,7 +100,7 @@ static void
i82801b11_bridge_class_init(ObjectClass *klass, void *data)
}
static const TypeInfo i82801b11_bridge_info = {
- .name = "i82801b11-bridge",
+ .name = TYPE_ICH_DMI_PCI_BRIDGE,
.parent = TYPE_PCI_BRIDGE,
.instance_size = sizeof(I82801b11Bridge),
.class_init = i82801b11_bridge_class_init,
