|
From: | Thomas Huth |
Subject: | Re: [RFC PATCH v2 19/19] hw/usb: Inline usb_bus_from_device() |
Date: | Mon, 13 Feb 2023 10:11:23 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0 |
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é <philmd@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
[Prev in Thread] | Current Thread | [Next in Thread] |