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>
---
 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