[PATCH v4 1/7] hwmon: max31827: Make code cleaner

From: Daniel Matyas
Date: Tue Sep 19 2023 - 05:35:52 EST


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

v3 -> v4: Put the wait time modification in another patch.

v2 -> v3: No change.

v2: Added patch.

drivers/hwmon/max31827.c | 125 +++++++++++++++++----------------------
1 file changed, 55 insertions(+), 70 deletions(-)

diff --git a/drivers/hwmon/max31827.c b/drivers/hwmon/max31827.c
index 602f4e4f81ff..47124d665fa7 100644
--- a/drivers/hwmon/max31827.c
+++ b/drivers/hwmon/max31827.c
@@ -27,18 +27,30 @@

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