| Message ID | 20220222173819.76568-3-shentey@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | isa: Resolve unneeded IRQ attributes from ISADevice | expand |
On Tue, 22 Feb 2022 at 17:41, Bernhard Beschow <shentey@gmail.com> wrote: > > Exposing the IRQ number as a QOM property not only allows it to be > configurable but also to be printed by standard QOM mechanisms. This allows > isabus_dev_print() to be retired eventually. > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > --- > hw/rtc/m48t59-isa.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/hw/rtc/m48t59-isa.c b/hw/rtc/m48t59-isa.c > index dc21fb10a5..d1c1d789a5 100644 > --- a/hw/rtc/m48t59-isa.c > +++ b/hw/rtc/m48t59-isa.c > @@ -42,6 +42,7 @@ struct M48txxISAState { > ISADevice parent_obj; > M48t59State state; > uint32_t io_base; > + uint32_t isairq; > MemoryRegion io; > }; > > @@ -79,6 +80,7 @@ static void m48txx_isa_toggle_lock(Nvram *obj, int lock) > static Property m48t59_isa_properties[] = { > DEFINE_PROP_INT32("base-year", M48txxISAState, state.base_year, 0), > DEFINE_PROP_UINT32("iobase", M48txxISAState, io_base, 0x74), > + DEFINE_PROP_UINT32("irq", M48txxISAState, isairq, 8), > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -99,7 +101,7 @@ static void m48t59_isa_realize(DeviceState *dev, Error **errp) > > s->model = u->info.model; > s->size = u->info.size; > - isa_init_irq(isadev, &s->IRQ, 8); > + isa_init_irq(isadev, &s->IRQ, d->isairq); > m48t59_realize_common(s, errp); > memory_region_init_io(&d->io, OBJECT(dev), &m48t59_io_ops, s, "m48t59", 4); > if (d->io_base != 0) { > -- > 2.35.1 If the user creates the device specifying irq=16 then isa_init_irq() will assert. Since these devices are creatable by users on the commandline, I think the realize method should check that the property value is in range (d->isairq < ISA_NUM_IRQS), and report a suitable error back if not. thanks -- PMM
Am 22. Februar 2022 17:47:56 UTC schrieb Peter Maydell <peter.maydell@linaro.org>: >On Tue, 22 Feb 2022 at 17:41, Bernhard Beschow <shentey@gmail.com> wrote: >> >> Exposing the IRQ number as a QOM property not only allows it to be >> configurable but also to be printed by standard QOM mechanisms. This allows >> isabus_dev_print() to be retired eventually. >> >> Signed-off-by: Bernhard Beschow <shentey@gmail.com> >> --- >> hw/rtc/m48t59-isa.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/hw/rtc/m48t59-isa.c b/hw/rtc/m48t59-isa.c >> index dc21fb10a5..d1c1d789a5 100644 >> --- a/hw/rtc/m48t59-isa.c >> +++ b/hw/rtc/m48t59-isa.c >> @@ -42,6 +42,7 @@ struct M48txxISAState { >> ISADevice parent_obj; >> M48t59State state; >> uint32_t io_base; >> + uint32_t isairq; >> MemoryRegion io; >> }; >> >> @@ -79,6 +80,7 @@ static void m48txx_isa_toggle_lock(Nvram *obj, int lock) >> static Property m48t59_isa_properties[] = { >> DEFINE_PROP_INT32("base-year", M48txxISAState, state.base_year, 0), >> DEFINE_PROP_UINT32("iobase", M48txxISAState, io_base, 0x74), >> + DEFINE_PROP_UINT32("irq", M48txxISAState, isairq, 8), >> DEFINE_PROP_END_OF_LIST(), >> }; >> >> @@ -99,7 +101,7 @@ static void m48t59_isa_realize(DeviceState *dev, Error **errp) >> >> s->model = u->info.model; >> s->size = u->info.size; >> - isa_init_irq(isadev, &s->IRQ, 8); >> + isa_init_irq(isadev, &s->IRQ, d->isairq); >> m48t59_realize_common(s, errp); >> memory_region_init_io(&d->io, OBJECT(dev), &m48t59_io_ops, s, "m48t59", 4); >> if (d->io_base != 0) { >> -- >> 2.35.1 > >If the user creates the device specifying irq=16 then isa_init_irq() >will assert. Since these devices are creatable by users on the >commandline, I think the realize method should check that the >property value is in range (d->isairq < ISA_NUM_IRQS), and report >a suitable error back if not. Ah, I missed that! Thanks for pointing out, I will fix it. Regards, Bernhard > >thanks >-- PMM
diff --git a/hw/rtc/m48t59-isa.c b/hw/rtc/m48t59-isa.c index dc21fb10a5..d1c1d789a5 100644 --- a/hw/rtc/m48t59-isa.c +++ b/hw/rtc/m48t59-isa.c @@ -42,6 +42,7 @@ struct M48txxISAState { ISADevice parent_obj; M48t59State state; uint32_t io_base; + uint32_t isairq; MemoryRegion io; }; @@ -79,6 +80,7 @@ static void m48txx_isa_toggle_lock(Nvram *obj, int lock) static Property m48t59_isa_properties[] = { DEFINE_PROP_INT32("base-year", M48txxISAState, state.base_year, 0), DEFINE_PROP_UINT32("iobase", M48txxISAState, io_base, 0x74), + DEFINE_PROP_UINT32("irq", M48txxISAState, isairq, 8), DEFINE_PROP_END_OF_LIST(), }; @@ -99,7 +101,7 @@ static void m48t59_isa_realize(DeviceState *dev, Error **errp) s->model = u->info.model; s->size = u->info.size; - isa_init_irq(isadev, &s->IRQ, 8); + isa_init_irq(isadev, &s->IRQ, d->isairq); m48t59_realize_common(s, errp); memory_region_init_io(&d->io, OBJECT(dev), &m48t59_io_ops, s, "m48t59", 4); if (d->io_base != 0) {
Exposing the IRQ number as a QOM property not only allows it to be configurable but also to be printed by standard QOM mechanisms. This allows isabus_dev_print() to be retired eventually. Signed-off-by: Bernhard Beschow <shentey@gmail.com> --- hw/rtc/m48t59-isa.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)