Re: [PATCH 02/22] hw/rtc/m48t59-isa: QOM'ify IRQ number

[Peter Maydell]

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