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

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

[Philippe Mathieu-Daudé]

Since v2:
- Propagate error in hw/i386/

Philippe Mathieu-Daudé (5):
  hw/i386/pc: Pass Error** argument to pc_basic_device_init()
  hw/i386/pc: Propagate error if HPET device creation failed
  hw/i386/pc: Inline legacy pcspk_init() in pc_basic_device_init()
  hw/isa/i82378: Inline legacy pcspk_init()
  hw/mips/jazz: Inline and remove legacy pcspk_init()

 include/hw/audio/pcspk.h | 10 ----------
 include/hw/i386/pc.h     |  5 +++--
 hw/i386/pc.c             | 17 +++++++++++++----
 hw/i386/pc_piix.c        |  2 +-
 hw/i386/pc_q35.c         |  2 +-
 hw/isa/i82378.c          |  7 ++++++-
 hw/mips/jazz.c           |  5 ++++-
 7 files changed, 28 insertions(+), 20 deletions(-)

-- 
2.41.0

[PATCH v3 1/5] hw/i386/pc: Pass Error** argument to pc_basic_device_init()

[Philippe Mathieu-Daudé]

pc_basic_device_init() creates devices which can fail,
allow to propagate error to caller.

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i386/pc.h | 5 +++--
 hw/i386/pc.c         | 7 +++++--
 hw/i386/pc_piix.c    | 2 +-
 hw/i386/pc_q35.c     | 2 +-
 4 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index bec38cb92c..069c27368d 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -174,11 +174,12 @@ void pc_memory_init(PCMachineState *pcms,
                     uint64_t pci_hole64_size);
 uint64_t pc_pci_hole64_start(void);
 DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus);
-void pc_basic_device_init(struct PCMachineState *pcms,
+bool pc_basic_device_init(struct PCMachineState *pcms,
                           ISABus *isa_bus, qemu_irq *gsi,
                           ISADevice *rtc_state,
                           bool create_fdctrl,
-                          uint32_t hpet_irqs);
+                          uint32_t hpet_irqs,
+                          Error **errp);
 void pc_cmos_init(PCMachineState *pcms,
                   BusState *ide0, BusState *ide1,
                   ISADevice *s);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index bb3854d1d0..c0477f0141 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1189,11 +1189,12 @@ static void pc_superio_init(ISABus *isa_bus, bool create_fdctrl,
     g_free(a20_line);
 }
 
