[PATCH v2 1/5] hwmon: (pmbus) Use -ENODATA for unhandled registers in MPS drivers

[PATCH v2 1/5] hwmon: (pmbus) Use -ENODATA for unhandled registers in MPS drivers

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

The read_word_data and write_word_data callbacks in mp2869, mp29502, and
mp2925 return -EINVAL for unhandled register addresses. In the PMBus core,
-ENODATA has a special meaning: it tells the core to fall through to the
standard PMBus register read/write path. Any other negative value (such
as -EINVAL) tells the core the register does not exist, causing valid
PMBus standard registers to be silently hidden.

Replace -EINVAL with -ENODATA in the default case of all affected
read_word_data and write_word_data callbacks so that standard PMBus
registers not handled by the driver are properly served by the core.

While at it, remove the explicit per-register -ENODATA cases in
mp2925_read_word_data() and the PMBUS_STATUS_WORD case in
mp29502_read_word_data() that are now redundant with the default.

Returning -ENODATA lets the PMBus core handle standard PMBus registers
through its normal fallback path. In mp2925 and mp29502, the existing
explicit -ENODATA cases are folded into the default case, preserving the
intended fallback behavior while simplifying the callback logic.

Fixes: a3a2923aaf7f ("hwmon: add MP2869,MP29608,MP29612 and MP29816 series driver")
Fixes: 90bad684e9ac ("hwmon: add MP29502 driver")
Fixes: a79472e30be4 ("hwmon: Add MP2925 and MP2929 driver")
Cc: stable@vger.kernel.org
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
v2:
- Folded explicit per-register -ENODATA cases into the default case
  per feedback.
- Reworded the fallback rationale to describe the intended PMBus core
  behavior more precisely.
---
 drivers/hwmon/pmbus/mp2869.c  |  4 ++--
 drivers/hwmon/pmbus/mp2925.c  | 21 ++-------------------
 drivers/hwmon/pmbus/mp29502.c |  7 ++-----
 3 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/drivers/hwmon/pmbus/mp2869.c b/drivers/hwmon/pmbus/mp2869.c
index cc69a1e91dfe..4f8543801298 100644
--- a/drivers/hwmon/pmbus/mp2869.c
+++ b/drivers/hwmon/pmbus/mp2869.c
@@ -391,7 +391,7 @@ static int mp2869_read_word_data(struct i2c_client *client, int page, int phase,
 		ret = (ret & GENMASK(7, 0)) * MP2869_POUT_OP_GAIN;
 		break;
 	default:
-		ret = -EINVAL;
+		ret = -ENODATA;
 		break;
 	}
 
@@ -536,7 +536,7 @@ static int mp2869_write_word_data(struct i2c_client *client, int page, int reg,
 								     MP2869_POUT_OP_GAIN)));
 		break;
 	default:
-		ret = -EINVAL;
+		ret = -ENODATA;
 		break;
 	}
 
diff --git a/drivers/hwmon/pmbus/mp2925.c b/drivers/hwmon/pmbus/mp2925.c
index ad094842cf2d..570e343fdf24 100644
--- a/drivers/hwmon/pmbus/mp2925.c
+++ b/drivers/hwmon/pmbus/mp2925.c
@@ -114,25 +114,8 @@ static int mp2925_read_word_data(struct i2c_client *client, int page, int phase,
 		ret = DIV_ROUND_CLOSEST((ret & GENMASK(11, 0)) * MP2925_VOUT_OVUV_UINT,
 					MP2925_VOUT_OVUV_DIV);
 		break;
-	case PMBUS_STATUS_WORD:
-	case PMBUS_READ_VIN:
-	case PMBUS_READ_IOUT:
-	case PMBUS_READ_POUT:
-	case PMBUS_READ_PIN:
-	case PMBUS_READ_IIN:
-	case PMBUS_READ_TEMPERATURE_1:
-	case PMBUS_VIN_OV_FAULT_LIMIT:
-	case PMBUS_VIN_OV_WARN_LIMIT:
-	case PMBUS_VIN_UV_WARN_LIMIT:
-	case PMBUS_VIN_UV_FAULT_LIMIT:
-	case PMBUS_IOUT_OC_FAULT_LIMIT:
-	case PMBUS_IOUT_OC_WARN_LIMIT:
-	case PMBUS_OT_FAULT_LIMIT:
-	case PMBUS_OT_WARN_LIMIT:
-		ret = -ENODATA;
-		break;
 	default:
-		ret = -EINVAL;
+		ret = -ENODATA;
 		break;
 	}
 
