[v3] isa: Resolve unneeded IRQ attributes from ISADevice

[PATCH v3 0/7] isa: Resolve unneeded IRQ attributes from ISADevice

Posted by Bernhard Beschow 2 years, 1 month ago

v3:
* Use DEFINE_PROP_UINT8 / uint8_t for new IRQ properties. (Philippe)
* Use "%u" format when printing maximum allowed IRQ number in error messages.
  (Philippe)
* Use object_property_get_uint() in pnv.c. (Philippe)
* Squash cleanup patches (7-22) into
  "Inline and remove one-line isa_init_irq()" (Philippe)
* Mention in the commit message of patch 1 that mc146818rtc has its IRQ number
  now displayed in the human monitor. Since the IRQ numbers of the other
  "QOM'ified" devices were already displayed I've only adapted this commit
  message. (Philippe)

v2:
* The newly QOM'ified devices now report an error to the user in their realize
  functions if the configured IRQ number is greater than 15. (PMM)

v1:
The IRQ attributes of ISADevice are hardcoded to support up to two IRQs per
device which creates an artificial limit. By not having the attributes in the
first place, this limitation can be avoided altogether.

The IRQ attributes are mostly used for printing ('info qtree') and there is one
user, hw/ppc/pnv, to use the attributes directly. As it turns out, the printing
is redundant if the IRQ numbers are exposed as QOM properties and hw/ppc/pnv
can be easily ported away.

The patch series is structured as follows: Patch 1-3 QOM'ify the last devices
which rely on printing their IRQ numbers via the ISADevice attributes. Patch
4 and 5 remove the last users of the ISADevice attributes such that they can be
removed in patch 6. The remainder of the patch series is cleanup.

Patch 6 turns isa_init_irq() into a trivial wrapper for isa_get_irq(). That is,
the former function becomes redundant. All users are therefore converted to use
isa_get_irq() directly. Finally, the last patch removes the now unused
isa_init_irq().


Bernhard Beschow (7):
  hw/rtc/mc146818rtc: QOM'ify IRQ number
  hw/rtc/m48t59-isa: QOM'ify IRQ number
  hw/input/pckbd: QOM'ify IRQ numbers
  hw/isa/isa-bus: Remove isabus_dev_print()
  hw/ppc/pnv: Determine ns16550's IRQ number from QOM property
  isa: Drop unused attributes from ISADevice
  isa: Inline and remove one-line isa_init_irq()

 hw/audio/cs4231a.c           |  2 +-
 hw/audio/gus.c               |  2 +-
 hw/audio/sb16.c              |  2 +-
 hw/block/fdc-isa.c           |  2 +-
 hw/char/parallel.c           |  2 +-
 hw/char/serial-isa.c         |  2 +-
 hw/ide/isa.c                 |  2 +-
 hw/input/pckbd.c             | 26 +++++++++++++++++++++----
 hw/ipmi/isa_ipmi_bt.c        |  2 +-
 hw/ipmi/isa_ipmi_kcs.c       |  2 +-
 hw/isa/isa-bus.c             | 37 +-----------------------------------
 hw/isa/piix4.c               |  2 +-
 hw/net/ne2000-isa.c          |  2 +-
 hw/ppc/pnv.c                 |  5 ++++-
 hw/rtc/m48t59-isa.c          |  9 ++++++++-
 hw/rtc/mc146818rtc.c         | 13 +++++++++++--
 hw/tpm/tpm_tis_isa.c         |  2 +-
 include/hw/isa/isa.h         |  3 ---
 include/hw/rtc/mc146818rtc.h |  1 +
 tests/qemu-iotests/172.out   | 26 -------------------------
 20 files changed, 59 insertions(+), 85 deletions(-)

-- 
2.35.1

Re: [PATCH v3 0/7] isa: Resolve unneeded IRQ attributes from ISADevice

Posted by Philippe Mathieu-Daudé 2 years, 1 month ago

On 1/3/22 23:00, Bernhard Beschow wrote:

> The IRQ attributes are mostly used for printing ('info qtree') and there is one
> user, hw/ppc/pnv, to use the attributes directly. As it turns out, the printing
> is redundant if the IRQ numbers are exposed as QOM properties and hw/ppc/pnv
> can be easily ported away.
> 
> The patch series is structured as follows: Patch 1-3 QOM'ify the last devices
> which rely on printing their IRQ numbers via the ISADevice attributes. Patch
> 4 and 5 remove the last users of the ISADevice attributes such that they can be
> removed in patch 6. The remainder of the patch series is cleanup.
> 
> Patch 6 turns isa_init_irq() into a trivial wrapper for isa_get_irq(). That is,
> the former function becomes redundant. All users are therefore converted to use
> isa_get_irq() directly. Finally, the last patch removes the now unused
> isa_init_irq().
> 
> 
> Bernhard Beschow (7):
>    hw/rtc/mc146818rtc: QOM'ify IRQ number
>    hw/rtc/m48t59-isa: QOM'ify IRQ number
>    hw/input/pckbd: QOM'ify IRQ numbers
>    hw/isa/isa-bus: Remove isabus_dev_print()
>    hw/ppc/pnv: Determine ns16550's IRQ number from QOM property
>    isa: Drop unused attributes from ISADevice
>    isa: Inline and remove one-line isa_init_irq()
> 
>   hw/audio/cs4231a.c           |  2 +-
>   hw/audio/gus.c               |  2 +-
>   hw/audio/sb16.c              |  2 +-
>   hw/block/fdc-isa.c           |  2 +-
>   hw/char/parallel.c           |  2 +-
>   hw/char/serial-isa.c         |  2 +-
>   hw/ide/isa.c                 |  2 +-
>   hw/input/pckbd.c             | 26 +++++++++++++++++++++----
>   hw/ipmi/isa_ipmi_bt.c        |  2 +-
>   hw/ipmi/isa_ipmi_kcs.c       |  2 +-
>   hw/isa/isa-bus.c             | 37 +-----------------------------------
>   hw/isa/piix4.c               |  2 +-
>   hw/net/ne2000-isa.c          |  2 +-
>   hw/ppc/pnv.c                 |  5 ++++-
>   hw/rtc/m48t59-isa.c          |  9 ++++++++-
>   hw/rtc/mc146818rtc.c         | 13 +++++++++++--
>   hw/tpm/tpm_tis_isa.c         |  2 +-
>   include/hw/isa/isa.h         |  3 ---
>   include/hw/rtc/mc146818rtc.h |  1 +
>   tests/qemu-iotests/172.out   | 26 -------------------------
>   20 files changed, 59 insertions(+), 85 deletions(-)
> 

