| Message ID | 20250121084452.81941-3-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/ipack: Minor dust removal | expand |
Am 21. Januar 2025 08:44:52 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>: >No need to dynamically allocate IRQ when we know before hands >how many we'll use. Declare the 2 of them in IPackDevice state >and initialize them in the DeviceRealize handler. > >Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >--- > include/hw/ipack/ipack.h | 7 ++----- > hw/char/ipoctal232.c | 4 ++-- > hw/ipack/ipack.c | 7 +++---- > hw/ipack/tpci200.c | 6 +++--- > 4 files changed, 10 insertions(+), 14 deletions(-) > >diff --git a/include/hw/ipack/ipack.h b/include/hw/ipack/ipack.h >index cbcdda509d3..00f397fd020 100644 >--- a/include/hw/ipack/ipack.h >+++ b/include/hw/ipack/ipack.h >@@ -12,6 +12,7 @@ > #define QEMU_IPACK_H > > #include "hw/qdev-core.h" >+#include "hw/irq.h" > #include "qom/object.h" > > >@@ -19,10 +20,8 @@ > OBJECT_DECLARE_SIMPLE_TYPE(IPackBus, IPACK_BUS) > > struct IPackBus { >- /*< private >*/ > BusState parent_obj; > >- /* All fields are private */ > uint8_t n_slots; > uint8_t free_slot; > qemu_irq_handler set_irq; >@@ -58,13 +57,11 @@ struct IPackDeviceClass { > }; > > struct IPackDevice { >- /*< private >*/ > DeviceState parent_obj; >- /*< public >*/ > > int32_t slot; > /* IRQ objects for the IndustryPack INT0# and INT1# */ >- qemu_irq *irq; >+ IRQState irq[2]; > }; > > extern const VMStateDescription vmstate_ipack_device; >diff --git a/hw/char/ipoctal232.c b/hw/char/ipoctal232.c >index d1e5f6dad2e..a2879977fb3 100644 >--- a/hw/char/ipoctal232.c >+++ b/hw/char/ipoctal232.c >@@ -184,9 +184,9 @@ static void update_irq(IPOctalState *dev, unsigned block) > unsigned intno = block / 2; > > if ((blk0->isr & blk0->imr) || (blk1->isr & blk1->imr)) { >- qemu_irq_raise(idev->irq[intno]); >+ qemu_irq_raise(&idev->irq[intno]); > } else { >- qemu_irq_lower(idev->irq[intno]); >+ qemu_irq_lower(&idev->irq[intno]); > } > } > >diff --git a/hw/ipack/ipack.c b/hw/ipack/ipack.c >index ed75f791832..eeb48dd331c 100644 >--- a/hw/ipack/ipack.c >+++ b/hw/ipack/ipack.c >@@ -55,22 +55,21 @@ static void ipack_device_realize(DeviceState *dev, Error **errp) > } > bus->free_slot = idev->slot + 1; > >- idev->irq = qemu_allocate_irqs(bus->set_irq, idev, 2); >+ for (int i = 0; i < ARRAY_SIZE(idev->irq); i++) { >+ qemu_init_irq(&idev->irq[i], bus->set_irq, idev, i); I wonder if we eventually need a qemu_init_irqs() (note the plural) as we remove further uses of qemu_allocate_irqs(). >+ } > > k->realize(dev, errp); > } > > static void ipack_device_unrealize(DeviceState *dev) > { >- IPackDevice *idev = IPACK_DEVICE(dev); > IPackDeviceClass *k = IPACK_DEVICE_GET_CLASS(dev); > > if (k->unrealize) { > k->unrealize(dev); > return; > } >- >- qemu_free_irqs(idev->irq, 2); > } > > static const Property ipack_device_props[] = { >diff --git a/hw/ipack/tpci200.c b/hw/ipack/tpci200.c >index 88eef4b8308..470a4203ae4 100644 >--- a/hw/ipack/tpci200.c >+++ b/hw/ipack/tpci200.c >@@ -275,11 +275,11 @@ static void tpci200_write_las0(void *opaque, hwaddr addr, uint64_t val, > if (ip != NULL) { > if (val & STATUS_INT(i, 0)) { > DPRINTF("Clear IP %c INT0# status\n", 'A' + i); >- qemu_irq_lower(ip->irq[0]); >+ qemu_irq_lower(&ip->irq[0]); > } > if (val & STATUS_INT(i, 1)) { > DPRINTF("Clear IP %c INT1# status\n", 'A' + i); >- qemu_irq_lower(ip->irq[1]); >+ qemu_irq_lower(&ip->irq[1]); > } > } > >@@ -344,7 +344,7 @@ static uint64_t tpci200_read_las1(void *opaque, hwaddr addr, unsigned size) > bool int_set = s->status & STATUS_INT(ip_n, intno); > bool int_edge_sensitive = s->ctrl[ip_n] & CTRL_INT_EDGE(intno); > if (int_set && !int_edge_sensitive) { >- qemu_irq_lower(ip->irq[intno]); >+ qemu_irq_lower(&ip->irq[intno]); > } > } > With the above addressed: Reviewed-by: Bernhard Beschow <shentey@gmail.com>
On 21/1/25 10:24, Bernhard Beschow wrote: > > > Am 21. Januar 2025 08:44:52 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>: >> No need to dynamically allocate IRQ when we know before hands >> how many we'll use. Declare the 2 of them in IPackDevice state >> and initialize them in the DeviceRealize handler. >> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> >> --- >> include/hw/ipack/ipack.h | 7 ++----- >> hw/char/ipoctal232.c | 4 ++-- >> hw/ipack/ipack.c | 7 +++---- >> hw/ipack/tpci200.c | 6 +++--- >> 4 files changed, 10 insertions(+), 14 deletions(-) >> >> diff --git a/include/hw/ipack/ipack.h b/include/hw/ipack/ipack.h >> index cbcdda509d3..00f397fd020 100644 >> --- a/include/hw/ipack/ipack.h >> +++ b/include/hw/ipack/ipack.h >> @@ -12,6 +12,7 @@ >> #define QEMU_IPACK_H >> >> #include "hw/qdev-core.h" >> +#include "hw/irq.h" >> #include "qom/object.h" >> >> >> @@ -19,10 +20,8 @@ >> OBJECT_DECLARE_SIMPLE_TYPE(IPackBus, IPACK_BUS) >> >> struct IPackBus { >> - /*< private >*/ >> BusState parent_obj; >> >> - /* All fields are private */ >> uint8_t n_slots; >> uint8_t free_slot; >> qemu_irq_handler set_irq; >> @@ -58,13 +57,11 @@ struct IPackDeviceClass { >> }; >> >> struct IPackDevice { >> - /*< private >*/ >> DeviceState parent_obj; >> - /*< public >*/ >> >> int32_t slot; >> /* IRQ objects for the IndustryPack INT0# and INT1# */ >> - qemu_irq *irq; >> + IRQState irq[2]; >> }; >> >> extern const VMStateDescription vmstate_ipack_device; >> diff --git a/hw/char/ipoctal232.c b/hw/char/ipoctal232.c >> index d1e5f6dad2e..a2879977fb3 100644 >> --- a/hw/char/ipoctal232.c >> +++ b/hw/char/ipoctal232.c >> @@ -184,9 +184,9 @@ static void update_irq(IPOctalState *dev, unsigned block) >> unsigned intno = block / 2; >> >> if ((blk0->isr & blk0->imr) || (blk1->isr & blk1->imr)) { >> - qemu_irq_raise(idev->irq[intno]); >> + qemu_irq_raise(&idev->irq[intno]); >> } else { >> - qemu_irq_lower(idev->irq[intno]); >> + qemu_irq_lower(&idev->irq[intno]); >> } >> } >> >> diff --git a/hw/ipack/ipack.c b/hw/ipack/ipack.c >> index ed75f791832..eeb48dd331c 100644 >> --- a/hw/ipack/ipack.c >> +++ b/hw/ipack/ipack.c >> @@ -55,22 +55,21 @@ static void ipack_device_realize(DeviceState *dev, Error **errp) >> } >> bus->free_slot = idev->slot + 1; >> >> - idev->irq = qemu_allocate_irqs(bus->set_irq, idev, 2); >> + for (int i = 0; i < ARRAY_SIZE(idev->irq); i++) { >> + qemu_init_irq(&idev->irq[i], bus->set_irq, idev, i); > > I wonder if we eventually need a qemu_init_irqs() (note the plural) as we remove further uses of qemu_allocate_irqs(). Yeah I thought about it, but looking at other "Initialize pre-allocated QOM object in place", we don't have helpers for arrays. Usually because we can set properties, and array initializer doesn't scale then. For now IRQ doesn't have properties, so can do. > With the above addressed: > Reviewed-by: Bernhard Beschow <shentey@gmail.com> Thanks!
diff --git a/include/hw/ipack/ipack.h b/include/hw/ipack/ipack.h index cbcdda509d3..00f397fd020 100644 --- a/include/hw/ipack/ipack.h +++ b/include/hw/ipack/ipack.h @@ -12,6 +12,7 @@ #define QEMU_IPACK_H #include "hw/qdev-core.h" +#include "hw/irq.h" #include "qom/object.h" @@ -19,10 +20,8 @@ OBJECT_DECLARE_SIMPLE_TYPE(IPackBus, IPACK_BUS) struct IPackBus { - /*< private >*/ BusState parent_obj; - /* All fields are private */ uint8_t n_slots; uint8_t free_slot; qemu_irq_handler set_irq; @@ -58,13 +57,11 @@ struct IPackDeviceClass { }; struct IPackDevice { - /*< private >*/ DeviceState parent_obj; - /*< public >*/ int32_t slot; /* IRQ objects for the IndustryPack INT0# and INT1# */ - qemu_irq *irq; + IRQState irq[2]; }; extern const VMStateDescription vmstate_ipack_device; diff --git a/hw/char/ipoctal232.c b/hw/char/ipoctal232.c index d1e5f6dad2e..a2879977fb3 100644 --- a/hw/char/ipoctal232.c +++ b/hw/char/ipoctal232.c @@ -184,9 +184,9 @@ static void update_irq(IPOctalState *dev, unsigned block) unsigned intno = block / 2; if ((blk0->isr & blk0->imr) || (blk1->isr & blk1->imr)) { - qemu_irq_raise(idev->irq[intno]); + qemu_irq_raise(&idev->irq[intno]); } else { - qemu_irq_lower(idev->irq[intno]); + qemu_irq_lower(&idev->irq[intno]); } } diff --git a/hw/ipack/ipack.c b/hw/ipack/ipack.c index ed75f791832..eeb48dd331c 100644 --- a/hw/ipack/ipack.c +++ b/hw/ipack/ipack.c @@ -55,22 +55,21 @@ static void ipack_device_realize(DeviceState *dev, Error **errp) } bus->free_slot = idev->slot + 1; - idev->irq = qemu_allocate_irqs(bus->set_irq, idev, 2); + for (int i = 0; i < ARRAY_SIZE(idev->irq); i++) { + qemu_init_irq(&idev->irq[i], bus->set_irq, idev, i); + } k->realize(dev, errp); } static void ipack_device_unrealize(DeviceState *dev) { - IPackDevice *idev = IPACK_DEVICE(dev); IPackDeviceClass *k = IPACK_DEVICE_GET_CLASS(dev); if (k->unrealize) { k->unrealize(dev); return; } - - qemu_free_irqs(idev->irq, 2); } static const Property ipack_device_props[] = { diff --git a/hw/ipack/tpci200.c b/hw/ipack/tpci200.c index 88eef4b8308..470a4203ae4 100644 --- a/hw/ipack/tpci200.c +++ b/hw/ipack/tpci200.c @@ -275,11 +275,11 @@ static void tpci200_write_las0(void *opaque, hwaddr addr, uint64_t val, if (ip != NULL) { if (val & STATUS_INT(i, 0)) { DPRINTF("Clear IP %c INT0# status\n", 'A' + i); - qemu_irq_lower(ip->irq[0]); + qemu_irq_lower(&ip->irq[0]); } if (val & STATUS_INT(i, 1)) { DPRINTF("Clear IP %c INT1# status\n", 'A' + i); - qemu_irq_lower(ip->irq[1]); + qemu_irq_lower(&ip->irq[1]); } } @@ -344,7 +344,7 @@ static uint64_t tpci200_read_las1(void *opaque, hwaddr addr, unsigned size) bool int_set = s->status & STATUS_INT(ip_n, intno); bool int_edge_sensitive = s->ctrl[ip_n] & CTRL_INT_EDGE(intno); if (int_set && !int_edge_sensitive) { - qemu_irq_lower(ip->irq[intno]); + qemu_irq_lower(&ip->irq[intno]); } }
No need to dynamically allocate IRQ when we know before hands how many we'll use. Declare the 2 of them in IPackDevice state and initialize them in the DeviceRealize handler. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- include/hw/ipack/ipack.h | 7 ++----- hw/char/ipoctal232.c | 4 ++-- hw/ipack/ipack.c | 7 +++---- hw/ipack/tpci200.c | 6 +++--- 4 files changed, 10 insertions(+), 14 deletions(-)