Re: [PATCH] thermal: Increase maximum number of trip points

[Andy Shevchenko]

On Tue, Sep 27, 2022 at 09:17:09PM +0530, Sumeet Pawnikar wrote:
> On one of the Chrome system, if we define more than 12 trip points,
> probe for thermal sensor fails with
> "int3403 thermal: probe of INTC1046:03 failed with error -22"
> and throws an error as
> "thermal_sys: Error: Incorrect number of thermal trips".
> 
> The thermal_zone_device_register() interface needs maximum
> number of trip points supported in a zone as an argument.
> This number can't exceed THERMAL_MAX_TRIPS, which is currently
> set to 12. To address this issue, THERMAL_MAX_TRIPS value
> has to be increased.
> 
> This interface also has an argument to specify a mask of trips
> which are writable. This mask is defined as an int.
> This mask sets the ceiling for increasing maximum number of
> supported trips. With the current implementation, maximum number
> of trips can be supported is 31.
> 
> Also, THERMAL_MAX_TRIPS macro is used in one place only.
> So, remove THERMAL_MAX_TRIPS macro and compare num_trips
> directly with using a macro BITS_PER_TYPE(int)-1.

FWIW,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>

> Signed-off-by: Sumeet Pawnikar <sumeet.r.pawnikar@xxxxxxxxx>
> ---
>  drivers/thermal/thermal_core.c | 15 ++++++++++++++-
>  include/linux/thermal.h        |  2 --
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
> index 50d50cec7774..589dd82fe10c 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -1212,7 +1212,20 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> -	if (num_trips > THERMAL_MAX_TRIPS || num_trips < 0 || mask >> num_trips) {
> +	/*
> +	 * Max trip count can't exceed 31 as the "mask >> num_trips" condition.
> +	 * For example, shifting by 32 will result in compiler warning:
> +	 * warning: right shift count >= width of type [-Wshift-count- overflow]
> +	 *
> +	 * Also "mask >> num_trips" will always be true with 32 bit shift.
> +	 * E.g. mask = 0x80000000 for trip id 31 to be RW. Then
> +	 * mask >> 32 = 0x80000000
> +	 * This will result in failure for the below condition.
> +	 *
> +	 * Check will be true when the bit 31 of the mask is set.
> +	 * 32 bit shift will cause overflow of 4 byte integer.
> +	 */
> +	if (num_trips > (BITS_PER_TYPE(int) - 1) || num_trips < 0 || mask >> num_trips) {
>  		pr_err("Incorrect number of thermal trips\n");
>  		return ERR_PTR(-EINVAL);
>  	}
> diff --git a/include/linux/thermal.h b/include/linux/thermal.h
> index 1386c713885d..c05f5c78a0f2 100644
> --- a/include/linux/thermal.h
> +++ b/include/linux/thermal.h
> @@ -17,8 +17,6 @@
>  #include <linux/workqueue.h>
>  #include <uapi/linux/thermal.h>
>  
> -#define THERMAL_MAX_TRIPS	12
> -
>  /* invalid cooling state */
>  #define THERMAL_CSTATE_INVALID -1UL
>  
> -- 
> 2.17.1
> 

-- 
With Best Regards,
Andy Shevchenko