On 13/2/23 10:11, Thomas Huth wrote:
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.
There is another helper, scsi_bus_from_device(), but is only used
twice. The previous patch "hw/scsi/scsi-bus: Inline two uses of
scsi_bus_from_device()" remove it.
> Thus I think your patch here is likely the better
> solution right now (when you add a proper patch description).
OK, thanks!
