[PATCH 0/2] hwmon: (cros_ec) Rework the temperature conversions

[PATCH 0/2] hwmon: (cros_ec) Rework the temperature conversions

[Thomas Weißschuh]

Align the temperature conversion to the one inside the EC itself
to preserve nice round integer degree values.
Also handle overflows.

Patch 1 was originally part of [0], but I'll need more time for the new
functionality, and these patches are useful either way.

[0] https://lore.kernel.org/lkml/20260529-cros_ec-hwmon-fan-curve-v1-0-da6792b3830f@weissschuh.net/

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Thomas Weißschuh (2):
      hwmon: (cros_ec) Implement custom kelvin to celsius conversions
      hwmon: (cros_ec) Handle temperature conversion overflows

 drivers/hwmon/cros_ec_hwmon.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20260630-cros_ec-hwmon-overflow-0381c8509df3

Best regards,
--  
Thomas Weißschuh <linux@weissschuh.net>

[PATCH 1/2] hwmon: (cros_ec) Implement custom kelvin to celsius conversions

[Thomas Weißschuh]

The ChromeOS EC APIs use integers representing degrees kelvin for
temperatures. The default conversions from linux/units.h will then
always convert these integer degrees celsius with a 150 millidegree
offset. This is a bit confusing, as it also differs from other CrOS EC
tooling. Internally the EC uses a kelvin to celsius offset of a round
273, so the current conversion is also not entirely accurate.

Implement a custom conversion which preserves round values.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/hwmon/cros_ec_hwmon.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
index ea24056ae646..44291799dd8e 100644
--- a/drivers/hwmon/cros_ec_hwmon.c
+++ b/drivers/hwmon/cros_ec_hwmon.c
@@ -147,9 +147,17 @@ static bool cros_ec_hwmon_is_error_temp(u8 temp)
 	       temp == EC_TEMP_SENSOR_NOT_CALIBRATED;
 }
 
+/* This differs slightly from the variant in units.h to avoid rounding inconsistencies. */
+#define CROS_EC_HWMON_ABSOLUTE_ZERO_MILLICELSIUS (-273000)
+
+static long cros_ec_hwmon_kelvin_to_millicelsius(long t)
+{
+	return t * MILLIDEGREE_PER_DEGREE + CROS_EC_HWMON_ABSOLUTE_ZERO_MILLICELSIUS;
+}
+
 static long cros_ec_hwmon_temp_to_millicelsius(u8 temp)
 {
-	return kelvin_to_millicelsius((((long)temp) + EC_TEMP_SENSOR_OFFSET));
+	return cros_ec_hwmon_kelvin_to_millicelsius((((long)temp) + EC_TEMP_SENSOR_OFFSET));
 }
 
 static bool cros_ec_hwmon_attr_is_temp_threshold(u32 attr)
@@ -228,7 +236,7 @@ static int cros_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 								cros_ec_hwmon_attr_to_thres(attr),
 								&threshold);
 			if (ret == 0)
-				*val = kelvin_to_millicelsius(threshold);
+				*val = cros_ec_hwmon_kelvin_to_millicelsius(threshold);
 		}
 	}
 

-- 
2.55.0

[PATCH 2/2] hwmon: (cros_ec) Handle temperature conversion overflows

[Thomas Weißschuh]

The calculations converting between the different temperature units can
overflow, resulting in incorrect data.

Detect these overflows and report them.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/hwmon/cros_ec_hwmon.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
index 44291799dd8e..4b402142b46f 100644
--- a/drivers/hwmon/cros_ec_hwmon.c
+++ b/drivers/hwmon/cros_ec_hwmon.c
@@ -5,11 +5,13 @@
  *  Copyright (C) 2024 Thomas Weißschuh <linux@weissschuh.net>
  */
 
+#include <linux/build_bug.h>
 #include <linux/device.h>
 #include <linux/hwmon.h>
 #include <linux/math.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/overflow.h>
 #include <linux/platform_device.h>
 #include <linux/platform_data/cros_ec_commands.h>
 #include <linux/platform_data/cros_ec_proto.h>
