On 12/21/20 10:32 AM, Eddie James wrote: > 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? >
sgtm Thanks, Guenter > Thanks for the quick reply, > Eddie > >> >> Thanks, >> Guenter >> >>> Signed-off-by: Eddie James <[email protected]> >>> --- >>> 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 */ >>> >>> >
