[PATCH] hw/audio/pcspk: Inline pcspk_init()

Philippe Mathieu-Daudé posted 1 patch 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20231017135058.44247-1-philmd@linaro.org
Maintainers: Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Hervé Poussineau" <hpoussin@reactos.org>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Gerd Hoffmann <kraxel@redhat.com>
There is a newer version of this series
include/hw/audio/pcspk.h | 10 ----------
hw/i386/pc.c             |  3 ++-
hw/isa/i82378.c          |  5 ++++-
hw/mips/jazz.c           |  5 ++++-
4 files changed, 10 insertions(+), 13 deletions(-)
[PATCH] hw/audio/pcspk: Inline pcspk_init()
Posted by Philippe Mathieu-Daudé 7 months ago
pcspk_init() is a legacy init function, inline and remove it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/audio/pcspk.h | 10 ----------
 hw/i386/pc.c             |  3 ++-
 hw/isa/i82378.c          |  5 ++++-
 hw/mips/jazz.c           |  5 ++++-
 4 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/include/hw/audio/pcspk.h b/include/hw/audio/pcspk.h
index 9506179587..6be75a6b86 100644
--- a/include/hw/audio/pcspk.h
+++ b/include/hw/audio/pcspk.h
@@ -25,16 +25,6 @@
 #ifndef HW_PCSPK_H
 #define HW_PCSPK_H
 
-#include "hw/isa/isa.h"
-#include "hw/qdev-properties.h"
-#include "qapi/error.h"
-
 #define TYPE_PC_SPEAKER "isa-pcspk"
 
-static inline void pcspk_init(ISADevice *isadev, ISABus *bus, ISADevice *pit)
-{
-    object_property_set_link(OBJECT(isadev), "pit", OBJECT(pit), NULL);
-    isa_realize_and_unref(isadev, bus, &error_fatal);
-}
-
 #endif /* HW_PCSPK_H */
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index bb3854d1d0..3d0b53a583 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1283,7 +1283,8 @@ void pc_basic_device_init(struct PCMachineState *pcms,
             /* connect PIT to output control line of the HPET */
             qdev_connect_gpio_out(hpet, 0, qdev_get_gpio_in(DEVICE(pit), 0));
         }
-        pcspk_init(pcms->pcspk, isa_bus, pit);
+        object_property_set_link(OBJECT(pcms->pcspk), "pit", OBJECT(pit), NULL);
+        isa_realize_and_unref(pcms->pcspk, isa_bus, &error_fatal);
     }
 
     /* Super I/O */
diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index 63e0857208..9474bf994c 100644
--- a/hw/isa/i82378.c
+++ b/hw/isa/i82378.c
@@ -67,6 +67,7 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
     uint8_t *pci_conf;
     ISABus *isabus;
     ISADevice *pit;
