On 12/08/21 11:18 am, Guenter Roeck wrote: > On Wed, Aug 11, 2021 at 10:19:44PM +0000, Chris Packham wrote: >> On 12/08/21 7:53 am, Guenter Roeck wrote: >>> On Wed, Aug 11, 2021 at 04:17:38PM +1200, Chris Packham wrote: >>>> BPD-RS600 modules running firmware v5.70 misreport the MFR_PIN_MAX. >>>> The indicate a maximum of 1640W instead of 700W. Detect the invalid >>>> reading and return a sensible value instead. >>>> >>>> Signed-off-by: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx> >>>> --- >>>> drivers/hwmon/pmbus/bpa-rs600.c | 21 ++++++++++++++++++++- >>>> 1 file changed, 20 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/hwmon/pmbus/bpa-rs600.c b/drivers/hwmon/pmbus/bpa-rs600.c >>>> index d495faa89799..f4baed9ce8a4 100644 >>>> --- a/drivers/hwmon/pmbus/bpa-rs600.c >>>> +++ b/drivers/hwmon/pmbus/bpa-rs600.c >>>> @@ -65,6 +65,24 @@ static int bpa_rs600_read_vin(struct i2c_client *client) >>>> return ret; >>>> } >>>> >>>> +/* >>>> + * The firmware on some BPD-RS600 models incorrectly reports 1640W >>>> + * for MFR_PIN_MAX. Deal with this by returning a sensible value. >>>> + */ >>>> +static int bpa_rs600_read_pin_max(struct i2c_client *client) >>>> +{ >>>> + int ret; >>>> + >>>> + ret = pmbus_read_word_data(client, 0, 0xff, PMBUS_MFR_PIN_MAX); >>>> + if (ret < 0) >>>> + return ret; >>>> + >>>> + if (ret == 0x0b34) >>>> + return 0x095e; >>> The comments from the descriotion need to be here. >> will update >>> Thanks, >>> Guenter >>> >>>> + >>>> + return ret; >>>> +} >>>> + >>>> static int bpa_rs600_read_word_data(struct i2c_client *client, int page, int phase, int reg) >>>> { >>>> int ret; >>>> @@ -92,7 +110,8 @@ static int bpa_rs600_read_word_data(struct i2c_client *client, int page, int pha >>>> ret = pmbus_read_word_data(client, 0, 0xff, PMBUS_MFR_IOUT_MAX); >>>> break; >>>> case PMBUS_PIN_OP_WARN_LIMIT: >>>> - ret = pmbus_read_word_data(client, 0, 0xff, PMBUS_MFR_PIN_MAX); >>>> + case PMBUS_MFR_PIN_MAX: >>>> + ret = bpa_rs600_read_pin_max(client); >>> So the idea is to return the same value for PMBUS_PIN_OP_WARN_LIMIT >>> (max_alarm) and PMBUS_MFR_PIN_MAX (rated_max) ? That doesn't really >>> make sense. The meaning of those limits is distinctly different. >> For the BPA-RS600/BPD-RS600 these appear to be treated the same. > What a mess. *sigh* I know. I've also got another 2 BluTek supplies I haven't got round to dealing with yet. > This needs to be documented in the driver, including the > behavior if any of those attributes is written into. Mercifully these attributes are all read-only. So at least we don't have to deal with that. It's probably not too late to return -ENXIO for the WARN_LIMITs and have lm-sensors display the rated_max (we also have a custom consumer of the sysfs API that I'd need to sort out). > > Guenter > >>> Guenter >>> >>>> break; >>>> case PMBUS_POUT_OP_WARN_LIMIT: >>>> ret = pmbus_read_word_data(client, 0, 0xff, PMBUS_MFR_POUT_MAX); >>>> -- >>>> 2.32.0 >>>>