[PATCH] thermal: fix Kconfig dependencies

[PATCH] thermal: fix Kconfig dependencies

[Heiko Carstens]

From: Heiko Carstens <heiko.carstens@de.ibm.com>

git commit 3152fb9f11cdd2fd8688c2c5cb805e5c09b53dd9
"thermal: fix generic thermal I/F for hwmon" adds a select HWMON
to THERMAL. This causes HWMON to be selected regardless of its
other dependencies. In this case depends on HAS_IOMEM gets ignored
which causes this build error on s390:

drivers/hwmon/w83627hf.c: In function 'superio_outb':
drivers/hwmon/w83627hf.c:117: error: implicit declaration of function 'outb'

Change the select to a depends on to fix this. Should work as well.

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Len Brown <len.brown@intel.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 drivers/thermal/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-2.6/drivers/thermal/Kconfig
===================================================================
--- linux-2.6.orig/drivers/thermal/Kconfig
+++ linux-2.6/drivers/thermal/Kconfig
@@ -4,7 +4,7 @@
 
 menuconfig THERMAL
 	bool "Generic Thermal sysfs driver"
-	select HWMON
+	depends on HWMON
 	default y
 	help
 	  Generic Thermal Sysfs driver offers a generic mechanism for

Re: [PATCH] thermal: fix Kconfig dependencies

[Linus Torvalds]

On Mon, 17 Mar 2008, Heiko Carstens wrote:
> 
> Change the select to a depends on to fix this. Should work as well.

Nope, that doesn't work. ACPI_THERMAL will select THERMAL, so now you have 
THERMAL selected without HWMON.

As a minimal fix, you'd at least need to make ACPI_THERMAL depend on 
THERMAL too.

		Linus

Re: [PATCH] thermal: fix Kconfig dependencies

[Heiko Carstens]

On Mon, Mar 17, 2008 at 09:55:13AM -0700, Linus Torvalds wrote:
> On Mon, 17 Mar 2008, Heiko Carstens wrote:
> > 
> > Change the select to a depends on to fix this. Should work as well.
> 
> Nope, that doesn't work. ACPI_THERMAL will select THERMAL, so now you have 
> THERMAL selected without HWMON.

Oh, missed that. Sorry.
 
> As a minimal fix, you'd at least need to make ACPI_THERMAL depend on 
> THERMAL too.

Updated patch:

Subject: [PATCH] thermal: fix Kconfig dependencies

From: Heiko Carstens <heiko.carstens@de.ibm.com>

git commit 3152fb9f11cdd2fd8688c2c5cb805e5c09b53dd9
"thermal: fix generic thermal I/F for hwmon" adds a select HWMON
to THERMAL. This causes HWMON to be selected regardless of its
other dependencies. In this case depends on HAS_IOMEM gets ignored
which causes this build error on s390:

drivers/hwmon/w83627hf.c: In function 'superio_outb':
drivers/hwmon/w83627hf.c:117: error: implicit declaration of function 'outb'

Change the select to a depends on. In addition change the select THERMAL
from ACPI_THERMAL to a depends on THERMAL. Otherwise THERMAL
could be selected by ACPI_THERMAL without HWMON being selected.

Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Len Brown <len.brown@intel.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---

 drivers/acpi/Kconfig    |    3 +--
 drivers/thermal/Kconfig |    2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

Index: linux-2.6/drivers/thermal/Kconfig
===================================================================
--- linux-2.6.orig/drivers/thermal/Kconfig
+++ linux-2.6/drivers/thermal/Kconfig
@@ -4,7 +4,7 @@
 
 menuconfig THERMAL
 	bool "Generic Thermal sysfs driver"
-	select HWMON
+	depends on HWMON
 	default y
 	help
 	  Generic Thermal Sysfs driver offers a generic mechanism for
Index: linux-2.6/drivers/acpi/Kconfig
===================================================================
--- linux-2.6.orig/drivers/acpi/Kconfig
+++ linux-2.6/drivers/acpi/Kconfig
@@ -187,8 +187,7 @@ config ACPI_HOTPLUG_CPU
 
 config ACPI_THERMAL
 	tristate "Thermal Zone"
-	depends on ACPI_PROCESSOR
-	select THERMAL
+	depends on ACPI_PROCESSOR && THERMAL
 	default y
 	help
 	  This driver adds support for ACPI thermal zones.  Most mobile and

