* [PATCH] thermal: Increase maximum number of trip points
@ 2022-09-27 15:47 Sumeet Pawnikar
2022-09-28 10:52 ` Andy Shevchenko
0 siblings, 1 reply; 3+ messages in thread
From: Sumeet Pawnikar @ 2022-09-27 15:47 UTC (permalink / raw)
To: rafael, srinivas.pandruvada, rui.zhang, daniel.lezcano, linux-pm,
linux-kernel
Cc: andriy.shevchenko, sumeet.r.pawnikar
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.
Signed-off-by: Sumeet Pawnikar <sumeet.r.pawnikar@intel.com>
---
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] thermal: Increase maximum number of trip points
2022-09-27 15:47 [PATCH] thermal: Increase maximum number of trip points Sumeet Pawnikar
@ 2022-09-28 10:52 ` Andy Shevchenko
2022-09-30 17:52 ` Rafael J. Wysocki
0 siblings, 1 reply; 3+ messages in thread
From: Andy Shevchenko @ 2022-09-28 10:52 UTC (permalink / raw)
To: Sumeet Pawnikar
Cc: rafael, srinivas.pandruvada, rui.zhang, daniel.lezcano, linux-pm,
linux-kernel
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@linux.intel.com>
> Signed-off-by: Sumeet Pawnikar <sumeet.r.pawnikar@intel.com>
> ---
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] thermal: Increase maximum number of trip points
2022-09-28 10:52 ` Andy Shevchenko
@ 2022-09-30 17:52 ` Rafael J. Wysocki
0 siblings, 0 replies; 3+ messages in thread
From: Rafael J. Wysocki @ 2022-09-30 17:52 UTC (permalink / raw)
To: Andy Shevchenko, Sumeet Pawnikar, daniel.lezcano
Cc: srinivas.pandruvada, rui.zhang, linux-pm, linux-kernel
On Wed, Sep 28, 2022 at 12:52 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> 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@linux.intel.com>
Applied as 6.1 material, thanks!
Daniel, please let me know if you have any concerns.
> > Signed-off-by: Sumeet Pawnikar <sumeet.r.pawnikar@intel.com>
> > ---
> > 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
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-30 17:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-27 15:47 [PATCH] thermal: Increase maximum number of trip points Sumeet Pawnikar
2022-09-28 10:52 ` Andy Shevchenko
2022-09-30 17:52 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).