Hi
On Wed, Oct 22, 2025 at 3:42 PM BALATON Zoltan <balaton@eik.bme.hu> wrote:
>
> On Wed, 22 Oct 2025, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > For consistency, use only qdev_device_add() to instantiate the devices.
> > We can't rely on automatic bus lookup for the "hda-duplex" device though
> > as it may end up on a different "intel-hda" bus...
>
> Maybe this needs a better commit message. My first question was how is
> this simpler when it's 6 lines more and at least to me less obvious what
> it does. The real goal may be to make it independent from PCI for the next
> patch so maybe that's what the commit message should also say.
I agree it's not as simple and generalized as I wish it would have ended.
But this allows to make init() callback bus-agnostic though. I will
add this to the commit message.
There might be other ways to do that.
> Regards,
> BALATON Zoltan
>
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > hw/audio/intel-hda.c | 14 ++++++++++----
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
> > index 6a0db0dd9e..14bcf1257d 100644
> > --- a/hw/audio/intel-hda.c
> > +++ b/hw/audio/intel-hda.c
> > @@ -21,6 +21,7 @@
> > #include "hw/pci/pci.h"
> > #include "hw/qdev-properties.h"
> > #include "hw/pci/msi.h"
> > +#include "monitor/qdev.h"
> > #include "qemu/timer.h"
> > #include "qemu/bitops.h"
> > #include "qemu/log.h"
> > @@ -30,6 +31,7 @@
> > #include "intel-hda.h"
> > #include "migration/vmstate.h"
> > #include "intel-hda-defs.h"
> > +#include "qobject/qdict.h"
> > #include "system/dma.h"
> > #include "qapi/error.h"
> > #include "qom/object.h"
> > @@ -1305,15 +1307,19 @@ static const TypeInfo hda_codec_device_type_info = {
> > */
> > static int intel_hda_and_codec_init(PCIBus *bus, const char *audiodev)
> > {
> > - DeviceState *controller;
> > + g_autoptr(QDict) props = qdict_new();
> > + DeviceState *intel_hda, *codec;
> > BusState *hdabus;
> > - DeviceState *codec;
> >
> > - controller = DEVICE(pci_create_simple(bus, -1, "intel-hda"));
> > - hdabus = QLIST_FIRST(&controller->child_bus);
> > + qdict_put_str(props, "driver", "intel-hda");
> > + intel_hda = qdev_device_add_from_qdict(props, false, &error_fatal);
> > + hdabus = QLIST_FIRST(&intel_hda->child_bus);
> > +
> > codec = qdev_new("hda-duplex");
> > qdev_prop_set_string(codec, "audiodev", audiodev);
> > qdev_realize_and_unref(codec, hdabus, &error_fatal);
> > + object_unref(intel_hda);
> > +
> > return 0;
> > }
> >
> >
--
Marc-André Lureau