devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Daniel Matyas <daniel.matyas@analog.com>
To: unlisted-recipients:; (no To-header on input)
Cc: Daniel Matyas <daniel.matyas@analog.com>,
	Jean Delvare <jdelvare@suse.com>,
	Guenter Roeck <linux@roeck-us.net>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Jonathan Corbet <corbet@lwn.net>, <linux-hwmon@vger.kernel.org>,
	<devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-doc@vger.kernel.org>
Subject: [PATCH 1/4] hwmon: max31827: Make code cleaner
Date: Wed, 13 Sep 2023 18:21:31 +0300	[thread overview]
Message-ID: <20230913152135.457892-2-daniel.matyas@analog.com> (raw)
In-Reply-To: <20230913152135.457892-1-daniel.matyas@analog.com>

Now the wait time for one-shot is 140ms, instead of the old 141
(removed the 1ms error).

Used enums and while loops to replace switch for selecting and getting
update interval from conversion rate bits.

Divided the write_alarm_val function into 2 functions. The new function
is more generic: it can be used not only for alarm writes, but for any
kind of writes which require the device to be in shutdown mode.

Signed-off-by: Daniel Matyas <daniel.matyas@analog.com>
---
 Documentation/hwmon/max31827.rst |   4 +-
 drivers/hwmon/max31827.c         | 127 ++++++++++++++-----------------
 2 files changed, 58 insertions(+), 73 deletions(-)

diff --git a/Documentation/hwmon/max31827.rst b/Documentation/hwmon/max31827.rst
index b0971d05b8a4..9a1055a007cf 100644
--- a/Documentation/hwmon/max31827.rst
+++ b/Documentation/hwmon/max31827.rst
@@ -73,8 +73,8 @@ the conversion frequency to 1 conv/s. The conversion time varies depending on
 the resolution. The conversion time doubles with every bit of increased
 resolution. For 10 bit resolution 35ms are needed, while for 12 bit resolution
 (default) 140ms. When chip is in shutdown mode and a read operation is
-requested, one-shot is triggered, the device waits for 140 (conversion time) + 1
-(error) ms, and only after that is the temperature value register read.
+requested, one-shot is triggered, the device waits for 140 (conversion time) ms,
+and only after that is the temperature value register read.
 
 The LSB of the temperature values is 0.0625 degrees Celsius, but the values of
 the temperatures are displayed in milli-degrees. This means, that some data is
diff --git a/drivers/hwmon/max31827.c b/drivers/hwmon/max31827.c
index 602f4e4f81ff..f05762219995 100644
--- a/drivers/hwmon/max31827.c
+++ b/drivers/hwmon/max31827.c
@@ -25,20 +25,32 @@
 #define MAX31827_CONFIGURATION_U_TEMP_STAT_MASK	BIT(14)
 #define MAX31827_CONFIGURATION_O_TEMP_STAT_MASK	BIT(15)
 
-#define MAX31827_12_BIT_CNV_TIME	141
-
-#define MAX31827_CNV_1_DIV_64_HZ	0x1
-#define MAX31827_CNV_1_DIV_32_HZ	0x2
-#define MAX31827_CNV_1_DIV_16_HZ	0x3
-#define MAX31827_CNV_1_DIV_4_HZ		0x4
-#define MAX31827_CNV_1_HZ		0x5
-#define MAX31827_CNV_4_HZ		0x6
-#define MAX31827_CNV_8_HZ		0x7
+#define MAX31827_12_BIT_CNV_TIME	140
 
 #define MAX31827_16_BIT_TO_M_DGR(x)	(sign_extend32(x, 15) * 1000 / 16)
 #define MAX31827_M_DGR_TO_16_BIT(x)	(((x) << 4) / 1000)
 #define MAX31827_DEVICE_ENABLE(x)	((x) ? 0xA : 0x0)
 
