Re: [PATCH v2 1/2] hw/char/serial: Ensure SerialState::irq is set when realizing

[Pierrick Bouvier]

On 1/10/25 10:27, Philippe Mathieu-Daudé wrote:

SerialState::irq must be set by parent when the object is realized.

Suggested-by: Paolo Bonzini <pbonz...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <phi...@linaro.org>
---
  hw/char/serial-pci-multi.c | 2 +-
  hw/char/serial-pci.c       | 2 +-
  hw/char/serial.c           | 2 ++
  3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/char/serial-pci-multi.c b/hw/char/serial-pci-multi.c
index 7578e863cfe..5ad2c0e8443 100644
--- a/hw/char/serial-pci-multi.c
+++ b/hw/char/serial-pci-multi.c
@@ -106,11 +106,11 @@ static void multi_serial_pci_realize(PCIDevice *dev, 
Error **errp)

for (i = 0; i < nports; i++) {

          s = pci->state + i;
+        s->irq = pci->irqs[i];
          if (!qdev_realize(DEVICE(s), NULL, errp)) {
              multi_serial_pci_exit(dev);
              return;
          }
-        s->irq = pci->irqs[i];
          pci->name[i] = g_strdup_printf("uart #%zu", i + 1);
          memory_region_init_io(&s->io, OBJECT(pci), &serial_io_ops, s,
                                pci->name[i], 8);
diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c
index 6659cef5d4b..d31a3ccf03b 100644
--- a/hw/char/serial-pci.c
+++ b/hw/char/serial-pci.c
@@ -49,13 +49,13 @@ static void serial_pci_realize(PCIDevice *dev, Error **errp)
      PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev);
      SerialState *s = &pci->state;

+ s->irq = pci_allocate_irq(&pci->dev);

      if (!qdev_realize(DEVICE(s), NULL, errp)) {
          return;
      }

pci->dev.config[PCI_CLASS_PROG] = pci->prog_if;

      pci->dev.config[PCI_INTERRUPT_PIN] = 0x01;
-    s->irq = pci_allocate_irq(&pci->dev);

memory_region_init_io(&s->io, OBJECT(pci), &serial_io_ops, s, "serial", 8);

      pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io);
diff --git a/hw/char/serial.c b/hw/char/serial.c
index 70044e14a0f..562931685e7 100644
--- a/hw/char/serial.c
+++ b/hw/char/serial.c
@@ -923,6 +923,8 @@ static void serial_realize(DeviceState *dev, Error **errp)
  {
      SerialState *s = SERIAL(dev);

+ assert(s->irq);

+
      s->modem_status_poll = timer_new_ns(QEMU_CLOCK_VIRTUAL, (QEMUTimerCB *) 
serial_update_msl, s);

s->fifo_timeout_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, (QEMUTimerCB *) fifo_timeout_int, s);

Reviewed-by: Pierrick Bouvier <pierrick.bouv...@linaro.org>