Re: [PATCH 2.6.26-rc] ACPI Thermal Zone driver breaks lm-sensors 2userspace
From: Zhang Rui
Date: Mon Jun 23 2008 - 23:47:20 EST
On Tue, 2008-06-24 at 11:07 +0800, Linus Torvalds wrote:
>
>
> On Mon, 23 Jun 2008, Robert Hancock wrote:
>
> > Mark M. Hoffman wrote:
> > >
> > > 1) The bug is in libsensors (2.10), not the kernel.
> >
> > This doesn't matter. Breaking userspace in such a fashion is
> severely frowned
> > upon unless essentially unavoidable, even if it is just triggering a
> bug.
>
> I do tend to agree. If it used to work, it should continue to work,
> even
> if it wasn't _meant_ to work.
>
> That's not always 100% possible, but the fact is, neither is updating
> user
> space always possible. The kernel needs to try its best to maintain
> stable
> interfaces, and no, "..but the interface was broken" is _not_ an
> acceptable excuse for unbreaking it if it causes problems.
>
> > > - but more importantly -
> > >
> > > 2) This patch is broken.
> >
> > You didn't indicate what was wrong with the patch.
>
> Yes. Leaving everybody wondering whether it's just an opinionated
> expression of the former problem, or whether there is a real and
> understandable reason why it was NACK'ed.
Multiple thermal zone devices may share the same hwmon device.
here is an example,
# grep . /sys/class/hwmon/hwmon0/*
/sys/class/hwmon/hwmon0/name:acpitz
/sys/class/hwmon/hwmon0/temp1_crit:127000
/sys/class/hwmon/hwmon0/temp1_input:40000
/sys/class/hwmon/hwmon0/temp2_crit:100000
/sys/class/hwmon/hwmon0/temp2_input:44000
temp1 and temp2 stand for different ACPI thermal zones.
This patch will break if the first ACPI thermal zone is unregistered
before the others, although it only happens theoretically.
Plus, as this is not a 1:1 binding, a symbol link from the hwmon device
to thermal zone device doesn't make sense.
Following is a possible solution which is also from Rene,
while I think CONFIG_THERMAL_HWMON should be "default n".
thanks,
rui
>From 7b41d17346cdd1f9db1223c024bd9f4604ee5c82 Mon Sep 17 00:00:00 2001
From: Rene Herman <rene.herman@xxxxxxxxx>
Date: Mon, 23 Jun 2008 21:50:25 +0200
Subject: [PATCH] acpi/thermal: allow disabling of thermal zone hwmon support
2.6.26-rc gained a hwmon interface to the Thermal Zone driver
which unfortunately breaks lm-sensors 2 userspace and renders
all other (subsequent) hwmon sensors inoperable also.
Many systems, current slackware and derivative systems among
them, are using lm-sensors 2 and would be affected. A new
lm-sensors 2.10.7 release is needed to fix this from the user
side.
This makes the hwmon support optional (enabled by default) and
comments that it needs a new enough lm-sensors userspace. This
also immediately schedules the option for removal again as this
is only intended to not break systems over the 2.6.26 upgrade
when new enough lm-sensors is not yet available or widely
deployed.
Signed-off-by: Rene Herman <rene.herman@xxxxxxxxx>
CC: Len Brown <lenb@xxxxxxxxxx>
CC: Hans de Goede <j.w.r.degoede@xxxxxx>
CC: Zhang Rui <rui.zhang@xxxxxxxxx>
CC: Mark M. Hoffman <mhoffman@xxxxxxxxxxxxx>
CC: Jean Delvare <khali@xxxxxxxxxxxx>
CC: linux-acpi@xxxxxxxxxxxxxxx
CC: lm-sensors@xxxxxxxxxxxxxx
CC: linux-kernel@xxxxxxxxxxxxxxx
---
Documentation/feature-removal-schedule.txt | 9 +++++++++
drivers/thermal/Kconfig | 10 ++++++++++
drivers/thermal/thermal_sys.c | 4 ++--
include/linux/thermal.h | 6 ++----
4 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
index 5b3f31f..46ece3f 100644
--- a/Documentation/feature-removal-schedule.txt
+++ b/Documentation/feature-removal-schedule.txt
@@ -312,3 +312,12 @@ When: 2.6.26
Why: Implementation became generic; users should now include
linux/semaphore.h instead.
Who: Matthew Wilcox <willy@xxxxxxxxxxxxxxx>
+
+---------------------------
+
+What: CONFIG_THERMAL_HWMON
+When: January 2009
+Why: This option was introduced just to allow older lm-sensors userspace
+ to keep working over the upgrade to 2.6.26. At the scheduled time of
+ removal fixed lm-sensors (2.x or 3.x) should be readily available.
+Who: Rene Herman <rene.herman@xxxxxxxxx>
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 4b62852..0b359ab 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -12,3 +12,13 @@ menuconfig THERMAL
cooling devices.
All platforms with ACPI thermal support can use this driver.
If you want this support, you should say Y or M here.
+
+config THERMAL_HWMON
+ bool "Hardware monitoring support"
+ depends on HWMON=y || HWMON=THERMAL
+ default y
+ help
+ The generic thermal sysfs driver's hardware monitoring support
+ requires a 2.10.7/3.0.2 or later lm-sensors userspace.
+
+ Say Y unless you need support for older lm-sensors.
diff --git a/drivers/thermal/thermal_sys.c b/drivers/thermal/thermal_sys.c
index 6098787..fe07462 100644
--- a/drivers/thermal/thermal_sys.c
+++ b/drivers/thermal/thermal_sys.c
@@ -295,8 +295,8 @@ thermal_cooling_device_trip_point_show(struct device *dev,
/* Device management */
-#if defined(CONFIG_HWMON) || \
- (defined(CONFIG_HWMON_MODULE) && defined(CONFIG_THERMAL_MODULE))
+#if defined(CONFIG_THERMAL_HWMON)
+
/* hwmon sys I/F */
#include <linux/hwmon.h>
static LIST_HEAD(thermal_hwmon_list);
diff --git a/include/linux/thermal.h b/include/linux/thermal.h
index 06d3e6e..917707e 100644
--- a/include/linux/thermal.h
+++ b/include/linux/thermal.h
@@ -66,8 +66,7 @@ struct thermal_cooling_device {
((long)t-2732+5)/10 : ((long)t-2732-5)/10)
#define CELSIUS_TO_KELVIN(t) ((t)*10+2732)
-#if defined(CONFIG_HWMON) || \
- (defined(CONFIG_HWMON_MODULE) && defined(CONFIG_THERMAL_MODULE))
+#if defined(CONFIG_THERMAL_HWMON)
/* thermal zone devices with the same type share one hwmon device */
struct thermal_hwmon_device {
char type[THERMAL_NAME_LENGTH];
@@ -94,8 +93,7 @@ struct thermal_zone_device {
struct idr idr;
struct mutex lock; /* protect cooling devices list */
struct list_head node;
-#if defined(CONFIG_HWMON) || \
- (defined(CONFIG_HWMON_MODULE) && defined(CONFIG_THERMAL_MODULE))
+#if defined(CONFIG_THERMAL_HWMON)
struct list_head hwmon_node;
struct thermal_hwmon_device *hwmon;
struct thermal_hwmon_attr temp_input; /* hwmon sys attr */
--
1.5.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/