-void pc_basic_device_init(struct PCMachineState *pcms,
+bool pc_basic_device_init(struct PCMachineState *pcms,
                           ISABus *isa_bus, qemu_irq *gsi,
                           ISADevice *rtc_state,
                           bool create_fdctrl,
-                          uint32_t hpet_irqs)
+                          uint32_t hpet_irqs,
+                          Error **errp)
 {
     int i;
     DeviceState *hpet = NULL;
@@ -1289,6 +1290,8 @@ void pc_basic_device_init(struct PCMachineState *pcms,
     /* Super I/O */
     pc_superio_init(isa_bus, create_fdctrl, pcms->i8042_enabled,
                     pcms->vmport != ON_OFF_AUTO_ON);
+
+    return true;
 }
 
 void pc_nic_init(PCMachineClass *pcmc, ISABus *isa_bus, PCIBus *pci_bus)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index e36a3262b2..0d9cdf773e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -320,7 +320,7 @@ static void pc_init1(MachineState *machine,
 
     /* init basic PC hardware */
     pc_basic_device_init(pcms, isa_bus, x86ms->gsi, rtc_state, true,
-                         0x4);
+                         0x4, &error_fatal);
 
     pc_nic_init(pcmc, isa_bus, pci_bus);
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index a7386f2ca2..e4b05e3139 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -301,7 +301,7 @@ static void pc_q35_init(MachineState *machine)
 
     /* init basic PC hardware */
     pc_basic_device_init(pcms, isa_bus, x86ms->gsi, rtc_state, !mc->no_floppy,
-                         0xff0104);
+                         0xff0104, &error_fatal);
 
     if (pcms->sata_enabled) {
         /* ahci and SATA device, for q35 1 ahci controller is built-in */
-- 
2.41.0

[PATCH v3 2/5] hw/i386/pc: Propagate error if HPET device creation failed

[Philippe Mathieu-Daudé]

Reported-by: Bernhard Beschow <shentey@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c0477f0141..c58a15182e 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1235,7 +1235,9 @@ bool pc_basic_device_init(struct PCMachineState *pcms,
         if (!compat) {
             qdev_prop_set_uint32(hpet, HPET_INTCAP, hpet_irqs);
         }
-        sysbus_realize_and_unref(SYS_BUS_DEVICE(hpet), &error_fatal);
+        if (!sysbus_realize_and_unref(SYS_BUS_DEVICE(hpet), errp)) {
+            return false;
+        }
         sysbus_mmio_map(SYS_BUS_DEVICE(hpet), 0, HPET_BASE);
 
         for (i = 0; i < IOAPIC_NUM_PINS; i++) {
-- 
2.41.0

[PATCH v3 3/5] hw/i386/pc: Inline legacy pcspk_init() in pc_basic_device_init()

[Philippe Mathieu-Daudé]

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/i386/pc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c58a15182e..3937d88355 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1286,7 +1286,11 @@ bool 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), &error_fatal);
+        if (!isa_realize_and_unref(pcms->pcspk, isa_bus, errp)) {
+            return false;
+        }
     }
 
     /* Super I/O */
-- 
2.41.0

[PATCH v3 4/5] hw/isa/i82378: Inline legacy pcspk_init()

[Philippe Mathieu-Daudé]

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/isa/i82378.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c
index 63e0857208..203b92c264 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,11 @@ 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), &error_fatal);
+    if (!isa_realize_and_unref(pcspk, isabus, errp)) {
+        return;
+    }
 
     /* 2 82C37 (dma) */
     isa_create_simple(isabus, "i82374");
-- 
2.41.0

[PATCH v3 5/5] hw/mips/jazz: Inline and remove legacy pcspk_init()

[Philippe Mathieu-Daudé]

pcspk_init() is a legacy init function, inline and remove it.

Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/audio/pcspk.h | 10 ----------
 hw/mips/jazz.c           |  5 ++++-
 2 files changed, 4 insertions(+), 11 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/mips/jazz.c b/hw/mips/jazz.c
index c32d2b0b0a..cdc37126c2 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), &error_fatal);
+    isa_realize_and_unref(pcspk, isa_bus, &error_fatal);
 
     /* Video card */
     switch (jazz_model) {
-- 
2.41.0

Re: [PATCH v3 0/5] hw/audio/pcspk: Inline pcspk_init()

[Richard Henderson]

On 10/20/23 03:54, Philippe Mathieu-Daudé wrote:
> Since v2:
> - Propagate error in hw/i386/
> 
> Philippe Mathieu-Daudé (5):
>    hw/i386/pc: Pass Error** argument to pc_basic_device_init()
>    hw/i386/pc: Propagate error if HPET device creation failed
>    hw/i386/pc: Inline legacy pcspk_init() in pc_basic_device_init()
>    hw/isa/i82378: Inline legacy pcspk_init()
>    hw/mips/jazz: Inline and remove legacy pcspk_init()
> 
>   include/hw/audio/pcspk.h | 10 ----------
>   include/hw/i386/pc.h     |  5 +++--
>   hw/i386/pc.c             | 17 +++++++++++++----
>   hw/i386/pc_piix.c        |  2 +-
>   hw/i386/pc_q35.c         |  2 +-
>   hw/isa/i82378.c          |  7 ++++++-
>   hw/mips/jazz.c           |  5 ++++-
>   7 files changed, 28 insertions(+), 20 deletions(-)
> 

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~