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

[Bernhard Beschow <shentey@gmail.com>]

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