On Mon, 2020-12-21 at 08:54 -0800, Guenter Roeck wrote:
> On 12/21/20 8:30 AM, Eddie James wrote:
> > Some PMBus chips don't respond with valid data when reading the
> > CAPABILITY register. For instance the register may report that the
> > chip supports PEC when in reality it does not. For such chips, PEC
> > must not be enabled while probing the chip, so add a flag so that
> > device drivers can force PEC off.
> >
>
> I think the flag should indicate that the capability register
> shall not be read/used. That the capability register is currently
> only used to check for PEC is secondary. We might,for example,
> start using it to check for alert support or to check the numeric
> format.
OK, that makes sense. I'll rename the flag in v2, how does
PMBUS_NO_CAPABILITY sound?
Thanks for the quick reply,
Eddie
>
> Thanks,
> Guenter
>
> > Signed-off-by: Eddie James <eajames@xxxxxxxxxxxxx>
> > ---
> > drivers/hwmon/pmbus/pmbus_core.c | 8 +++++---
> > include/linux/pmbus.h | 10 ++++++++++
> > 2 files changed, 15 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/hwmon/pmbus/pmbus_core.c
> > b/drivers/hwmon/pmbus/pmbus_core.c
> > index 192442b3b7a2..3de1657dde35 100644
> > --- a/drivers/hwmon/pmbus/pmbus_core.c
> > +++ b/drivers/hwmon/pmbus/pmbus_core.c
> > @@ -2204,9 +2204,11 @@ static int pmbus_init_common(struct
> > i2c_client *client, struct pmbus_data *data,
> > }
> >
> > /* Enable PEC if the controller supports it */
> > - ret = i2c_smbus_read_byte_data(client, PMBUS_CAPABILITY);
> > - if (ret >= 0 && (ret & PB_CAPABILITY_ERROR_CHECK))
> > - client->flags |= I2C_CLIENT_PEC;
> > + if (!(data->flags & PMBUS_NO_PEC)) {
> > + ret = i2c_smbus_read_byte_data(client,
> > PMBUS_CAPABILITY);
> > + if (ret >= 0 && (ret & PB_CAPABILITY_ERROR_CHECK))
> > + client->flags |= I2C_CLIENT_PEC;
> > + }
> >
> > /*
> > * Check if the chip is write protected. If it is, we can not
> > clear
> > diff --git a/include/linux/pmbus.h b/include/linux/pmbus.h
> > index 1ea5bae708a1..9bdc8a581b03 100644
> > --- a/include/linux/pmbus.h
> > +++ b/include/linux/pmbus.h
> > @@ -34,6 +34,16 @@
> > */
> > #define PMBUS_WRITE_PROTECTED BIT(1)
> >
> > +/*
> > + * PMBUS_NO_PEC
> > + *
> > + * Some PMBus chips don't respond with valid data when reading the
> > CAPABILITY
> > + * register. In this case, the register may report that the chip
> > supports PEC
> > + * with bit 7 (PB_CAPABILITY_ERROR_CHECK) when in reality it's not
> > supported.
> > + * For such chips, PEC must not be enabled before probing the
> > chip.
> > + */
> > +#define PMBUS_NO_PEC BIT(2)
> > +
> > struct pmbus_platform_data {
> > u32 flags; /* Device specific flags */
> >
> >
