2022-04-21 13:04:15

by Adam Wujek

[permalink] [raw]
Subject: Re: [PATCH] hwmod: (pmbus) disable PEC if not enabled

------- Original Message -------
On Wednesday, April 20th, 2022 at 00:00, Guenter Roeck <[email protected]> wrote:
>
>
> On 4/19/22 13:53, Adam Wujek wrote:
>
> > Explicitly disable PEC when the client does not support it.
> > Without the explicit disable, when the device with the PEC support is removed
> > later when a device without PEC support is inserted into the same address,
> > the driver uses the old value of client->flags which contains the I2C_CLIENT_PEC
> > flag. As a consequence the PEC is used when it should not.
>
>
> How can that happen ? I would assume the I2C device gets deleted and re-created
> in that case, which should clear the PEC flag.
>
> Guenter
In my case it was when I unloaded the driver for the I2C slave, changed the advertised PEC value in PMBUS_CAPABILITY register on slave. Then loaded the driver. When the switch was from disable->enable it worked as expected (this case was already covered), but when the PEC was set in the slave from enabled->disabled it was still using PEC to communicate.

Adam
>
> > Signed-off-by: Adam Wujek [email protected]
> > ---
> > drivers/hwmon/pmbus/pmbus_core.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> > index 82c3754e21e3..f8ca36759b0a 100644
> > --- a/drivers/hwmon/pmbus/pmbus_core.c
> > +++ b/drivers/hwmon/pmbus/pmbus_core.c
> > @@ -2014,6 +2014,8 @@ static int pmbus_init_common(struct i2c_client *client, struct pmbus_data *data,
> > ret = i2c_smbus_read_byte_data(client, PMBUS_CAPABILITY);
> > if (ret >= 0 && (ret & PB_CAPABILITY_ERROR_CHECK))
> > client->flags |= I2C_CLIENT_PEC;
> > + else
> > + client->flags &= ~I2C_CLIENT_PEC;
> >
> > pmbus_clear_faults(client);
> >
> > --
> > 2.17.1


2022-04-22 21:15:29

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH] hwmod: (pmbus) disable PEC if not enabled

On 4/19/22 15:10, wujek dev wrote:
> ------- Original Message -------
> On Wednesday, April 20th, 2022 at 00:00, Guenter Roeck <[email protected]> wrote:
>>
>>
>> On 4/19/22 13:53, Adam Wujek wrote:
>>
>>> Explicitly disable PEC when the client does not support it.
>>> Without the explicit disable, when the device with the PEC support is removed
>>> later when a device without PEC support is inserted into the same address,
>>> the driver uses the old value of client->flags which contains the I2C_CLIENT_PEC
>>> flag. As a consequence the PEC is used when it should not.
>>
>>
>> How can that happen ? I would assume the I2C device gets deleted and re-created
>> in that case, which should clear the PEC flag.
>>
>> Guenter
> In my case it was when I unloaded the driver for the I2C slave, changed the advertised PEC value in PMBUS_CAPABILITY register on slave. Then loaded the driver. When the switch was from disable->enable it worked as expected (this case was already covered), but when the PEC was set in the slave from enabled->disabled it was still using PEC to communicate.

So it is really the same device, only you unload the driver, change the
device configuration (presumably with i2cset commands), and load it
again. Please explain that in more detail in the commit description.

Thanks,
Guenter