@@ -150,14 +152,28 @@ static bool cros_ec_hwmon_is_error_temp(u8 temp)
 /* This differs slightly from the variant in units.h to avoid rounding inconsistencies. */
 #define CROS_EC_HWMON_ABSOLUTE_ZERO_MILLICELSIUS (-273000)
 
-static long cros_ec_hwmon_kelvin_to_millicelsius(long t)
+static bool cros_ec_hwmon_kelvin_to_millicelsius_overflow(long t, long *ret)
 {
-	return t * MILLIDEGREE_PER_DEGREE + CROS_EC_HWMON_ABSOLUTE_ZERO_MILLICELSIUS;
+	if (check_mul_overflow(t, MILLIDEGREE_PER_DEGREE, ret))
+		return true;
+
+	if (check_add_overflow(*ret, CROS_EC_HWMON_ABSOLUTE_ZERO_MILLICELSIUS, ret))
+		return true;
+
+	return false;
 }
 
-static long cros_ec_hwmon_temp_to_millicelsius(u8 temp)
+static long __flatten cros_ec_hwmon_temp_to_millicelsius(u8 temp)
 {
-	return cros_ec_hwmon_kelvin_to_millicelsius((((long)temp) + EC_TEMP_SENSOR_OFFSET));
+	long ret;
+
+	if (check_add_overflow(temp, EC_TEMP_SENSOR_OFFSET, &ret))
+		BUILD_BUG();
+
+	if (cros_ec_hwmon_kelvin_to_millicelsius_overflow(ret, &ret))
+		BUILD_BUG();
+
+	return ret;
 }
 
 static bool cros_ec_hwmon_attr_is_temp_threshold(u32 attr)
@@ -235,8 +251,10 @@ static int cros_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
 			ret = cros_ec_hwmon_read_temp_threshold(priv->cros_ec, channel,
 								cros_ec_hwmon_attr_to_thres(attr),
 								&threshold);
-			if (ret == 0)
-				*val = cros_ec_hwmon_kelvin_to_millicelsius(threshold);
+			if (ret == 0) {
+				if (cros_ec_hwmon_kelvin_to_millicelsius_overflow(threshold, val))
+					*val = LONG_MAX;
+			}
 		}
 	}
 

-- 
2.55.0

Re: [PATCH 2/2] hwmon: (cros_ec) Handle temperature conversion overflows

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Using `BUILD_BUG()` with runtime variables introduces build fragility because it relies on compiler Value Range Propagation (VRP) to eliminate the branch.
--

commit a22cd7f1036247302ddc232e1cbd36ceb641e202
Author: Thomas Weißschuh <linux@weissschuh.net>

hwmon: (cros_ec) Handle temperature conversion overflows

The calculations converting between the different temperature units can
overflow, resulting in incorrect data. Detect these overflows and report them.

> diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
> index 44291799dd8e4..4b402142b46f3 100644
> --- a/drivers/hwmon/cros_ec_hwmon.c
> +++ b/drivers/hwmon/cros_ec_hwmon.c

[ ... ]

> @@ -150,14 +152,28 @@ static bool cros_ec_hwmon_is_error_temp(u8 temp)
>  /* This differs slightly from the variant in units.h to avoid rounding inconsistencies. */
>  #define CROS_EC_HWMON_ABSOLUTE_ZERO_MILLICELSIUS (-273000)

[ ... ]