Re: [PATCH] thermal: fix Kconfig dependencies

[Len Brown]

On Monday 17 March 2008, Heiko Carstens wrote:
> On Mon, Mar 17, 2008 at 09:55:13AM -0700, Linus Torvalds wrote:
> > On Mon, 17 Mar 2008, Heiko Carstens wrote:
> > > 
> > > Change the select to a depends on to fix this. Should work as well.
> > 
> > Nope, that doesn't work. ACPI_THERMAL will select THERMAL, so now you have 
> > THERMAL selected without HWMON.
> 
> Oh, missed that. Sorry.
>  
> > As a minimal fix, you'd at least need to make ACPI_THERMAL depend on 
> > THERMAL too.

Although the "select" that started this thread was certainly erroneous,
this doesn't look right either.

THERMAL should not depend on or select HWMON.
Instead, part of its code that is there for
the benefit of HWMON should depend on HWMON.

Based on Jean's last message, that code should
probably get its own sub-config option, CONFIG_THERMAL_HWMON
that is default N for the benefit of old libraries.

Similarly, ACPI_THERMAL should not depend on THERMAL,
instead the code that registers with THERMAL should
simply depend on if THERMAL is selected or not.

Also, the "default y" should go.

I'll tinker with this a bit after lunch.

thanks,
-Len


> Updated patch:
> 
> Subject: [PATCH] thermal: fix Kconfig dependencies
> 
> From: Heiko Carstens <heiko.carstens@de.ibm.com>
> 
> git commit 3152fb9f11cdd2fd8688c2c5cb805e5c09b53dd9
> "thermal: fix generic thermal I/F for hwmon" adds a select HWMON
> to THERMAL. This causes HWMON to be selected regardless of its
> other dependencies. In this case depends on HAS_IOMEM gets ignored
> which causes this build error on s390:
> 
> drivers/hwmon/w83627hf.c: In function 'superio_outb':
> drivers/hwmon/w83627hf.c:117: error: implicit declaration of function 'outb'
> 
> Change the select to a depends on. In addition change the select THERMAL
> from ACPI_THERMAL to a depends on THERMAL. Otherwise THERMAL
> could be selected by ACPI_THERMAL without HWMON being selected.
> 
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Len Brown <len.brown@intel.com>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> ---
> 
>  drivers/acpi/Kconfig    |    3 +--
>  drivers/thermal/Kconfig |    2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6/drivers/thermal/Kconfig
> ===================================================================
> --- linux-2.6.orig/drivers/thermal/Kconfig
> +++ linux-2.6/drivers/thermal/Kconfig
> @@ -4,7 +4,7 @@
>  
>  menuconfig THERMAL
>  	bool "Generic Thermal sysfs driver"
> -	select HWMON
> +	depends on HWMON
>  	default y
>  	help
>  	  Generic Thermal Sysfs driver offers a generic mechanism for
> Index: linux-2.6/drivers/acpi/Kconfig
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/Kconfig
> +++ linux-2.6/drivers/acpi/Kconfig
> @@ -187,8 +187,7 @@ config ACPI_HOTPLUG_CPU
>  
>  config ACPI_THERMAL
>  	tristate "Thermal Zone"
> -	depends on ACPI_PROCESSOR
> -	select THERMAL
> +	depends on ACPI_PROCESSOR && THERMAL
>  	default y
>  	help
>  	  This driver adds support for ACPI thermal zones.  Most mobile and
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [PATCH] thermal: fix Kconfig dependencies

[Linus Torvalds]

On Mon, 17 Mar 2008, Len Brown wrote:
> 
> Similarly, ACPI_THERMAL should not depend on THERMAL,
> instead the code that registers with THERMAL should
> simply depend on if THERMAL is selected or not.
> 
> Also, the "default y" should go.
> 
> I'll tinker with this a bit after lunch.

