On 2/6/22 11:05, Christian Lamparter wrote:
Adds thermal_cooling device support to the tc654/tc655
driver. This make it possible to integrate it into a
device-tree supported thermal-zone node as a
cooling device.
I have been using this patch as part of the Netgear WNDR4700
Centria NAS Router support within OpenWrt since 2016.
Signed-off-by: Christian Lamparter <chunkeey@xxxxxxxxx>
---
v1 -> v2: - Drop imply THERMAL
- aligned ( so . everything is lining up
---
drivers/hwmon/tc654.c | 94 +++++++++++++++++++++++++++++++++++--------
1 file changed, 77 insertions(+), 17 deletions(-)
diff --git a/drivers/hwmon/tc654.c b/drivers/hwmon/tc654.c
index cf2a3acd5c91..c8f511a60769 100644
--- a/drivers/hwmon/tc654.c
+++ b/drivers/hwmon/tc654.c
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/slab.h>
+#include <linux/thermal.h>
#include <linux/util_macros.h>
enum tc654_regs {
@@ -367,36 +368,30 @@ static ssize_t pwm_mode_store(struct device *dev, struct device_attribute *da,
static const int tc654_pwm_map[16] = { 77, 88, 102, 112, 124, 136, 148, 160,
172, 184, 196, 207, 219, 231, 243, 255};
+static int get_pwm(struct tc654_data *data)
+{
+ if (data->config & TC654_REG_CONFIG_SDM)
+ return 0;
+ else
Nit: else after return is unnecessary. Sorry for not noticing before.
+ return tc654_pwm_map[data->duty_cycle];
+}
+
static ssize_t pwm_show(struct device *dev, struct device_attribute *da,
char *buf)
{
struct tc654_data *data = tc654_update_client(dev);
- int pwm;
if (IS_ERR(data))
return PTR_ERR(data);
- if (data->config & TC654_REG_CONFIG_SDM)
- pwm = 0;
- else
- pwm = tc654_pwm_map[data->duty_cycle];
-
- return sprintf(buf, "%d\n", pwm);
+ return sprintf(buf, "%d\n", get_pwm(data));
}
-static ssize_t pwm_store(struct device *dev, struct device_attribute *da,
- const char *buf, size_t count)
+static int _set_pwm(struct tc654_data *data, unsigned long val)
{
- struct tc654_data *data = dev_get_drvdata(dev);
struct i2c_client *client = data->client;
- unsigned long val;
int ret;
- if (kstrtoul(buf, 10, &val))
- return -EINVAL;
- if (val > 255)
- return -EINVAL;
-
mutex_lock(&data->update_lock);
if (val == 0)
@@ -416,6 +411,22 @@ static ssize_t pwm_store(struct device *dev, struct device_attribute *da,
out:
mutex_unlock(&data->update_lock);
+ return ret;
+}
+
+static ssize_t pwm_store(struct device *dev, struct device_attribute *da,
+ const char *buf, size_t count)
+{
+ struct tc654_data *data = dev_get_drvdata(dev);
+ unsigned long val;
+ int ret;
+
+ if (kstrtoul(buf, 10, &val))
+ return -EINVAL;
+ if (val > 255)
+ return -EINVAL;
+
+ ret = _set_pwm(data, val);
return ret < 0 ? ret : count;
}
@@ -447,6 +458,44 @@ static struct attribute *tc654_attrs[] = {
ATTRIBUTE_GROUPS(tc654);
+/* cooling device */
+
+static int tc654_get_max_state(struct thermal_cooling_device *cdev,
+ unsigned long *state)
+{
+ *state = 255;
+ return 0;
+}
+
+static int tc654_get_cur_state(struct thermal_cooling_device *cdev,
+ unsigned long *state)
+{
+ struct tc654_data *data = tc654_update_client(cdev->devdata);
+
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+
+ *state = get_pwm(data);
+ return 0;
+}
+
+static int tc654_set_cur_state(struct thermal_cooling_device *cdev,
+ unsigned long state)
+{
+ struct tc654_data *data = tc654_update_client(cdev->devdata);
+
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+
+ return _set_pwm(data, clamp_val(state, 0, 255));
+}
Looking into this further, wouldn't it be more appropriate to limit the
cooling states to [0 .. 15], or in other words use data->duty_cycle
directly ? I don't know how the thermal subsystem reacts if it requests
to set the state to a certain value and the actual state is set to
something completely different. Also, it doesn't seem to make much sense
to bother the thermal subsystem with 256 states if the chip really only
supports 16 states.
Thanks,
Guenter