+    ISADevice *pcspk;
 
     pci_conf = pci->config;
     pci_set_word(pci_conf + PCI_COMMAND,
@@ -102,7 +103,9 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
     pit = i8254_pit_init(isabus, 0x40, 0, NULL);
 
     /* speaker */
-    pcspk_init(isa_new(TYPE_PC_SPEAKER), isabus, pit);
+    pcspk = isa_new(TYPE_PC_SPEAKER);
+    object_property_set_link(OBJECT(pcspk), "pit", OBJECT(pit), NULL);
+    isa_realize_and_unref(pcspk, isabus, &error_fatal);
 
     /* 2 82C37 (dma) */
     isa_create_simple(isabus, "i82374");
diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
index c32d2b0b0a..aac851747c 100644
--- a/hw/mips/jazz.c
+++ b/hw/mips/jazz.c
@@ -177,6 +177,7 @@ static void mips_jazz_init(MachineState *machine,
     SysBusDevice *sysbus;
     ISABus *isa_bus;
     ISADevice *pit;
+    ISADevice *pcspk;
     DriveInfo *fds[MAX_FD];
     MemoryRegion *bios = g_new(MemoryRegion, 1);
     MemoryRegion *bios2 = g_new(MemoryRegion, 1);
@@ -279,7 +280,9 @@ static void mips_jazz_init(MachineState *machine,
     isa_bus_register_input_irqs(isa_bus, i8259);
     i8257_dma_init(isa_bus, 0);
     pit = i8254_pit_init(isa_bus, 0x40, 0, NULL);
-    pcspk_init(isa_new(TYPE_PC_SPEAKER), isa_bus, pit);
+    pcspk = isa_new(TYPE_PC_SPEAKER);
+    object_property_set_link(OBJECT(pcspk), "pit", OBJECT(pit), NULL);
+    isa_realize_and_unref(pcspk, isa_bus, &error_fatal);
 
     /* Video card */
     switch (jazz_model) {
-- 
2.41.0


Re: [PATCH] hw/audio/pcspk: Inline pcspk_init()
Posted by Mark Cave-Ayland 7 months ago
On 17/10/2023 14:50, Philippe Mathieu-Daudé wrote:

> pcspk_init() is a legacy init function, inline and remove it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/hw/audio/pcspk.h | 10 ----------
>   hw/i386/pc.c             |  3 ++-
>   hw/isa/i82378.c          |  5 ++++-
>   hw/mips/jazz.c           |  5 ++++-
>   4 files changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/include/hw/audio/pcspk.h b/include/hw/audio/pcspk.h
> index 9506179587..6be75a6b86 100644
> --- a/include/hw/audio/pcspk.h
> +++ b/include/hw/audio/pcspk.h
> @@ -25,16 +25,6 @@
>   #ifndef HW_PCSPK_H
>   #define HW_PCSPK_H
>   
> -#include "hw/isa/isa.h"
> -#include "hw/qdev-properties.h"
> -#include "qapi/error.h"
> -
>   #define TYPE_PC_SPEAKER "isa-pcspk"
>   
> -static inline void pcspk_init(ISADevice *isadev, ISABus *bus, ISADevice *pit)
> -{
> -    object_property_set_link(OBJECT(isadev), "pit", OBJECT(pit), NULL);
> -    isa_realize_and_unref(isadev, bus, &error_fatal);
> -}
> -
>   #endif /* HW_PCSPK_H */
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index bb3854d1d0..3d0b53a583 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1283,7 +1283,8 @@ void pc_basic_device_init(struct PCMachineState *pcms,
>               /* connect PIT to output control line of the HPET */
>               qdev_connect_gpio_out(hpet, 0, qdev_get_gpio_in(DEVICE(pit), 0));
>           }
> -        pcspk_init(pcms->pcspk, isa_bus, pit);
> +        object_property_set_link(OBJECT(pcms->pcspk), "pit", OBJECT(pit), NULL);
> +        isa_realize_and_unref(pcms->pcspk, isa_bus, &error_fatal);
>       }
>   
>       /* Super I/O */
> diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
> index 63e0857208..9474bf994c 100644
> --- a/hw/isa/i82378.c
> +++ b/hw/isa/i82378.c
> @@ -67,6 +67,7 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
>       uint8_t *pci_conf;
>       ISABus *isabus;
>       ISADevice *pit;
> +    ISADevice *pcspk;
>   
>       pci_conf = pci->config;
>       pci_set_word(pci_conf + PCI_COMMAND,
> @@ -102,7 +103,9 @@ static void i82378_realize(PCIDevice *pci, Error **errp)
>       pit = i8254_pit_init(isabus, 0x40, 0, NULL);
>   
>       /* speaker */
> -    pcspk_init(isa_new(TYPE_PC_SPEAKER), isabus, pit);
> +    pcspk = isa_new(TYPE_PC_SPEAKER);
> +    object_property_set_link(OBJECT(pcspk), "pit", OBJECT(pit), NULL);
> +    isa_realize_and_unref(pcspk, isabus, &error_fatal);
>   
>       /* 2 82C37 (dma) */
>       isa_create_simple(isabus, "i82374");
> diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
> index c32d2b0b0a..aac851747c 100644
> --- a/hw/mips/jazz.c
> +++ b/hw/mips/jazz.c
> @@ -177,6 +177,7 @@ static void mips_jazz_init(MachineState *machine,
>       SysBusDevice *sysbus;
>       ISABus *isa_bus;
>       ISADevice *pit;
> +    ISADevice *pcspk;
>       DriveInfo *fds[MAX_FD];
>       MemoryRegion *bios = g_new(MemoryRegion, 1);
>       MemoryRegion *bios2 = g_new(MemoryRegion, 1);
> @@ -279,7 +280,9 @@ static void mips_jazz_init(MachineState *machine,
>       isa_bus_register_input_irqs(isa_bus, i8259);
>       i8257_dma_init(isa_bus, 0);
>       pit = i8254_pit_init(isa_bus, 0x40, 0, NULL);
> -    pcspk_init(isa_new(TYPE_PC_SPEAKER), isa_bus, pit);
> +    pcspk = isa_new(TYPE_PC_SPEAKER);
> +    object_property_set_link(OBJECT(pcspk), "pit", OBJECT(pit), NULL);
> +    isa_realize_and_unref(pcspk, isa_bus, &error_fatal);
>   
>       /* Video card */
>       switch (jazz_model) {

Possibly you might want to pass errp instead of NULL for the last parameter of 
object_property_set_link() in i82378_realize()? But regardless:

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


ATB,

Mark.


Re: [PATCH] hw/audio/pcspk: Inline pcspk_init()
Posted by Philippe Mathieu-Daudé 7 months ago
On 17/10/23 22:08, Mark Cave-Ayland wrote:
> On 17/10/2023 14:50, Philippe Mathieu-Daudé wrote:
> 
>> pcspk_init() is a legacy init function, inline and remove it.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   include/hw/audio/pcspk.h | 10 ----------
>>   hw/i386/pc.c             |  3 ++-
>>   hw/isa/i82378.c          |  5 ++++-
>>   hw/mips/jazz.c           |  5 ++++-
>>   4 files changed, 10 insertions(+), 13 deletions(-)


>> diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
>> index c32d2b0b0a..aac851747c 100644
>> --- a/hw/mips/jazz.c
>> +++ b/hw/mips/jazz.c
>> @@ -177,6 +177,7 @@ static void mips_jazz_init(MachineState *machine,
>>       SysBusDevice *sysbus;
>>       ISABus *isa_bus;
>>       ISADevice *pit;
>> +    ISADevice *pcspk;
>>       DriveInfo *fds[MAX_FD];
>>       MemoryRegion *bios = g_new(MemoryRegion, 1);
>>       MemoryRegion *bios2 = g_new(MemoryRegion, 1);
>> @@ -279,7 +280,9 @@ static void mips_jazz_init(MachineState *machine,
>>       isa_bus_register_input_irqs(isa_bus, i8259);
>>       i8257_dma_init(isa_bus, 0);
>>       pit = i8254_pit_init(isa_bus, 0x40, 0, NULL);
>> -    pcspk_init(isa_new(TYPE_PC_SPEAKER), isa_bus, pit);
>> +    pcspk = isa_new(TYPE_PC_SPEAKER);
>> +    object_property_set_link(OBJECT(pcspk), "pit", OBJECT(pit), NULL);
>> +    isa_realize_and_unref(pcspk, isa_bus, &error_fatal);
>>       /* Video card */
>>       switch (jazz_model) {
> 
> Possibly you might want to pass errp instead of NULL for the last 
> parameter of object_property_set_link() in i82378_realize()?

Oops you are right. I'll use &error_fatal.

> But 
> regardless:
> 
> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

Thanks!