Hmm.. It looks like the same commit that caused these Kconfig issues is 
also the one that causes the problems with lmsensors (I didn't immediately 
realize that it's the exact same commit).

I think that right now the right thing to do is to just revert it, since 
apparently there won't be a released lmsensors version by the time 2.6.25 
gets released that can handle the new sysfs layout, and that we should 
give this some more time to be resolved.

So I'm inclined to revert commit 3152fb9f11cdd2fd8688c2c5cb805e5c09b53dd9 
and plan on revisiting this for 2.6.26. I already got an ack for that from 
Jean Delvare, but I thought I'd mention it in this thread too before I 
actually do the final revert.

Any really strong objections?

		Linus

Re: [PATCH] thermal: fix Kconfig dependencies

[Zhang, Rui]

On Tue, 2008-03-18 at 10:25 +0800, Linus Torvalds wrote:
> 
> 
> On Mon, 17 Mar 2008, Len Brown wrote:
> >
> > Similarly, ACPI_THERMAL should not depend on THERMAL,
> > instead the code that registers with THERMAL should
> > simply depend on if THERMAL is selected or not.
> >
> > Also, the "default y" should go.
> >
> > I'll tinker with this a bit after lunch.
> 
> Hmm.. It looks like the same commit that caused these Kconfig issues
> is
> also the one that causes the problems with lmsensors
Yes.
>  (I didn't immediately
> realize that it's the exact same commit).
> 
> I think that right now the right thing to do is to just revert it,
> since
> apparently there won't be a released lmsensors version by the time
> 2.6.25
> gets released that can handle the new sysfs layout, and that we should
> give this some more time to be resolved.
> 
> So I'm inclined to revert commit
> 3152fb9f11cdd2fd8688c2c5cb805e5c09b53dd9
> and plan on revisiting this for 2.6.26. I already got an ack for that
> from
> Jean Delvare, but I thought I'd mention it in this thread too before I
> actually do the final revert.
> 
> Any really strong objections?
No.
As we still need some discussions before the problem is solved, please
revert it. :)

And please apply this patch after reverting the old one.

thanks,
rui

From: Zhang Rui <rui.zhang@intel.com>

The generic thermal driver shows temperature in millidegree Celsius.
Update the documentation.

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
---
 Documentation/thermal/sysfs-api.txt |   22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

Index: linux-2.6/Documentation/thermal/sysfs-api.txt
===================================================================
--- linux-2.6.orig/Documentation/thermal/sysfs-api.txt
+++ linux-2.6/Documentation/thermal/sysfs-api.txt
@@ -143,10 +143,10 @@ type				Strings which represent the ther
 				This is given by thermal zone driver as part of registration.
 				Eg: "ACPI thermal zone" indicates it's a ACPI thermal device
 				RO
-				Optional
+				Required
 
 temp				Current temperature as reported by thermal zone (sensor)
-				Unit: degree Celsius
+				Unit: millidegree Celsius
 				RO
 				Required
 
@@ -163,7 +163,7 @@ mode				One of the predefined values in 
 					  charge of the thermal management.
 
 trip_point_[0-*]_temp		The temperature above which trip point will be fired
-				Unit: degree Celsius
+				Unit: millidegree Celsius
 				RO
 				Optional
 
@@ -193,7 +193,7 @@ type				String which represents the type
 				eg. For memory controller device on intel_menlow platform:
 				this should be "Memory controller"
 				RO
-				Optional
+				Required
 
 max_state			The maximum permissible cooling state of this cooling device.
 				RO
@@ -219,16 +219,16 @@ the sys I/F structure will be built like
 
 |thermal_zone1:
 	|-----type:			ACPI thermal zone
-	|-----temp:			37
+	|-----temp:			37000
 	|-----mode:			kernel
-	|-----trip_point_0_temp:	100
+	|-----trip_point_0_temp:	100000
 	|-----trip_point_0_type:	critical
-	|-----trip_point_1_temp:	80
+	|-----trip_point_1_temp:	80000
 	|-----trip_point_1_type:	passive
-	|-----trip_point_2_temp:	70
-	|-----trip_point_2_type:	active[0]
-	|-----trip_point_3_temp:	60
-	|-----trip_point_3_type:	active[1]
+	|-----trip_point_2_temp:	70000
+	|-----trip_point_2_type:	active0
+	|-----trip_point_3_temp:	60000
+	|-----trip_point_3_type:	active1
 	|-----cdev0:			--->/sys/class/thermal/cooling_device0
 	|-----cdev0_trip_point:		1	/* cdev0 can be used for passive */
 	|-----cdev1:			--->/sys/class/thermal/cooling_device3

Re: [PATCH] thermal: fix Kconfig dependencies

[Len Brown]

i've reverted the offending thermal/hwmon commit,
and re-applied this documentation update to thermal.

thanks,
-Len

On Monday 17 March 2008, Zhang, Rui wrote:
> 
> On Tue, 2008-03-18 at 10:25 +0800, Linus Torvalds wrote:
> > 
> > 
> > On Mon, 17 Mar 2008, Len Brown wrote:
> > >
> > > Similarly, ACPI_THERMAL should not depend on THERMAL,
> > > instead the code that registers with THERMAL should
> > > simply depend on if THERMAL is selected or not.
> > >
> > > Also, the "default y" should go.
> > >
> > > I'll tinker with this a bit after lunch.
> > 
> > Hmm.. It looks like the same commit that caused these Kconfig issues
> > is
> > also the one that causes the problems with lmsensors
> Yes.
> >  (I didn't immediately
> > realize that it's the exact same commit).
> > 
> > I think that right now the right thing to do is to just revert it,
> > since
> > apparently there won't be a released lmsensors version by the time
> > 2.6.25
> > gets released that can handle the new sysfs layout, and that we should
> > give this some more time to be resolved.
> > 
> > So I'm inclined to revert commit
> > 3152fb9f11cdd2fd8688c2c5cb805e5c09b53dd9
> > and plan on revisiting this for 2.6.26. I already got an ack for that
> > from
> > Jean Delvare, but I thought I'd mention it in this thread too before I
> > actually do the final revert.
> > 
> > Any really strong objections?
> No.
> As we still need some discussions before the problem is solved, please
> revert it. :)
> 
> And please apply this patch after reverting the old one.
> 
> thanks,
> rui
> 
> From: Zhang Rui <rui.zhang@intel.com>
> 
> The generic thermal driver shows temperature in millidegree Celsius.
> Update the documentation.
> 
> Signed-off-by: Zhang Rui <rui.zhang@intel.com>
> ---
>  Documentation/thermal/sysfs-api.txt |   22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> Index: linux-2.6/Documentation/thermal/sysfs-api.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/thermal/sysfs-api.txt
> +++ linux-2.6/Documentation/thermal/sysfs-api.txt
> @@ -143,10 +143,10 @@ type				Strings which represent the ther
>  				This is given by thermal zone driver as part of registration.
>  				Eg: "ACPI thermal zone" indicates it's a ACPI thermal device
>  				RO
> -				Optional
> +				Required
>  
>  temp				Current temperature as reported by thermal zone (sensor)
> -				Unit: degree Celsius
> +				Unit: millidegree Celsius
>  				RO
>  				Required
>  
> @@ -163,7 +163,7 @@ mode				One of the predefined values in 
>  					  charge of the thermal management.
>  
>  trip_point_[0-*]_temp		The temperature above which trip point will be fired
> -				Unit: degree Celsius
> +				Unit: millidegree Celsius
>  				RO
>  				Optional
>  
> @@ -193,7 +193,7 @@ type				String which represents the type
>  				eg. For memory controller device on intel_menlow platform:
>  				this should be "Memory controller"
>  				RO
> -				Optional
> +				Required
>  
>  max_state			The maximum permissible cooling state of this cooling device.
>  				RO
> @@ -219,16 +219,16 @@ the sys I/F structure will be built like
>  
>  |thermal_zone1:
>  	|-----type:			ACPI thermal zone
> -	|-----temp:			37
> +	|-----temp:			37000
>  	|-----mode:			kernel
> -	|-----trip_point_0_temp:	100
> +	|-----trip_point_0_temp:	100000
>  	|-----trip_point_0_type:	critical
> -	|-----trip_point_1_temp:	80
> +	|-----trip_point_1_temp:	80000
>  	|-----trip_point_1_type:	passive
> -	|-----trip_point_2_temp:	70
> -	|-----trip_point_2_type:	active[0]
> -	|-----trip_point_3_temp:	60
> -	|-----trip_point_3_type:	active[1]
> +	|-----trip_point_2_temp:	70000
> +	|-----trip_point_2_type:	active0
> +	|-----trip_point_3_temp:	60000
> +	|-----trip_point_3_type:	active1
>  	|-----cdev0:			--->/sys/class/thermal/cooling_device0
>  	|-----cdev0_trip_point:		1	/* cdev0 can be used for passive */
>  	|-----cdev1:			--->/sys/class/thermal/cooling_device3
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>