diff mbox series

[RFC,v2,2/4] hwmon: (jc42) Convert to regmap's built-in caching

Message ID 20221020210320.1624617-3-martin.blumenstingl@googlemail.com (mailing list archive)
State Superseded
Headers show
Series hwmon: (jc42) regmap conversion and resume fix | expand

Commit Message

Martin Blumenstingl Oct. 20, 2022, 9:03 p.m. UTC 
Move over to regmap's built-in caching instead of adding a custom
caching implementation. This works for JC42_REG_TEMP_UPPER,
JC42_REG_TEMP_LOWER and JC42_REG_TEMP_CRITICAL as these values never
change except when explicitly written. For JC42_REG_TEMP a cache
variable is still kept as regmap cannot cache this register (because
it's volatile, meaning it can change at any time).

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 drivers/hwmon/jc42.c | 97 ++++++++++++++++++++++++--------------------
 1 file changed, 54 insertions(+), 43 deletions(-)

Comments

Guenter Roeck Oct. 20, 2022, 10:04 p.m. UTC | #1 
On Thu, Oct 20, 2022 at 11:03:18PM +0200, Martin Blumenstingl wrote:
> Move over to regmap's built-in caching instead of adding a custom
> caching implementation. This works for JC42_REG_TEMP_UPPER,
> JC42_REG_TEMP_LOWER and JC42_REG_TEMP_CRITICAL as these values never
> change except when explicitly written. For JC42_REG_TEMP a cache
> variable is still kept as regmap cannot cache this register (because
> it's volatile, meaning it can change at any time).
> 

Just drop that one as well, together with jc42_update_device(),
and read the temperature directly where needed. In practice
caching of 'hot' registers isn't really worth the trouble.

Thanks,
Guenter

> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  drivers/hwmon/jc42.c | 97 ++++++++++++++++++++++++--------------------
>  1 file changed, 54 insertions(+), 43 deletions(-)
> 
> diff --git a/drivers/hwmon/jc42.c b/drivers/hwmon/jc42.c
> index 329a80264556..3f524ab5451c 100644
> --- a/drivers/hwmon/jc42.c
> +++ b/drivers/hwmon/jc42.c
> @@ -200,21 +200,6 @@ static struct jc42_chips jc42_chips[] = {
>  	{ STM_MANID, STTS3000_DEVID, STTS3000_DEVID_MASK },
>  };
>  
> -enum temp_index {
> -	t_input = 0,
> -	t_crit,
> -	t_min,
> -	t_max,
> -	t_num_temp
> -};
> -
> -static const u8 temp_regs[t_num_temp] = {
> -	[t_input] = JC42_REG_TEMP,
> -	[t_crit] = JC42_REG_TEMP_CRITICAL,
> -	[t_min] = JC42_REG_TEMP_LOWER,
> -	[t_max] = JC42_REG_TEMP_UPPER,
> -};
> -
>  /* Each client has this additional data */
>  struct jc42_data {
>  	struct regmap	*regmap;
> @@ -224,7 +209,7 @@ struct jc42_data {
>  	unsigned long	last_updated;	/* In jiffies */
>  	u16		orig_config;	/* original configuration */
>  	u16		config;		/* current configuration */
> -	u16		temp[t_num_temp];/* Temperatures */
> +	u16		temp;		/* Cached temperature register value */
diff mbox series

Patch

diff --git a/drivers/hwmon/jc42.c b/drivers/hwmon/jc42.c
index 329a80264556..3f524ab5451c 100644
--- a/drivers/hwmon/jc42.c
+++ b/drivers/hwmon/jc42.c
@@ -200,21 +200,6 @@  static struct jc42_chips jc42_chips[] = {
 	{ STM_MANID, STTS3000_DEVID, STTS3000_DEVID_MASK },
 };
 
-enum temp_index {
-	t_input = 0,
-	t_crit,
-	t_min,
-	t_max,
-	t_num_temp
-};
-
-static const u8 temp_regs[t_num_temp] = {
-	[t_input] = JC42_REG_TEMP,
-	[t_crit] = JC42_REG_TEMP_CRITICAL,
-	[t_min] = JC42_REG_TEMP_LOWER,
-	[t_max] = JC42_REG_TEMP_UPPER,
-};
-
 /* Each client has this additional data */
 struct jc42_data {
 	struct regmap	*regmap;
@@ -224,7 +209,7 @@  struct jc42_data {
 	unsigned long	last_updated;	/* In jiffies */
 	u16		orig_config;	/* original configuration */
 	u16		config;		/* current configuration */
-	u16		temp[t_num_temp];/* Temperatures */
+	u16		temp;		/* Cached temperature register value */
 };
 
 #define JC42_TEMP_MIN_EXTENDED	(-40000)
@@ -252,18 +237,17 @@  static int jc42_temp_from_reg(s16 reg)
 static struct jc42_data *jc42_update_device(struct device *dev)
 {
 	struct jc42_data *data = dev_get_drvdata(dev);
-	unsigned int i, val;
+	unsigned int val;
 	int ret;
 
 	mutex_lock(&data->update_lock);
 
 	if (time_after(jiffies, data->last_updated + HZ) || !data->valid) {
-		for (i = 0; i < t_num_temp; i++) {
-			ret = regmap_read(data->regmap, temp_regs[i], &val);
-			if (ret)
-				goto abort;
-			data->temp[i] = val;
-		}
+		ret = regmap_read(data->regmap, JC42_REG_TEMP, &val);
+		if (ret)
+			goto abort;
+
+		data->temp = val;
 		data->last_updated = jiffies;
 		data->valid = true;
 	}
@@ -276,44 +260,67 @@  static int jc42_read(struct device *dev, enum hwmon_sensor_types type,
 		     u32 attr, int channel, long *val)
 {
 	struct jc42_data *data = jc42_update_device(dev);
-	int temp, hyst;
+	unsigned int regval;
+	int ret, temp, hyst;
 
 	if (IS_ERR(data))
 		return PTR_ERR(data);
 
 	switch (attr) {
 	case hwmon_temp_input:
-		*val = jc42_temp_from_reg(data->temp[t_input]);
+		*val = jc42_temp_from_reg(data->temp);
 		return 0;
 	case hwmon_temp_min:
-		*val = jc42_temp_from_reg(data->temp[t_min]);
+		ret = regmap_read(data->regmap, JC42_REG_TEMP_LOWER, &regval);
+		if (ret)
+			return ret;
+
+		*val = jc42_temp_from_reg(regval);
 		return 0;
 	case hwmon_temp_max:
-		*val = jc42_temp_from_reg(data->temp[t_max]);
+		ret = regmap_read(data->regmap, JC42_REG_TEMP_UPPER, &regval);
+		if (ret)
+			return ret;
+
+		*val = jc42_temp_from_reg(regval);
 		return 0;
 	case hwmon_temp_crit:
-		*val = jc42_temp_from_reg(data->temp[t_crit]);
+		ret = regmap_read(data->regmap, JC42_REG_TEMP_CRITICAL,
+				  &regval);
+		if (ret)
+			return ret;
+
+		*val = jc42_temp_from_reg(regval);
 		return 0;
 	case hwmon_temp_max_hyst:
-		temp = jc42_temp_from_reg(data->temp[t_max]);
+		ret = regmap_read(data->regmap, JC42_REG_TEMP_UPPER, &regval);
+		if (ret)
+			return ret;
+
+		temp = jc42_temp_from_reg(regval);
 		hyst = jc42_hysteresis[(data->config & JC42_CFG_HYST_MASK)
 						>> JC42_CFG_HYST_SHIFT];
 		*val = temp - hyst;
 		return 0;
 	case hwmon_temp_crit_hyst:
-		temp = jc42_temp_from_reg(data->temp[t_crit]);
+		ret = regmap_read(data->regmap, JC42_REG_TEMP_CRITICAL,
+				  &regval);
+		if (ret)
+			return ret;
+
+		temp = jc42_temp_from_reg(regval);
 		hyst = jc42_hysteresis[(data->config & JC42_CFG_HYST_MASK)
 						>> JC42_CFG_HYST_SHIFT];
 		*val = temp - hyst;
 		return 0;
 	case hwmon_temp_min_alarm:
-		*val = (data->temp[t_input] >> JC42_ALARM_MIN_BIT) & 1;
+		*val = (data->temp >> JC42_ALARM_MIN_BIT) & 1;
 		return 0;
 	case hwmon_temp_max_alarm:
-		*val = (data->temp[t_input] >> JC42_ALARM_MAX_BIT) & 1;
+		*val = (data->temp >> JC42_ALARM_MAX_BIT) & 1;
 		return 0;
 	case hwmon_temp_crit_alarm:
-		*val = (data->temp[t_input] >> JC42_ALARM_CRIT_BIT) & 1;
+		*val = (data->temp >> JC42_ALARM_CRIT_BIT) & 1;
 		return 0;
 	default:
 		return -EOPNOTSUPP;
@@ -324,6 +331,7 @@  static int jc42_write(struct device *dev, enum hwmon_sensor_types type,
 		      u32 attr, int channel, long val)
 {
 	struct jc42_data *data = dev_get_drvdata(dev);
+	unsigned int regval;
 	int diff, hyst;
 	int ret;
 
@@ -331,21 +339,23 @@  static int jc42_write(struct device *dev, enum hwmon_sensor_types type,
 
 	switch (attr) {
 	case hwmon_temp_min:
-		data->temp[t_min] = jc42_temp_to_reg(val, data->extended);
-		ret = regmap_write(data->regmap, temp_regs[t_min],
-				   data->temp[t_min]);
+		ret = regmap_write(data->regmap, JC42_REG_TEMP_LOWER,
+				   jc42_temp_to_reg(val, data->extended));
 		break;
 	case hwmon_temp_max:
-		data->temp[t_max] = jc42_temp_to_reg(val, data->extended);
-		ret = regmap_write(data->regmap, temp_regs[t_max],
-				   data->temp[t_max]);
+		ret = regmap_write(data->regmap, JC42_REG_TEMP_UPPER,
+				   jc42_temp_to_reg(val, data->extended));
 		break;
 	case hwmon_temp_crit:
-		data->temp[t_crit] = jc42_temp_to_reg(val, data->extended);
-		ret = regmap_write(data->regmap, temp_regs[t_crit],
-				   data->temp[t_crit]);
+		ret = regmap_write(data->regmap, JC42_REG_TEMP_CRITICAL,
+				   jc42_temp_to_reg(val, data->extended));
 		break;
 	case hwmon_temp_crit_hyst:
+		ret = regmap_read(data->regmap, JC42_REG_TEMP_CRITICAL,
+				  &regval);
+		if (ret)
+			return ret;
+
 		/*
 		 * JC42.4 compliant chips only support four hysteresis values.
 		 * Pick best choice and go from there.
@@ -353,7 +363,7 @@  static int jc42_write(struct device *dev, enum hwmon_sensor_types type,
 		val = clamp_val(val, (data->extended ? JC42_TEMP_MIN_EXTENDED
 						     : JC42_TEMP_MIN) - 6000,
 				JC42_TEMP_MAX);
-		diff = jc42_temp_from_reg(data->temp[t_crit]) - val;
+		diff = jc42_temp_from_reg(regval) - val;
 		hyst = 0;
 		if (diff > 0) {
 			if (diff < 2250)
@@ -491,6 +501,7 @@  static const struct regmap_config jc42_regmap_config = {
 	.writeable_reg = jc42_writable_reg,
 	.readable_reg = jc42_readable_reg,
 	.volatile_reg = jc42_volatile_reg,
+	.cache_type = REGCACHE_RBTREE,
 };
 
 static int jc42_probe(struct i2c_client *client)