Hi

On Wed, Oct 22, 2025 at 3:42 PM BALATON Zoltan <[email protected]> wrote:
>
> On Wed, 22 Oct 2025, [email protected] wrote:
> > From: Marc-André Lureau <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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

Reply via email to