All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: linux-hwmon@vger.kernel.org
Cc: Arthur Korn <akorn@google.com>, Guenter Roeck <linux@roeck-us.net>
Subject: [RFT PATCH 1/2] hwmon: (pmbus/adm1275) Prepare for protected write to PMON_CONFIG
Date: Wed, 14 Jun 2023 09:36:04 -0700	[thread overview]
Message-ID: <20230614163605.3688964-2-linux@roeck-us.net> (raw)
In-Reply-To: <20230614163605.3688964-1-linux@roeck-us.net>

According to ADI, changing PMON_CONFIG while ADC is running can have
unexpected results. ADI recommends halting the ADC with PMON_CONTROL
before setting PMON_CONFIG and then resume after.

To prepare for this change, rename adm1275_read_pmon_config()
and adm1275_write_pmon_config() to adm1275_read_samples() and
adm1275_write_samples() to more accurately reflect the functionality
of the code. Introduce new function adm1275_write_pmon_config()
and use it for all code writing into the PMON_CONFIG register.

Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/pmbus/adm1275.c | 56 +++++++++++++++++++----------------
 1 file changed, 31 insertions(+), 25 deletions(-)

diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c
index b8543c06d022..eaa691b98c14 100644
--- a/drivers/hwmon/pmbus/adm1275.c
+++ b/drivers/hwmon/pmbus/adm1275.c
@@ -173,8 +173,8 @@ static const struct coefficients adm1293_coefficients[] = {
 	[18] = { 7658, 0, -3 },		/* power, 21V, irange200 */
 };
 
