On 13/02/2023 09.44, Philippe Mathieu-Daudé wrote:
On 13/2/23 09:11, Thomas Huth wrote:
On 13/02/2023 08.08, Philippe Mathieu-Daudé wrote:
Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
RFC Other devices don't use such helper. Maybe it should
be the other way around, introduce more bus_from_device()
helpers?
---
hw/usb/bus.c | 10 +++++-----
hw/usb/core.c | 6 +++---
hw/usb/dev-hub.c | 4 ++--
hw/usb/dev-serial.c | 10 +++++-----
hw/usb/hcd-xhci.c | 2 +-
include/hw/usb.h | 5 -----
6 files changed, 16 insertions(+), 21 deletions(-)
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index d7c3c71435..4a1b67761c 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -427,7 +427,7 @@ void usb_unregister_port(USBBus *bus, USBPort *port)
void usb_claim_port(USBDevice *dev, Error **errp)
{
- USBBus *bus = usb_bus_from_device(dev);
+ USBBus *bus = USB_BUS(qdev_get_parent_bus(DEVICE(dev)));
You're certainly missing a proper justification in the patch description
here. The "other devices don't use such a helper" does not sound like a
real justification to me, since the code lines rather get longer this way.
Thus this rather looks like unnecessary code churn to me --> rather drop
the patch?
The idea is to avoid having 7 different ways of implementing something
with 3 different APIs and 2 unfinished API conversions in flight.
Ok, then please add such information to the patch description.
I'm wondering if the QOM DECLARE_xxx() macros could also define some
xxx_BUS_FROM_DEV() or xxx_PARENT_BUS() macros. So here it would become:
USBBus *bus = USB_PARENT_BUS(dev);
Sounds more readable at a first glance, but when looking at the output of:
grep -r '(qdev_get_parent_bus' hw/
it seems like there aren't that many other places using this pattern (many
places rather use BUS() instead), so it's maybe hard to justify such a
change. Thus I think your patch here is likely the better solution right now
(when you add a proper patch description).
Thomas
