[PATCH v2 4/5] hwmon: (sch5627) Add support for writing limit registers

From: Armin Wolf
Date: Thu Sep 07 2023 - 01:27:17 EST

Next message: Armin Wolf: "[PATCH v2 5/5] hwmon: (sch5627) Document behaviour of limit registers"
Previous message: Armin Wolf: "[PATCH v2 3/5] hwmon: (sch5627) Use regmap for pwm map register caching"
In reply to: Armin Wolf: "[PATCH v2 3/5] hwmon: (sch5627) Use regmap for pwm map register caching"
Next in thread: Armin Wolf: "[PATCH v2 5/5] hwmon: (sch5627) Document behaviour of limit registers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

After some testing on a Fujitsu Esprimo P720, it turned out that
the limit registers are indeed writable and affect the fan control
algorithm. This is supported by the datasheet, which says that the
fan control functions are based on the limit and parameter registers.
Since accessing those registers is very inefficient, the existing
regmap cache is used to cache those registers values.

Tested on a Fujitsu Esprimo P720.

Signed-off-by: Armin Wolf <W_Armin@xxxxxx>
---
drivers/hwmon/sch5627.c | 174 ++++++++++++++++++++++-----------
drivers/hwmon/sch56xx-common.c | 31 ++++++
drivers/hwmon/sch56xx-common.h | 3 +
3 files changed, 153 insertions(+), 55 deletions(-)

diff --git a/drivers/hwmon/sch5627.c b/drivers/hwmon/sch5627.c
index 85f8352cb3cf..1891d4d75aa9 100644
--- a/drivers/hwmon/sch5627.c
+++ b/drivers/hwmon/sch5627.c
@@ -7,6 +7,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

