Re: [PATCH 11/13] vt82c686: Support machine-default audiodev with fallback

[BALATON Zoltan]

On Sat, 23 Sep 2023, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> hw/isa/vt82c686.c   |  2 ++
> hw/mips/fuloong2e.c | 13 ++++++++++---
> hw/ppc/pegasos2.c   | 10 ++++++++--
> 3 files changed, 20 insertions(+), 5 deletions(-)

This looks better but I still wonder if this machine audiodev propery is 
needed at all. If there's one -audiodev option specified it's already 
picked up by default devices and if there are more one could use -global 
to set it. Why isn't that enough?

If you still want a machine audiodev propery then could the device handle 
it without needing changes to the machine? Like in via_isa_realize() add

if (current_machine->audiodev) {
    qdev_prop_set_string(DEVICE(pci_dev), "audiodev", machine->audiodev);
}

before qdev_realize(DEVICE(&s->ac97) then no need to change the device 
creation in board code.

Regards,
BALATON Zoltan

> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 57bdfb4e78c..3ec8e43708a 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -578,6 +578,8 @@ static void via_isa_init(Object *obj)
>     object_initialize_child(obj, "uhci2", &s->uhci[1], TYPE_VT82C686B_USB_UHCI);
>     object_initialize_child(obj, "ac97", &s->ac97, TYPE_VIA_AC97);
>     object_initialize_child(obj, "mc97", &s->mc97, TYPE_VIA_MC97);
> +
> +    object_property_add_alias(obj, "audiodev", OBJECT(&s->ac97), "audiodev");
> }
>
> static const TypeInfo via_isa_info = {
> diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
> index c827f615f3b..df2be188257 100644
> --- a/hw/mips/fuloong2e.c
> +++ b/hw/mips/fuloong2e.c
> @@ -41,6 +41,7 @@
> #include "sysemu/reset.h"
> #include "sysemu/sysemu.h"
> #include "qemu/error-report.h"
> +#include "audio/audio.h"
>
> #define ENVP_PADDR              0x2000
> #define ENVP_VADDR              cpu_mips_phys_to_kseg0(NULL, ENVP_PADDR)
> @@ -295,9 +296,13 @@ static void mips_fuloong2e_init(MachineState *machine)
>     pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));
>
>     /* South bridge -> IP5 */
> -    pci_dev = pci_create_simple_multifunction(pci_bus,
> -                                              PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
> -                                              TYPE_VT82C686B_ISA);
> +    pci_dev = pci_new_multifunction(PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
> +                                    TYPE_VT82C686B_ISA);
> +    if (machine->audiodev) {
> +        qdev_prop_set_string(DEVICE(pci_dev), "audiodev", machine->audiodev);
> +    }
> +    pci_realize_and_unref(pci_dev, pci_bus, &error_abort);
> +
>     object_property_add_alias(OBJECT(machine), "rtc-time",
>                               object_resolve_path_component(OBJECT(pci_dev),
>                                                             "rtc"),
> @@ -337,6 +342,8 @@ static void mips_fuloong2e_machine_init(MachineClass *mc)
>     mc->default_ram_size = 256 * MiB;
>     mc->default_ram_id = "fuloong2e.ram";
>     mc->minimum_page_bits = 14;
> +
> +    machine_add_audiodev_property(mc);
> }
>
> DEFINE_MACHINE("fuloong2e", mips_fuloong2e_machine_init)
> diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
> index bd397cf2b5c..61c302895c9 100644
> --- a/hw/ppc/pegasos2.c
> +++ b/hw/ppc/pegasos2.c
> @@ -37,6 +37,7 @@
> #include "qemu/datadir.h"
> #include "sysemu/device_tree.h"
> #include "hw/ppc/vof.h"
> +#include "audio/audio.h"
>
> #include <libfdt.h>
>
> @@ -180,8 +181,11 @@ static void pegasos2_init(MachineState *machine)
>     pci_bus_irqs(pci_bus, pegasos2_pci_irq, pm, PCI_NUM_PINS);
>
>     /* VIA VT8231 South Bridge (multifunction PCI device) */
> -    via = OBJECT(pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0),
> -                                                 TYPE_VT8231_ISA));
> +    via = OBJECT(pci_new_multifunction(PCI_DEVFN(12, 0), TYPE_VT8231_ISA));
> +    if (machine->audiodev) {
> +        qdev_prop_set_string(DEVICE(via), "audiodev", machine->audiodev);
> +    }
> +    pci_realize_and_unref(PCI_DEVICE(via), pci_bus, &error_abort);
>     for (i = 0; i < PCI_NUM_PINS; i++) {
>         pm->via_pirq[i] = qdev_get_gpio_in_named(DEVICE(via), "pirq", i);
>     }
> @@ -564,6 +568,8 @@ static void pegasos2_machine_class_init(ObjectClass *oc, 
> void *data)
>     vhc->encode_hpt_for_kvm_pr = vhyp_encode_hpt_for_kvm_pr;
>
>     vmc->setprop = pegasos2_setprop;
> +
> +    machine_add_audiodev_property(mc);
> }
>
> static const TypeInfo pegasos2_machine_info = {