| Message ID | 20240608-cros_ec-hwmon-pwm-v1-0-d29dfc26fbc3@weissschuh.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | hwmon: (cros_ec): fan target, fan pwm control and temperature thresholds | expand |
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/
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 <linux@weissschuh.net> --- 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,