Please avoid posting 2 series going in different directions but touching
the same files, and expect the same person to take them both and resolve
resulting conflicts. Post one, then the second one based on the previous
one (and so on if multiple steps). Anyway, for now I adapted this series
on top of your "malta: Fix PCI IRQ levels" series and queued to mips.

Thanks,

Phil.

Re: [PATCH v3 0/7] isa: Resolve unneeded IRQ attributes from ISADevice

Posted by Bernhard Beschow 2 years, 1 month ago

Am 7. März 2022 00:34:27 UTC schrieb "Philippe Mathieu-Daudé" <philippe.mathieu.daude@gmail.com>:
>On 1/3/22 23:00, Bernhard Beschow wrote:
>
>> The IRQ attributes are mostly used for printing ('info qtree') and there is one
>> user, hw/ppc/pnv, to use the attributes directly. As it turns out, the printing
>> is redundant if the IRQ numbers are exposed as QOM properties and hw/ppc/pnv
>> can be easily ported away.
>> 
>> The patch series is structured as follows: Patch 1-3 QOM'ify the last devices
>> which rely on printing their IRQ numbers via the ISADevice attributes. Patch
>> 4 and 5 remove the last users of the ISADevice attributes such that they can be
>> removed in patch 6. The remainder of the patch series is cleanup.
>> 
>> Patch 6 turns isa_init_irq() into a trivial wrapper for isa_get_irq(). That is,
>> the former function becomes redundant. All users are therefore converted to use
>> isa_get_irq() directly. Finally, the last patch removes the now unused
>> isa_init_irq().
>> 
>> 
>> Bernhard Beschow (7):
>>    hw/rtc/mc146818rtc: QOM'ify IRQ number
>>    hw/rtc/m48t59-isa: QOM'ify IRQ number
>>    hw/input/pckbd: QOM'ify IRQ numbers
>>    hw/isa/isa-bus: Remove isabus_dev_print()
>>    hw/ppc/pnv: Determine ns16550's IRQ number from QOM property
>>    isa: Drop unused attributes from ISADevice
>>    isa: Inline and remove one-line isa_init_irq()
>> 
>>   hw/audio/cs4231a.c           |  2 +-
>>   hw/audio/gus.c               |  2 +-
>>   hw/audio/sb16.c              |  2 +-
>>   hw/block/fdc-isa.c           |  2 +-
>>   hw/char/parallel.c           |  2 +-
>>   hw/char/serial-isa.c         |  2 +-
>>   hw/ide/isa.c                 |  2 +-
>>   hw/input/pckbd.c             | 26 +++++++++++++++++++++----
>>   hw/ipmi/isa_ipmi_bt.c        |  2 +-
>>   hw/ipmi/isa_ipmi_kcs.c       |  2 +-
>>   hw/isa/isa-bus.c             | 37 +-----------------------------------
>>   hw/isa/piix4.c               |  2 +-
>>   hw/net/ne2000-isa.c          |  2 +-
>>   hw/ppc/pnv.c                 |  5 ++++-
>>   hw/rtc/m48t59-isa.c          |  9 ++++++++-
>>   hw/rtc/mc146818rtc.c         | 13 +++++++++++--
>>   hw/tpm/tpm_tis_isa.c         |  2 +-
>>   include/hw/isa/isa.h         |  3 ---
>>   include/hw/rtc/mc146818rtc.h |  1 +
>>   tests/qemu-iotests/172.out   | 26 -------------------------
>>   20 files changed, 59 insertions(+), 85 deletions(-)
>> 
>
>Please avoid posting 2 series going in different directions but touching
>the same files, and expect the same person to take them both and resolve
>resulting conflicts. Post one, then the second one based on the previous
>one (and so on if multiple steps). Anyway, for now I adapted this series
>on top of your "malta: Fix PCI IRQ levels" series and queued to mips.

Hi Phil,

I'm sorry to have caused you issues with my two patch series. In hindsight I should have communicated the merge conflict beforehand. I didn't because I considered it a special case where two logically independent series happened to be created by the same person. Now I know that communicating it earlier had allowed us to find a solution beforehand. After all, I'm more than happy helping out to accellerate integration of my patches.

Anyway, thanks for having picked this up and also thanks a lot for your steady stream of motivating review comments!

Best regards,
Bernhard

>
>Thanks,
>
>Phil.