diff mbox

[7/9] Thermal: Make PER_ZONE values configurable

Message ID 1357542806-20449-8-git-send-email-durgadoss.r@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Zhang Rui
Headers show

Commit Message

durgadoss.r@intel.com Jan. 7, 2013, 7:13 a.m. UTC
This patch makes MAX_SENSORS_PER_ZONE and
MAX_CDEVS_PER_ZONE values configurable. The
default value is 1, and range is 1-12.

Signed-off-by: Durgadoss R <durgadoss.r@intel.com>
---
 drivers/thermal/Kconfig |   14 ++++++++++++++
 include/linux/thermal.h |    6 +++---
 2 files changed, 17 insertions(+), 3 deletions(-)

Comments

Greg Kroah-Hartman Jan. 7, 2013, 7:24 p.m. UTC | #1
On Mon, Jan 07, 2013 at 12:43:24PM +0530, Durgadoss R wrote:
> This patch makes MAX_SENSORS_PER_ZONE and
> MAX_CDEVS_PER_ZONE values configurable. The
> default value is 1, and range is 1-12.

Why would we ever want to change this?  Why make this configurable at
all, how is a distro supposed to set this value?

Shouldn't it be specified from the driver itself?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
durgadoss.r@intel.com Jan. 9, 2013, 9:12 a.m. UTC | #2
Hi Greg,

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, January 08, 2013 12:54 AM
> To: R, Durgadoss
> Cc: Zhang, Rui; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org;
> eduardo.valentin@ti.com; hongbo.zhang@linaro.org; wni@nvidia.com
> Subject: Re: [PATCH 7/9] Thermal: Make PER_ZONE values configurable
> 
> On Mon, Jan 07, 2013 at 12:43:24PM +0530, Durgadoss R wrote:
> > This patch makes MAX_SENSORS_PER_ZONE and
> > MAX_CDEVS_PER_ZONE values configurable. The
> > default value is 1, and range is 1-12.
> 
> Why would we ever want to change this?  Why make this configurable at
> all, how is a distro supposed to set this value?
> 
> Shouldn't it be specified from the driver itself?

These are platform level parameters, that can differ for various platforms.
(Mostly due to board design and thermistor layouts). Stand-alone thermal
sensor drivers are not (need not be) aware of these values.
That's why these values are made configurable.

Thanks,
Durga
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg Kroah-Hartman Jan. 9, 2013, 5 p.m. UTC | #3
On Wed, Jan 09, 2013 at 09:12:29AM +0000, R, Durgadoss wrote:
> Hi Greg,
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Tuesday, January 08, 2013 12:54 AM
> > To: R, Durgadoss
> > Cc: Zhang, Rui; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org;
> > eduardo.valentin@ti.com; hongbo.zhang@linaro.org; wni@nvidia.com
> > Subject: Re: [PATCH 7/9] Thermal: Make PER_ZONE values configurable
> > 
> > On Mon, Jan 07, 2013 at 12:43:24PM +0530, Durgadoss R wrote:
> > > This patch makes MAX_SENSORS_PER_ZONE and
> > > MAX_CDEVS_PER_ZONE values configurable. The
> > > default value is 1, and range is 1-12.
> > 
> > Why would we ever want to change this?  Why make this configurable at
> > all, how is a distro supposed to set this value?
> > 
> > Shouldn't it be specified from the driver itself?
> 
> These are platform level parameters, that can differ for various platforms.
> (Mostly due to board design and thermistor layouts). Stand-alone thermal
> sensor drivers are not (need not be) aware of these values.

Ok, and how does anyone know how to set them properly?

> That's why these values are made configurable.

Pushing work onto other people, without telling them what they need to
do, isn't nice.  Please, either make it auto-configurable, or don't make
it configurable at all.  As it is, this is useless for a distro, or any
"normal" user, right?

If these are platform specific things, shouldn't they be defined in the
platform data for the hardware?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
durgadoss.r@intel.com Jan. 10, 2013, 12:43 p.m. UTC | #4
Hi Greg,

> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Wednesday, January 09, 2013 10:30 PM
> To: R, Durgadoss
> Cc: Zhang, Rui; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org;
> eduardo.valentin@ti.com; hongbo.zhang@linaro.org; wni@nvidia.com
> Subject: Re: [PATCH 7/9] Thermal: Make PER_ZONE values configurable
> 
> On Wed, Jan 09, 2013 at 09:12:29AM +0000, R, Durgadoss wrote:
> > Hi Greg,
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Tuesday, January 08, 2013 12:54 AM
> > > To: R, Durgadoss
> > > Cc: Zhang, Rui; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > eduardo.valentin@ti.com; hongbo.zhang@linaro.org; wni@nvidia.com
> > > Subject: Re: [PATCH 7/9] Thermal: Make PER_ZONE values configurable
> > >
> > > On Mon, Jan 07, 2013 at 12:43:24PM +0530, Durgadoss R wrote:
> > > > This patch makes MAX_SENSORS_PER_ZONE and
> > > > MAX_CDEVS_PER_ZONE values configurable. The
> > > > default value is 1, and range is 1-12.
> > >
> > > Why would we ever want to change this?  Why make this configurable at
> > > all, how is a distro supposed to set this value?
> > >
> > > Shouldn't it be specified from the driver itself?
> >
> > These are platform level parameters, that can differ for various platforms.
> > (Mostly due to board design and thermistor layouts). Stand-alone thermal
> > sensor drivers are not (need not be) aware of these values.
> 
> Ok, and how does anyone know how to set them properly?
> 
> > That's why these values are made configurable.
> 
> Pushing work onto other people, without telling them what they need to
> do, isn't nice.  Please, either make it auto-configurable, or don't make
> it configurable at all.  As it is, this is useless for a distro, or any
> "normal" user, right?

