[RFC] docs: thermal/drivers/intel: Reading trip point previously set to 0

[RFC] docs: thermal/drivers/intel: Reading trip point previously set to 0

[Vasilis Liaskovitis]

Hi,

Since commit:
eb8500b8 "thermal/drivers/intel: Initialize RW trip to THERMAL_TEMP_INVALID"

writing 0 into thermal trip point temp sysfs files cannot be read back:
~ # echo 0  > /sys/devices/virtual/thermal/thermal_zone1/trip_point_0_temp
~ # cat /sys/devices/virtual/thermal/thermal_zone1/trip_point_0_temp 

-274000

Prior to this change, the value 0 could be "read". Afaict only because 0 
was always returned for uninitialized trip points, and not only when 0 
was actually written into the trip_point_*_temp by userspace.

A customer uses scripts to set the trip_point_*_temp value to 0, and 
then checks that the value is indeed 0. Their userspace test suite 
breaks because of this change.

Should userspace still be able to read the set value of 0 (meaning the 
thermal subsystem confirms no notifications will be sent)? I understand 
this is a corner case (maybe it's even non-sensical to check a value of 
0 is returned, after setting it), but I wanted to ask since the sysfs 
value read by userspace is now THERMAL_TEMP_INVALID.

Or should we instead update documentation to reflect the fact that 
uninitialized trip points and trip points set to 0 return an invalid 
value THERMAL_TEMP_INVALID? E.g.:


diff --git 
a/Documentation/driver-api/thermal/x86_pkg_temperature_thermal.rst 
b/Documentation/driver-api/thermal/x86_pkg_temperature_thermal.rst
index 2ac42ccd236f6..2e6f2728b5b94 100644
--- a/Documentation/driver-api/thermal/x86_pkg_temperature_thermal.rst
+++ b/Documentation/driver-api/thermal/x86_pkg_temperature_thermal.rst
@@ -43,8 +43,10 @@ User can set any temperature between 0 to TJ-Max 
temperature. Temperature units
  are in milli-degree Celsius. Refer to 
"Documentation/driver-api/thermal/sysfs-api.rst" for
  thermal sys-fs details.

+An uninitialized trip_point_*_temp returns an invalid value 
(THERMAL_TEMP_INVALID).
  Any value other than 0 in these trip points, can trigger thermal 
notifications.
-Setting 0, stops sending thermal notifications.
+Setting 0, stops sending thermal notifications. Having set 0, the value 
returned by the
+trip point remains invalid (THERMAL_TEMP_INVALID).

  Thermal notifications:
  To get kobject-uevent notifications, set the thermal zone

Re: [RFC] docs: thermal/drivers/intel: Reading trip point previously set to 0

[Vasilis LIaskovitis]

Hi,

ping, any thoughts on this?

> Since commit:
> eb8500b8 "thermal/drivers/intel: Initialize RW trip to 
> THERMAL_TEMP_INVALID"
>
> writing 0 into thermal trip point temp sysfs files cannot be read back:
> ~ # echo 0  > 
> /sys/devices/virtual/thermal/thermal_zone1/trip_point_0_temp
> ~ # cat /sys/devices/virtual/thermal/thermal_zone1/trip_point_0_temp
> -274000
>
> Prior to this change, the value 0 could be "read". Afaict only because 
> 0 was always returned for uninitialized trip points, and not only when 
> 0 was actually written into the trip_point_*_temp by userspace.
>
> A customer uses scripts to set the trip_point_*_temp value to 0, and 
> then checks that the value is indeed 0. Their userspace test suite 
> breaks because of this change.
>
> Should userspace still be able to read the set value of 0 (meaning the 
> thermal subsystem confirms no notifications will be sent)? I 
> understand this is a corner case (maybe it's even non-sensical to 
> check a value of 0 is returned, after setting it), but I wanted to ask 
> since the sysfs value read by userspace is now THERMAL_TEMP_INVALID.
>
> Or should we instead update documentation to reflect the fact that 
> uninitialized trip points and trip points set to 0 return an invalid 
> value THERMAL_TEMP_INVALID? E.g.:
>
>
> diff --git 
> a/Documentation/driver-api/thermal/x86_pkg_temperature_thermal.rst 
> b/Documentation/driver-api/thermal/x86_pkg_temperature_thermal.rst
> index 2ac42ccd236f6..2e6f2728b5b94 100644
> --- a/Documentation/driver-api/thermal/x86_pkg_temperature_thermal.rst
> +++ b/Documentation/driver-api/thermal/x86_pkg_temperature_thermal.rst
> @@ -43,8 +43,10 @@ User can set any temperature between 0 to TJ-Max 
> temperature. Temperature units
>  are in milli-degree Celsius. Refer to 
> "Documentation/driver-api/thermal/sysfs-api.rst" for
>  thermal sys-fs details.
>
> +An uninitialized trip_point_*_temp returns an invalid value 
> (THERMAL_TEMP_INVALID).
>  Any value other than 0 in these trip points, can trigger thermal 
> notifications.
> -Setting 0, stops sending thermal notifications.
> +Setting 0, stops sending thermal notifications. Having set 0, the 
> value returned by the
> +trip point remains invalid (THERMAL_TEMP_INVALID).
>
>  Thermal notifications:
>  To get kobject-uevent notifications, set the thermal zone
>