@@ -203,7 +186,7 @@ static int mp2925_write_word_data(struct i2c_client *client, int page, int reg,
 										 ret)));
 		break;
 	default:
-		ret = -EINVAL;
+		ret = -ENODATA;
 		break;
 	}
 
diff --git a/drivers/hwmon/pmbus/mp29502.c b/drivers/hwmon/pmbus/mp29502.c
index 7241373f1557..aad4c57a0a2c 100644
--- a/drivers/hwmon/pmbus/mp29502.c
+++ b/drivers/hwmon/pmbus/mp29502.c
@@ -293,9 +293,6 @@ static int mp29502_read_word_data(struct i2c_client *client, int page,
 	int ret;
 
 	switch (reg) {
-	case PMBUS_STATUS_WORD:
-		ret = -ENODATA;
-		break;
 	case PMBUS_READ_VIN:
 		/*
 		 * The MP29502 PMBUS_READ_VIN[10:0] is the vin value, the vin scale is
@@ -456,7 +453,7 @@ static int mp29502_read_word_data(struct i2c_client *client, int page,
 		ret = (ret & GENMASK(7, 0)) - MP29502_TEMP_LIMIT_OFFSET;
 		break;
 	default:
-		ret = -EINVAL;
+		ret = -ENODATA;
 		break;
 	}
 
@@ -555,7 +552,7 @@ static int mp29502_write_word_data(struct i2c_client *client, int page, int reg,
 						   word + MP29502_TEMP_LIMIT_OFFSET));
 		break;
 	default:
-		ret = -EINVAL;
+		ret = -ENODATA;
 		break;
 	}
 
-- 
2.34.1

[PATCH v2 2/5] hwmon: (pmbus) Fix return type truncation in MPS reg2data_linear11()

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

mp2869_reg2data_linear11() and mp29502_reg2data_linear11() decode
a Linear11 PMBus value using signed intermediates but return u16.
This can silently truncate negative or oversized results before they
are consumed by the driver read_word_data() callback path.

Those helpers feed values later returned through the driver
read_word_data() callback path. In that path, negative integers are
reserved for errors, so successful decoded values must remain in a
non-negative bounded range.

Change the helper return type to int and clamp the result to
[0, 0xffff]. This makes the bounded result explicit instead of
relying on implicit truncation to u16, and keeps the conversion
behavior consistent for all helper users.

Fixes: a3a2923aaf7f ("hwmon: add MP2869,MP29608,MP29612 and MP29816 series driver")
Fixes: 90bad684e9ac ("hwmon: add MP29502 driver")
Cc: stable@vger.kernel.org
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
v2:
- No changes to this patch in this version.
---
 drivers/hwmon/pmbus/mp2869.c  | 4 ++--
 drivers/hwmon/pmbus/mp29502.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/pmbus/mp2869.c b/drivers/hwmon/pmbus/mp2869.c
index 4f8543801298..fc4ce854c9c3 100644
--- a/drivers/hwmon/pmbus/mp2869.c
+++ b/drivers/hwmon/pmbus/mp2869.c
@@ -65,7 +65,7 @@ static const int mp2869_iout_sacle[8] = {32, 1, 2, 4, 8, 16, 32, 64};
 
 #define to_mp2869_data(x)	container_of(x, struct mp2869_data, info)
 
-static u16 mp2869_reg2data_linear11(u16 word)
+static int mp2869_reg2data_linear11(u16 word)
 {
 	s16 exponent;
 	s32 mantissa;
@@ -80,7 +80,7 @@ static u16 mp2869_reg2data_linear11(u16 word)
 	else
 		val >>= -exponent;
 
-	return val;
+	return clamp_val(val, 0, 0xffff);
 }
 
 static int
diff --git a/drivers/hwmon/pmbus/mp29502.c b/drivers/hwmon/pmbus/mp29502.c
index 4556bc8350ae..1457809aa7e4 100644
--- a/drivers/hwmon/pmbus/mp29502.c
+++ b/drivers/hwmon/pmbus/mp29502.c
@@ -52,7 +52,7 @@ struct mp29502_data {
 
 #define to_mp29502_data(x)	container_of(x, struct mp29502_data, info)
 
-static u16 mp29502_reg2data_linear11(u16 word)
+static int mp29502_reg2data_linear11(u16 word)
 {
 	s16 exponent;
 	s32 mantissa;
@@ -67,7 +67,7 @@ static u16 mp29502_reg2data_linear11(u16 word)
 	else
 		val >>= -exponent;
 
-	return val;
+	return clamp_val(val, 0, 0xffff);
 }
 
 static int
-- 
2.34.1

[PATCH v2 3/5] hwmon: (pmbus/mp9945) Replace raw I2C calls with PMBus core API

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

The mp9945 read_byte_data, read_word_data, and mp9945_read_vout
callbacks use raw i2c_smbus_write_byte_data() to set PMBUS_PAGE and
raw i2c_smbus_read_word_data() to read registers. These raw page
writes desynchronize the PMBus core's internal page cache: after a raw
write to PMBUS_PAGE, the core still believes the previous page is
selected and may skip the page-select on the next pmbus_read_word_data()
call, causing reads from the wrong page. As a secondary benefit,
switching to the core helpers also routes all post-probe accesses
through the update_lock mutex, closing a potential race with concurrent
sysfs reads.

Replace the raw I2C calls with pmbus_read_word_data(), which handles
page selection, page cache coherency, and locking internally. Remove
the now-unnecessary manual PMBUS_PAGE writes from read_byte_data and
read_word_data. The identify() function retains raw I2C because it
runs during probe before pmbus_do_probe() registers the device.

Fixes: 6923e2827d58 ("hwmon: (pmbus) add driver for MPS MP9945")
Cc: stable@vger.kernel.org
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
v2:
- No changes to this patch in this version.
---
 drivers/hwmon/pmbus/mp9945.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/hwmon/pmbus/mp9945.c b/drivers/hwmon/pmbus/mp9945.c
index 34822e0de812..1723ef84eb0c 100644
--- a/drivers/hwmon/pmbus/mp9945.c
+++ b/drivers/hwmon/pmbus/mp9945.c
@@ -43,11 +43,12 @@ struct mp9945_data {
 
 #define to_mp9945_data(x) container_of(x, struct mp9945_data, info)
 
-static int mp9945_read_vout(struct i2c_client *client, struct mp9945_data *data)
+static int mp9945_read_vout(struct i2c_client *client, struct mp9945_data *data,
+			   int page, int phase)
 {
 	int ret;
 
-	ret = i2c_smbus_read_word_data(client, PMBUS_READ_VOUT);
+	ret = pmbus_read_word_data(client, page, phase, PMBUS_READ_VOUT);
 	if (ret < 0)
 		return ret;
 
@@ -73,12 +74,6 @@ static int mp9945_read_vout(struct i2c_client *client, struct mp9945_data *data)
 
 static int mp9945_read_byte_data(struct i2c_client *client, int page, int reg)
 {
-	int ret;
-
-	ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
-	if (ret < 0)
-		return ret;
-
 	switch (reg) {
 	case PMBUS_VOUT_MODE:
 		/*
@@ -98,17 +93,13 @@ static int mp9945_read_word_data(struct i2c_client *client, int page, int phase,
 	struct mp9945_data *data = to_mp9945_data(info);
 	int ret;
 
-	ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
-	if (ret < 0)
-		return ret;
-
 	switch (reg) {
 	case PMBUS_READ_VOUT:
-		ret = mp9945_read_vout(client, data);
+		ret = mp9945_read_vout(client, data, page, phase);
 		break;
 	case PMBUS_VOUT_OV_FAULT_LIMIT:
 	case PMBUS_VOUT_UV_FAULT_LIMIT:
-		ret = i2c_smbus_read_word_data(client, reg);
+		ret = pmbus_read_word_data(client, page, phase, reg);
 		if (ret < 0)
 			return ret;
 
@@ -116,7 +107,7 @@ static int mp9945_read_word_data(struct i2c_client *client, int page, int phase,
 		ret = DIV_ROUND_CLOSEST((ret & GENMASK(11, 0)) * 39, 20);
 		break;
 	case PMBUS_VOUT_UV_WARN_LIMIT:
-		ret = i2c_smbus_read_word_data(client, reg);
+		ret = pmbus_read_word_data(client, page, phase, reg);
 		if (ret < 0)
 			return ret;
 
-- 
2.34.1

[PATCH v2 4/5] hwmon: (pmbus/mp29502) Replace raw I2C calls with PMBus core API

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

The mp29502 read_byte_data, read_vout_ov_limit, write_vout_ov_limit,
and write_word_data callbacks use raw i2c_smbus_write_byte_data() to
set PMBUS_PAGE and raw i2c_smbus_read/write_word_data() for register
access. These raw page writes desynchronize the PMBus core's internal
page cache: after a raw write to PMBUS_PAGE, the core still believes
the previous page is selected and may skip the page-select on the
next pmbus_read_word_data() call, reading from the wrong page. As a
secondary benefit, switching to the core helpers also routes all
post-probe accesses through the update_lock mutex, closing a potential
race with concurrent sysfs reads.

Replace the raw I2C calls in read_vout_ov_limit and write_vout_ov_limit
with pmbus_read_word_data(client, 1, 0xff, reg) and
pmbus_write_word_data(client, 1, reg, word), which handle page
selection, page cache coherency, and locking internally. Page 1 is
selected explicitly as the OV limit registers reside on page 1 per the
datasheet; the phase argument 0xff indicates phase is not applicable.
Remove the manual PMBUS_PAGE writes from read_byte_data and
write_word_data, and simplify read_byte_data to use direct returns.

Fixes: 90bad684e9ac ("hwmon: add MP29502 driver")
Cc: stable@vger.kernel.org
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
v2:
- No changes to this patch in this version.
---
 drivers/hwmon/pmbus/mp29502.c | 68 +++++++++--------------------------
 1 file changed, 17 insertions(+), 51 deletions(-)

diff --git a/drivers/hwmon/pmbus/mp29502.c b/drivers/hwmon/pmbus/mp29502.c
index 1457809aa7e4..aef9d957bdf1 100644
--- a/drivers/hwmon/pmbus/mp29502.c
+++ b/drivers/hwmon/pmbus/mp29502.c
@@ -210,31 +210,18 @@ mp29502_identify_iout_scale(struct i2c_client *client, struct pmbus_driver_info
 static int mp29502_read_vout_ov_limit(struct i2c_client *client, struct mp29502_data *data)
 {
 	int ret;
-	int ov_value;
 
 	/*
-	 * This is because the vout ov fault limit value comes from
-	 * page1 MFR_TSNS_FLT_SET reg, and other telemetry and limit
-	 * value comes from page0 reg. So the page should be set to
-	 * 0 after the reading of vout ov limit.
+	 * The vout ov fault limit value comes from page 1
+	 * MFR_TSNS_FLT_SET register.
 	 */
-	ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 1);
+	ret = pmbus_read_word_data(client, 1, 0xff, MFR_TSNS_FLT_SET);
 	if (ret < 0)
 		return ret;
 
-	ret = i2c_smbus_read_word_data(client, MFR_TSNS_FLT_SET);
-	if (ret < 0)
-		return ret;
-
-	ov_value = DIV_ROUND_CLOSEST(FIELD_GET(GENMASK(12, 7), ret) *
-						   MP28502_VOUT_OV_GAIN * MP28502_VOUT_OV_SCALE,
-						   data->ovp_div);
-
-	ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
-	if (ret < 0)
-		return ret;
-
-	return ov_value;
+	return DIV_ROUND_CLOSEST(FIELD_GET(GENMASK(12, 7), ret) *
+				 MP28502_VOUT_OV_GAIN * MP28502_VOUT_OV_SCALE,
+				 data->ovp_div);
 }
 
 static int mp29502_write_vout_ov_limit(struct i2c_client *client, u16 word,
@@ -243,46 +230,29 @@ static int mp29502_write_vout_ov_limit(struct i2c_client *client, u16 word,
 	int ret;
 
 	/*
-	 * This is because the vout ov fault limit value comes from
-	 * page1 MFR_TSNS_FLT_SET reg, and other telemetry and limit
-	 * value comes from page0 reg. So the page should be set to
-	 * 0 after the writing of vout ov limit.
+	 * The vout ov fault limit value is in page 1
+	 * MFR_TSNS_FLT_SET register.
 	 */
-	ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 1);
-	if (ret < 0)
-		return ret;
-
-	ret = i2c_smbus_read_word_data(client, MFR_TSNS_FLT_SET);
+	ret = pmbus_read_word_data(client, 1, 0xff, MFR_TSNS_FLT_SET);
 	if (ret < 0)
 		return ret;
 
-	ret = i2c_smbus_write_word_data(client, MFR_TSNS_FLT_SET,
-					(ret & ~GENMASK(12, 7)) |
-		FIELD_PREP(GENMASK(12, 7),
-			   DIV_ROUND_CLOSEST(word * data->ovp_div,
-					     MP28502_VOUT_OV_GAIN * MP28502_VOUT_OV_SCALE)));
-
-	return i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
+	return pmbus_write_word_data(client, 1, MFR_TSNS_FLT_SET,
+				    (ret & ~GENMASK(12, 7)) |
+				FIELD_PREP(GENMASK(12, 7),
+					   DIV_ROUND_CLOSEST(word * data->ovp_div,
+							     MP28502_VOUT_OV_GAIN *
+							     MP28502_VOUT_OV_SCALE)));
 }
 
 static int mp29502_read_byte_data(struct i2c_client *client, int page, int reg)
 {
-	int ret;
-
-	ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
-	if (ret < 0)
-		return ret;
-
 	switch (reg) {
 	case PMBUS_VOUT_MODE:
-		ret = PB_VOUT_MODE_DIRECT;
-		break;
+		return PB_VOUT_MODE_DIRECT;
 	default:
-		ret = -ENODATA;
-		break;
+		return -ENODATA;
 	}
-
-	return ret;
 }
 
 static int mp29502_read_word_data(struct i2c_client *client, int page,
@@ -470,10 +440,6 @@ static int mp29502_write_word_data(struct i2c_client *client, int page, int reg,
 	struct mp29502_data *data = to_mp29502_data(info);
 	int ret;
 
-	ret = i2c_smbus_write_byte_data(client, PMBUS_PAGE, 0);
-	if (ret < 0)
-		return ret;
-
 	switch (reg) {
 	case PMBUS_VIN_OV_FAULT_LIMIT:
 		/*
-- 
2.34.1

[PATCH v2 5/5] hwmon: (pmbus/mp29502) Prevent division by zero from hardware register

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@juniper.net>

mp29502_identify_vout_divider() and mp29502_identify_ovp_divider() read
divider values from hardware registers (MFR_VOUT_PROT1 bits [11:0] and
MFR_SLOPE_CNT_SET bits [9:0]) into data->vout_bottom_div and
data->ovp_div respectively. These divisors are used in
DIV_ROUND_CLOSEST() calculations across multiple read and write paths:
vout_bottom_div feeds the PMBUS_READ_VOUT, PMBUS_READ_POUT, and
PMBUS_VOUT_UV_FAULT_LIMIT handlers in addition to the OV-limit helpers,
while ovp_div is used in mp29502_read_vout_ov_limit() and
mp29502_write_vout_ov_limit(). If the hardware returns zero for either
field, a division-by-zero exception occurs at runtime.

Add zero-value guards that return -EINVAL when a divisor is zero,
indicating the hardware returned an invalid configuration. This causes
probe to fail gracefully rather than crashing with a divide exception.

Fixes: 90bad684e9ac ("hwmon: add MP29502 driver")
Cc: stable@vger.kernel.org
Signed-off-by: Sanman Pradhan <psanman@juniper.net>
---
v2:
- No changes to this patch in this version.
---
 drivers/hwmon/pmbus/mp29502.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hwmon/pmbus/mp29502.c b/drivers/hwmon/pmbus/mp29502.c
index aef9d957bdf1..bbcf018e5d05 100644
--- a/drivers/hwmon/pmbus/mp29502.c
+++ b/drivers/hwmon/pmbus/mp29502.c
@@ -134,6 +134,8 @@ mp29502_identify_vout_divider(struct i2c_client *client, struct pmbus_driver_inf
 		return ret;
 
 	data->vout_bottom_div = FIELD_GET(GENMASK(11, 0), ret);
+	if (!data->vout_bottom_div)
+		return -EINVAL;
 
 	ret = i2c_smbus_read_word_data(client, MFR_VOUT_PROT2);
 	if (ret < 0)
@@ -160,6 +162,8 @@ mp29502_identify_ovp_divider(struct i2c_client *client, struct pmbus_driver_info
 		return ret;
 
 	data->ovp_div = FIELD_GET(GENMASK(9, 0), ret);
+	if (!data->ovp_div)
+		return -EINVAL;
 
 	return 0;
 }
-- 
2.34.1