> -static long cros_ec_hwmon_temp_to_millicelsius(u8 temp)
> +static long __flatten cros_ec_hwmon_temp_to_millicelsius(u8 temp)
>  {
> -	return cros_ec_hwmon_kelvin_to_millicelsius((((long)temp) + EC_TEMP_SENSOR_OFFSET));
> +	long ret;
> +
> +	if (check_add_overflow(temp, EC_TEMP_SENSOR_OFFSET, &ret))
> +		BUILD_BUG();

[Severity: Low]
Does the use of BUILD_BUG() with the runtime variable temp risk spurious build
failures?

Relying on the compiler's Value Range Propagation (VRP) to eliminate the
check_add_overflow() branch can lead to broken builds when compiling with
CONFIG_CC_OPTIMIZE_FOR_DEBUGGING (which uses -Og) or on older toolchains where
VRP fails to eliminate the branch.

> +
> +	if (cros_ec_hwmon_kelvin_to_millicelsius_overflow(ret, &ret))
> +		BUILD_BUG();
> +
> +	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-cros_ec-hwmon-overflow-v1-0-3d2ecd3eb0f2@weissschuh.net?part=2

Re: [PATCH 1/2] hwmon: (cros_ec) Implement custom kelvin to celsius conversions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing hwmon core serialization in thermal cooling device callbacks, causing potential race conditions.
--

commit ee3fdfc1d667eaecb682b673010b74478f9bb80e
Author: Thomas Weißschuh <linux@weissschuh.net>

hwmon: (cros_ec) Implement custom kelvin to celsius conversions

The ChromeOS EC APIs use integers representing degrees kelvin for
temperatures. This commit implements a custom conversion which
preserves round values.

> diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
> index ea24056ae6467..44291799dd8e4 100644
> --- a/drivers/hwmon/cros_ec_hwmon.c
> +++ b/drivers/hwmon/cros_ec_hwmon.c

[ ... ]

> @@ -228,7 +236,7 @@ static int cros_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>  								cros_ec_hwmon_attr_to_thres(attr),
>  								&threshold);
>  			if (ret == 0)
> -				*val = kelvin_to_millicelsius(threshold);
> +				*val = cros_ec_hwmon_kelvin_to_millicelsius(threshold);
>  		}
>  	}
>  

While reviewing this driver, I noticed the following in the same file:

> static int cros_ec_hwmon_cooling_set_cur_state(struct thermal_cooling_device *cdev,
> 					       unsigned long val)
> {
> 	const struct cros_ec_hwmon_cooling_priv *priv = cdev->devdata;
> 
> 	return cros_ec_hwmon_write_pwm_input(priv->hwmon_priv->cros_ec, priv->index, val);
> }

[Severity: High]
This is a pre-existing issue, but does this cooling device callback need to
take the hwmon lock to prevent race conditions?

The thermal subsystem calls cros_ec_hwmon_cooling_set_cur_state()
asynchronously, which bypasses the hwmon core serialization.

Looking at cros_ec_hwmon_write_pwm_input(), it performs a read-modify-write
sequence on the EC PWM state:

cros_ec_hwmon_write_pwm_input()
    cros_ec_hwmon_read_pwm_enable(cros_ec, index, &control_method);
    if (control_method != 1) 
        return -EOPNOTSUPP;
    cros_ec_hwmon_set_fan_pwm_val(cros_ec, index, val);

Can this race with a concurrent sysfs write modifying the fan PWM, leading
to an incorrect fan control state? 

The hardware monitoring subsystem guidelines require drivers to use
hwmon_lock() and hwmon_unlock() for attributes registered outside the core
info parameter, such as these thermal cooling state callbacks.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-cros_ec-hwmon-overflow-v1-0-3d2ecd3eb0f2@weissschuh.net?part=1

Re: [PATCH 2/2] hwmon: (cros_ec) Handle temperature conversion overflows

[Thomas Weißschuh]

