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

["Cédric Le Goater" <clegoate@redhat.com>]

On 10/10/23 22:35, Miles Glenn wrote:
> 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!

yes. My bad, I didn't look close enough. Thanks Mark

C.