-static int adm1275_read_pmon_config(const struct adm1275_data *data,
-				    struct i2c_client *client, bool is_power)
+static int adm1275_read_samples(const struct adm1275_data *data,
+				struct i2c_client *client, bool is_power)
 {
 	int shift, ret;
 	u16 mask;
@@ -200,8 +200,23 @@ static int adm1275_read_pmon_config(const struct adm1275_data *data,
 }
 
 static int adm1275_write_pmon_config(const struct adm1275_data *data,
-				     struct i2c_client *client,
-				     bool is_power, u16 word)
+				     struct i2c_client *client, u16 word)
+{
+	int ret;
+
+	if (data->have_power_sampling)
+		ret = i2c_smbus_write_word_data(client, ADM1275_PMON_CONFIG,
+						word);
+	else
+		ret = i2c_smbus_write_byte_data(client, ADM1275_PMON_CONFIG,
+						word);
+
+	return ret;
+}
+
+static int adm1275_write_samples(const struct adm1275_data *data,
+				 struct i2c_client *client,
+				 bool is_power, u16 word)
 {
 	int shift, ret;
 	u16 mask;
@@ -219,14 +234,8 @@ static int adm1275_write_pmon_config(const struct adm1275_data *data,
 		return ret;
 
 	word = (ret & ~mask) | ((word << shift) & mask);
-	if (data->have_power_sampling)
-		ret = i2c_smbus_write_word_data(client, ADM1275_PMON_CONFIG,
-						word);
-	else
-		ret = i2c_smbus_write_byte_data(client, ADM1275_PMON_CONFIG,
-						word);
 
-	return ret;
+	return adm1275_write_pmon_config(data, client, word);
 }
 
 static int adm1275_read_word_data(struct i2c_client *client, int page,
@@ -321,14 +330,14 @@ static int adm1275_read_word_data(struct i2c_client *client, int page,
 	case PMBUS_VIRT_POWER_SAMPLES:
 		if (!data->have_power_sampling)
 			return -ENXIO;
-		ret = adm1275_read_pmon_config(data, client, true);
+		ret = adm1275_read_samples(data, client, true);
 		if (ret < 0)
 			break;
 		ret = BIT(ret);
 		break;
 	case PMBUS_VIRT_IN_SAMPLES:
 	case PMBUS_VIRT_CURR_SAMPLES:
-		ret = adm1275_read_pmon_config(data, client, false);
+		ret = adm1275_read_samples(data, client, false);
 		if (ret < 0)
 			break;
 		ret = BIT(ret);
@@ -381,14 +390,12 @@ static int adm1275_write_word_data(struct i2c_client *client, int page, int reg,
 		if (!data->have_power_sampling)
 			return -ENXIO;
 		word = clamp_val(word, 1, ADM1275_SAMPLES_AVG_MAX);
-		ret = adm1275_write_pmon_config(data, client, true,
-						ilog2(word));
+		ret = adm1275_write_samples(data, client, true, ilog2(word));
 		break;
 	case PMBUS_VIRT_IN_SAMPLES:
 	case PMBUS_VIRT_CURR_SAMPLES:
 		word = clamp_val(word, 1, ADM1275_SAMPLES_AVG_MAX);
-		ret = adm1275_write_pmon_config(data, client, false,
-						ilog2(word));
+		ret = adm1275_write_samples(data, client, false, ilog2(word));
 		break;
 	default:
 		ret = -ENODATA;
@@ -466,13 +473,14 @@ static const struct i2c_device_id adm1275_id[] = {
 MODULE_DEVICE_TABLE(i2c, adm1275_id);
 
 /* Enable VOUT & TEMP1 if not enabled (disabled by default) */
-static int adm1275_enable_vout_temp(struct i2c_client *client, int config)
+static int adm1275_enable_vout_temp(struct adm1275_data *data,
+				    struct i2c_client *client, int config)
 {
 	int ret;
 
 	if ((config & ADM1278_PMON_DEFCONFIG) != ADM1278_PMON_DEFCONFIG) {
 		config |= ADM1278_PMON_DEFCONFIG;
-		ret = i2c_smbus_write_word_data(client, ADM1275_PMON_CONFIG, config);
+		ret = adm1275_write_pmon_config(data, client, config);
 		if (ret < 0) {
 			dev_err(&client->dev, "Failed to enable VOUT/TEMP1 monitoring\n");
 			return ret;
@@ -634,7 +642,7 @@ static int adm1275_probe(struct i2c_client *client)
 			PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
 			PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
 
-		ret = adm1275_enable_vout_temp(client, config);
+		ret = adm1275_enable_vout_temp(data, client, config);
 		if (ret)
 			return ret;
 
@@ -694,7 +702,7 @@ static int adm1275_probe(struct i2c_client *client)
 			PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
 			PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
 
-		ret = adm1275_enable_vout_temp(client, config);
+		ret = adm1275_enable_vout_temp(data, client, config);
 		if (ret)
 			return ret;
 
@@ -766,8 +774,7 @@ static int adm1275_probe(struct i2c_client *client)
 				"Invalid number of power samples");
 			return -EINVAL;
 		}
-		ret = adm1275_write_pmon_config(data, client, true,
-						ilog2(avg));
+		ret = adm1275_write_samples(data, client, true, ilog2(avg));
 		if (ret < 0) {
 			dev_err(&client->dev,
 				"Setting power sample averaging failed with error %d",
@@ -784,8 +791,7 @@ static int adm1275_probe(struct i2c_client *client)
 				"Invalid number of voltage/current samples");
 			return -EINVAL;
 		}
-		ret = adm1275_write_pmon_config(data, client, false,
-						ilog2(avg));
+		ret = adm1275_write_samples(data, client, false, ilog2(avg));
 		if (ret < 0) {
 			dev_err(&client->dev,
 				"Setting voltage and current sample averaging failed with error %d",
-- 
2.39.2


  reply	other threads:[~2023-06-14 16:36 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-14 16:36 [RFT PATCH 0/2] hwmon: (pmbus/adm1275) Protect writes to PMON_CONFIG Guenter Roeck
2023-06-14 16:36 ` Guenter Roeck [this message]
2023-06-14 16:36 ` [RFT PATCH 2/2] hwmon: (pmbus/adm1275) Disable ADC while updating PMON_CONFIG Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230614163605.3688964-2-linux@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=akorn@google.com \
    --cc=linux-hwmon@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.