Re: [PATCH] thermal: kconfig: select THERMAL_WRITABLE_TRIPS for x86 thermal

[Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>]

On Mon, 2015-08-03 at 10:19 -0700, Eduardo Valentin wrote:
> On Mon, Aug 03, 2015 at 08:49:03AM -0700, Srinivas Pandruvada wrote:
> > On Sun, 2015-08-02 at 20:10 -0700, Eduardo Valentin wrote:
> > > Srinivas,
> > > 
> > > On Sat, Aug 01, 2015 at 10:46:41PM -0700, Srinivas Pandruvada wrote:
> > > > After the commit "thermal: core: Add Kconfig option to enable writable trips",
> > > > user space tools like thermal daemon and Chrome OS DPTF thermal controller
> > > > can no longer receive async events for thermal thresholds. Since we need to
> > > > enable the new config introduced by above commit to allow writable trips.
> > > > Selecting CONFIG_THERMAL_WRITABLE_TRIPS for x86 thermal drivers.
> > > 
> > > I am fine with enabling the config on x86 drivers. However the
> > > description is somewhat confusing. Are you sure you are not receiving
> > > async events or you are not able to write on trip point files?
> > > 
> > > Because that's all this option does (quoting thermal_core.c):
> > >                 if (IS_ENABLED(CONFIG_THERMAL_WRITABLE_TRIPS) &&
> > >                     mask & (1 << indx)) {
> > >                         tz->trip_temp_attrs[indx].attr.attr.mode |= S_IWUSR;
> > >                         tz->trip_temp_attrs[indx].attr.store =
> > >                                                         trip_point_temp_store;
> > >                 }
> > > 
> > > 
> > > I am not really sure if this change is going to give your events back.
> > > 
> > > 
> > > can you please confirm what you are trying to achieve here?
> > > 
> > Sorry for the confusion. Since we can't write thresholds, we can't get
> > asynchronous events for thresholds. This config prevents writable trips,
> > hence we need to select this config.
> 
> OK... Can you then please rephrase your commit log? I don't see why the
> kernel would not send you the events, even if you cannot write the
> threshold. The events will go for the thresholds set by the kernel.
> If you are not allowed to change the threshold, you are going to receive
> the events at the wrong time, probably, right? Or did I miss something?
> 
The EC/FW allows us to set a temperature threshold, where the user space
thermal controller will take some action (and not poll for temperature).
On these platforms there is no thermal control at kernel level (user
space governor only).

> > 
> > Thanks,
> > Srinivas
> > 
> > > > 
> > > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > > > ---
> > > >  drivers/thermal/Kconfig | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > > > index 118938e..0e3b576 100644
> > > > --- a/drivers/thermal/Kconfig
> > > > +++ b/drivers/thermal/Kconfig
> > > > @@ -275,6 +275,7 @@ config X86_PKG_TEMP_THERMAL
> > > >  	tristate "X86 package temperature thermal driver"
> > > >  	depends on X86_THERMAL_VECTOR
> > > >  	select THERMAL_GOV_USER_SPACE
> > > > +	select THERMAL_WRITABLE_TRIPS
> > > >  	default m
> > > >  	help
> > > >  	  Enable this to register CPU digital sensor for package temperature as
> > > > @@ -296,6 +297,7 @@ config INTEL_SOC_DTS_THERMAL
> > > >  	tristate "Intel SoCs DTS thermal driver"
> > > >  	depends on X86
> > > >  	select INTEL_SOC_DTS_IOSF_CORE
> > > > +	select THERMAL_WRITABLE_TRIPS
> > > >  	help
> > > >  	  Enable this to register Intel SoCs (e.g. Bay Trail) platform digital
> > > >  	  temperature sensor (DTS). These SoCs have two additional DTSs in
> > > > @@ -322,6 +324,7 @@ config INT340X_THERMAL
> > > >  	select ACPI_THERMAL_REL
> > > >  	select ACPI_FAN
> > > >  	select INTEL_SOC_DTS_IOSF_CORE
> > > > +	select THERMAL_WRITABLE_TRIPS
> > > >  	help
> > > >  	  Newer laptops and tablets that use ACPI may have thermal sensors and
> > > >  	  other devices with thermal control capabilities outside the core
> > > > -- 
> > > > 2.4.3
> > > > 
> > 
> >