Re: [PATCH 4/8] hw/gpio/pca9554: New abstract base "pca9554-base"

[Markus Armbruster <armbru@redhat.com>]

Mark Cave-Ayland <mark.caveayland@nutanix.com> writes:

> On 09/11/2026 09:02, Mark Cave-Ayland wrote:
>
>> On 06/11/2026 11:04, Markus Armbruster wrote:
>> 
>>> Commit 5eceb48bbe86 (hw/gpio: pca9554: add PCA9536 support) made type
>>> "pca9535" a child of concrete type "pca9555".  This is inconvenient
>>> for the next commit.  Make them both children of new abstract type
>>> "pca9554-base".
>>>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>  include/hw/gpio/pca9554.h |  1 +
>>>  hw/gpio/pca9554.c         | 55 +++++++++++++++++++++++----------------
>>>  2 files changed, 33 insertions(+), 23 deletions(-)
>>> 
>>> diff --git a/include/hw/gpio/pca9554.h b/include/hw/gpio/pca9554.h
>>> index 1b33a8a752..0d841ff017 100644
>>> --- a/include/hw/gpio/pca9554.h
>>> +++ b/include/hw/gpio/pca9554.h
>>> @@ -8,6 +8,7 @@
>>>  #ifndef PCA9554_H
>>>  #define PCA9554_H
>>>  
>>> +#define TYPE_PCA9554_BASE "pca9554-base"
>>>  #define TYPE_PCA9554 "pca9554"
>>>  #define TYPE_PCA9536 "pca9536"
>>>  
>>> diff --git a/hw/gpio/pca9554.c b/hw/gpio/pca9554.c
>>> index caa73f63c9..1362b605c2 100644
>>> --- a/hw/gpio/pca9554.c
>>> +++ b/hw/gpio/pca9554.c
>>> @@ -26,7 +26,7 @@
>>>  #define PCA9554_PIN_COUNT 8
>>>  #define PCA9536_PIN_COUNT 4
>>>  
>>> -OBJECT_DECLARE_TYPE(PCA9554State, PCA9554Class, PCA9554)
>>> +OBJECT_DECLARE_TYPE(PCA9554State, PCA9554Class, PCA9554_BASE)
>>>  
>>>  struct PCA9554State {
>>>      /*< private >*/

[...]

>> The basic change looks good to me, however I found it tricky to follow given that PCA9554Class is now related to PCA9554_BASE_CLASS() and PCA9554State is now related to PCA9554_BASE().
>> Would it make sense to rename PCA9554Class to PCA9554BaseClass and PCA9554State to PCA9554BaseState here? I think it would help make the hierarchy easier to follow.
> Another follow-up question: how is PCA9554BaseState related to PCA955xState? The naming suggests that the latter is already assuming this role.

There is no PCA9554BaseState.

Have a look at the OBJECT_DECLARE_TYPE(), and its definition in
qom/object.h.  It does the following:

* typedef struct PCA9554State PCA9554State

* typedef struct PCA9554Class PCA9554Class

* Make g_autoptr(PCA9554State) work

* Define PCA9554_BASE(), PCA9554_BASE_CLASS(), PCA9554_GET_BASE_CLASS()

Or let's approach this from another angle.

Initially, we had just one type ("pca9554").  It had its own class and
instance struct (PCA9554Class and PCA9554State), which came with the
usual macros PCI9554(), PCA9554_CLASS(), PCA9554_GET_CLASS().

Then we added a second type ("pca9536") as a subtype of the first.
Instead of giving it its own class and instance struct, we reused the
existing ones, and we didn't define any macros.

This patch adds a third type ("pca9554-base").  It is abstract.  It
again reuses the existing class and instance struct.  However, because
this third type is the supertype of the existing two, we can't cleanly
reuse the existing macros.  So we *replace* them by PCA9554_BASE(),
PCA9554_BASE_CLASS(), PCA9554_BASE_GET_CLASS().  Except this is actually
a *rename*, because we still use them same class and instance struct.

Summary:

* "pca9536" ceases to be a subtype of "pca9554"

* "pca9554" and "pca9536" become subtypes of "pca9554-base"

* Their class and instance structs remain the same: PCA9554Class and
  PCA9554State.  There are no new C types.

* Macros PCA9554(), PCA9554_CLASS(), PCA9554_GET_CLASS() are renamed to
  PCA9554_BASE(), PCA9554_BASE_CLASS(), PCA9554_BASE_GET_CLASS().

Questions?