These values are for OEM's to configure according to their platform design.
So, not making them configurable won't be good IMHO.

But, as you said, for normal users, it does not matter; so they can
leave it as it is, which will set it to the default value (i.e 1).

Thanks,
Durga
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Greg Kroah-Hartman Jan. 10, 2013, 2:27 p.m. UTC | #5
On Thu, Jan 10, 2013 at 12:43:16PM +0000, R, Durgadoss wrote:
> Hi Greg,
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Wednesday, January 09, 2013 10:30 PM
> > To: R, Durgadoss
> > Cc: Zhang, Rui; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org;
> > eduardo.valentin@ti.com; hongbo.zhang@linaro.org; wni@nvidia.com
> > Subject: Re: [PATCH 7/9] Thermal: Make PER_ZONE values configurable
> > 
> > On Wed, Jan 09, 2013 at 09:12:29AM +0000, R, Durgadoss wrote:
> > > Hi Greg,
> > >
> > > > -----Original Message-----
> > > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > > Sent: Tuesday, January 08, 2013 12:54 AM
> > > > To: R, Durgadoss
> > > > Cc: Zhang, Rui; linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > > eduardo.valentin@ti.com; hongbo.zhang@linaro.org; wni@nvidia.com
> > > > Subject: Re: [PATCH 7/9] Thermal: Make PER_ZONE values configurable
> > > >
> > > > On Mon, Jan 07, 2013 at 12:43:24PM +0530, Durgadoss R wrote:
> > > > > This patch makes MAX_SENSORS_PER_ZONE and
> > > > > MAX_CDEVS_PER_ZONE values configurable. The
> > > > > default value is 1, and range is 1-12.
> > > >
> > > > Why would we ever want to change this?  Why make this configurable at
> > > > all, how is a distro supposed to set this value?
> > > >
> > > > Shouldn't it be specified from the driver itself?
> > >
> > > These are platform level parameters, that can differ for various platforms.
> > > (Mostly due to board design and thermistor layouts). Stand-alone thermal
> > > sensor drivers are not (need not be) aware of these values.
> > 
> > Ok, and how does anyone know how to set them properly?
> > 
> > > That's why these values are made configurable.
> > 
> > Pushing work onto other people, without telling them what they need to
> > do, isn't nice.  Please, either make it auto-configurable, or don't make
> > it configurable at all.  As it is, this is useless for a distro, or any
> > "normal" user, right?
> 
> These values are for OEM's to configure according to their platform design.
> So, not making them configurable won't be good IMHO.

Ok, but you are guaranteeing that the same kernel can not work on
multiple platforms, which is something that people have been working
very hard to solve.

So again, please make this a platform data field, or autoconfigurable,
and not a kernel build time configuration option, that isn't acceptable
anymore.

thanks,

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

Patch

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index d96da07..c5ba3340 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -15,6 +15,20 @@  menuconfig THERMAL
 
 if THERMAL
 
+config THERMAL_MAX_SENSORS_PER_ZONE
+	int "Maximum number of sensors allowed per thermal zone"
+	default 1
+	range 1 12
+	---help---
+	  Specify the number of sensors allowed per zone
+
+config THERMAL_MAX_CDEVS_PER_ZONE
+	int "Maximum number of cooling devices allowed per thermal zone"
+	default 1
+	range 1 12
+	---help---
+	  Specify the number of cooling devices allowed per zone
+
 config THERMAL_HWMON
 	bool
 	depends on HWMON=y || HWMON=THERMAL
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 187fadb..cf19fba 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -50,9 +50,9 @@ 
 /* Default Thermal Governor: Does Linear Throttling */
 #define DEFAULT_THERMAL_GOVERNOR	"step_wise"
 
-#define MAX_SENSORS_PER_ZONE		5
-
-#define MAX_CDEVS_PER_ZONE		5
+/* Maximum number of sensors/cdevs per zone, defined through Kconfig */
+#define MAX_SENSORS_PER_ZONE	CONFIG_THERMAL_MAX_SENSORS_PER_ZONE
+#define MAX_CDEVS_PER_ZONE	CONFIG_THERMAL_MAX_CDEVS_PER_ZONE
 
 /* If we map each sensor with every possible cdev for a zone */
 #define MAX_MAPS_PER_ZONE	(MAX_SENSORS_PER_ZONE * MAX_CDEVS_PER_ZONE)