On Fri, 3 Jul 2020, Philippe Mathieu-Daudé wrote:
By using the TYPE_* definitions for devices, we can:
- quickly find where devices are used with 'git-grep'
You could just as well grep for the type name but it's true if some files
use name and others the constant then you need to grep for both.
- easily rename a non-user-creatable device (one-line change).
But most devices are user creatable and thus their name is part of the CLI
so inlikely to change due to preserving backward compatibility of command
lines. So usefulness of this change seems limited to me.
But my problem with it is not the above. It's that hcd-ohci.h is not in
include but in hw/usb so it's internal to the implementation of the device
model and defines things that users of the device should not need,
therefore they should not include this header. So if you want to use the
defined constant then that should be split off to some public header
instead of including hw/usb/hcd-ohci.h. Maybe we need a new header for
these TYPE_* constants similar to qemu/typedefs.h?
Regards,
BALATON Zoltan
Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
---
hw/display/sm501.c | 3 ++-
hw/ppc/sam460ex.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index 9cccc68c35..c122a4eca5 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -36,6 +36,7 @@
#include "hw/qdev-properties.h"
#include "hw/i2c/i2c.h"
#include "hw/display/i2c-ddc.h"
+#include "hw/usb/hcd-ohci.h"
#include "qemu/range.h"
#include "ui/pixel_ops.h"
#include "qemu/bswap.h"
@@ -1961,7 +1962,7 @@ static void sm501_realize_sysbus(DeviceState *dev, Error
**errp)
sysbus_init_mmio(sbd, &s->state.mmio_region);
/* bridge to usb host emulation module */
- usb_dev = qdev_new("sysbus-ohci");
+ usb_dev = qdev_new(TYPE_SYSBUS_OHCI);
qdev_prop_set_uint32(usb_dev, "num-ports", 2);
qdev_prop_set_uint64(usb_dev, "dma-offset", s->base);
sysbus_realize_and_unref(SYS_BUS_DEVICE(usb_dev), &error_fatal);
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 1a106a68de..593436937a 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -35,6 +35,7 @@
#include "hw/char/serial.h"
#include "hw/i2c/ppc4xx_i2c.h"
#include "hw/i2c/smbus_eeprom.h"
+#include "hw/usb/hcd-ohci.h"
#include "hw/usb/hcd-ehci.h"
#include "hw/ppc/fdt.h"
#include "hw/qdev-properties.h"
@@ -370,7 +371,7 @@ static void sam460ex_init(MachineState *machine)
/* USB */
sysbus_create_simple(TYPE_PPC4xx_EHCI, 0x4bffd0400, uic[2][29]);
- dev = qdev_new("sysbus-ohci");
+ dev = qdev_new(TYPE_SYSBUS_OHCI);
qdev_prop_set_string(dev, "masterbus", "usb-bus.0");
qdev_prop_set_uint32(dev, "num-ports", 6);
sbdev = SYS_BUS_DEVICE(dev);
