Re: [PATCH 2/4] hw: Replace dev->parent_bus by qdev_get_parent_bus(dev)

[Philippe Mathieu-Daudé]

On 14/2/23 00:19, Richard Henderson wrote:
> On 2/12/23 13:03, Philippe Mathieu-Daudé wrote:
> > On 12/2/23 23:47, Philippe Mathieu-Daudé wrote:
> > > DeviceState::parent_bus is an internal field and should be
> > > accessed by the qdev_get_parent_bus() helper. Replace all
> > > uses in hw/ except the QDev uses in hw/core/.
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > > ---
> > >   hw/audio/intel-hda.c                |  2 +-
> > >   hw/block/fdc.c                      |  2 +-
> > >   hw/block/swim.c                     |  2 +-
> > >   hw/ide/qdev.c                       |  4 ++--
> > >   hw/net/virtio-net.c                 |  2 +-
> > >   hw/pci-bridge/pci_expander_bridge.c |  2 +-
> > >   hw/scsi/scsi-bus.c                  |  2 +-
> > >   hw/usb/bus.c                        |  2 +-
> > >   hw/usb/desc.c                       |  2 +-
> > >   hw/usb/dev-smartcard-reader.c       | 16 ++++++++--------
> > >   10 files changed, 18 insertions(+), 18 deletions(-)
> >
> > I missed:
>
> Did you use a temporary rename of the field to catch all the uses?

No, git-grep. Good idea.

> >   void hda_codec_response(HDACodecDevice *dev, bool solicited, 
> > uint32_t response)
> >   {
> > -    HDACodecBus *bus = HDA_BUS(dev->qdev.parent_bus);
> > +    HDACodecBus *bus = HDA_BUS(qdev_get_parent_bus(DEVICE(dev)));
>
> I'm never sure the cast is clearer than &dev->qdev.

Maybe this one isn't obvious, but see for QOM macros use:
20230213170145.45666-3-philmd@linaro.org/:">https://lore.kernel.org/qemu-devel/20230213170145.45666-3-philmd@linaro.org/:

-    vcdev->vdev.dev = &vcdev->cdev.parent_obj.parent_obj;
+    vcdev->vdev.dev = DEVICE(vcdev);

We should agree on how we want to use this API. If the
DeviceState::parent_bus field isn't considered internal,
the we should remove the qdev_get_parent_bus() helper which
is simply:

hw/core/qdev.c:333:BusState *qdev_get_parent_bus(DeviceState *dev)
hw/core/qdev.c-334-{
hw/core/qdev.c-335-    return dev->parent_bus;
hw/core/qdev.c-336-}

Note the alternate series expanding QDev macros:
20230213105609.6173-1-philmd@linaro.org/">https://lore.kernel.org/qemu-devel/20230213105609.6173-1-philmd@linaro.org/

> But it seems the normal way in qemu...
>
> Acked-by: Richard Henderson <richard.henderson@linaro.org>

Thanks!