#include <linux/bits.h>
+#include <linux/minmax.h>
#include <linux/module.h>
#include <linux/mod_devicetable.h>
#include <linux/pm.h>
@@ -77,9 +78,6 @@ struct sch5627_data {
struct regmap *regmap;
unsigned short addr;
u8 control;
- u8 temp_max[SCH5627_NO_TEMPS];
- u8 temp_crit[SCH5627_NO_TEMPS];
- u16 fan_min[SCH5627_NO_FANS];

struct mutex update_lock;
unsigned long last_battery; /* In jiffies */
@@ -95,7 +93,17 @@ struct sch5627_data {
};

static const struct regmap_range sch5627_tunables_ranges[] = {
+ regmap_reg_range(0x57, 0x57),
+ regmap_reg_range(0x59, 0x59),
+ regmap_reg_range(0x5B, 0x5B),
+ regmap_reg_range(0x5D, 0x5D),
+ regmap_reg_range(0x5F, 0x5F),
+ regmap_reg_range(0x61, 0x69),
+ regmap_reg_range(0x96, 0x9B),
regmap_reg_range(0xA0, 0xA3),
+ regmap_reg_range(0x184, 0x184),
+ regmap_reg_range(0x186, 0x186),
+ regmap_reg_range(0x1A8, 0x1A9),
};

static const struct regmap_access_table sch5627_tunables_table = {
@@ -200,38 +208,6 @@ static int sch5627_update_in(struct sch5627_data *data)
return ret;
}

-static int sch5627_read_limits(struct sch5627_data *data)
-{
- int i, val;
-
- for (i = 0; i < SCH5627_NO_TEMPS; i++) {
- /*
- * Note what SMSC calls ABS, is what lm_sensors calls max
- * (aka high), and HIGH is what lm_sensors calls crit.
- */
- val = sch56xx_read_virtual_reg(data->addr,
- SCH5627_REG_TEMP_ABS[i]);
- if (val < 0)
- return val;
- data->temp_max[i] = val;
-
- val = sch56xx_read_virtual_reg(data->addr,
- SCH5627_REG_TEMP_HIGH[i]);
- if (val < 0)
- return val;
- data->temp_crit[i] = val;
- }
- for (i = 0; i < SCH5627_NO_FANS; i++) {
- val = sch56xx_read_virtual_reg16(data->addr,
- SCH5627_REG_FAN_MIN[i]);
- if (val < 0)
- return val;
- data->fan_min[i] = val;
- }
-
- return 0;
-}
-
static int reg_to_temp(u16 reg)
{
return (reg * 625) / 10 - 64000;
@@ -252,6 +228,25 @@ static int reg_to_rpm(u16 reg)
return 5400540 / reg;
}

+static u8 sch5627_temp_limit_to_reg(long value)
+{
+ long limit = (value / 1000) + 64;
+
+ return clamp_val(limit, 0, U8_MAX);
+}
+
+static u16 sch5627_rpm_to_reg(long value)
+{
+ long pulses;
+
+ if (value <= 0)
+ return U16_MAX - 1;
+
+ pulses = 5400540 / value;
+
+ return clamp_val(pulses, 1, U16_MAX - 1);
+}
+
static umode_t sch5627_is_visible(const void *drvdata, enum hwmon_sensor_types type, u32 attr,
int channel)
{
@@ -263,8 +258,35 @@ static umode_t sch5627_is_visible(const void *drvdata, enum hwmon_sensor_types t
if (data->control & SCH5627_CTRL_LOCK)
return 0444;

- if (type == hwmon_pwm && attr == hwmon_pwm_auto_channels_temp)
- return 0644;
+ switch (type) {
+ case hwmon_temp:
+ switch (attr) {
+ case hwmon_temp_max:
+ case hwmon_temp_crit:
+ return 0644;
+ default:
+ break;
+ }
+ break;
+ case hwmon_fan:
+ switch (attr) {
+ case hwmon_fan_min:
+ return 0644;
+ default:
+ break;
+ }
+ break;
+ case hwmon_pwm:
+ switch (attr) {
+ case hwmon_pwm_auto_channels_temp:
+ return 0644;
+ default:
+ break;
+ }
+ break;
+ default:
+ break;
+ }

return 0444;
}
@@ -277,20 +299,33 @@ static int sch5627_read(struct device *dev, enum hwmon_sensor_types type, u32 at

switch (type) {
case hwmon_temp:
- ret = sch5627_update_temp(data);
- if (ret < 0)
- return ret;
switch (attr) {
case hwmon_temp_input:
+ ret = sch5627_update_temp(data);
+ if (ret < 0)
+ return ret;
+
*val = reg_to_temp(data->temp[channel]);
return 0;
case hwmon_temp_max:
- *val = reg_to_temp_limit(data->temp_max[channel]);
+ ret = regmap_read(data->regmap, SCH5627_REG_TEMP_ABS[channel], &value);
+ if (ret < 0)
+ return ret;
+
+ *val = reg_to_temp_limit((u8)value);
return 0;
case hwmon_temp_crit:
- *val = reg_to_temp_limit(data->temp_crit[channel]);
+ ret = regmap_read(data->regmap, SCH5627_REG_TEMP_HIGH[channel], &value);
+ if (ret < 0)
+ return ret;
+
+ *val = reg_to_temp_limit((u8)value);
return 0;
case hwmon_temp_fault:
+ ret = sch5627_update_temp(data);
+ if (ret < 0)
+ return ret;
+
*val = (data->temp[channel] == 0);
return 0;
default:
@@ -298,23 +333,35 @@ static int sch5627_read(struct device *dev, enum hwmon_sensor_types type, u32 at
}
break;
case hwmon_fan:
- ret = sch5627_update_fan(data);
- if (ret < 0)
- return ret;
switch (attr) {
case hwmon_fan_input:
+ ret = sch5627_update_fan(data);
+ if (ret < 0)
+ return ret;
+
ret = reg_to_rpm(data->fan[channel]);
if (ret < 0)
return ret;
+
*val = ret;
return 0;
case hwmon_fan_min:
- ret = reg_to_rpm(data->fan_min[channel]);
+ ret = sch56xx_regmap_read16(data->regmap, SCH5627_REG_FAN_MIN[channel],
+ &value);
if (ret < 0)
return ret;
+
+ ret = reg_to_rpm((u16)value);
+ if (ret < 0)
+ return ret;
+
*val = ret;
return 0;
case hwmon_fan_fault:
+ ret = sch5627_update_fan(data);
+ if (ret < 0)
+ return ret;
+
*val = (data->fan[channel] == 0xffff);
return 0;
default:
@@ -378,8 +425,33 @@ static int sch5627_write(struct device *dev, enum hwmon_sensor_types type, u32 a
long val)
{
struct sch5627_data *data = dev_get_drvdata(dev);
+ u16 fan;
+ u8 temp;

switch (type) {
+ case hwmon_temp:
+ temp = sch5627_temp_limit_to_reg(val);
+
+ switch (attr) {
+ case hwmon_temp_max:
+ return regmap_write(data->regmap, SCH5627_REG_TEMP_ABS[channel], temp);
+ case hwmon_temp_crit:
+ return regmap_write(data->regmap, SCH5627_REG_TEMP_HIGH[channel], temp);
+ default:
+ break;
+ }
+ break;
+ case hwmon_fan:
+ switch (attr) {
+ case hwmon_fan_min:
+ fan = sch5627_rpm_to_reg(val);
+
+ return sch56xx_regmap_write16(data->regmap, SCH5627_REG_FAN_MIN[channel],
+ fan);
+ default:
+ break;
+ }
+ break;
case hwmon_pwm:
switch (attr) {
case hwmon_pwm_auto_channels_temp:
@@ -449,7 +521,7 @@ static int sch5627_probe(struct platform_device *pdev)
{
struct sch5627_data *data;
struct device *hwmon_dev;
- int err, build_code, build_id, hwmon_rev, val;
+ int build_code, build_id, hwmon_rev, val;

data = devm_kzalloc(&pdev->dev, sizeof(struct sch5627_data),
GFP_KERNEL);
@@ -525,14 +597,6 @@ static int sch5627_probe(struct platform_device *pdev)
sch56xx_write_virtual_reg(data->addr, SCH5627_REG_CTRL, data->control | SCH5627_CTRL_VBAT);
data->last_battery = jiffies;

- /*
- * Read limits, we do this only once as reading a register on
- * the sch5627 is quite expensive (and they don't change).
- */
- err = sch5627_read_limits(data);
- if (err)
- return err;
-
pr_info("found %s chip at %#hx\n", DEVNAME, data->addr);
pr_info("firmware build: code 0x%02X, id 0x%04X, hwmon: rev 0x%02X\n",
build_code, build_id, hwmon_rev);
diff --git a/drivers/hwmon/sch56xx-common.c b/drivers/hwmon/sch56xx-common.c
index a3abafdaa3e6..36b38626dcdf 100644
--- a/drivers/hwmon/sch56xx-common.c
+++ b/drivers/hwmon/sch56xx-common.c
@@ -253,6 +253,37 @@ EXPORT_SYMBOL(sch56xx_read_virtual_reg12);
* Regmap support
*/

+int sch56xx_regmap_read16(struct regmap *map, unsigned int reg, unsigned int *val)
+{
+ int lsb, msb, ret;
+
+ /* See sch56xx_read_virtual_reg16() */
+ ret = regmap_read(map, reg, &lsb);
+ if (ret < 0)
+ return ret;
+
+ ret = regmap_read(map, reg + 1, &msb);
+ if (ret < 0)
+ return ret;
+
+ *val = lsb | (msb << 8);
+
+ return 0;
+}
+EXPORT_SYMBOL(sch56xx_regmap_read16);
+
+int sch56xx_regmap_write16(struct regmap *map, unsigned int reg, unsigned int val)
+{
+ int ret;
+
+ ret = regmap_write(map, reg, val & 0xff);
+ if (ret < 0)
+ return ret;
+
+ return regmap_write(map, reg + 1, (val >> 8) & 0xff);
+}
+EXPORT_SYMBOL(sch56xx_regmap_write16);
+
static int sch56xx_reg_write(void *context, unsigned int reg, unsigned int val)
{
struct sch56xx_bus_context *bus = context;
diff --git a/drivers/hwmon/sch56xx-common.h b/drivers/hwmon/sch56xx-common.h
index 3fb1cddbf977..7479a549a026 100644
--- a/drivers/hwmon/sch56xx-common.h
+++ b/drivers/hwmon/sch56xx-common.h
@@ -11,6 +11,9 @@ struct sch56xx_watchdog_data;

struct regmap *devm_regmap_init_sch56xx(struct device *dev, struct mutex *lock, u16 addr,
const struct regmap_config *config);
+int sch56xx_regmap_read16(struct regmap *map, unsigned int reg, unsigned int *val);
+int sch56xx_regmap_write16(struct regmap *map, unsigned int reg, unsigned int val);
+
int sch56xx_read_virtual_reg(u16 addr, u16 reg);
int sch56xx_write_virtual_reg(u16 addr, u16 reg, u8 val);
int sch56xx_read_virtual_reg16(u16 addr, u16 reg);
--
2.39.2

Next message: Armin Wolf: "[PATCH v2 5/5] hwmon: (sch5627) Document behaviour of limit registers"
Previous message: Armin Wolf: "[PATCH v2 3/5] hwmon: (sch5627) Use regmap for pwm map register caching"
In reply to: Armin Wolf: "[PATCH v2 3/5] hwmon: (sch5627) Use regmap for pwm map register caching"
Next in thread: Armin Wolf: "[PATCH v2 5/5] hwmon: (sch5627) Document behaviour of limit registers"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]