+enum max31827_cnv {
+	MAX31827_CNV_1_DIV_64_HZ = 1,
+	MAX31827_CNV_1_DIV_32_HZ,
+	MAX31827_CNV_1_DIV_16_HZ,
+	MAX31827_CNV_1_DIV_4_HZ,
+	MAX31827_CNV_1_HZ,
+	MAX31827_CNV_4_HZ,
+	MAX31827_CNV_8_HZ,
+};
+
+static const u16 max31827_conversions[] = {
+	[MAX31827_CNV_1_DIV_64_HZ] = 64000,
+	[MAX31827_CNV_1_DIV_32_HZ] = 32000,
+	[MAX31827_CNV_1_DIV_16_HZ] = 16000,
+	[MAX31827_CNV_1_DIV_4_HZ] = 4000,
+	[MAX31827_CNV_1_HZ] = 1000,
+	[MAX31827_CNV_4_HZ] = 250,
+	[MAX31827_CNV_8_HZ] = 125,
+};
+
 struct max31827_state {
 	/*
 	 * Prevent simultaneous access to the i2c client.
@@ -54,15 +66,13 @@ static const struct regmap_config max31827_regmap = {
 	.max_register = 0xA,
 };
 
-static int write_alarm_val(struct max31827_state *st, unsigned int reg,
-			   long val)
+static int shutdown_write(struct max31827_state *st, unsigned int reg,
+			  unsigned int val)
 {
 	unsigned int cfg;
-	unsigned int tmp;
+	unsigned int cnv_rate;
 	int ret;
 
-	val = MAX31827_M_DGR_TO_16_BIT(val);
-
 	/*
 	 * Before the Temperature Threshold Alarm and Alarm Hysteresis Threshold
 	 * register values are changed over I2C, the part must be in shutdown
@@ -82,9 +92,10 @@ static int write_alarm_val(struct max31827_state *st, unsigned int reg,
 	if (ret)
 		goto unlock;
 
-	tmp = cfg & ~(MAX31827_CONFIGURATION_1SHOT_MASK |
+	cnv_rate = MAX31827_CONFIGURATION_CNV_RATE_MASK & cfg;
+	cfg = cfg & ~(MAX31827_CONFIGURATION_1SHOT_MASK |
 		      MAX31827_CONFIGURATION_CNV_RATE_MASK);
-	ret = regmap_write(st->regmap, MAX31827_CONFIGURATION_REG, tmp);
+	ret = regmap_write(st->regmap, MAX31827_CONFIGURATION_REG, cfg);
 	if (ret)
 		goto unlock;
 
@@ -92,13 +103,23 @@ static int write_alarm_val(struct max31827_state *st, unsigned int reg,
 	if (ret)
 		goto unlock;
 
-	ret = regmap_write(st->regmap, MAX31827_CONFIGURATION_REG, cfg);
+	ret = regmap_update_bits(st->regmap, MAX31827_CONFIGURATION_REG,
+				 MAX31827_CONFIGURATION_CNV_RATE_MASK,
+				 cnv_rate);
 
 unlock:
 	mutex_unlock(&st->lock);
 	return ret;
 }
 
+static int write_alarm_val(struct max31827_state *st, unsigned int reg,
+			   long val)
+{
+	val = MAX31827_M_DGR_TO_16_BIT(val);
+
+	return shutdown_write(st, reg, val);
+}
+
 static umode_t max31827_is_visible(const void *state,
 				   enum hwmon_sensor_types type, u32 attr,
 				   int channel)
@@ -243,32 +264,7 @@ static int max31827_read(struct device *dev, enum hwmon_sensor_types type,
 
 			uval = FIELD_GET(MAX31827_CONFIGURATION_CNV_RATE_MASK,
 					 uval);
-			switch (uval) {
-			case MAX31827_CNV_1_DIV_64_HZ:
-				*val = 64000;
-				break;
-			case MAX31827_CNV_1_DIV_32_HZ:
-				*val = 32000;
-				break;
-			case MAX31827_CNV_1_DIV_16_HZ:
-				*val = 16000;
-				break;
-			case MAX31827_CNV_1_DIV_4_HZ:
-				*val = 4000;
-				break;
-			case MAX31827_CNV_1_HZ:
-				*val = 1000;
-				break;
-			case MAX31827_CNV_4_HZ:
-				*val = 250;
-				break;
-			case MAX31827_CNV_8_HZ:
-				*val = 125;
-				break;
-			default:
-				*val = 0;
-				break;
-			}
+			*val = max31827_conversions[uval];
 		}
 		break;
 
@@ -284,6 +280,7 @@ static int max31827_write(struct device *dev, enum hwmon_sensor_types type,
 			  u32 attr, int channel, long val)
 {
 	struct max31827_state *st = dev_get_drvdata(dev);
+	int res = 1;
 	int ret;
 
 	switch (type) {
@@ -333,39 +330,27 @@ static int max31827_write(struct device *dev, enum hwmon_sensor_types type,
 			if (!st->enable)
 				return -EINVAL;
 
-			switch (val) {
-			case 125:
-				val = MAX31827_CNV_8_HZ;
-				break;
-			case 250:
-				val = MAX31827_CNV_4_HZ;
-				break;
-			case 1000:
-				val = MAX31827_CNV_1_HZ;
-				break;
-			case 4000:
-				val = MAX31827_CNV_1_DIV_4_HZ;
-				break;
-			case 16000:
-				val = MAX31827_CNV_1_DIV_16_HZ;
-				break;
-			case 32000:
-				val = MAX31827_CNV_1_DIV_32_HZ;
-				break;
-			case 64000:
-				val = MAX31827_CNV_1_DIV_64_HZ;
-				break;
-			default:
-				return -EINVAL;
-			}
+			/*
+			 * Convert the desired conversion rate into register
+			 * bits. res is already initialized with 1.
+			 *
+			 * This was inspired by lm73 driver.
+			 */
+			while (res < ARRAY_SIZE(max31827_conversions) &&
+			       val < max31827_conversions[res])
+				res++;
+
+			if (res == ARRAY_SIZE(max31827_conversions) ||
+			    val != max31827_conversions[res])
+				return -EOPNOTSUPP;
 
-			val = FIELD_PREP(MAX31827_CONFIGURATION_CNV_RATE_MASK,
-					 val);
+			res = FIELD_PREP(MAX31827_CONFIGURATION_CNV_RATE_MASK,
+					 res);
 
 			return regmap_update_bits(st->regmap,
 						  MAX31827_CONFIGURATION_REG,
 						  MAX31827_CONFIGURATION_CNV_RATE_MASK,
-						  val);
+						  res);
 		}
 		break;
 
