[PATCH] hwmon: (cros_ec): Hide unconfigured temperature thresholds

[PATCH] hwmon: (cros_ec): Hide unconfigured temperature thresholds

[Daniel Schaefer via B4 Relay]

From: Daniel Schaefer <dhs@frame.work>

The EC reports a threshold of 0 K for host thresholds that are not
configured. Most Framework Computer systems don't use all thresholds.
E.g. temp_host_warn is left unset and stays at its -273 C / 0 K
default), see the EC code in zephyr/shim/src/thermal.c:

  DT_PROP_OR(node_id, temp_host_warn,  -273)

Previously these unconfigured thresholds were exposed as sysfs limits
that read back as -273.15 C, which userspace (e.g. s-tui) interpreted as
a real, permanently-exceeded limit.

To avoid this we can hide the sysfs attribute for that threshold.
See truncated example below.

Before:

> sensors cros_ec-isa-000c
cpu_f75303@4d:       +36.9°C  (crit = +74.8°C, emerg = +84.8°C)

After:

> sensors cros_ec-isa-000c
cpu_f75303@4d:       +36.9°C  (high = -273.1°C, crit = +74.8°C)
                              (emerg = +84.8°C)

Cc: Tzung-Bi Shih <tzungbi@kernel.org>
Cc: Thomas Weißschuh <linux@weissschuh.net>
Cc: linux@frame.work
Signed-off-by: Daniel Schaefer <dhs@frame.work>
---
 drivers/hwmon/cros_ec_hwmon.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
