| Message ID | 20210428083833.4136346-1-kubernat@cesnet.cz (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] Fix FSP-3Y YH-5151E non-compliant vout encoding | expand |
On 4/28/21 1:38 AM, Václav Kubernát wrote: > I didn't properly test the driver for YH-5151E, so it was completely > broken. Firstly, the log/real mapping was incorrect in one case. > Secondly, PMBus specifies that output voltages should be in the linear16 > encoding. However, the PDU is non-compliant and uses linear11. YM-2151E > isn't affected by this. Fix this by converting the values inside the > read functions. linear16 gets the exponent from the VOUT_MODE command. > The device doesn't support it, so I have to manually supply the value > for it. > > Both supported devices have now been tested to report correct vout > values. > Fixes: 1734b4135a62 ("hwmon: Add driver for fsp-3y PSUs and PDUs") > Signed-off-by: Václav Kubernát <kubernat@cesnet.cz> > --- > drivers/hwmon/pmbus/fsp-3y.c | 26 ++++++++++++++++++++++++-- > 1 file changed, 24 insertions(+), 2 deletions(-) > > diff --git a/drivers/hwmon/pmbus/fsp-3y.c b/drivers/hwmon/pmbus/fsp-3y.c > index 564649e87e34..dc147a79d6aa 100644 > --- a/drivers/hwmon/pmbus/fsp-3y.c > +++ b/drivers/hwmon/pmbus/fsp-3y.c > @@ -57,7 +57,7 @@ static int page_log_to_page_real(int page_log, enum chips chip) > case YH5151E_PAGE_12V_LOG: > return YH5151E_PAGE_12V_REAL; > case YH5151E_PAGE_5V_LOG: > - return YH5151E_PAGE_5V_LOG; > + return YH5151E_PAGE_5V_REAL; > case YH5151E_PAGE_3V3_LOG: > return YH5151E_PAGE_3V3_REAL; > } > @@ -103,17 +103,28 @@ static int set_page(struct i2c_client *client, int page_log) > > static int fsp3y_read_byte_data(struct i2c_client *client, int page, int reg) > { > + const struct pmbus_driver_info *info = pmbus_get_driver_info(client); > + struct fsp3y_data *data = to_fsp3y_data(info); > int rv; > > rv = set_page(client, page); > if (rv < 0) > return rv; > > + /* > + * YH5151-E outputs vout in linear11. The conversion is done later, but > + * I have to inject pmbus_core with the correct exponent. s/I/we/ Also, it might be useful to mention the actual exponent (-6 ?) in the comment. > + */ > + if (data->chip == yh5151e && reg == PMBUS_VOUT_MODE) > + return 0x1A; Can this code be moved ahead of set_page() ? > + > return i2c_smbus_read_byte_data(client, reg); > } > > static int fsp3y_read_word_data(struct i2c_client *client, int page, int phase, int reg) > { > + const struct pmbus_driver_info *info = pmbus_get_driver_info(client); > + struct fsp3y_data *data = to_fsp3y_data(info); > int rv; > > /* > @@ -144,7 +155,18 @@ static int fsp3y_read_word_data(struct i2c_client *client, int page, int phase, > if (rv < 0) > return rv; > > - return i2c_smbus_read_word_data(client, reg); > + rv = i2c_smbus_read_word_data(client, reg); > + if (rv < 0) > + return rv; > + > + /* > + * The PDU is non-compliant and outputs output voltages in linear11 s/The PDU/YH-5151E/ > + * instead of linear16. > + */ > + if (data->chip == yh5151e && reg == PMBUS_READ_VOUT) > + rv = ((s16)((rv & 0x7ff) << 5)) >> 5; Problem with this is that it auto-converts to int and will return a negative value if bit 10 is set (I wrote some test code and confirmed it, just to be sure). rv = sign_extend32(rv, 10) & 0xffff; should work here. Thanks, Guenter > + > + return rv; > } > > struct pmbus_driver_info fsp3y_info[] = { >
diff --git a/drivers/hwmon/pmbus/fsp-3y.c b/drivers/hwmon/pmbus/fsp-3y.c index 564649e87e34..dc147a79d6aa 100644 --- a/drivers/hwmon/pmbus/fsp-3y.c +++ b/drivers/hwmon/pmbus/fsp-3y.c @@ -57,7 +57,7 @@ static int page_log_to_page_real(int page_log, enum chips chip) case YH5151E_PAGE_12V_LOG: return YH5151E_PAGE_12V_REAL; case YH5151E_PAGE_5V_LOG: - return YH5151E_PAGE_5V_LOG; + return YH5151E_PAGE_5V_REAL; case YH5151E_PAGE_3V3_LOG: return YH5151E_PAGE_3V3_REAL; } @@ -103,17 +103,28 @@ static int set_page(struct i2c_client *client, int page_log) static int fsp3y_read_byte_data(struct i2c_client *client, int page, int reg) { + const struct pmbus_driver_info *info = pmbus_get_driver_info(client); + struct fsp3y_data *data = to_fsp3y_data(info); int rv; rv = set_page(client, page); if (rv < 0) return rv; + /* + * YH5151-E outputs vout in linear11. The conversion is done later, but + * I have to inject pmbus_core with the correct exponent. + */ + if (data->chip == yh5151e && reg == PMBUS_VOUT_MODE) + return 0x1A; + return i2c_smbus_read_byte_data(client, reg); } static int fsp3y_read_word_data(struct i2c_client *client, int page, int phase, int reg) { + const struct pmbus_driver_info *info = pmbus_get_driver_info(client); + struct fsp3y_data *data = to_fsp3y_data(info); int rv; /* @@ -144,7 +155,18 @@ static int fsp3y_read_word_data(struct i2c_client *client, int page, int phase, if (rv < 0) return rv; - return i2c_smbus_read_word_data(client, reg); + rv = i2c_smbus_read_word_data(client, reg); + if (rv < 0) + return rv; + + /* + * The PDU is non-compliant and outputs output voltages in linear11 + * instead of linear16. + */ + if (data->chip == yh5151e && reg == PMBUS_READ_VOUT) + rv = ((s16)((rv & 0x7ff) << 5)) >> 5; + + return rv; } struct pmbus_driver_info fsp3y_info[] = {
I didn't properly test the driver for YH-5151E, so it was completely broken. Firstly, the log/real mapping was incorrect in one case. Secondly, PMBus specifies that output voltages should be in the linear16 encoding. However, the PDU is non-compliant and uses linear11. YM-2151E isn't affected by this. Fix this by converting the values inside the read functions. linear16 gets the exponent from the VOUT_MODE command. The device doesn't support it, so I have to manually supply the value for it. Both supported devices have now been tested to report correct vout values. Signed-off-by: Václav Kubernát <kubernat@cesnet.cz> --- drivers/hwmon/pmbus/fsp-3y.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-)