-- 
2.34.1


  reply	other threads:[~2023-09-13 15:22 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-13 15:21 [PATCH 0/4] Why no v2? Daniel Matyas
2023-09-13 15:21 ` Daniel Matyas [this message]
2023-09-13 15:21 ` [PATCH 2/4] dt-bindings: hwmon: Added new properties to the devicetree Daniel Matyas
2023-09-13 15:40   ` Krzysztof Kozlowski
2023-09-13 15:48     ` Matyas, Daniel
2023-09-14  5:43       ` Krzysztof Kozlowski
2023-09-13 16:43     ` Guenter Roeck
2023-09-14  5:41       ` Krzysztof Kozlowski
2023-09-14  6:07         ` Guenter Roeck
2023-09-13 15:21 ` [PATCH 3/4] hwmon: max31827: Handle new properties from " Daniel Matyas
2023-09-13 16:44   ` Guenter Roeck
2023-09-14  4:39   ` kernel test robot
2023-09-13 15:21 ` [PATCH 4/4] hwmon: max31827: Add custom attribute for resolution Daniel Matyas
2023-09-14 13:34   ` kernel test robot
2023-09-13 16:01 ` [PATCH 0/4] Why no v2? 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=20230913152135.457892-2-daniel.matyas@analog.com \
    --to=daniel.matyas@analog.com \
    --cc=conor+dt@kernel.org \
    --cc=corbet@lwn.net \
    --cc=devicetree@vger.kernel.org \
    --cc=jdelvare@suse.com \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=robh+dt@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).