Reviewed-by: Glenn Miles <[email protected]> Thanks,
Glenn On Thu, 2026-07-09 at 17:23 +0200, Emmanuel Blot wrote: > The PCA9536 is a 4-bit I/O expander that's register-compatible with the > PCA9554 but only has 4 pins. Rather than duplicating the whole driver, > make the existing PCA9554 model parameterizable and register PCA9536 as > a subtype. > > Introduce a PCA9554Class with a pin_count property, and replace every > hard-coded PCA9554_PIN_COUNT reference in the driver with the class > value. The reset function now computes the correct pin mask from > pin_count instead of assuming 0xFF. > > Signed-off-by: Emmanuel Blot <[email protected]> > --- > hw/gpio/pca9554.c | 79 > +++++++++++++++++++++++++++++++---------------- > include/hw/gpio/pca9554.h | 2 ++ > 2 files changed, 54 insertions(+), 27 deletions(-) > > diff --git a/hw/gpio/pca9554.c b/hw/gpio/pca9554.c > index 8427e01e9b2..b44ec0d9991 100644 > --- a/hw/gpio/pca9554.c > +++ b/hw/gpio/pca9554.c > @@ -24,6 +24,8 @@ struct PCA9554Class { > /*< private >*/ > I2CSlaveClass parent_class; > /*< public >*/ > + > + uint8_t pin_count; > }; > typedef struct PCA9554Class PCA9554Class; > > @@ -37,12 +39,13 @@ static const char *pin_state[] = {"low", "high"}; > > static void pca9554_update_pin_input(PCA9554State *s) > { > + PCA9554Class *pc = PCA9554_GET_CLASS(s); > int i; > uint8_t config = s->regs[PCA9554_CONFIG]; > uint8_t output = s->regs[PCA9554_OUTPUT]; > uint8_t internal_state = config | output; > > - for (i = 0; i < PCA9554_PIN_COUNT; i++) { > + for (i = 0; i < pc->pin_count; i++) { > uint8_t bit_mask = 1 << i; > uint8_t internal_pin_state = (internal_state >> i) & 0x1; > uint8_t old_value = s->regs[PCA9554_INPUT] & bit_mask; > @@ -67,7 +70,7 @@ static void pca9554_update_pin_input(PCA9554State *s) > break; > } > > - /* update irq state only if pin state changed */ > + /* drive the per-pin GPIO output only if the pin level changed */ > new_value = s->regs[PCA9554_INPUT] & bit_mask; > if (new_value != old_value) { > if (new_value) { > @@ -99,6 +102,12 @@ static uint8_t pca9554_read(PCA9554State *s, uint8_t reg) > > static void pca9554_write(PCA9554State *s, uint8_t reg, uint8_t data) > { > + PCA9554Class *pc = PCA9554_GET_CLASS(s); > + uint8_t pin_mask = (1 << pc->pin_count) - 1; > + > + /* Variants narrower than 8 bits ignore the unimplemented upper pins. */ > + data &= pin_mask; > + > switch (reg) { > case PCA9554_OUTPUT: > case PCA9554_CONFIG: > @@ -157,7 +166,7 @@ static void pca9554_get_pin(Object *obj, Visitor *v, > const char *name, > error_setg(errp, "%s: error reading %s", __func__, name); > return; > } > - if (pin < 0 || pin >= PCA9554_PIN_COUNT) { > + if (pin < 0 || pin >= PCA9554_GET_CLASS(s)->pin_count) { > error_setg(errp, "%s invalid pin %s", __func__, name); > return; > } > @@ -184,7 +193,7 @@ static void pca9554_set_pin(Object *obj, Visitor *v, > const char *name, > error_setg(errp, "%s: error reading %s", __func__, name); > return; > } > - if (pin < 0 || pin >= PCA9554_PIN_COUNT) { > + if (pin < 0 || pin >= PCA9554_GET_CLASS(s)->pin_count) { > error_setg(errp, "%s invalid pin %s", __func__, name); > return; > } > @@ -232,13 +241,15 @@ static const VMStateDescription pca9554_vmstate = { > static void pca9554_reset(DeviceState *dev) > { > PCA9554State *s = PCA9554(dev); > + PCA9554Class *pc = PCA9554_GET_CLASS(s); > + uint8_t pin_mask = (1 << pc->pin_count) - 1; > > - s->regs[PCA9554_INPUT] = 0xFF; > - s->regs[PCA9554_OUTPUT] = 0xFF; > + s->regs[PCA9554_INPUT] = pin_mask; > + s->regs[PCA9554_OUTPUT] = pin_mask; > s->regs[PCA9554_POLARITY] = 0x0; /* No pins are inverted */ > - s->regs[PCA9554_CONFIG] = 0xFF; /* All pins are inputs */ > + s->regs[PCA9554_CONFIG] = pin_mask; /* All pins are inputs */ > > - memset(s->ext_state, PCA9554_PIN_HIZ, PCA9554_PIN_COUNT); > + memset(s->ext_state, PCA9554_PIN_HIZ, pc->pin_count); > pca9554_update_pin_input(s); > > s->pointer = 0x0; > @@ -247,9 +258,10 @@ static void pca9554_reset(DeviceState *dev) > > static void pca9554_initfn(Object *obj) > { > + PCA9554Class *pc = PCA9554_GET_CLASS(obj); > int pin; > > - for (pin = 0; pin < PCA9554_PIN_COUNT; pin++) { > + for (pin = 0; pin < pc->pin_count; pin++) { > char *name; > > name = g_strdup_printf("pin%d", pin); > @@ -269,23 +281,24 @@ static void pca9554_set_ext_state(PCA9554State *s, int > pin, int level) > > static void pca9554_gpio_in_handler(void *opaque, int pin, int level) > { > - > PCA9554State *s = PCA9554(opaque); > + PCA9554Class *pc = PCA9554_GET_CLASS(s); > > - assert((pin >= 0) && (pin < PCA9554_PIN_COUNT)); > + assert((pin >= 0) && (pin < pc->pin_count)); > pca9554_set_ext_state(s, pin, level); > } > > static void pca9554_realize(DeviceState *dev, Error **errp) > { > PCA9554State *s = PCA9554(dev); > + PCA9554Class *pc = PCA9554_GET_CLASS(s); > > if (!s->description) { > - s->description = g_strdup("pca9554"); > + s->description = g_strdup(object_get_typename(OBJECT(dev))); > } > > - qdev_init_gpio_out(dev, s->gpio_out, PCA9554_PIN_COUNT); > - qdev_init_gpio_in(dev, pca9554_gpio_in_handler, PCA9554_PIN_COUNT); > + qdev_init_gpio_out(dev, s->gpio_out, pc->pin_count); > + qdev_init_gpio_in(dev, pca9554_gpio_in_handler, pc->pin_count); > } > > static const Property pca9554_properties[] = { > @@ -296,6 +309,7 @@ static void pca9554_class_init(ObjectClass *klass, const > void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > I2CSlaveClass *k = I2C_SLAVE_CLASS(klass); > + PCA9554Class *pc = PCA9554_CLASS(klass); > > k->event = pca9554_event; > k->recv = pca9554_recv; > @@ -304,21 +318,32 @@ static void pca9554_class_init(ObjectClass *klass, > const void *data) > device_class_set_legacy_reset(dc, pca9554_reset); > dc->vmsd = &pca9554_vmstate; > device_class_set_props(dc, pca9554_properties); > + > + pc->pin_count = PCA9554_PIN_COUNT; > } > > -static const TypeInfo pca9554_info = { > - .name = TYPE_PCA9554, > - .parent = TYPE_I2C_SLAVE, > - .instance_init = pca9554_initfn, > - .instance_size = sizeof(PCA9554State), > - .class_init = pca9554_class_init, > - .class_size = sizeof(PCA9554Class), > - .abstract = false, > -}; > - > -static void pca9554_register_types(void) > +static void pca9536_class_init(ObjectClass *klass, const void *data) > { > - type_register_static(&pca9554_info); > + PCA9554Class *pc = PCA9554_CLASS(klass); > + > + pc->pin_count = PCA9536_PIN_COUNT; > } > > -type_init(pca9554_register_types) > +static const TypeInfo pca9554_types[] = { > + { > + .name = TYPE_PCA9554, > + .parent = TYPE_I2C_SLAVE, > + .instance_init = pca9554_initfn, > + .instance_size = sizeof(PCA9554State), > + .class_init = pca9554_class_init, > + .class_size = sizeof(PCA9554Class), > + .abstract = false, > + }, > + { > + .name = TYPE_PCA9536, > + .parent = TYPE_PCA9554, > + .class_init = pca9536_class_init, > + } > +}; > + > +DEFINE_TYPES(pca9554_types); > diff --git a/include/hw/gpio/pca9554.h b/include/hw/gpio/pca9554.h > index 54bfc4c4c7a..c09108e8b65 100644 > --- a/include/hw/gpio/pca9554.h > +++ b/include/hw/gpio/pca9554.h > @@ -12,12 +12,14 @@ > #include "qom/object.h" > > #define TYPE_PCA9554 "pca9554" > +#define TYPE_PCA9536 "pca9536" > typedef struct PCA9554State PCA9554State; > DECLARE_INSTANCE_CHECKER(PCA9554State, PCA9554, > TYPE_PCA9554) > > #define PCA9554_NR_REGS 4 > #define PCA9554_PIN_COUNT 8 > +#define PCA9536_PIN_COUNT 4 > > struct PCA9554State { > /*< private >*/ >
