Re: [PATCH v3] misc/pca9552: Fix for pca9552 not getting reset

[Miles Glenn <milesg@linux.vnet.ibm.com>]

On Tue, 2023-10-10 at 21:31 +0100, Mark Cave-Ayland wrote:
> On 10/10/2023 20:52, Glenn Miles wrote:
> 
> > Testing of the pca9552 device on the powernv platform
> > showed that the reset method was not being called when
> > an instance of the device was realized.  This was causing
> > the INPUT0/INPUT1 POR values to be incorrect.
> > 
> > Fixed by overriding the parent pca955x_realize method with a
> > new pca9552_realize method which first calls
> > the parent pca955x_realize method followed by the
> > pca9552_reset function.
> > 
> > Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>
> > ---
> >   hw/misc/pca9552.c | 7 +++++++
> >   1 file changed, 7 insertions(+)
> > 
> > diff --git a/hw/misc/pca9552.c b/hw/misc/pca9552.c
> > index fff19e369a..4e183cc554 100644
> > --- a/hw/misc/pca9552.c
> > +++ b/hw/misc/pca9552.c
> > @@ -384,6 +384,12 @@ static void pca955x_realize(DeviceState *dev,
> > Error **errp)
> >       qdev_init_gpio_out(dev, s->gpio, k->pin_count);
> >   }
> >   
> > +static void pca9552_realize(DeviceState *dev, Error **errp)
> > +{
> > +    pca955x_realize(dev, errp);
> > +    pca9552_reset(dev);
> > +}
> > +
> >   static Property pca955x_properties[] = {
> >       DEFINE_PROP_STRING("description", PCA955xState, description),
> >       DEFINE_PROP_END_OF_LIST(),
> > @@ -417,6 +423,7 @@ static void pca9552_class_init(ObjectClass *oc,
> > void *data)
> >       PCA955xClass *pc = PCA955X_CLASS(oc);
> >   
> >       dc->reset = pca9552_reset;
> > +    dc->realize = pca9552_realize;
> >       dc->vmsd = &pca9552_vmstate;
> >       pc->max_reg = PCA9552_LS3;
> >       pc->pin_count = 16;
> 
> The reason that the reset function isn't being called here is because
> TYPE_I2C_SLAVE 
> is derived from TYPE_DEVICE, and for various historical reasons the
> DeviceClass reset 
> function is only called for devices that inherit from
> TYPE_SYS_BUS_DEVICE.
> 
> Probably the best way to make this work instead of mixing up the
> reset and realize 
> parts of the object lifecycle is to convert pca9552_reset() to use
> the new Resettable 
> interface for TYPE_PCA9552: take a look at commit d43e967f69 ("q800-
> glue.c: convert 
> to Resettable interface") as an example, along with the documentation
> at 
> https://www.qemu.org/docs/master/devel/reset.html.
> 

Ahh, that's very helpful.  Thanks, Mark!

-Glenn

> 
> ATB,
> 
> Mark.
>