| Message ID | 20230213105609.6173-1-philmd@linaro.org |
|---|---|
| Headers | show |
| Series | qdev: Introduce QDEV_DECLARE_DEV_BUS_TYPES() macro | expand |
On Mon, 13 Feb 2023 11:56:06 +0100 Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > Experiment after discussing with Thomas around qdev_get_parent_bus: > https://lore.kernel.org/qemu-devel/ad356f64-dca0-8117-d22a-a530e620ddb0@redhat.com/ > > When a QDev plug on a QBus, we'll always use qdev_get_parent_bus() > at least once with this type. Why not provide a consistent defined > macro instead of: > 1/ adding an inlined helper such usb_bus_from_device() > or scsi_bus_from_device() with different type checks, > 2/ open-code calls to qdev_get_parent_bus() with unsafe casts > ? > > This RFC series introduce a QDev-equivalent of QOM DECLARE_TYPES > macro, to be used with a (device, bus) tuple, and declaring the > equivalent device_GET_BUS() macro. it's already bad having 2 ways to declare types (though SIMPLE was a huge LOC saving) so question is where do we stop (API explosion ain't a good thing either). I my opinion this is just code churn for nor tangible benefit, given how qdev_get_parent_bus() is used. Fixing unsafe casts and getting rid of DO_UPCAST you mentioned before, would better use of resources. > hw/usb/ is converted as an example. > > Philippe Mathieu-Daudé (3): > hw/qdev: Introduce QDEV_DECLARE_DEV_BUS_TYPES() macro > hw/usb: Declare QOM macros using QDEV_DECLARE_DEV_BUS_TYPES() > hw/usb: Use USB_DEVICE_GET_BUS() macro > > 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/qdev-core.h | 28 ++++++++++++++++++++++++++++ > include/hw/usb.h | 13 ++++--------- > 7 files changed, 48 insertions(+), 25 deletions(-) >
On Wed, Mar 01, 2023 at 03:42:44PM +0100, Igor Mammedov wrote: > On Mon, 13 Feb 2023 11:56:06 +0100 > Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > > Experiment after discussing with Thomas around qdev_get_parent_bus: > > https://lore.kernel.org/qemu-devel/ad356f64-dca0-8117-d22a-a530e620ddb0@redhat.com/ > > > > When a QDev plug on a QBus, we'll always use qdev_get_parent_bus() > > at least once with this type. Why not provide a consistent defined > > macro instead of: > > 1/ adding an inlined helper such usb_bus_from_device() > > or scsi_bus_from_device() with different type checks, > > 2/ open-code calls to qdev_get_parent_bus() with unsafe casts > > ? > > > > This RFC series introduce a QDev-equivalent of QOM DECLARE_TYPES > > macro, to be used with a (device, bus) tuple, and declaring the > > equivalent device_GET_BUS() macro. > it's already bad having 2 ways to declare types (though SIMPLE was a huge LOC saving) > so question is where do we stop (API explosion ain't a good thing either). > > I my opinion this is just code churn for nor tangible benefit, > given how qdev_get_parent_bus() is used. Right so in the USB conversion the only improvement is the elimination of this method: -static inline USBBus *usb_bus_from_device(USBDevice *d) -{ - return DO_UPCAST(USBBus, qbus, qdev_get_parent_bus(DEVICE(d))); -} It strikes me that we don't have especially many (bus-type, device-type) pairs that would benefit from this macro. With regards, Daniel
Experiment after discussing with Thomas around qdev_get_parent_bus: https://lore.kernel.org/qemu-devel/ad356f64-dca0-8117-d22a-a530e620ddb0@redhat.com/ When a QDev plug on a QBus, we'll always use qdev_get_parent_bus() at least once with this type. Why not provide a consistent defined macro instead of: 1/ adding an inlined helper such usb_bus_from_device() or scsi_bus_from_device() with different type checks, 2/ open-code calls to qdev_get_parent_bus() with unsafe casts ? This RFC series introduce a QDev-equivalent of QOM DECLARE_TYPES macro, to be used with a (device, bus) tuple, and declaring the equivalent device_GET_BUS() macro. hw/usb/ is converted as an example. Philippe Mathieu-Daudé (3): hw/qdev: Introduce QDEV_DECLARE_DEV_BUS_TYPES() macro hw/usb: Declare QOM macros using QDEV_DECLARE_DEV_BUS_TYPES() hw/usb: Use USB_DEVICE_GET_BUS() macro 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/qdev-core.h | 28 ++++++++++++++++++++++++++++ include/hw/usb.h | 13 ++++--------- 7 files changed, 48 insertions(+), 25 deletions(-)