index 03bfcc40bb7c..57158443dafe 100644
--- a/drivers/hwmon/cros_ec_hwmon.c
+++ b/drivers/hwmon/cros_ec_hwmon.c
@@ -319,6 +319,7 @@ static umode_t cros_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_type
 					u32 attr, int channel)
 {
 	const struct cros_ec_hwmon_priv *priv = data;
+	u32 threshold;
 	u16 speed;
 
 	if (type == hwmon_fan) {
@@ -334,8 +335,17 @@ static umode_t cros_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_type
 	} else if (type == hwmon_temp) {
 		if (priv->temp_sensor_names[channel]) {
 			if (cros_ec_hwmon_attr_is_temp_threshold(attr)) {
-				if (priv->temp_threshold_supported)
-					return 0444;
+				if (!priv->temp_threshold_supported)
+					return 0;
+
+				if (cros_ec_hwmon_read_temp_threshold(priv->cros_ec, channel,
+								      cros_ec_hwmon_attr_to_thres(attr),
+								      &threshold) != 0)
+					return 0;
+				if (threshold == 0)
+					return 0;
+
+				return 0444;
 			} else {
 				return 0444;
 			}

---
base-commit: 58717b2a1365d06c8c64b72aa948541b53fe31eb
change-id: 20260729-crosec-hwmon-warn-default-9f3ca86ed809

Best regards,
-- 
Daniel Schaefer <dhs@frame.work>

Re: [PATCH] hwmon: (cros_ec): Hide unconfigured temperature thresholds

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Transient EC communication errors during driver probe permanently hide thermal threshold sysfs attributes.
--

--- Patch [1]: [PATCH] hwmon: (cros_ec): Hide unconfigured temperature thresholds ---
commit de61a2547d7ffb8ebe260dea1df08c0623451745
Author: Daniel Schaefer <dhs@frame.work>

hwmon: (cros_ec): Hide unconfigured temperature thresholds

This commit updates the cros_ec_hwmon driver to hide sysfs temperature
threshold attributes when the EC reports they are unconfigured (0 K). It
achieves this by evaluating the threshold value during the is_visible
callback and returning 0 when the threshold is 0 or if the read fails.

> diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
> index 03bfcc40bb7c3..57158443dafe6 100644
> --- a/drivers/hwmon/cros_ec_hwmon.c
> +++ b/drivers/hwmon/cros_ec_hwmon.c

[ ... ]

> @@ -334,8 +335,17 @@ static umode_t cros_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_type
>  	} else if (type == hwmon_temp) {
>  		if (priv->temp_sensor_names[channel]) {
>  			if (cros_ec_hwmon_attr_is_temp_threshold(attr)) {
> -				if (priv->temp_threshold_supported)
> -					return 0444;
> +				if (!priv->temp_threshold_supported)
> +					return 0;
> +
> +				if (cros_ec_hwmon_read_temp_threshold(priv->cros_ec, channel,
> +								      cros_ec_hwmon_attr_to_thres(attr),
> +								      &threshold) != 0)
> +					return 0;

[Severity: Medium]
Will this permanently hide valid thermal threshold attributes if there
is a transient EC communication error during driver probe?

The hwmon subsystem calls the is_visible callback during registration.
If cros_ec_hwmon_read_temp_threshold() encounters a temporary transport
error (such as -EIO or -ETIMEDOUT), it returns a non-zero error code.

Since this check returns 0 on any error, the attribute is permanently
suppressed. Should this perhaps only return 0 if the error is exactly
-EOPNOTSUPP, similar to how hwmon_fan_target is handled earlier in
this function?

> +				if (threshold == 0)
> +					return 0;
> +
> +				return 0444;
>  			} else {
>  				return 0444;
>  			}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-crosec-hwmon-warn-default-v1-1-d1eb39656a60@frame.work?part=1

Re: [PATCH] hwmon: (cros_ec): Hide unconfigured temperature thresholds

[Guenter Roeck]

On 7/28/26 09:38, Daniel Schaefer via B4 Relay wrote:
> From: Daniel Schaefer <dhs@frame.work>
> 
> The EC reports a threshold of 0 K for host thresholds that are not
> configured. Most Framework Computer systems don't use all thresholds.
> E.g. temp_host_warn is left unset and stays at its -273 C / 0 K
> default), see the EC code in zephyr/shim/src/thermal.c:
> 
>    DT_PROP_OR(node_id, temp_host_warn,  -273)
> 
> Previously these unconfigured thresholds were exposed as sysfs limits
> that read back as -273.15 C, which userspace (e.g. s-tui) interpreted as
> a real, permanently-exceeded limit.
> 
> To avoid this we can hide the sysfs attribute for that threshold.
> See truncated example below.
> 

Is the limit just not configured or indeed unsupported ?

If it is indeed unsupported, this is ok. If it is supported but not set
(and it is configurable), this is not ok and will require a better solution.

Guenter

> Before:
> 
>> sensors cros_ec-isa-000c
> cpu_f75303@4d:       +36.9°C  (crit = +74.8°C, emerg = +84.8°C)
> 
> After:
> 
>> sensors cros_ec-isa-000c
> cpu_f75303@4d:       +36.9°C  (high = -273.1°C, crit = +74.8°C)
>                                (emerg = +84.8°C)
> 
> Cc: Tzung-Bi Shih <tzungbi@kernel.org>
> Cc: Thomas Weißschuh <linux@weissschuh.net>
> Cc: linux@frame.work
> Signed-off-by: Daniel Schaefer <dhs@frame.work>
> ---
>   drivers/hwmon/cros_ec_hwmon.c | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
> index 03bfcc40bb7c..57158443dafe 100644
> --- a/drivers/hwmon/cros_ec_hwmon.c
> +++ b/drivers/hwmon/cros_ec_hwmon.c
> @@ -319,6 +319,7 @@ static umode_t cros_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_type
>   					u32 attr, int channel)
>   {
>   	const struct cros_ec_hwmon_priv *priv = data;
> +	u32 threshold;
>   	u16 speed;
>   
>   	if (type == hwmon_fan) {
> @@ -334,8 +335,17 @@ static umode_t cros_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_type
>   	} else if (type == hwmon_temp) {
>   		if (priv->temp_sensor_names[channel]) {
>   			if (cros_ec_hwmon_attr_is_temp_threshold(attr)) {
> -				if (priv->temp_threshold_supported)
> -					return 0444;
> +				if (!priv->temp_threshold_supported)
> +					return 0;
> +
> +				if (cros_ec_hwmon_read_temp_threshold(priv->cros_ec, channel,
> +								      cros_ec_hwmon_attr_to_thres(attr),
> +								      &threshold) != 0)
> +					return 0;
> +				if (threshold == 0)
> +					return 0;
> +
> +				return 0444;
>   			} else {
>   				return 0444;
>   			}
> 
> ---
> base-commit: 58717b2a1365d06c8c64b72aa948541b53fe31eb
> change-id: 20260729-crosec-hwmon-warn-default-9f3ca86ed809
> 
> Best regards,

Re: [PATCH] hwmon: (cros_ec): Hide unconfigured temperature thresholds

[Daniel Schaefer]

> Is the limit just not configured or indeed unsupported ?
>
> If it is indeed unsupported, this is ok. If it is supported but not set
> (and it is configurable), this is not ok and will require a better solution.

This driver only supports reading thresholds, not setting them.
So I would say not configured and not supported are equivalent.

In our case if there is nothing useful for the EC to do with a
specific threshold (e.g. warn), then we don't set it.
I think chromebooks send a notification to a userspace daemon on warn threshold,
but we don't have that, so we don't do anything and don't set a threshold.

Re: [PATCH] hwmon: (cros_ec): Hide unconfigured temperature thresholds

[Thomas Weißschuh]

On 2026-07-29 00:38:16+0800, Daniel Schaefer via B4 Relay wrote:
> From: Daniel Schaefer <dhs@frame.work>

Welcome to upstream!

> The EC reports a threshold of 0 K for host thresholds that are not
> configured. Most Framework Computer systems don't use all thresholds.
> E.g. temp_host_warn is left unset and stays at its -273 C / 0 K
> default), see the EC code in zephyr/shim/src/thermal.c:
> 
>   DT_PROP_OR(node_id, temp_host_warn,  -273)
> 
> Previously these unconfigured thresholds were exposed as sysfs limits
> that read back as -273.15 C, which userspace (e.g. s-tui) interpreted as
> a real, permanently-exceeded limit.

In current hwmon-next this should read back as -273C. See commit
9f14280c5ef3 ("hwmon: (cros_ec) Implement custom kelvin to celsius conversions")

> To avoid this we can hide the sysfs attribute for that threshold.

"Avoid this by hiding ..."

> See truncated example below.
> 
> Before:
> 
> > sensors cros_ec-isa-000c
> cpu_f75303@4d:       +36.9°C  (crit = +74.8°C, emerg = +84.8°C)
> 
> After:
> 
> > sensors cros_ec-isa-000c
> cpu_f75303@4d:       +36.9°C  (high = -273.1°C, crit = +74.8°C)
>                               (emerg = +84.8°C)

"Before:" and "After:" are swapped?

> Cc: Tzung-Bi Shih <tzungbi@kernel.org>
> Cc: Thomas Weißschuh <linux@weissschuh.net>
> Cc: linux@frame.work
> Signed-off-by: Daniel Schaefer <dhs@frame.work>
> ---
>  drivers/hwmon/cros_ec_hwmon.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
> index 03bfcc40bb7c..57158443dafe 100644
> --- a/drivers/hwmon/cros_ec_hwmon.c
> +++ b/drivers/hwmon/cros_ec_hwmon.c
> @@ -319,6 +319,7 @@ static umode_t cros_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_type
>  					u32 attr, int channel)
>  {
>  	const struct cros_ec_hwmon_priv *priv = data;
> +	u32 threshold;
>  	u16 speed;
>  
>  	if (type == hwmon_fan) {
> @@ -334,8 +335,17 @@ static umode_t cros_ec_hwmon_is_visible(const void *data, enum hwmon_sensor_type
>  	} else if (type == hwmon_temp) {
>  		if (priv->temp_sensor_names[channel]) {
>  			if (cros_ec_hwmon_attr_is_temp_threshold(attr)) {
> -				if (priv->temp_threshold_supported)
> -					return 0444;
> +				if (!priv->temp_threshold_supported)
> +					return 0;
> +
> +				if (cros_ec_hwmon_read_temp_threshold(priv->cros_ec, channel,
> +								      cros_ec_hwmon_attr_to_thres(attr),
> +								      &threshold) != 0)

Probing would be a bit faster by not calling this for each threshold,
but only doing it once. cros_ec_hwmon_priv::temp_threshold_supported
could become a bitmask for supported thresholds to be read in is_visible().
This can also replace is_cros_ec_cmd_available(EC_CMD_THERMAL_GET_THRESHOLD).

> +					return 0;
> +				if (threshold == 0)
> +					return 0;
> +
> +				return 0444;
>  			} else {
>  				return 0444;
>  			}