[PATCH] Fix FSP-3Y YH-5151E non-compliant vout encoding

[PATCH] Fix FSP-3Y YH-5151E non-compliant vout encoding

[Václav Kubernát]

I didn't properly test the driver for the PDU, 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. The PSU
isn't affected by this.

pmbus_core didn't allow forcing linear11 format for output voltages, so
I added a way to force that.

Signed-off-by: Václav Kubernát <kubernat@cesnet.cz>
---
 drivers/hwmon/pmbus/fsp-3y.c     | 3 ++-
 drivers/hwmon/pmbus/pmbus.h      | 6 +++++-
 drivers/hwmon/pmbus/pmbus_core.c | 3 ++-
 3 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/pmbus/fsp-3y.c b/drivers/hwmon/pmbus/fsp-3y.c
index 564649e87e34..b4ea1e63272e 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;
 		}
@@ -164,6 +164,7 @@ struct pmbus_driver_info fsp3y_info[] = {
 	},
 	[yh5151e] = {
 		.pages = 3,
+		.format[PSC_VOLTAGE_OUT] = force_linear11,
 		.func[YH5151E_PAGE_12V_LOG] =
 			PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT |
 			PMBUS_HAVE_POUT  |
diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
index 4c30ec89f5bf..4d79a43fc965 100644
--- a/drivers/hwmon/pmbus/pmbus.h
+++ b/drivers/hwmon/pmbus/pmbus.h
@@ -405,7 +405,11 @@ enum pmbus_sensor_classes {
 #define PMBUS_PHASE_VIRTUAL	BIT(30)	/* Phases on this page are virtual */
 #define PMBUS_PAGE_VIRTUAL	BIT(31)	/* Page is virtual */
 
-enum pmbus_data_format { linear = 0, direct, vid };
+/*
+ * force_linear11 is for non-compliant devices that output VOUT in linear11
+ * instead of linear16.
+ */
+enum pmbus_data_format { linear = 0, force_linear11, direct, vid };
 enum vrm_version { vr11 = 0, vr12, vr13, imvp9, amd625mv };
 
 struct pmbus_driver_info {
diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 192442b3b7a2..45a0d0303c19 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -589,7 +589,8 @@ static s64 pmbus_reg2data_linear(struct pmbus_data *data,
 	s32 mantissa;
 	s64 val;
 
-	if (sensor->class == PSC_VOLTAGE_OUT) {	/* LINEAR16 */
+	if (sensor->class == PSC_VOLTAGE_OUT &&	/* LINEAR16 */
+	    data->info->format[sensor->class] != force_linear11) {
 		exponent = data->exponent[sensor->page];
 		mantissa = (u16) sensor->data;
 	} else {				/* LINEAR11 */
-- 
2.31.1

Re: [PATCH] Fix FSP-3Y YH-5151E non-compliant vout encoding

[Guenter Roeck]

On Tue, Apr 27, 2021 at 03:58:06PM +0200, Václav Kubernát wrote:
> I didn't properly test the driver for the PDU, 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. The PSU

It would be better to refer to YH5151E and YM2151; the reader won't know
how PSU and PDU map to the to the two power supplies supported by the
driver (and I don't know either).

> isn't affected by this.
> 
> pmbus_core didn't allow forcing linear11 format for output voltages, so
> I added a way to force that.
> 

Please don't do that. The driver should convert the linear11 values
to linear16 for the affected device(s) in fsp3y_read_word_data().

Thanks,
Guenter

> Signed-off-by: Václav Kubernát <kubernat@cesnet.cz>
> ---
>  drivers/hwmon/pmbus/fsp-3y.c     | 3 ++-
>  drivers/hwmon/pmbus/pmbus.h      | 6 +++++-
>  drivers/hwmon/pmbus/pmbus_core.c | 3 ++-
>  3 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/fsp-3y.c b/drivers/hwmon/pmbus/fsp-3y.c
> index 564649e87e34..b4ea1e63272e 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;
>  		}
> @@ -164,6 +164,7 @@ struct pmbus_driver_info fsp3y_info[] = {
>  	},
>  	[yh5151e] = {
>  		.pages = 3,
> +		.format[PSC_VOLTAGE_OUT] = force_linear11,
>  		.func[YH5151E_PAGE_12V_LOG] =
>  			PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT |
>  			PMBUS_HAVE_POUT  |
> diff --git a/drivers/hwmon/pmbus/pmbus.h b/drivers/hwmon/pmbus/pmbus.h
> index 4c30ec89f5bf..4d79a43fc965 100644
> --- a/drivers/hwmon/pmbus/pmbus.h
> +++ b/drivers/hwmon/pmbus/pmbus.h
> @@ -405,7 +405,11 @@ enum pmbus_sensor_classes {
>  #define PMBUS_PHASE_VIRTUAL	BIT(30)	/* Phases on this page are virtual */
>  #define PMBUS_PAGE_VIRTUAL	BIT(31)	/* Page is virtual */
>  
> -enum pmbus_data_format { linear = 0, direct, vid };
> +/*
> + * force_linear11 is for non-compliant devices that output VOUT in linear11
> + * instead of linear16.
> + */
> +enum pmbus_data_format { linear = 0, force_linear11, direct, vid };
>  enum vrm_version { vr11 = 0, vr12, vr13, imvp9, amd625mv };
>  
>  struct pmbus_driver_info {
> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 192442b3b7a2..45a0d0303c19 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c
> @@ -589,7 +589,8 @@ static s64 pmbus_reg2data_linear(struct pmbus_data *data,
>  	s32 mantissa;
>  	s64 val;
>  
> -	if (sensor->class == PSC_VOLTAGE_OUT) {	/* LINEAR16 */
> +	if (sensor->class == PSC_VOLTAGE_OUT &&	/* LINEAR16 */
> +	    data->info->format[sensor->class] != force_linear11) {
>  		exponent = data->exponent[sensor->page];
>  		mantissa = (u16) sensor->data;
>  	} else {				/* LINEAR11 */
> -- 
> 2.31.1
>