Re: [RFC] docs: thermal/drivers/intel: Reading trip point previously set to 0

[srinivas pandruvada]

Hi Vasilis,

On Thu, 2023-09-28 at 20:05 +0200, Vasilis LIaskovitis wrote:
> Hi,
> 
> ping, any thoughts on this?
> 
> > Since commit:
> > eb8500b8 "thermal/drivers/intel: Initialize RW trip to 
> > THERMAL_TEMP_INVALID"
> > 
> > writing 0 into thermal trip point temp sysfs files cannot be read
> > back:
> > ~ # echo 0  > 
> > /sys/devices/virtual/thermal/thermal_zone1/trip_point_0_temp
> > ~ # cat
> > /sys/devices/virtual/thermal/thermal_zone1/trip_point_0_temp
> > -274000
> > 
> > Prior to this change, the value 0 could be "read". Afaict only
> > because 
> > 0 was always returned for uninitialized trip points, and not only
> > when 
> > 0 was actually written into the trip_point_*_temp by userspace.
> > 
> > A customer uses scripts to set the trip_point_*_temp value to 0,
> > and 
> > then checks that the value is indeed 0. Their userspace test suite 
> > breaks because of this change.
> > 
> > Should userspace still be able to read the set value of 0 (meaning
> > the 
> > thermal subsystem confirms no notifications will be sent)? I 
> > understand this is a corner case (maybe it's even non-sensical to 
> > check a value of 0 is returned, after setting it), but I wanted to
> > ask 
> > since the sysfs value read by userspace is now
> > THERMAL_TEMP_INVALID.
> > 
> > Or should we instead update documentation to reflect the fact that 
> > uninitialized trip points and trip points set to 0 return an
> > invalid 
> > value THERMAL_TEMP_INVALID? E.g.:
> > 

I think better to update documentation, Please submit a change, if you
can.

Thanks,
Srinivas

> > 
> > diff --git 
> > a/Documentation/driver-api/thermal/x86_pkg_temperature_thermal.rst 
> > b/Documentation/driver-api/thermal/x86_pkg_temperature_thermal.rst
> > index 2ac42ccd236f6..2e6f2728b5b94 100644
> > --- a/Documentation/driver-
> > api/thermal/x86_pkg_temperature_thermal.rst
> > +++ b/Documentation/driver-
> > api/thermal/x86_pkg_temperature_thermal.rst
> > @@ -43,8 +43,10 @@ User can set any temperature between 0 to TJ-Max
> > temperature. Temperature units
> >  are in milli-degree Celsius. Refer to 
> > "Documentation/driver-api/thermal/sysfs-api.rst" for
> >  thermal sys-fs details.
> > 
> > +An uninitialized trip_point_*_temp returns an invalid value 
> > (THERMAL_TEMP_INVALID).
> >  Any value other than 0 in these trip points, can trigger thermal 
> > notifications.
> > -Setting 0, stops sending thermal notifications.
> > +Setting 0, stops sending thermal notifications. Having set 0, the 
> > value returned by the
> > +trip point remains invalid (THERMAL_TEMP_INVALID).
> > 
> >  Thermal notifications:
> >  To get kobject-uevent notifications, set the thermal zone
> >