Add support to cros_ec_hwmon for
* fan target speed for fan 1
* fan pwm control for all fans
* fan temperature thresholds (RW) for all temp sensors
The EC also supports tempY_auto_pointZ_{pwm,temp} but that does not yet
seem to be usable from "struct hwmon_channel_info".
Guenter, is this intentional, do you want me to add support for it?
Patch 1 introduces a utility function cros_ec_cmd_versions() which wraps
EC_CMD_GET_CMD_VERSIONS.
That is used as it seems inadvisable to call EC_CMD_PWM_SET_FAN_DUTY
during probing.
I'm planning on also using this utility function in my CrOS EC charge
control driver.
Also there are some other places throughout the tree that could use it.
This series is meant to be merged through the chrome-platform tree,
as cros_ec_hwmon itself is only available there and because of
the new cros_ec_cmd_version().
To test it you *also* need
commit 27e669820c ("mfd: cros_ec: Register hardware monitoring subdevice")
from the MFD tree (branch mfd-for-next).
Tested on a Framework 13 AMD.
Signed-off-by: Thomas Weißschuh <[email protected]>
---
Thomas Weißschuh (5):
platform/chrome: cros_ec_proto: Introduce cros_ec_cmd_versions()
hwmon: (cros_ec) Add support for fan target speed
hwmon: (cros_ec) Add support for PWM fan control
hwmon: (cros_ec) Split temperature channel params
hwmon: (cros_ec) Add support for temperature thresholds
Documentation/hwmon/cros_ec_hwmon.rst | 8 +-
drivers/hwmon/cros_ec_hwmon.c | 284 +++++++++++++++++++++++++---
drivers/platform/chrome/cros_ec_proto.c | 26 +++
include/linux/platform_data/cros_ec_proto.h | 2 +
4 files changed, 290 insertions(+), 30 deletions(-)
---
base-commit: c8a4bdca928debacf49524d1b09dbf27e88e1f18
change-id: 20240529-cros_ec-hwmon-pwm-0dcc610ba0dc
Best regards,
--
Thomas Weißschuh <[email protected]>
Implement reading and writing temperature thresholds through
EC_CMD_THERMAL_GET_THRESHOLD/EC_CMD_THERMAL_SET_THRESHOLD.
Thresholds are mapped as follows between the EC and hwmon:
hwmon_temp_max - EC_TEMP_THRESH_WARN
hwmon_temp_crit - EC_TEMP_THRESH_HIGH
hwmon_temp_emergency - EC_TEMP_THRESH_HALT
Signed-off-by: Thomas Weißschuh <[email protected]>
---
Documentation/hwmon/cros_ec_hwmon.rst | 1 +
drivers/hwmon/cros_ec_hwmon.c | 82 +++++++++++++++++++++++++++++++++--
2 files changed, 80 insertions(+), 3 deletions(-)
diff --git a/Documentation/hwmon/cros_ec_hwmon.rst b/Documentation/hwmon/cros_ec_hwmon.rst
index 3cc345425aac..24ff261ae232 100644
--- a/Documentation/hwmon/cros_ec_hwmon.rst
+++ b/Documentation/hwmon/cros_ec_hwmon.rst
@@ -29,3 +29,4 @@ Supported features:
- Target fan speed (for fan 1 only)
- PWM-based fan control
- Current temperature
+ - Temperature thresholds
diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
index 5cddf78cfe0e..009b94af6df4 100644
--- a/drivers/hwmon/cros_ec_hwmon.c
+++ b/drivers/hwmon/cros_ec_hwmon.c
@@ -29,6 +29,7 @@ struct cros_ec_hwmon_priv {
const char *temp_sensor_names[EC_TEMP_SENSOR_ENTRIES + EC_TEMP_SENSOR_B_ENTRIES];
u8 usable_fans;
bool has_fan_pwm;
+ bool has_temp_threshold;
u8 fan_pwm[EC_FAN_SPEED_ENTRIES];
enum cros_ec_hwmon_fan_mode fan_mode[EC_FAN_SPEED_ENTRIES];
};
@@ -97,6 +98,42 @@ static int cros_ec_hwmon_read_temp(struct cros_ec_device *cros_ec, u8 index, u8
return 0;
}
+static int cros_ec_hwmon_read_temp_threshold(struct cros_ec_device *cros_ec, u8 index,
+ enum ec_temp_thresholds threshold, u32 *temp)
+{
+ struct ec_params_thermal_get_threshold_v1 req = {};
+ struct ec_thermal_config resp;
+ int ret;
+
+ req.sensor_num = index;
+ ret = cros_ec_cmd(cros_ec, 1, EC_CMD_THERMAL_GET_THRESHOLD,
+ &req, sizeof(req), &resp, sizeof(resp));
+ if (ret < 0)
+ return ret;
+
+ *temp = resp.temp_host[threshold];
+ return 0;
+}
+
+static int cros_ec_hwmon_write_temp_threshold(struct cros_ec_device *cros_ec, u8 index,
+ enum ec_temp_thresholds threshold, u32 temp)
+{
+ struct ec_params_thermal_get_threshold_v1 get_req = {};
+ struct ec_params_thermal_set_threshold_v1 set_req = {};
+ int ret;
+
+ get_req.sensor_num = index;
+ ret = cros_ec_cmd(cros_ec, 1, EC_CMD_THERMAL_GET_THRESHOLD,
+ &get_req, sizeof(get_req), &set_req.cfg, sizeof(set_req.cfg));
+ if (ret < 0)
+ return ret;
+
+ set_req.sensor_num = index;
+ set_req.cfg.temp_host[threshold] = temp;
+ return cros_ec_cmd(cros_ec, 1, EC_CMD_THERMAL_SET_THRESHOLD,
+ &set_req, sizeof(set_req), NULL, 0);
+}
+
static bool cros_ec_hwmon_is_error_fan(u16 speed)
{
return speed == EC_FAN_SPEED_NOT_PRESENT || speed == EC_FAN_SPEED_STALLED;
@@ -115,11 +152,24 @@ static long cros_ec_hwmon_temp_to_millicelsius(u8 temp)
return kelvin_to_millicelsius((((long)temp) + EC_TEMP_SENSOR_OFFSET));
}
+static enum ec_temp_thresholds cros_ec_hwmon_attr_to_thres(u32 attr)
+{
+ if (attr == hwmon_temp_max)
+ return EC_TEMP_THRESH_WARN;
+ else if (attr == hwmon_temp_crit)
+ return EC_TEMP_THRESH_HIGH;
+ else if (attr == hwmon_temp_emergency)
+ return EC_TEMP_THRESH_HALT;
+ else
+ return 0;
+}
+
static int cros_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
u32 attr, int channel, long *val)
{
struct cros_ec_hwmon_priv *priv = dev_get_drvdata(dev);
int ret = -EOPNOTSUPP;
+ u32 threshold;
u16 speed;
u8 temp;
@@ -166,6 +216,14 @@ static int cros_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
ret = cros_ec_hwmon_read_temp(priv->cros_ec, channel, &temp);
if (ret == 0)
*val = cros_ec_hwmon_is_error_temp(temp);
+
+ } else if (attr == hwmon_temp_max || attr == hwmon_temp_crit ||
+ attr == hwmon_temp_emergency) {
+ ret = cros_ec_hwmon_read_temp_threshold(priv->cros_ec, channel,
+ cros_ec_hwmon_attr_to_thres(attr),
+ &threshold);
+ if (ret == 0)
+ *val = kelvin_to_millicelsius(threshold);
}
}
@@ -235,6 +293,10 @@ static int cros_ec_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
else if (attr == hwmon_pwm_enable)
ret = cros_ec_hwmon_write_pwm_enable(priv, channel, val);
+ } else if (type == hwmon_temp) {
+ ret = cros_ec_hwmon_write_temp_threshold(priv->cros_ec, channel,
+ cros_ec_hwmon_attr_to_thres(attr),
+ millicelsius_to_kelvin(val));
}
return ret;
@@ -259,8 +321,16 @@ static umode_t cros_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_type
return 0644;
} else if (type == hwmon_temp) {
- if (priv->temp_sensor_names[channel])
- return 0444;
+ if (priv->temp_sensor_names[channel]) {
+ if (attr == hwmon_temp_max ||
+ attr == hwmon_temp_crit ||
+ attr == hwmon_temp_emergency) {
+ if (priv->has_temp_threshold)
+ return 0644;
+ } else {
+ return 0444;
+ }
+ }
}
return 0;
@@ -278,7 +348,8 @@ static const struct hwmon_channel_info * const cros_ec_hwmon_info[] = {
HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
HWMON_PWM_INPUT | HWMON_PWM_ENABLE),
-#define CROS_EC_HWMON_TEMP_PARAMS (HWMON_T_INPUT | HWMON_T_FAULT | HWMON_T_LABEL)
+#define CROS_EC_HWMON_TEMP_PARAMS (HWMON_T_INPUT | HWMON_T_FAULT | HWMON_T_LABEL | \
+ HWMON_T_MAX | HWMON_T_CRIT | HWMON_T_EMERGENCY)
HWMON_CHANNEL_INFO(temp,
CROS_EC_HWMON_TEMP_PARAMS,
CROS_EC_HWMON_TEMP_PARAMS,
@@ -325,9 +396,14 @@ static void cros_ec_hwmon_probe_temp_sensors(struct device *dev, struct cros_ec_
struct ec_params_temp_sensor_get_info req = {};
struct ec_response_temp_sensor_get_info resp;
size_t candidates, i, sensor_name_size;
+ u32 threshold;
int ret;
u8 temp;
+ ret = cros_ec_hwmon_read_temp_threshold(priv->cros_ec, 0, EC_TEMP_THRESH_HIGH, &threshold);
+ if (ret == 0)
+ priv->has_temp_threshold = 1;
+
if (thermal_version < 2)
candidates = EC_TEMP_SENSOR_ENTRIES;
else
--
2.45.2
On 6/8/24 01:12, Thomas Weißschuh wrote:
> Add support to cros_ec_hwmon for
> * fan target speed for fan 1
> * fan pwm control for all fans
> * fan temperature thresholds (RW) for all temp sensors
>
> The EC also supports tempY_auto_pointZ_{pwm,temp} but that does not yet
> seem to be usable from "struct hwmon_channel_info".
> Guenter, is this intentional, do you want me to add support for it?
>
When I wrote the _info API, I figured it was too chip specific to make
it generic. It is also at least somewhat questionable if it makes sense
to have all that configurable through sysfs in the first place; normally
the ranges are device specific and should be configured through the
thermal subsystem using devicetree properties and not be touched by
the user. There might even be warranty implications by playing with
thermal parameters if someone manages to overheat the system as result.
I really don't want to help enabling that.
Which leads to the next question - we are going way beyond just reporting
system operational parameters with your patches. What is the actual use
case ?
Thanks,
Guenter
On 2024-06-10 18:18:05+0000, Guenter Roeck wrote:
> On 6/8/24 01:12, Thomas Weißschuh wrote:
> > Add support to cros_ec_hwmon for
> > * fan target speed for fan 1
> > * fan pwm control for all fans
> > * fan temperature thresholds (RW) for all temp sensors
> >
> > The EC also supports tempY_auto_pointZ_{pwm,temp} but that does not yet
> > seem to be usable from "struct hwmon_channel_info".
> > Guenter, is this intentional, do you want me to add support for it?
> >
>
> When I wrote the _info API, I figured it was too chip specific to make
> it generic. It is also at least somewhat questionable if it makes sense
> to have all that configurable through sysfs in the first place; normally
> the ranges are device specific and should be configured through the
> thermal subsystem using devicetree properties and not be touched by
> the user. There might even be warranty implications by playing with
> thermal parameters if someone manages to overheat the system as result.
> I really don't want to help enabling that.
Fair enough.
> Which leads to the next question - we are going way beyond just reporting
> system operational parameters with your patches. What is the actual use
> case ?
The pwm control is something that many users want.
There are a custom daemon [0], its gnome-shell-extension [1] and quite a
few scripts shared in the Framework forums to adjust the fan pwm through
ectool.
Personally I'd like to avoid some thermal throttling for shorter compute
tasks where the default curves are not aggressive enough.
For the others, it's mostly because the information was already there.
I'd be fine with dropping the write access to the thresholds,
not even ectool exposes that.
[0] https://github.com/TamtamHero/fw-fanctrl
[1] https://extensions.gnome.org/extension/7053/fw-fanctrl/