RE: [PATCHv4 11/15] Thermal: Introduce a step_wise thermal governor

[Zhang Rui <rui.zhang@intel.com>]

On 二, 2012-09-18 at 02:11 -0600, R, Durgadoss wrote:
> Hi Rui,
> 
> 
> > -----Original Message-----
> > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi-
> > owner@vger.kernel.org] On Behalf Of Zhang Rui
> > Sent: Tuesday, September 18, 2012 12:44 PM
> > To: R, Durgadoss
> > Cc: lenb@kernel.org; linux-acpi@vger.kernel.org; eduardo.valentin@ti.com
> > Subject: Re: [PATCHv4 11/15] Thermal: Introduce a step_wise thermal
> > governor
> > 
> > On 二, 2012-09-18 at 11:05 +0530, Durgadoss R wrote:
> > > This patch adds a simple step_wise governor to the
> > > generic thermal layer. This algorithm throttles the
> > > cooling devices in a linear fashion. If the 'trend'
> > > is heating, it throttles by one step. And if the
> > > thermal trend is cooling it de-throttles by one step.
> > >
> > > This actually moves the throttling logic from thermal_sys.c
> > > and puts inside step_wise.c, without any change. This is
> > > always selected as the default governor.
> > >
> > > Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
> > > ---
> > >  drivers/thermal/Kconfig     |    7 ++
> > >  drivers/thermal/Makefile    |    1 +
> > >  drivers/thermal/step_wise.c |  194
> > +++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 202 insertions(+)
> > >  create mode 100644 drivers/thermal/step_wise.c
> > >
> > > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > > index 2200912..9f6a3aa 100644
> > > --- a/drivers/thermal/Kconfig
> > > +++ b/drivers/thermal/Kconfig
> > > @@ -4,6 +4,7 @@
> > >
> > >  menuconfig THERMAL
> > >  	tristate "Generic Thermal sysfs driver"
> > > +	select STEP_WISE
> > >  	help
> > >  	  Generic Thermal Sysfs driver offers a generic mechanism for
> > >  	  thermal management. Usually it's made up of one or more thermal
> > > @@ -59,3 +60,9 @@ config FAIR_SHARE
> > >  	depends on THERMAL
> > >  	help
> > >  	  Enable this to manage platform thermals using fair-share governor.
> > > +
> > > +config STEP_WISE
> > > +	bool "Step_wise thermal governor"
> > > +	depends on THERMAL
> > > +	help
> > > +	  Enable this to manage platform thermals using a simple linear
> > 
> > if we always wants CONFIG_STEP_WISE built in, why bother to export this
> > config option to users?
> > 
> > IMO, something like
> >  menuconfig THERMAL
> >   	tristate "Generic Thermal sysfs driver"
> > 	select STEP_WISE
> > 
> >  config STEP_WISE
> > 	bool
> > 	depends on THERMAL
> > 
> > is enough, SETP_WISE will always be selected.
> > 
> > further more, we even do not need a config option for this governor,
> > just "obj-y += step_wise.o" in Makefile.
> 
> Yes, Thought through this. But I thought it would be good to align this
> with the rest of the governors.
> 

we'd better follow cpufreq governor to allow users to select the default
governor, or else this configure option is needless because we never
want it to be cleared with CONFIG_THERMAL=y.

> > 
> > another question would be,
> > as this governor registration function is fs_initcall(),
> > what if a platform thermal driver is registered and gets a thermal event
> > before this governor being registered?
> 
> Wonder if this can happen with the current code, since the
> thermal_sys.c also loads with fs_initcall.
> 
you're right. this is not a problem.

thanks,
rui

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html