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(-)

-- 
2.38.1

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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|