On 2026-06-30 21:10:27+0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Using `BUILD_BUG()` with runtime variables introduces build fragility because it relies on compiler Value Range Propagation (VRP) to eliminate the branch.
> --
> 
> commit a22cd7f1036247302ddc232e1cbd36ceb641e202
> Author: Thomas Weißschuh <linux@weissschuh.net>
> 
> hwmon: (cros_ec) Handle temperature conversion overflows
> 
> The calculations converting between the different temperature units can
> overflow, resulting in incorrect data. Detect these overflows and report them.
> 
> > diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
> > index 44291799dd8e4..4b402142b46f3 100644
> > --- a/drivers/hwmon/cros_ec_hwmon.c
> > +++ b/drivers/hwmon/cros_ec_hwmon.c
> 
> [ ... ]
> 
> > @@ -150,14 +152,28 @@ static bool cros_ec_hwmon_is_error_temp(u8 temp)
> >  /* This differs slightly from the variant in units.h to avoid rounding inconsistencies. */
> >  #define CROS_EC_HWMON_ABSOLUTE_ZERO_MILLICELSIUS (-273000)
> 
> [ ... ]
> 
> > -static long cros_ec_hwmon_temp_to_millicelsius(u8 temp)
> > +static long __flatten cros_ec_hwmon_temp_to_millicelsius(u8 temp)
> >  {
> > -	return cros_ec_hwmon_kelvin_to_millicelsius((((long)temp) + EC_TEMP_SENSOR_OFFSET));
> > +	long ret;
> > +
> > +	if (check_add_overflow(temp, EC_TEMP_SENSOR_OFFSET, &ret))
> > +		BUILD_BUG();
> 
> [Severity: Low]
> Does the use of BUILD_BUG() with the runtime variable temp risk spurious build
> failures?
> Relying on the compiler's Value Range Propagation (VRP) to eliminate the
> check_add_overflow() branch can lead to broken builds when compiling with
> CONFIG_CC_OPTIMIZE_FOR_DEBUGGING (which uses -Og) or on older toolchains where
> VRP fails to eliminate the branch.

CC_OPTIMIZE_FOR_DEBUGGING is not a thing (anymore?).

This patch passed the 0day build farm, so all relevant compilers should
be handling this correctly.

> > +
> > +	if (cros_ec_hwmon_kelvin_to_millicelsius_overflow(ret, &ret))
> > +		BUILD_BUG();
> > +
> > +	return ret;
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260630-cros_ec-hwmon-overflow-v1-0-3d2ecd3eb0f2@weissschuh.net?part=2

Re: [PATCH 2/2] hwmon: (cros_ec) Handle temperature conversion overflows

[Guenter Roeck]

On Tue, Jun 30, 2026 at 10:57:52PM +0200, Thomas Weißschuh wrote:
> The calculations converting between the different temperature units can
> overflow, resulting in incorrect data.
> 
> Detect these overflows and report them.
> 

I did not see how this can happen in practice, so I asked an AI agent.
Its response is below.

Given that, I am not going to apply this patch. I am all for checking
against overflows, but only if they are real.

Guenter

---
Yes, an overflow is possible, but only on 32-bit architectures.
  
  Here is the breakdown of why this happens and its limitations:
  
  ### 1.  cros_ec_hwmon_read_temp_threshold 
  
  The function  cros_ec_hwmon_read_temp_threshold  populates the  threshold  variable using  resp.temp_host[threshold] , which is an array of  uint32_t .
  
  In  cros_ec_hwmon_read , this  u32  threshold is passed to the conversion function which takes a  long t :
  
    static bool cros_ec_hwmon_kelvin_to_millicelsius_overflow(long t, long *ret)
    
  • On a 32-bit system:  long  is a 32-bit signed integer with a maximum value of  2,147,483,647 . If the EC hardware returns a bogus or uninitialized positive  u32  value (e.g.,  3,000,000 ), the
  multiplication  t * MILLIDEGREE_PER_DEGREE  ( 3,000,000 * 1000 = 3,000,000,000 ) will exceed  LONG_MAX  and cause a signed integer overflow.                                                           
  • On a 64-bit system:  long  is a 64-bit integer. The maximum possible  u32  value multiplied by  1000  is  4,294,967,295,000 , which comfortably fits within the 64-bit limit of  9.22 x 10^18 . An
  overflow here is mathematically impossible.                                                                                                                                                            
  
  ### 2. A Quirky Edge Case on 32-bit
  
  If the EC returns a standard error state like  0xFFFFFFFF , the implicit cast from  u32 threshold  to  long t  on a 32-bit system will result in  t = -1 .
  Interestingly, the new  check_mul_overflow(-1, 1000, ret)  check will not trigger, because  -1000  fits perfectly inside a 32-bit  long . Instead, it will silently return a bogus value of  -274000 
  (which corresponds to -274°C). The patch does not actually prevent this specific "incorrect data" case.                                                                                                
  
  ### 3.  cros_ec_hwmon_temp_to_millicelsius 
  
  For the standard temperature readings (where  temp  is a  u8 ), an overflow is impossible on all architectures.
  The maximum value of  u8  is 255. When the offset of 200 is added, the maximum base is 455. Multiplying  455 * 1000  is  455,000 , which easily fits in a 32-bit (and even 16-bit) integer. The patch
  author is clearly aware of this, which is why the patch asserts its impossibility via  BUILD_BUG() :                                                                                                   
  
        if (check_add_overflow(temp, EC_TEMP_SENSOR_OFFSET, &ret))
            BUILD_BUG();
    
        if (cros_ec_hwmon_kelvin_to_millicelsius_overflow(ret, &ret))
            BUILD_BUG();
    
  ### Conclusion
  
  The patch's claim that calculations "can overflow" is accurate, but the scope of the fix is limited strictly to threshold registers running on 32-bit systems, and it will still emit bogus negative
  temperatures for unsigned integer values larger than  LONG_MAX  (such as  0xFFFFFFFF ).

Re: [PATCH 1/2] hwmon: (cros_ec) Implement custom kelvin to celsius conversions

[Guenter Roeck]

On Tue, Jun 30, 2026 at 10:57:51PM +0200, Thomas Weißschuh wrote:
> The ChromeOS EC APIs use integers representing degrees kelvin for
> temperatures. The default conversions from linux/units.h will then
> always convert these integer degrees celsius with a 150 millidegree
> offset. This is a bit confusing, as it also differs from other CrOS EC
> tooling. Internally the EC uses a kelvin to celsius offset of a round
> 273, so the current conversion is also not entirely accurate.
> 
> Implement a custom conversion which preserves round values.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>

Applied.

Thanks,
Guenter

> ---
>  drivers/hwmon/cros_ec_hwmon.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
> index ea24056ae646..44291799dd8e 100644
> --- a/drivers/hwmon/cros_ec_hwmon.c
> +++ b/drivers/hwmon/cros_ec_hwmon.c
> @@ -147,9 +147,17 @@ static bool cros_ec_hwmon_is_error_temp(u8 temp)
>  	       temp == EC_TEMP_SENSOR_NOT_CALIBRATED;
>  }
>  
> +/* This differs slightly from the variant in units.h to avoid rounding inconsistencies. */
> +#define CROS_EC_HWMON_ABSOLUTE_ZERO_MILLICELSIUS (-273000)
> +
> +static long cros_ec_hwmon_kelvin_to_millicelsius(long t)
> +{
> +	return t * MILLIDEGREE_PER_DEGREE + CROS_EC_HWMON_ABSOLUTE_ZERO_MILLICELSIUS;
> +}
> +
>  static long cros_ec_hwmon_temp_to_millicelsius(u8 temp)
>  {
> -	return kelvin_to_millicelsius((((long)temp) + EC_TEMP_SENSOR_OFFSET));
> +	return cros_ec_hwmon_kelvin_to_millicelsius((((long)temp) + EC_TEMP_SENSOR_OFFSET));
>  }
>  
>  static bool cros_ec_hwmon_attr_is_temp_threshold(u32 attr)
> @@ -228,7 +236,7 @@ static int cros_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
>  								cros_ec_hwmon_attr_to_thres(attr),
>  								&threshold);
>  			if (ret == 0)
> -				*val = kelvin_to_millicelsius(threshold);
> +				*val = cros_ec_hwmon_kelvin_to_millicelsius(threshold);
>  		}
>  	}
>