public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] thinkpad-acpi sysfs support part 1
@ 2007-04-24 14:48 Henrique de Moraes Holschuh
  2007-04-24 14:48 ` [PATCH 1/9] ACPI: thinkpad-acpi: register with the device model Henrique de Moraes Holschuh
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-04-24 14:48 UTC (permalink / raw)
  To: lenb; +Cc: ibm-acpi-devel, linux-acpi

Len,

Please pull from:
git://repo.or.cz/linux-2.6/linux-acpi-2.6/ibm-acpi-2.6.git for-upstream/acpi-test

to receive the following patches *already* in acpi-test:
      ACPI: ibm-acpi: kill trailing whitespace
      ACPI: ibm-acpi: rename some identifiers
      ACPI: ibm-acpi: add header file
      ACPI: ibm-acpi: organize code
      ACPI: ibm-acpi: update copyright notice
      ACPI: ibm-acpi: update documentation
      ACPI: ibm-acpi: move driver to drivers/misc hierarchy
      ACPI: ibm-acpi: rename driver to thinkpad-acpi
      ACPI: thinkpad-acpi: cleanup Kconfig for thinkpad-acpi
      ACPI: thinkpad-acpi: add compatibility MODULE_ALIAS entry
      ACPI: thinkpad-acpi: cleanup after rename
      ACPI: thinkpad-acpi: update MAINTAINERS
      ACPI: thinkpad-acpi: rename register_ibmacpi_subdriver
      ACPI: thinkpad-acpi: rename one stray use of ibm-acpi in a comment
      ACPI: thinkpad-acpi: rename module glue
      ACPI: thinkpad-acpi: rename thinkpad constants
      ACPI: thinkpad-acpi: update fan firmware documentation
      ACPI: thinkpad-acpi: add debug mode
      ACPI: thinkpad-acpi: clean up probing and move init to subdrivers
      ACPI: thinkpad-acpi: add subdriver debug statements
      ACPI: thinkpad-acpi: uncouple subdriver init from ibms struct
      ACPI: thinkpad-acpi: improve thinkpad detection
      ACPI: thinkpad-acpi: use bitfields to hold subdriver flags
      ACPI: thinkpad-acpi: use bitfields for module flags
      ACPI: thinkpad-acpi: prepare for device model conversion
      ACPI: thinkpad-acpi: mark acpi helper functions __must_check
      ACPI: thinkpad-acpi: clean up hotkey subdriver
      ACPI: thinkpad-acpi: cleanup bluetooth and wan for sysfs conversion
      ACPI: thinkpad-acpi: cleanup video subdriver
      ACPI: thinkpad-acpi: clean up CMOS commands subdriver
      ACPI: thinkpad-acpi: cleanup thermal subdriver for sysfs conversion
      ACPI: thinkpad-acpi: improve fan watchdog messages

      (the above should not cause any probles if you already pulled
      from that branch, as I have not rebased the branch or modified
      any of the above patches)

and to receive the following *new* patches:

      ACPI: thinkpad-acpi: register with the device model
      ACPI: thinkpad-acpi: driver sysfs conversion
      ACPI: thinkpad-acpi: add infrastructure for the sysfs device attributes
      ACPI: thinkpad-acpi: protect fan and hotkey data structures
      ACPI: thinkpad-acpi: add sysfs support to the thermal subdriver
      ACPI: thinkpad-acpi: add sysfs support to fan subdriver
      ACPI: thinkpad-acpi: add a safety net for TPEC fan control mode
      ACPI: thinkpad-acpi: add sysfs support to the cmos command subdriver
      ACPI: thinkpad-acpi: update brightness sysfs interface support

This is the first part of the sysfs conversion work.  It uses standard
generic interfaces (hwmon and backlight classes) for the most part, so it
is not subject to much change later on.

I am still working on the rest of the interfaces.  Also, I am sure at lest
the thermal sysfs conversion can be further enhanced coding-wise to be less
wasteful of memory, but that can wait for a later patch.

Please merge for 2.6.22.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/9] ACPI: thinkpad-acpi: register with the device model
  2007-04-24 14:48 [GIT PULL] thinkpad-acpi sysfs support part 1 Henrique de Moraes Holschuh
@ 2007-04-24 14:48 ` Henrique de Moraes Holschuh
  2007-04-24 14:48 ` [PATCH 3/9] ACPI: thinkpad-acpi: add infrastructure for the sysfs device attributes Henrique de Moraes Holschuh
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-04-24 14:48 UTC (permalink / raw)
  To: lenb; +Cc: ibm-acpi-devel, linux-acpi, Henrique de Moraes Holschuh

Register thinkpad-acpi platform driver and platform device for the device
model.  Also register the platform device with the hwmon class.

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
---
 Documentation/thinkpad-acpi.txt |   40 ++++++++++++++++++++++++----
 drivers/misc/Kconfig            |    1 +
 drivers/misc/thinkpad_acpi.c    |   54 +++++++++++++++++++++++++++++++++++++++
 drivers/misc/thinkpad_acpi.h    |    8 ++++++
 4 files changed, 97 insertions(+), 6 deletions(-)

diff --git a/Documentation/thinkpad-acpi.txt b/Documentation/thinkpad-acpi.txt
index 1a42b77..0e4e053 100644
--- a/Documentation/thinkpad-acpi.txt
+++ b/Documentation/thinkpad-acpi.txt
@@ -1,7 +1,7 @@
 		     ThinkPad ACPI Extras Driver
 
                             Version 0.14
-                          March 26th, 2007
+                          April 21st, 2007
 
                Borislav Deianov <borislav@users.sf.net>
 	     Henrique de Moraes Holschuh <hmh@hmh.eng.br>
@@ -67,11 +67,39 @@ thinkpad-specific bay functionality.
 Features
 --------
 
-The driver creates the /proc/acpi/ibm directory. There is a file under
-that directory for each feature described below. Note that while the
-driver is still in the alpha stage, the exact proc file format and
-commands supported by the various features is guaranteed to change
-frequently.
+The driver exports two different interfaces to userspace, which can be
+used to access the features it provides.  One is a legacy procfs-based
+interface, which will be removed at some time in the distant future.
+The other is a new sysfs-based interface which is not complete yet.
+
+The procfs interface creates the /proc/acpi/ibm directory.  There is a
+file under that directory for each feature it supports.  The procfs
+interface is mostly frozen, and will change very little if at all: it
+will not be extended to add any new functionality in the driver, instead
+all new functionality will be implemented on the sysfs interface.
+
+The sysfs interface tries to blend in the generic Linux sysfs subsystems
+and classes as much as possible.  Since some of these subsystems are not
+yet ready or stabilized, it is expected that this interface will change,
+and any and all userspace programs must deal with it.
+
+
+Notes about the sysfs interface:
+
+Unlike what was done with the procfs interface, correctness when talking
+to the sysfs interfaces will be enforced, as will correctness in the
+thinkpad-acpi's implementation of sysfs interfaces.
+
+Also, any bugs in the thinkpad-acpi sysfs driver code or in the
+thinkpad-acpi's implementation of the sysfs interfaces will be fixed for
+maximum correctness, even if that means changing an interface in
+non-compatible ways.  As these interfaces mature both in the kernel and
+in thinkpad-acpi, such changes should become quite rare.
+
+Applications interfacing to the thinkpad-acpi sysfs interfaces must
+follow all sysfs guidelines and correctly process all errors (the sysfs
+interface makes extensive use of errors).  File descriptors and open /
+close operations to the sysfs inodes must also be properly implemented.
 
 Driver version -- /proc/acpi/ibm/driver
 ---------------------------------------
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 44e4c8f..445c4b1 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -126,6 +126,7 @@ config THINKPAD_ACPI
 	tristate "ThinkPad ACPI Laptop Extras"
 	depends on X86 && ACPI
 	select BACKLIGHT_CLASS_DEVICE
+	select HWMON
 	---help---
 	  This is a driver for the IBM and Lenovo ThinkPad laptops. It adds
 	  support for Fn-Fx key combinations, Bluetooth control, video
diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index 9b4eea4..e47eaf7 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
@@ -477,6 +477,25 @@ static char *next_cmd(char **cmds)
 /****************************************************************************
  ****************************************************************************
  *
+ * Device model: hwmon and platform
+ *
+ ****************************************************************************
+ ****************************************************************************/
+
+static struct platform_device *tpacpi_pdev = NULL;
+static struct class_device *tpacpi_hwmon = NULL;
+
+static struct platform_driver tpacpi_pdriver = {
+	.driver = {
+		.name = IBM_DRVR_NAME,
+		.owner = THIS_MODULE,
+	},
+};
+
+
+/****************************************************************************
+ ****************************************************************************
+ *
  * Subdrivers
  *
  ****************************************************************************
@@ -3225,10 +3244,12 @@ static int __init thinkpad_acpi_module_init(void)
 {
 	int ret, i;
 
+	/* Driver-level probe */
 	ret = probe_for_thinkpad();
 	if (ret)
 		return ret;
 
+	/* Driver initialization */
 	ibm_thinkpad_ec_found = check_dmi_for_ec();
 	IBM_ACPIHANDLE_INIT(ecrd);
 	IBM_ACPIHANDLE_INIT(ecwr);
@@ -3241,6 +3262,31 @@ static int __init thinkpad_acpi_module_init(void)
 	}
 	proc_dir->owner = THIS_MODULE;
 
+	ret = platform_driver_register(&tpacpi_pdriver);
+	if (ret) {
+		printk(IBM_ERR "unable to register platform driver\n");
+		thinkpad_acpi_module_exit();
+		return ret;
+	}
+
+	/* Device initialization */
+	tpacpi_pdev = platform_device_register_simple(IBM_DRVR_NAME, -1,
+							NULL, 0);
+	if (IS_ERR(tpacpi_pdev)) {
+		ret = PTR_ERR(tpacpi_pdev);
+		tpacpi_pdev = NULL;
+		printk(IBM_ERR "unable to register platform device\n");
+		thinkpad_acpi_module_exit();
+		return ret;
+	}
+	tpacpi_hwmon = hwmon_device_register(&tpacpi_pdev->dev);
+	if (IS_ERR(tpacpi_hwmon)) {
+		ret = PTR_ERR(tpacpi_hwmon);
+		tpacpi_hwmon = NULL;
+		printk(IBM_ERR "unable to register hwmon device\n");
+		thinkpad_acpi_module_exit();
+		return ret;
+	}
 	for (i = 0; i < ARRAY_SIZE(ibms_init); i++) {
 		ret = ibm_init(&ibms_init[i]);
 		if (ret >= 0 && *ibms_init[i].param)
@@ -3266,6 +3312,14 @@ static void thinkpad_acpi_module_exit(void)
 
 	dbg_printk(TPACPI_DBG_INIT, "finished subdriver exit path...\n");
 
+	if (tpacpi_hwmon)
+		hwmon_device_unregister(tpacpi_hwmon);
+
+	if (tpacpi_pdev)
+		platform_device_unregister(tpacpi_pdev);
+
+	platform_driver_unregister(&tpacpi_pdriver);
+
 	if (proc_dir)
 		remove_proc_entry(IBM_PROC_DIR, acpi_root_dir);
 
diff --git a/drivers/misc/thinkpad_acpi.h b/drivers/misc/thinkpad_acpi.h
index 6432b28..fea5809 100644
--- a/drivers/misc/thinkpad_acpi.h
+++ b/drivers/misc/thinkpad_acpi.h
@@ -34,6 +34,8 @@
 #include <linux/proc_fs.h>
 #include <linux/backlight.h>
 #include <linux/fb.h>
+#include <linux/platform_device.h>
+#include <linux/hwmon.h>
 #include <asm/uaccess.h>
 
 #include <linux/dmi.h>
@@ -56,6 +58,7 @@
 
 #define IBM_PROC_DIR "ibm"
 #define IBM_ACPI_EVENT_PREFIX "ibm"
+#define IBM_DRVR_NAME IBM_FILE
 
 #define IBM_LOG IBM_FILE ": "
 #define IBM_ERR	   KERN_ERR    IBM_LOG
@@ -130,6 +133,11 @@ static int dispatch_procfs_write(struct file *file,
 		unsigned long count, void *data);
 static char *next_cmd(char **cmds);
 
+/* Device model */
+static struct platform_device *tpacpi_pdev;
+static struct class_device *tpacpi_hwmon;
+static struct platform_driver tpacpi_pdriver;
+
 /* Module */
 static int experimental;
 static u32 dbg_level;
-- 
1.5.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/9] ACPI: thinkpad-acpi: driver sysfs conversion
       [not found] ` <11774261003184-git-send-email-hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
@ 2007-04-24 14:48   ` Henrique de Moraes Holschuh
  2007-04-24 14:48   ` [PATCH 4/9] ACPI: thinkpad-acpi: protect fan and hotkey data structures Henrique de Moraes Holschuh
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-04-24 14:48 UTC (permalink / raw)
  To: lenb-DgEjT+Ai2ygdnm+yROfE0A
  Cc: ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Henrique de Moraes Holschuh, linux-acpi-u79uwXL29TY76Z2rM5mHXA

Add the sysfs attributes for the platform driver.

Signed-off-by: Henrique de Moraes Holschuh <hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
---
 Documentation/thinkpad-acpi.txt |   42 +++++++++++++++++-
 drivers/misc/thinkpad_acpi.c    |   90 +++++++++++++++++++++++++++++++++++++++
 drivers/misc/thinkpad_acpi.h    |    3 +
 3 files changed, 133 insertions(+), 2 deletions(-)

diff --git a/Documentation/thinkpad-acpi.txt b/Documentation/thinkpad-acpi.txt
index 0e4e053..cc079af 100644
--- a/Documentation/thinkpad-acpi.txt
+++ b/Documentation/thinkpad-acpi.txt
@@ -101,11 +101,39 @@ follow all sysfs guidelines and correctly process all errors (the sysfs
 interface makes extensive use of errors).  File descriptors and open /
 close operations to the sysfs inodes must also be properly implemented.
 
-Driver version -- /proc/acpi/ibm/driver
----------------------------------------
+The version of thinkpad-acpi's sysfs interface is exported by the driver
+as a driver attribute (see below).
+
+Sysfs driver attributes are on the driver's sysfs attribute space,
+for 2.6.20 this is /sys/bus/platform/drivers/thinkpad-acpi/.
+
+Sysfs device attributes are on the driver's sysfs attribute space,
+for 2.6.20 this is /sys/devices/platform/thinkpad-acpi/.
+
+Driver version
+--------------
+
+procfs: /proc/acpi/ibm/driver
+sysfs driver attribute: version
 
 The driver name and version. No commands can be written to this file.
 
+Sysfs interface version
+-----------------------
+
+sysfs driver attribute: interface_version
+
+Version of the thinkpad-acpi sysfs interface, as an unsigned long
+(output in hex format: 0xAAAABBCC), where:
+	AAAA - major revision
+	BB - minor revision
+	CC - bugfix revision
+
+The sysfs interface version changelog for the driver can be found at the
+end of this document.  Changes to the sysfs interface done by the kernel
+subsystems are not documented here, nor are they tracked by this
+attribute.
+
 Hot keys -- /proc/acpi/ibm/hotkey
 ---------------------------------
 
@@ -745,9 +773,19 @@ to enable more than one output class, just add their values.
 There is also a kernel build option to enable more debugging
 information, which may be necessary to debug driver problems.
 
+The level of debugging information output by the driver can be changed
+at runtime through sysfs, using the driver attribute debug_level.  The
+attribute takes the same bitmask as the debug module parameter above.
+
 Force loading of module
 -----------------------
 
 If thinkpad-acpi refuses to detect your ThinkPad, you can try to specify
 the module parameter force_load=1.  Regardless of whether this works or
 not, please contact ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org with a report.
+
+
+Sysfs interface changelog:
+
+0x000100:	Initial sysfs support, as a single platform driver and
+		device.
diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index e47eaf7..a31d00d 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
@@ -22,6 +22,7 @@
  */
 
 #define IBM_VERSION "0.14"
+#define TPACPI_SYSFS_VERSION 0x000100
 
 /*
  *  Changelog:
@@ -493,6 +494,87 @@ static struct platform_driver tpacpi_pdriver = {
 };
 
 
+/*************************************************************************
+ * thinkpad-acpi driver attributes
+ */
+
+/* interface_version --------------------------------------------------- */
+static ssize_t tpacpi_driver_interface_version_show(
+				struct device_driver *drv,
+				char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "0x%08x\n", TPACPI_SYSFS_VERSION);
+}
+
+static DRIVER_ATTR(interface_version, S_IRUGO,
+		tpacpi_driver_interface_version_show, NULL);
+
+/* debug_level --------------------------------------------------------- */
+static ssize_t tpacpi_driver_debug_show(struct device_driver *drv,
+						char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "0x%04x\n", dbg_level);
+}
+
+static ssize_t tpacpi_driver_debug_store(struct device_driver *drv,
+						const char *buf, size_t count)
+{
+	unsigned long t;
+	char *endp;
+
+	t = simple_strtoul(buf, &endp, 0);
+	while (*endp && isspace(*endp))
+		endp++;
+	if (*endp)
+		return -EINVAL;
+
+	dbg_level = t;
+
+	return count;
+}
+
+static DRIVER_ATTR(debug_level, S_IWUSR | S_IRUGO,
+		tpacpi_driver_debug_show, tpacpi_driver_debug_store);
+
+/* version ------------------------------------------------------------- */
+static ssize_t tpacpi_driver_version_show(struct device_driver *drv,
+						char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%s v%s\n", IBM_DESC, IBM_VERSION);
+}
+
+static DRIVER_ATTR(version, S_IRUGO,
+		tpacpi_driver_version_show, NULL);
+
+/* --------------------------------------------------------------------- */
+
+static struct driver_attribute* tpacpi_driver_attributes[] = {
+	&driver_attr_debug_level, &driver_attr_version,
+	&driver_attr_interface_version,
+};
+
+static int __init tpacpi_create_driver_attributes(struct device_driver *drv)
+{
+	int i, res;
+
+	i = 0;
+	res = 0;
+	while (!res && i < ARRAY_SIZE(tpacpi_driver_attributes)) {
+		res = driver_create_file(drv, tpacpi_driver_attributes[i]);
+		i++;
+	}
+
+	return res;
+}
+
+static void tpacpi_remove_driver_attributes(struct device_driver *drv)
+{
+	int i;
+
+	for(i = 0; i < ARRAY_SIZE(tpacpi_driver_attributes); i++)
+		driver_remove_file(drv, tpacpi_driver_attributes[i]);
+}
+
 /****************************************************************************
  ****************************************************************************
  *
@@ -3268,6 +3350,13 @@ static int __init thinkpad_acpi_module_init(void)
 		thinkpad_acpi_module_exit();
 		return ret;
 	}
+	ret = tpacpi_create_driver_attributes(&tpacpi_pdriver.driver);
+	if (ret) {
+		printk(IBM_ERR "unable to create sysfs driver attributes\n");
+		thinkpad_acpi_module_exit();
+		return ret;
+	}
+
 
 	/* Device initialization */
 	tpacpi_pdev = platform_device_register_simple(IBM_DRVR_NAME, -1,
@@ -3318,6 +3407,7 @@ static void thinkpad_acpi_module_exit(void)
 	if (tpacpi_pdev)
 		platform_device_unregister(tpacpi_pdev);
 
+	tpacpi_remove_driver_attributes(&tpacpi_pdriver.driver);
 	platform_driver_unregister(&tpacpi_pdriver);
 
 	if (proc_dir)
diff --git a/drivers/misc/thinkpad_acpi.h b/drivers/misc/thinkpad_acpi.h
index fea5809..3786058 100644
--- a/drivers/misc/thinkpad_acpi.h
+++ b/drivers/misc/thinkpad_acpi.h
@@ -32,6 +32,7 @@
 #include <linux/list.h>
 
 #include <linux/proc_fs.h>
+#include <linux/sysfs.h>
 #include <linux/backlight.h>
 #include <linux/fb.h>
 #include <linux/platform_device.h>
@@ -137,6 +138,8 @@ static char *next_cmd(char **cmds);
 static struct platform_device *tpacpi_pdev;
 static struct class_device *tpacpi_hwmon;
 static struct platform_driver tpacpi_pdriver;
+static int tpacpi_create_driver_attributes(struct device_driver *drv);
+static void tpacpi_remove_driver_attributes(struct device_driver *drv);
 
 /* Module */
 static int experimental;
-- 
1.5.1


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/9] ACPI: thinkpad-acpi: add infrastructure for the sysfs device attributes
  2007-04-24 14:48 [GIT PULL] thinkpad-acpi sysfs support part 1 Henrique de Moraes Holschuh
  2007-04-24 14:48 ` [PATCH 1/9] ACPI: thinkpad-acpi: register with the device model Henrique de Moraes Holschuh
@ 2007-04-24 14:48 ` Henrique de Moraes Holschuh
  2007-04-24 14:48 ` [PATCH 6/9] ACPI: thinkpad-acpi: add sysfs support to fan subdriver Henrique de Moraes Holschuh
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-04-24 14:48 UTC (permalink / raw)
  To: lenb; +Cc: ibm-acpi-devel, linux-acpi, Henrique de Moraes Holschuh

Add infrastructure to deal with sysfs attributes and grouping, and helpers
for common sysfs parsing.  Switch driver attributes to use them.

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
---
 drivers/misc/thinkpad_acpi.c |   86 +++++++++++++++++++++++++++++++++++++++--
 drivers/misc/thinkpad_acpi.h |   21 ++++++++++
 2 files changed, 102 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index a31d00d..ca6d15c 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
@@ -520,12 +520,8 @@ static ssize_t tpacpi_driver_debug_store(struct device_driver *drv,
 						const char *buf, size_t count)
 {
 	unsigned long t;
-	char *endp;
 
-	t = simple_strtoul(buf, &endp, 0);
-	while (*endp && isspace(*endp))
-		endp++;
-	if (*endp)
+	if (parse_strtoul(buf, 0xffff, &t))
 		return -EINVAL;
 
 	dbg_level = t;
@@ -575,6 +571,86 @@ static void tpacpi_remove_driver_attributes(struct device_driver *drv)
 		driver_remove_file(drv, tpacpi_driver_attributes[i]);
 }
 
+/*************************************************************************
+ * sysfs support helpers
+ */
+
+struct attribute_set_obj {
+	struct attribute_set s;
+	struct attribute *a;
+} __attribute__((packed));
+
+static struct attribute_set *create_attr_set(unsigned int max_members,
+						const char* name)
+{
+	struct attribute_set_obj *sobj;
+
+	if (max_members == 0)
+		return NULL;
+
+	/* Allocates space for implicit NULL at the end too */
+	sobj = kzalloc(sizeof(struct attribute_set_obj) +
+		    max_members * sizeof(struct attribute *),
+		    GFP_KERNEL);
+	if (!sobj)
+		return NULL;
+	sobj->s.max_members = max_members;
+	sobj->s.group.attrs = &sobj->a;
+	sobj->s.group.name = name;
+
+	return &sobj->s;
+}
+
+/* not multi-threaded safe, use it in a single thread per set */
+static int add_to_attr_set(struct attribute_set* s, struct attribute *attr)
+{
+	if (!s || !attr)
+		return -EINVAL;
+
+	if (s->members >= s->max_members)
+		return -ENOMEM;
+
+	s->group.attrs[s->members] = attr;
+	s->members++;
+
+	return 0;
+}
+
+static int add_many_to_attr_set(struct attribute_set* s,
+			struct attribute **attr,
+			unsigned int count)
+{
+	int i, res;
+
+	for (i = 0; i < count; i++) {
+		res = add_to_attr_set(s, attr[i]);
+		if (res)
+			return res;
+	}
+
+	return 0;
+}
+
+static void delete_attr_set(struct attribute_set* s, struct kobject *kobj)
+{
+	sysfs_remove_group(kobj, &s->group);
+	destroy_attr_set(s);
+}
+
+static int parse_strtoul(const char *buf,
+		unsigned long max, unsigned long *value)
+{
+	char *endp;
+
+	*value = simple_strtoul(buf, &endp, 0);
+	while (*endp && isspace(*endp))
+		endp++;
+	if (*endp || *value > max)
+		return -EINVAL;
+
+	return 0;
+}
+
 /****************************************************************************
  ****************************************************************************
  *
diff --git a/drivers/misc/thinkpad_acpi.h b/drivers/misc/thinkpad_acpi.h
index 3786058..84fdefe 100644
--- a/drivers/misc/thinkpad_acpi.h
+++ b/drivers/misc/thinkpad_acpi.h
@@ -134,6 +134,27 @@ static int dispatch_procfs_write(struct file *file,
 		unsigned long count, void *data);
 static char *next_cmd(char **cmds);
 
+/* sysfs support */
+struct attribute_set {
+	unsigned int members, max_members;
+	struct attribute_group group;
+};
+
+static struct attribute_set *create_attr_set(unsigned int max_members,
+						const char* name);
+#define destroy_attr_set(_set) \
+	kfree(_set);
+static int add_to_attr_set(struct attribute_set* s, struct attribute *attr);
+static int add_many_to_attr_set(struct attribute_set* s,
+			struct attribute **attr,
+			unsigned int count);
+#define register_attr_set_with_sysfs(_attr_set, _kobj) \
+	sysfs_create_group(_kobj, &_attr_set->group)
+static void delete_attr_set(struct attribute_set* s, struct kobject *kobj);
+
+static int parse_strtoul(const char *buf, unsigned long max,
+			unsigned long *value);
+
 /* Device model */
 static struct platform_device *tpacpi_pdev;
 static struct class_device *tpacpi_hwmon;
-- 
1.5.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/9] ACPI: thinkpad-acpi: protect fan and hotkey data structures
       [not found] ` <11774261003184-git-send-email-hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
  2007-04-24 14:48   ` [PATCH 2/9] ACPI: thinkpad-acpi: driver sysfs conversion Henrique de Moraes Holschuh
@ 2007-04-24 14:48   ` Henrique de Moraes Holschuh
  2007-04-24 14:48   ` [PATCH 5/9] ACPI: thinkpad-acpi: add sysfs support to the thermal subdriver Henrique de Moraes Holschuh
  2007-04-24 14:48   ` [PATCH 8/9] ACPI: thinkpad-acpi: add sysfs support to the cmos command subdriver Henrique de Moraes Holschuh
  3 siblings, 0 replies; 14+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-04-24 14:48 UTC (permalink / raw)
  To: lenb-DgEjT+Ai2ygdnm+yROfE0A
  Cc: ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Henrique de Moraes Holschuh, linux-acpi-u79uwXL29TY76Z2rM5mHXA

Add proper mutex locking to some data structures access subject to races
due to concurrent access of driver functions on the hotkey and fan
subdrivers.

Signed-off-by: Henrique de Moraes Holschuh <hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
---
 drivers/misc/thinkpad_acpi.c |  114 ++++++++++++++++++++++++++++++++----------
 drivers/misc/thinkpad_acpi.h |    5 ++
 2 files changed, 92 insertions(+), 27 deletions(-)

diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index ca6d15c..aa69ff0 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
@@ -704,6 +704,7 @@ static int __init hotkey_init(struct ibm_init_struct *iibm)
 	vdbg_printk(TPACPI_DBG_INIT, "initializing hotkey subdriver\n");
 
 	IBM_ACPIHANDLE_INIT(hkey);
+	mutex_init(&hotkey_mutex);
 
 	/* hotkey not supported on 570 */
 	tp_features.hotkey = hkey_handle != NULL;
@@ -752,6 +753,9 @@ static void hotkey_notify(struct ibm_struct *ibm, u32 event)
 	}
 }
 
+/*
+ * Call with hotkey_mutex held
+ */
 static int hotkey_get(int *status, int *mask)
 {
 	if (!acpi_evalf(hkey_handle, status, "DHKC", "d"))
@@ -764,6 +768,9 @@ static int hotkey_get(int *status, int *mask)
 	return 0;
 }
 
+/*
+ * Call with hotkey_mutex held
+ */
 static int hotkey_set(int status, int mask)
 {
 	int i;
@@ -792,7 +799,11 @@ static int hotkey_read(char *p)
 		return len;
 	}
 
+	res = mutex_lock_interruptible(&hotkey_mutex);
+	if (res < 0)
+		return res;
 	res = hotkey_get(&status, &mask);
+	mutex_unlock(&hotkey_mutex);
 	if (res)
 		return res;
 
@@ -818,10 +829,15 @@ static int hotkey_write(char *buf)
 	if (!tp_features.hotkey)
 		return -ENODEV;
 
+	res = mutex_lock_interruptible(&hotkey_mutex);
+	if (res < 0)
+		return res;
+
 	res = hotkey_get(&status, &mask);
 	if (res)
-		return res;
+		goto errexit;
 
+	res = 0;
 	while ((cmd = next_cmd(&buf))) {
 		if (strlencmp(cmd, "enable") == 0) {
 			status = 1;
@@ -834,18 +850,19 @@ static int hotkey_write(char *buf)
 			/* mask set */
 		} else if (sscanf(cmd, "%x", &mask) == 1) {
 			/* mask set */
-		} else
-			return -EINVAL;
+		} else {
+			res = -EINVAL;
+			goto errexit;
+		}
 		do_cmd = 1;
 	}
 
-	if (do_cmd) {
+	if (do_cmd)
 		res = hotkey_set(status, mask);
-		if (res)
-			return res;
-	}
 
-	return 0;
+errexit:
+	mutex_unlock(&hotkey_mutex);
+	return res;
 }
 
 static struct tp_acpi_drv_struct ibm_hotkey_acpidriver = {
@@ -2575,6 +2592,7 @@ static int __init fan_init(struct ibm_init_struct *iibm)
 {
 	vdbg_printk(TPACPI_DBG_INIT, "initializing fan subdriver\n");
 
+	mutex_init(&fan_mutex);
 	fan_status_access_mode = TPACPI_FAN_NONE;
 	fan_control_access_mode = TPACPI_FAN_WR_NONE;
 	fan_control_commands = 0;
@@ -2764,10 +2782,17 @@ static void fan_watchdog_reset(void)
 
 static int fan_set_level(int level)
 {
+	int res;
+
 	switch (fan_control_access_mode) {
 	case TPACPI_FAN_WR_ACPI_SFAN:
 		if (level >= 0 && level <= 7) {
-			if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", level))
+			res = mutex_lock_interruptible(&fan_mutex);
+			if (res < 0)
+				return res;
+			res = acpi_evalf(sfan_handle, NULL, NULL, "vd", level);
+			mutex_unlock(&fan_mutex);
+			if (!res)
 				return -EIO;
 		} else
 			return -EINVAL;
@@ -2780,7 +2805,12 @@ static int fan_set_level(int level)
 		    ((level < 0) || (level > 7)))
 			return -EINVAL;
 
-		if (!acpi_ec_write(fan_status_offset, level))
+		res = mutex_lock_interruptible(&fan_mutex);
+		if (res < 0)
+			return res;
+		res = acpi_ec_write(fan_status_offset, level);
+		mutex_unlock(&fan_mutex);
+		if (!res)
 			return -EIO;
 		else
 			tp_features.fan_ctrl_status_undef = 0;
@@ -2797,25 +2827,33 @@ static int fan_set_enable(void)
 	u8 s;
 	int rc;
 
+	rc = mutex_lock_interruptible(&fan_mutex);
+	if (rc < 0)
+		return rc;
+
 	switch (fan_control_access_mode) {
 	case TPACPI_FAN_WR_ACPI_FANS:
 	case TPACPI_FAN_WR_TPEC:
-		if ((rc = fan_get_status(&s)) < 0)
-			return rc;
+		rc = fan_get_status(&s);
+		if (rc < 0)
+			break;
 
 		/* Don't go out of emergency fan mode */
 		if (s != 7)
 			s = TP_EC_FAN_AUTO;
 
 		if (!acpi_ec_write(fan_status_offset, s))
-			return -EIO;
-		else
+			rc = -EIO;
+		else {
 			tp_features.fan_ctrl_status_undef = 0;
+			rc = 0;
+		}
 		break;
 
 	case TPACPI_FAN_WR_ACPI_SFAN:
-		if ((rc = fan_get_status(&s)) < 0)
-			return rc;
+		rc = fan_get_status(&s);
+		if (rc < 0)
+			break;
 
 		s &= 0x07;
 
@@ -2824,53 +2862,75 @@ static int fan_set_enable(void)
 			s = 4;
 
 		if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", s))
-			return -EIO;
+			rc= -EIO;
+		else
+			rc = 0;
 		break;
 
 	default:
-		return -ENXIO;
+		rc = -ENXIO;
 	}
-	return 0;
+
+	mutex_unlock(&fan_mutex);
+	return rc;
 }
 
 static int fan_set_disable(void)
 {
+	int rc;
+
+	rc = mutex_lock_interruptible(&fan_mutex);
+	if (rc < 0)
+		return rc;
+
+	rc = 0;
 	switch (fan_control_access_mode) {
 	case TPACPI_FAN_WR_ACPI_FANS:
 	case TPACPI_FAN_WR_TPEC:
 		if (!acpi_ec_write(fan_status_offset, 0x00))
-			return -EIO;
+			rc = -EIO;
 		else
 			tp_features.fan_ctrl_status_undef = 0;
 		break;
 
 	case TPACPI_FAN_WR_ACPI_SFAN:
 		if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", 0x00))
-			return -EIO;
+			rc = -EIO;
 		break;
 
 	default:
-		return -ENXIO;
+		rc = -ENXIO;
 	}
-	return 0;
+
+	mutex_unlock(&fan_mutex);
+	return rc;
 }
 
 static int fan_set_speed(int speed)
 {
+	int rc;
+
+	rc = mutex_lock_interruptible(&fan_mutex);
+	if (rc < 0)
+		return rc;
+
+	rc = 0;
 	switch (fan_control_access_mode) {
 	case TPACPI_FAN_WR_ACPI_FANS:
 		if (speed >= 0 && speed <= 65535) {
 			if (!acpi_evalf(fans_handle, NULL, NULL, "vddd",
 					speed, speed, speed))
-				return -EIO;
+				rc = -EIO;
 		} else
-			return -EINVAL;
+			rc = -EINVAL;
 		break;
 
 	default:
-		return -ENXIO;
+		rc = -ENXIO;
 	}
-	return 0;
+
+	mutex_unlock(&fan_mutex);
+	return rc;
 }
 
 static int fan_read(char *p)
diff --git a/drivers/misc/thinkpad_acpi.h b/drivers/misc/thinkpad_acpi.h
index 84fdefe..a9feb53 100644
--- a/drivers/misc/thinkpad_acpi.h
+++ b/drivers/misc/thinkpad_acpi.h
@@ -30,6 +30,7 @@
 #include <linux/types.h>
 #include <linux/string.h>
 #include <linux/list.h>
+#include <linux/mutex.h>
 
 #include <linux/proc_fs.h>
 #include <linux/sysfs.h>
@@ -375,6 +376,8 @@ static enum fan_control_commands fan_control_commands;
 static u8 fan_control_initial_status;
 static int fan_watchdog_maxinterval;
 
+struct mutex fan_mutex;
+
 static acpi_handle fans_handle, gfan_handle, sfan_handle;
 
 static int fan_init(struct ibm_init_struct *iibm);
@@ -403,6 +406,8 @@ static int fan_write_cmd_watchdog(const char *cmd, int *rc);
 static int hotkey_orig_status;
 static int hotkey_orig_mask;
 
+static struct mutex hotkey_mutex;
+
 static int hotkey_init(struct ibm_init_struct *iibm);
 static void hotkey_exit(void);
 static int hotkey_get(int *status, int *mask);
-- 
1.5.1


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/9] ACPI: thinkpad-acpi: add sysfs support to the thermal subdriver
       [not found] ` <11774261003184-git-send-email-hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
  2007-04-24 14:48   ` [PATCH 2/9] ACPI: thinkpad-acpi: driver sysfs conversion Henrique de Moraes Holschuh
  2007-04-24 14:48   ` [PATCH 4/9] ACPI: thinkpad-acpi: protect fan and hotkey data structures Henrique de Moraes Holschuh
@ 2007-04-24 14:48   ` Henrique de Moraes Holschuh
  2007-04-24 14:48   ` [PATCH 8/9] ACPI: thinkpad-acpi: add sysfs support to the cmos command subdriver Henrique de Moraes Holschuh
  3 siblings, 0 replies; 14+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-04-24 14:48 UTC (permalink / raw)
  To: lenb-DgEjT+Ai2ygdnm+yROfE0A
  Cc: ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Henrique de Moraes Holschuh, linux-acpi-u79uwXL29TY76Z2rM5mHXA

Export thinkpad thermal sensors to sysfs, following the hwmon
specification for thermal monitoring sensors.

ThinkPad thermal monitoring is done by the EC.  Sensors can show up or
disappear at runtime when they are inside hotswappable hardware, such as
batteries.  Sensors that are not available return -ENXIO when accessed.

Up to 16 thermal sensors are supported on new firmware (but nobody has
reported a ThinkPad with more than 12 sensors so far), and 8 sensors are
supported on older firmware.  Thermal sensor mapping is model-specific.
Precision varies, it is 1 degree Celcius on new ThinkPads, but higher on
some older models.

Signed-off-by: Henrique de Moraes Holschuh <hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
---
 Documentation/thinkpad-acpi.txt |   26 ++++++--
 drivers/misc/thinkpad_acpi.c    |  122 ++++++++++++++++++++++++++++++++++++++-
 drivers/misc/thinkpad_acpi.h    |    2 +
 3 files changed, 143 insertions(+), 7 deletions(-)

diff --git a/Documentation/thinkpad-acpi.txt b/Documentation/thinkpad-acpi.txt
index cc079af..80c0bf2 100644
--- a/Documentation/thinkpad-acpi.txt
+++ b/Documentation/thinkpad-acpi.txt
@@ -458,17 +458,17 @@ X40:
 	16 - one medium-pitched beep repeating constantly, stop with 17
 	17 - stop 16
 
-Temperature sensors -- /proc/acpi/ibm/thermal
----------------------------------------------
+Temperature sensors
+-------------------
+
+procfs: /proc/acpi/ibm/thermal
+sysfs device attributes: (hwmon) temp*_input
 
 Most ThinkPads include six or more separate temperature sensors but
 only expose the CPU temperature through the standard ACPI methods.
 This feature shows readings from up to eight different sensors on older
 ThinkPads, and it has experimental support for up to sixteen different
-sensors on newer ThinkPads.  Readings from sensors that are not available
-return -128.
-
-No commands can be written to this file.
+sensors on newer ThinkPads.
 
 EXPERIMENTAL: The 16-sensors feature is marked EXPERIMENTAL because the
 implementation directly accesses hardware registers and may not work as
@@ -525,6 +525,20 @@ The A31 has a very atypical layout for the thermal sensors
 8:  Bay Battery: secondary sensor
 
 
+Procfs notes:
+	Readings from sensors that are not available return -128.
+	No commands can be written to this file.
+
+Sysfs notes:
+	Sensors that are not available return the ENXIO error.  This
+	status may change at runtime, as there are hotplug thermal
+	sensors, like those inside the batteries and docks.
+
+	thinkpad-acpi thermal sensors are reported through the hwmon
+	subsystem, and follow all of the hwmon guidelines at
+	Documentation/hwmon.
+
+
 EXPERIMENTAL: Embedded controller register dump -- /proc/acpi/ibm/ecdump
 ------------------------------------------------------------------------
 
diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index aa69ff0..d5526e8 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
@@ -1992,11 +1992,91 @@ static struct ibm_struct beep_driver_data = {
 
 static enum thermal_access_mode thermal_read_mode;
 
+/* sysfs temp##_input -------------------------------------------------- */
+
+static ssize_t thermal_temp_input_show(struct device *dev,
+			   struct device_attribute *attr,
+			   char *buf)
+{
+	struct sensor_device_attribute *sensor_attr =
+					to_sensor_dev_attr(attr);
+	int idx = sensor_attr->index;
+	s32 value;
+	int res;
+
+	res = thermal_get_sensor(idx, &value);
+	if (res)
+		return res;
+	if (value == TP_EC_THERMAL_TMP_NA * 1000)
+		return -ENXIO;
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", value);
+}
+
+#define THERMAL_SENSOR_ATTR_TEMP(_idxA, _idxB) \
+	 SENSOR_ATTR(temp##_idxA##_input, S_IRUGO, thermal_temp_input_show, NULL, _idxB)
+
+static struct sensor_device_attribute sensor_dev_attr_thermal_temp_input[] = {
+	THERMAL_SENSOR_ATTR_TEMP(1, 0),
+	THERMAL_SENSOR_ATTR_TEMP(2, 1),
+	THERMAL_SENSOR_ATTR_TEMP(3, 2),
+	THERMAL_SENSOR_ATTR_TEMP(4, 3),
+	THERMAL_SENSOR_ATTR_TEMP(5, 4),
+	THERMAL_SENSOR_ATTR_TEMP(6, 5),
+	THERMAL_SENSOR_ATTR_TEMP(7, 6),
+	THERMAL_SENSOR_ATTR_TEMP(8, 7),
+	THERMAL_SENSOR_ATTR_TEMP(9, 8),
+	THERMAL_SENSOR_ATTR_TEMP(10, 9),
+	THERMAL_SENSOR_ATTR_TEMP(11, 10),
+	THERMAL_SENSOR_ATTR_TEMP(12, 11),
+	THERMAL_SENSOR_ATTR_TEMP(13, 12),
+	THERMAL_SENSOR_ATTR_TEMP(14, 13),
+	THERMAL_SENSOR_ATTR_TEMP(15, 14),
+	THERMAL_SENSOR_ATTR_TEMP(16, 15),
+};
+
+#define THERMAL_ATTRS(X) \
+	&sensor_dev_attr_thermal_temp_input[X].dev_attr.attr
+
+static struct attribute *thermal_temp_input_attr[] = {
+	THERMAL_ATTRS(8),
+	THERMAL_ATTRS(9),
+	THERMAL_ATTRS(10),
+	THERMAL_ATTRS(11),
+	THERMAL_ATTRS(12),
+	THERMAL_ATTRS(13),
+	THERMAL_ATTRS(14),
+	THERMAL_ATTRS(15),
+	THERMAL_ATTRS(0),
+	THERMAL_ATTRS(1),
+	THERMAL_ATTRS(2),
+	THERMAL_ATTRS(3),
+	THERMAL_ATTRS(4),
+	THERMAL_ATTRS(5),
+	THERMAL_ATTRS(6),
+	THERMAL_ATTRS(7),
+	NULL
+};
+
+static const struct attribute_group thermal_temp_input16_group = {
+	.attrs = thermal_temp_input_attr
+};
+
+static const struct attribute_group thermal_temp_input8_group = {
+	.attrs = &thermal_temp_input_attr[8]
+};
+
+#undef THERMAL_SENSOR_ATTR_TEMP
+#undef THERMAL_ATTRS
+
+/* --------------------------------------------------------------------- */
+
 static int __init thermal_init(struct ibm_init_struct *iibm)
 {
 	u8 t, ta1, ta2;
 	int i;
 	int acpi_tmp7;
+	int res;
 
 	vdbg_printk(TPACPI_DBG_INIT, "initializing thermal subdriver\n");
 
@@ -2060,7 +2140,46 @@ static int __init thermal_init(struct ibm_init_struct *iibm)
 		str_supported(thermal_read_mode != TPACPI_THERMAL_NONE),
 		thermal_read_mode);
 
-	return (thermal_read_mode != TPACPI_THERMAL_NONE)? 0 : 1;
+	switch(thermal_read_mode) {
+	case TPACPI_THERMAL_TPEC_16:
+		res = sysfs_create_group(&tpacpi_pdev->dev.kobj,
+				&thermal_temp_input16_group);
+		if (res)
+			return res;
+		break;
+	case TPACPI_THERMAL_TPEC_8:
+	case TPACPI_THERMAL_ACPI_TMP07:
+	case TPACPI_THERMAL_ACPI_UPDT:
+		res = sysfs_create_group(&tpacpi_pdev->dev.kobj,
+				&thermal_temp_input8_group);
+		if (res)
+			return res;
+		break;
+	case TPACPI_THERMAL_NONE:
+	default:
+		return 1;
+	}
+
+	return 0;
+}
+
+static void thermal_exit(void)
+{
+	switch(thermal_read_mode) {
+	case TPACPI_THERMAL_TPEC_16:
+		sysfs_remove_group(&tpacpi_pdev->dev.kobj,
+				   &thermal_temp_input16_group);
+		break;
+	case TPACPI_THERMAL_TPEC_8:
+	case TPACPI_THERMAL_ACPI_TMP07:
+	case TPACPI_THERMAL_ACPI_UPDT:
+		sysfs_remove_group(&tpacpi_pdev->dev.kobj,
+				   &thermal_temp_input16_group);
+		break;
+	case TPACPI_THERMAL_NONE:
+	default:
+		break;
+	}
 }
 
 /* idx is zero-based */
@@ -2168,6 +2287,7 @@ static int thermal_read(char *p)
 static struct ibm_struct thermal_driver_data = {
 	.name = "thermal",
 	.read = thermal_read,
+	.exit = thermal_exit,
 };
 
 /*************************************************************************
diff --git a/drivers/misc/thinkpad_acpi.h b/drivers/misc/thinkpad_acpi.h
index a9feb53..e833ff3 100644
--- a/drivers/misc/thinkpad_acpi.h
+++ b/drivers/misc/thinkpad_acpi.h
@@ -38,6 +38,7 @@
 #include <linux/fb.h>
 #include <linux/platform_device.h>
 #include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
 #include <asm/uaccess.h>
 
 #include <linux/dmi.h>
@@ -467,6 +468,7 @@ enum thermal_access_mode {
 enum { /* TPACPI_THERMAL_TPEC_* */
 	TP_EC_THERMAL_TMP0 = 0x78,	/* ACPI EC regs TMP 0..7 */
 	TP_EC_THERMAL_TMP8 = 0xC0,	/* ACPI EC regs TMP 8..15 */
+	TP_EC_THERMAL_TMP_NA = -128,	/* ACPI EC sensor not available */
 };
 
 #define TPACPI_MAX_THERMAL_SENSORS 16	/* Max thermal sensors supported */
-- 
1.5.1


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 6/9] ACPI: thinkpad-acpi: add sysfs support to fan subdriver
  2007-04-24 14:48 [GIT PULL] thinkpad-acpi sysfs support part 1 Henrique de Moraes Holschuh
  2007-04-24 14:48 ` [PATCH 1/9] ACPI: thinkpad-acpi: register with the device model Henrique de Moraes Holschuh
  2007-04-24 14:48 ` [PATCH 3/9] ACPI: thinkpad-acpi: add infrastructure for the sysfs device attributes Henrique de Moraes Holschuh
@ 2007-04-24 14:48 ` Henrique de Moraes Holschuh
  2007-04-25  5:58   ` Len Brown
  2007-04-24 14:48 ` [PATCH 7/9] ACPI: thinkpad-acpi: add a safety net for TPEC fan control mode Henrique de Moraes Holschuh
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-04-24 14:48 UTC (permalink / raw)
  To: lenb; +Cc: ibm-acpi-devel, linux-acpi, Henrique de Moraes Holschuh

Export sysfs attributes to monitor and control the internal thinkpad fan
(some thinkpads have more than one fan, but thinkpad-acpi doesn't support
the second fan yet).  The sysfs interface follows the hwmon design guide
for fan devices.

Also, fix some stray "thermal" files in the fan procfs description that
have been there forever, and officially support "full-speed" as the name
for the PWM-disabled state of the fan controller to keep it in line with
the hwmon interface.  It is much better a name for that mode than the
unobvious "disengaged" anyway.  Change the procfs interface to also accept
full-speed as a fan level, but still report it as disengaged for backwards
compatibility.

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
---
 Documentation/thinkpad-acpi.txt |  157 +++++++++++++------
 drivers/misc/thinkpad_acpi.c    |  326 ++++++++++++++++++++++++++++++++++++---
 drivers/misc/thinkpad_acpi.h    |    6 +
 3 files changed, 415 insertions(+), 74 deletions(-)

diff --git a/Documentation/thinkpad-acpi.txt b/Documentation/thinkpad-acpi.txt
index 80c0bf2..339ce21 100644
--- a/Documentation/thinkpad-acpi.txt
+++ b/Documentation/thinkpad-acpi.txt
@@ -642,8 +642,11 @@ distinct. The unmute the volume after the mute command, use either the
 up or down command (the level command will not unmute the volume).
 The current volume level and mute state is shown in the file.
 
-EXPERIMENTAL: fan speed, fan enable/disable -- /proc/acpi/ibm/fan
------------------------------------------------------------------
+EXPERIMENTAL: fan speed, fan enable/disable
+-------------------------------------------
+
+procfs: /proc/acpi/ibm/fan
+sysfs device attributes: (hwmon) fan_input, pwm1, pwm1_enable
 
 This feature is marked EXPERIMENTAL because the implementation
 directly accesses hardware registers and may not work as expected. USE
@@ -656,27 +659,26 @@ from the hardware registers of the embedded controller.  This is known
 to work on later R, T and X series ThinkPads but may show a bogus
 value on other models.
 
-Most ThinkPad fans work in "levels".  Level 0 stops the fan.  The higher
-the level, the higher the fan speed, although adjacent levels often map
-to the same fan speed.  7 is the highest level, where the fan reaches
-the maximum recommended speed.  Level "auto" means the EC changes the
-fan level according to some internal algorithm, usually based on
-readings from the thermal sensors.  Level "disengaged" means the EC
-disables the speed-locked closed-loop fan control, and drives the fan as
-fast as it can go, which might exceed hardware limits, so use this level
-with caution.
+Fan levels:
 
-The fan usually ramps up or down slowly from one speed to another,
-and it is normal for the EC to take several seconds to react to fan
-commands.
+Most ThinkPad fans work in "levels" at the firmware interface.  Level 0
+stops the fan.  The higher the level, the higher the fan speed, although
+adjacent levels often map to the same fan speed.  7 is the highest
+level, where the fan reaches the maximum recommended speed.
 
-The fan may be enabled or disabled with the following commands:
+Level "auto" means the EC changes the fan level according to some
+internal algorithm, usually based on readings from the thermal sensors.
 
-	echo enable  >/proc/acpi/ibm/fan
-	echo disable >/proc/acpi/ibm/fan
+There is also a "full-speed" level, also known as "disengaged" level.
+In this level, the EC disables the speed-locked closed-loop fan control,
+and drives the fan as fast as it can go, which might exceed hardware
+limits, so use this level with caution.
 
-Placing a fan on level 0 is the same as disabling it.  Enabling a fan
-will try to place it in a safe level if it is too slow or disabled.
+The fan usually ramps up or down slowly from one speed to another, and
+it is normal for the EC to take several seconds to react to fan
+commands.  The full-speed level may take up to two minutes to ramp up to
+maximum speed, and in some ThinkPads, the tachometer readings go stale
+while the EC is transitioning to the full-speed level.
 
 WARNING WARNING WARNING: do not leave the fan disabled unless you are
 monitoring all of the temperature sensor readings and you are ready to
@@ -694,48 +696,101 @@ fan is turned off when the CPU temperature drops to 49 degrees and the
 HDD temperature drops to 41 degrees.  These thresholds cannot
 currently be controlled.
 
+The ThinkPad's ACPI DSDT code will reprogram the fan on its own when
+certain conditions are met.  It will override any fan programming done
+through thinkpad-acpi.
+
+The thinkpad-acpi kernel driver can be programmed to revert the fan
+level to a safe setting if userspace does not issue one of the procfs
+fan commands: "enable", "disable", "level" or "watchdog", or if there
+are no writes to pwm1_enable (or to pwm1 *if and only if* pwm1_enable is
+set to 1, manual mode) within a configurable amount of time of up to
+120 seconds.  This functionality is called fan safety watchdog.
+
+Note that the watchdog timer stops after it enables the fan.  It will be
+rearmed again automatically (using the same interval) when one of the
+above mentioned fan commands is received.  The fan watchdog is,
+therefore, not suitable to protect against fan mode changes made through
+means other than the "enable", "disable", and "level" procfs fan
+commands, or the hwmon fan control sysfs interface.
+
+Procfs notes:
+
+The fan may be enabled or disabled with the following commands:
+
+	echo enable  >/proc/acpi/ibm/fan
+	echo disable >/proc/acpi/ibm/fan
+
+Placing a fan on level 0 is the same as disabling it.  Enabling a fan
+will try to place it in a safe level if it is too slow or disabled.
+
 The fan level can be controlled with the command:
 
-	echo 'level <level>' > /proc/acpi/ibm/thermal
+	echo 'level <level>' > /proc/acpi/ibm/fan
 
-Where <level> is an integer from 0 to 7, or one of the words "auto"
-or "disengaged" (without the quotes).  Not all ThinkPads support the
-"auto" and "disengaged" levels.
+Where <level> is an integer from 0 to 7, or one of the words "auto" or
+"full-speed" (without the quotes).  Not all ThinkPads support the "auto"
+and "full-speed" levels.  The driver accepts "disengaged" as an alias for
+"full-speed", and reports it as "disengaged" for backwards
+compatibility.
 
 On the X31 and X40 (and ONLY on those models), the fan speed can be
-controlled to a certain degree. Once the fan is running, it can be
+controlled to a certain degree.  Once the fan is running, it can be
 forced to run faster or slower with the following command:
 
-	echo 'speed <speed>' > /proc/acpi/ibm/thermal
+	echo 'speed <speed>' > /proc/acpi/ibm/fan
 
-The sustainable range of fan speeds on the X40 appears to be from
-about 3700 to about 7350. Values outside this range either do not have
-any effect or the fan speed eventually settles somewhere in that
-range. The fan cannot be stopped or started with this command.
+The sustainable range of fan speeds on the X40 appears to be from about
+3700 to about 7350. Values outside this range either do not have any
+effect or the fan speed eventually settles somewhere in that range.  The
+fan cannot be stopped or started with this command.  This functionality
+is incomplete, and not available through the sysfs interface.
 
-The ThinkPad's ACPI DSDT code will reprogram the fan on its own when
-certain conditions are met.  It will override any fan programming done
-through thinkpad-acpi.
+To program the safety watchdog, use the "watchdog" command.
+
+	echo 'watchdog <interval in seconds>' > /proc/acpi/ibm/fan
+
+If you want to disable the watchdog, use 0 as the interval.
+
+Sysfs notes:
+
+The sysfs interface follows the hwmon subsystem guidelines for the most
+part, and the exception is the fan safety watchdog.
+
+hwmon device attribute pwm1_enable:
+	0: PWM offline (fan is set to full-speed mode)
+	1: Manual PWM control (use pwm1 to set fan level)
+	2: Hardware PWM control (EC "auto" mode)
+	3: reserved (Software PWM control, not implemented yet)
+
+	Modes 0 and 2 are not supported by all ThinkPads, and the driver
+	is not always able to detect this.  If it does know a mode is
+	unsupported, it will return -EINVAL.
+
+hwmon device attribute pwm1:
+	Fan level, scaled from the firmware values of 0-7 to the hwmon
+	scale of 0-255.  0 means fan stopped, 255 means highest normal
+	speed (level 7).
+
+	This attribute only commands the fan if pmw1_enable is set to 1
+	(manual PWM control).
+
+hwmon device attribute fan1_input:
+	Fan tachometer reading, in RPM.  May go stale on certain
+	ThinkPads while the EC transitions the PWM to offline mode,
+	which can take up to two minutes.  May return rubbish on older
+	ThinkPads.
+
+driver attribute fan_watchdog:
+	Fan safety watchdog timer interval, in seconds.  Minimum is
+	1 second, maximum is 120 seconds.  0 disables the watchdog.
+
+To stop the fan: set pwm1 to zero, and pwm1_enable to 1.
+
+To start the fan in a safe mode: set pwm1_enable to 2.  If that fails
+with ENOTSUP, set it to 1 and set pwm1 to at least 128 (255 would be the
+safest choice, though).
 
-The thinkpad-acpi kernel driver can be programmed to revert the fan
-level to a safe setting if userspace does not issue one of the fan
-commands: "enable", "disable", "level" or "watchdog" within a
-configurable ammount of time.  To do this, use the "watchdog" command.
-
-	echo 'watchdog <interval>' > /proc/acpi/ibm/fan
-
-Interval is the ammount of time in seconds to wait for one of the
-above mentioned fan commands before reseting the fan level to a safe
-one.  If set to zero, the watchdog is disabled (default).  When the
-watchdog timer runs out, it does the exact equivalent of the "enable"
-fan command.
-
-Note that the watchdog timer stops after it enables the fan.  It will
-be rearmed again automatically (using the same interval) when one of
-the above mentioned fan commands is received.  The fan watchdog is,
-therefore, not suitable to protect against fan mode changes made
-through means other than the "enable", "disable", and "level" fan
-commands.
 
 EXPERIMENTAL: WAN -- /proc/acpi/ibm/wan
 ---------------------------------------
diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index d5526e8..a4d7ee4 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
@@ -2695,6 +2695,7 @@ static enum fan_control_access_mode fan_control_access_mode;
 static enum fan_control_commands fan_control_commands;
 
 static u8 fan_control_initial_status;
+static u8 fan_control_desired_level;
 
 static void fan_watchdog_fire(struct work_struct *ignored);
 static int fan_watchdog_maxinterval;
@@ -2708,8 +2709,222 @@ IBM_HANDLE(sfan, ec, "SFAN",	/* 570 */
 	   "JFNS",		/* 770x-JL */
 	   );			/* all others */
 
+/*
+ * SYSFS fan layout: hwmon compatible (device)
+ *
+ * pwm*_enable:
+ * 	0: "disengaged" mode
+ * 	1: manual mode
+ * 	2: native EC "auto" mode (recommended, hardware default)
+ *
+ * pwm*: set speed in manual mode, ignored otherwise.
+ * 	0 is level 0; 255 is level 7. Intermediate points done with linear
+ * 	interpolation.
+ *
+ * fan*_input: tachometer reading, RPM
+ *
+ *
+ * SYSFS fan layout: extensions
+ *
+ * fan_watchdog (driver):
+ * 	fan watchdog interval in seconds, 0 disables (default), max 120
+ */
+
+/* sysfs fan pwm1_enable ----------------------------------------------- */
+static ssize_t fan_pwm1_enable_show(struct device *dev,
+				    struct device_attribute *attr,
+				    char *buf)
+{
+	int res, mode;
+	u8 status;
+
+	res = fan_get_status_safe(&status);
+	if (res)
+		return res;
+
+	if (unlikely(tp_features.fan_ctrl_status_undef)) {
+		if (status != fan_control_initial_status) {
+			tp_features.fan_ctrl_status_undef = 0;
+		} else {
+			/* Return most likely status. In fact, it
+			 * might be the only possible status */
+			status = TP_EC_FAN_AUTO;
+		}
+	}
+
+	if (status & TP_EC_FAN_FULLSPEED) {
+		mode = 0;
+	} else if (status & TP_EC_FAN_AUTO) {
+		mode = 2;
+	} else
+		mode = 1;
+
+	return snprintf(buf, PAGE_SIZE, "%d\n", mode);
+}
+
+static ssize_t fan_pwm1_enable_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	unsigned long t;
+	int res, level;
+
+	if (parse_strtoul(buf, 2, &t))
+		return -EINVAL;
+
+	switch (t) {
+	case 0:
+		level = TP_EC_FAN_FULLSPEED;
+		break;
+	case 1:
+		level = TPACPI_FAN_LAST_LEVEL;
+		break;
+	case 2:
+		level = TP_EC_FAN_AUTO;
+		break;
+	case 3:
+		/* reserved for software-controlled auto mode */
+		return -ENOSYS;
+	default:
+		return -EINVAL;
+	}
+
+	res = fan_set_level_safe(level);
+	if (res < 0)
+		return res;
+
+	fan_watchdog_reset();
+
+	return count;
+}
+
+static struct device_attribute dev_attr_fan_pwm1_enable =
+	__ATTR(pwm1_enable, S_IWUSR | S_IRUGO,
+		fan_pwm1_enable_show, fan_pwm1_enable_store);
+
+/* sysfs fan pwm1 ------------------------------------------------------ */
+static ssize_t fan_pwm1_show(struct device *dev,
+			     struct device_attribute *attr,
+			     char *buf)
+{
+	int res;
+	u8 status;
+
+	res = fan_get_status_safe(&status);
+	if (res)
+		return res;
+
+	if (unlikely(tp_features.fan_ctrl_status_undef)) {
+		if (status != fan_control_initial_status) {
+			tp_features.fan_ctrl_status_undef = 0;
+		} else {
+			status = TP_EC_FAN_AUTO;
+		}
+	}
+
+	if ((status &
+	     (TP_EC_FAN_AUTO | TP_EC_FAN_FULLSPEED)) != 0)
+		status = fan_control_desired_level;
+
+	if (status > 7)
+		status = 7;
+
+	return snprintf(buf, PAGE_SIZE, "%u\n", (status * 255) / 7);
+}
+
+static ssize_t fan_pwm1_store(struct device *dev,
+			      struct device_attribute *attr,
+			      const char *buf, size_t count)
+{
+	unsigned long s;
+	int rc;
+	u8 status, newlevel;
+
+	if (parse_strtoul(buf, 255, &s))
+		return -EINVAL;
+
+	/* scale down from 0-255 to 0-7 */
+	newlevel = (s >> 5) & 0x07;
+
+	rc = mutex_lock_interruptible(&fan_mutex);
+	if (rc < 0)
+		return rc;
+
+	rc = fan_get_status(&status);
+	if (!rc && (status &
+		    (TP_EC_FAN_AUTO | TP_EC_FAN_FULLSPEED)) == 0) {
+		rc = fan_set_level(newlevel);
+		if (!rc)
+			fan_update_desired_level(newlevel);
+		fan_watchdog_reset();
+	}
+
+	mutex_unlock(&fan_mutex);
+	return (rc)? rc : count;
+}
+
+static struct device_attribute dev_attr_fan_pwm1 =
+	__ATTR(pwm1, S_IWUSR | S_IRUGO,
+		fan_pwm1_show, fan_pwm1_store);
+
+/* sysfs fan fan1_input ------------------------------------------------ */
+static ssize_t fan_fan1_input_show(struct device *dev,
+			   struct device_attribute *attr,
+			   char *buf)
+{
+	int res;
+	unsigned int speed;
+
+	res = fan_get_speed(&speed);
+	if (res < 0)
+		return res;
+
+	return snprintf(buf, PAGE_SIZE, "%u\n", speed);
+}
+
+static struct device_attribute dev_attr_fan_fan1_input =
+	__ATTR(fan1_input, S_IRUGO,
+		fan_fan1_input_show, NULL);
+
+/* sysfs fan fan_watchdog (driver) ------------------------------------- */
+static ssize_t fan_fan_watchdog_show(struct device_driver *drv,
+				     char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%u\n", fan_watchdog_maxinterval);
+}
+
+static ssize_t fan_fan_watchdog_store(struct device_driver *drv,
+				      const char *buf, size_t count)
+{
+	unsigned long t;
+
+	if (parse_strtoul(buf, 120, &t))
+		return -EINVAL;
+
+	fan_watchdog_maxinterval = t;
+	fan_watchdog_reset();
+
+	return count;
+}
+
+static DRIVER_ATTR(fan_watchdog, S_IWUSR | S_IRUGO,
+		fan_fan_watchdog_show, fan_fan_watchdog_store);
+
+/* --------------------------------------------------------------------- */
+static struct attribute *fan_attributes[] = {
+	&dev_attr_fan_pwm1_enable.attr, &dev_attr_fan_pwm1.attr,
+	&dev_attr_fan_fan1_input.attr,
+	NULL
+};
+
+static const struct attribute_group fan_attr_group = {
+	.attrs = fan_attributes,
+};
+
 static int __init fan_init(struct ibm_init_struct *iibm)
 {
+	int rc;
+
 	vdbg_printk(TPACPI_DBG_INIT, "initializing fan subdriver\n");
 
 	mutex_init(&fan_mutex);
@@ -2718,6 +2933,7 @@ static int __init fan_init(struct ibm_init_struct *iibm)
 	fan_control_commands = 0;
 	fan_watchdog_maxinterval = 0;
 	tp_features.fan_ctrl_status_undef = 0;
+	fan_control_desired_level = 7;
 
 	IBM_ACPIHANDLE_INIT(fans);
 	IBM_ACPIHANDLE_INIT(gfan);
@@ -2796,9 +3012,36 @@ static int __init fan_init(struct ibm_init_struct *iibm)
 		  fan_control_access_mode != TPACPI_FAN_WR_NONE),
 		fan_status_access_mode, fan_control_access_mode);
 
-	return (fan_status_access_mode != TPACPI_FAN_NONE ||
-	        fan_control_access_mode != TPACPI_FAN_WR_NONE)?
-			0 : 1;
+	/* update fan_control_desired_level */
+	if (fan_status_access_mode != TPACPI_FAN_NONE)
+		fan_get_status_safe(NULL);
+
+	if (fan_status_access_mode != TPACPI_FAN_NONE ||
+	    fan_control_access_mode != TPACPI_FAN_WR_NONE) {
+		rc = sysfs_create_group(&tpacpi_pdev->dev.kobj,
+					 &fan_attr_group);
+		if (!(rc < 0))
+			rc = driver_create_file(&tpacpi_pdriver.driver,
+					&driver_attr_fan_watchdog);
+		if (rc < 0)
+			return rc;
+		return 0;
+	} else
+		return 1;
+}
+
+/*
+ * Call with fan_mutex held
+ */
+static void fan_update_desired_level(u8 status)
+{
+	if ((status &
+	     (TP_EC_FAN_AUTO | TP_EC_FAN_FULLSPEED)) == 0) {
+		if (status > 7)
+			fan_control_desired_level = 7;
+		else
+			fan_control_desired_level = status;
+	}
 }
 
 static int fan_get_status(u8 *status)
@@ -2837,9 +3080,33 @@ static int fan_get_status(u8 *status)
 	return 0;
 }
 
+static int fan_get_status_safe(u8 *status)
+{
+	int rc;
+	u8 s;
+
+	rc = mutex_lock_interruptible(&fan_mutex);
+	if (rc < 0)
+		return rc;
+	rc = fan_get_status(&s);
+	if (!rc)
+		fan_update_desired_level(s);
+	mutex_unlock(&fan_mutex);
+
+	if (status)
+		*status = s;
+
+	return rc;
+}
+
 static void fan_exit(void)
 {
 	vdbg_printk(TPACPI_DBG_EXIT, "cancelling any pending fan watchdog tasks\n");
+
+	/* FIXME: can we really do this unconditionally? */
+	sysfs_remove_group(&tpacpi_pdev->dev.kobj, &fan_attr_group);
+	driver_remove_file(&tpacpi_pdriver.driver, &driver_attr_fan_watchdog);
+
 	cancel_delayed_work(&fan_watchdog_task);
 	flush_scheduled_work();
 }
@@ -2902,17 +3169,10 @@ static void fan_watchdog_reset(void)
 
 static int fan_set_level(int level)
 {
-	int res;
-
 	switch (fan_control_access_mode) {
 	case TPACPI_FAN_WR_ACPI_SFAN:
 		if (level >= 0 && level <= 7) {
-			res = mutex_lock_interruptible(&fan_mutex);
-			if (res < 0)
-				return res;
-			res = acpi_evalf(sfan_handle, NULL, NULL, "vd", level);
-			mutex_unlock(&fan_mutex);
-			if (!res)
+			if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", level))
 				return -EIO;
 		} else
 			return -EINVAL;
@@ -2925,12 +3185,7 @@ static int fan_set_level(int level)
 		    ((level < 0) || (level > 7)))
 			return -EINVAL;
 
-		res = mutex_lock_interruptible(&fan_mutex);
-		if (res < 0)
-			return res;
-		res = acpi_ec_write(fan_status_offset, level);
-		mutex_unlock(&fan_mutex);
-		if (!res)
+		if (!acpi_ec_write(fan_status_offset, level))
 			return -EIO;
 		else
 			tp_features.fan_ctrl_status_undef = 0;
@@ -2942,6 +3197,25 @@ static int fan_set_level(int level)
 	return 0;
 }
 
+static int fan_set_level_safe(int level)
+{
+	int rc;
+
+	rc = mutex_lock_interruptible(&fan_mutex);
+	if (rc < 0)
+		return rc;
+
+	if (level == TPACPI_FAN_LAST_LEVEL)
+		level = fan_control_desired_level;
+
+	rc = fan_set_level(level);
+	if (!rc)
+		fan_update_desired_level(level);
+
+	mutex_unlock(&fan_mutex);
+	return rc;
+}
+
 static int fan_set_enable(void)
 {
 	u8 s;
@@ -3009,19 +3283,24 @@ static int fan_set_disable(void)
 	case TPACPI_FAN_WR_TPEC:
 		if (!acpi_ec_write(fan_status_offset, 0x00))
 			rc = -EIO;
-		else
+		else {
+			fan_control_desired_level = 0;
 			tp_features.fan_ctrl_status_undef = 0;
+		}
 		break;
 
 	case TPACPI_FAN_WR_ACPI_SFAN:
 		if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", 0x00))
 			rc = -EIO;
+		else
+			fan_control_desired_level = 0;
 		break;
 
 	default:
 		rc = -ENXIO;
 	}
 
+
 	mutex_unlock(&fan_mutex);
 	return rc;
 }
@@ -3063,7 +3342,7 @@ static int fan_read(char *p)
 	switch (fan_status_access_mode) {
 	case TPACPI_FAN_RD_ACPI_GFAN:
 		/* 570, 600e/x, 770e, 770x */
-		if ((rc = fan_get_status(&status)) < 0)
+		if ((rc = fan_get_status_safe(&status)) < 0)
 			return rc;
 
 		len += sprintf(p + len, "status:\t\t%s\n"
@@ -3073,7 +3352,7 @@ static int fan_read(char *p)
 
 	case TPACPI_FAN_RD_TPEC:
 		/* all except 570, 600e/x, 770e, 770x */
-		if ((rc = fan_get_status(&status)) < 0)
+		if ((rc = fan_get_status_safe(&status)) < 0)
 			return rc;
 
 		if (unlikely(tp_features.fan_ctrl_status_undef)) {
@@ -3117,7 +3396,7 @@ static int fan_read(char *p)
 
 		default:
 			len += sprintf(p + len, " (<level> is 0-7, "
-				       "auto, disengaged)\n");
+				       "auto, disengaged, full-speed)\n");
 			break;
 		}
 	}
@@ -3140,12 +3419,13 @@ static int fan_write_cmd_level(const char *cmd, int *rc)
 
 	if (strlencmp(cmd, "level auto") == 0)
 		level = TP_EC_FAN_AUTO;
-	else if (strlencmp(cmd, "level disengaged") == 0)
+	else if ((strlencmp(cmd, "level disengaged") == 0) |
+	         (strlencmp(cmd, "level full-speed") == 0))
 		level = TP_EC_FAN_FULLSPEED;
 	else if (sscanf(cmd, "level %d", &level) != 1)
 		return 0;
 
-	if ((*rc = fan_set_level(level)) == -ENXIO)
+	if ((*rc = fan_set_level_safe(level)) == -ENXIO)
 		printk(IBM_ERR "level command accepted for unsupported "
 		       "access mode %d", fan_control_access_mode);
 
diff --git a/drivers/misc/thinkpad_acpi.h b/drivers/misc/thinkpad_acpi.h
index e833ff3..2fe4d61 100644
--- a/drivers/misc/thinkpad_acpi.h
+++ b/drivers/misc/thinkpad_acpi.h
@@ -349,6 +349,8 @@ enum {					/* Fan control constants */
 
 	TP_EC_FAN_FULLSPEED = 0x40,	/* EC fan mode: full speed */
 	TP_EC_FAN_AUTO	    = 0x80,	/* EC fan mode: auto fan control */
+
+	TPACPI_FAN_LAST_LEVEL = 0x100,	/* Use cached last-seen fan level */
 };
 
 enum fan_status_access_mode {
@@ -375,6 +377,7 @@ static enum fan_status_access_mode fan_status_access_mode;
 static enum fan_control_access_mode fan_control_access_mode;
 static enum fan_control_commands fan_control_commands;
 static u8 fan_control_initial_status;
+static u8 fan_control_desired_level;
 static int fan_watchdog_maxinterval;
 
 struct mutex fan_mutex;
@@ -384,10 +387,13 @@ static acpi_handle fans_handle, gfan_handle, sfan_handle;
 static int fan_init(struct ibm_init_struct *iibm);
 static void fan_exit(void);
 static int fan_get_status(u8 *status);
+static int fan_get_status_safe(u8 *status);
 static int fan_get_speed(unsigned int *speed);
+static void fan_update_desired_level(u8 status);
 static void fan_watchdog_fire(struct work_struct *ignored);
 static void fan_watchdog_reset(void);
 static int fan_set_level(int level);
+static int fan_set_level_safe(int level);
 static int fan_set_enable(void);
 static int fan_set_disable(void);
 static int fan_set_speed(int speed);
-- 
1.5.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 7/9] ACPI: thinkpad-acpi: add a safety net for TPEC fan control mode
  2007-04-24 14:48 [GIT PULL] thinkpad-acpi sysfs support part 1 Henrique de Moraes Holschuh
                   ` (2 preceding siblings ...)
  2007-04-24 14:48 ` [PATCH 6/9] ACPI: thinkpad-acpi: add sysfs support to fan subdriver Henrique de Moraes Holschuh
@ 2007-04-24 14:48 ` Henrique de Moraes Holschuh
       [not found] ` <11774261003184-git-send-email-hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-04-24 14:48 UTC (permalink / raw)
  To: lenb; +Cc: ibm-acpi-devel, linux-acpi, Henrique de Moraes Holschuh

The Linux ThinkPad community is not positive that all ThinkPads that do
HFSP EC fan control do implement full-speed and auto modes, some of the
earlier ones supporting HFSP might not.

If the EC ignores the AUTO or FULL-SPEED bits, it will pay attention to the
lower three bits that set the fan level.  And as thinkpad-acpi was leaving
these set to zero, it would stop(!) the fan, which is Not A Good Thing.

So, as a safety net, we now make sure to also set the fan level part of the
HFSP register to speed 7 for full-speed, and a minimum of speed 4 for auto
mode.

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
---
 drivers/misc/thinkpad_acpi.c |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index a4d7ee4..79abc68 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
@@ -3185,6 +3185,13 @@ static int fan_set_level(int level)
 		    ((level < 0) || (level > 7)))
 			return -EINVAL;
 
+		/* safety net should the EC not support AUTO
+		 * or FULLSPEED mode bits and just ignore them */
+		if (level & TP_EC_FAN_FULLSPEED)
+			level |= 7;	/* safety min speed 7 */
+		else if (level & TP_EC_FAN_FULLSPEED)
+			level |= 4;	/* safety min speed 4 */
+
 		if (!acpi_ec_write(fan_status_offset, level))
 			return -EIO;
 		else
@@ -3233,8 +3240,10 @@ static int fan_set_enable(void)
 			break;
 
 		/* Don't go out of emergency fan mode */
-		if (s != 7)
-			s = TP_EC_FAN_AUTO;
+		if (s != 7) {
+			s &= 0x07;
+			s |= TP_EC_FAN_AUTO | 4; /* min fan speed 4 */
+		}
 
 		if (!acpi_ec_write(fan_status_offset, s))
 			rc = -EIO;
@@ -3252,8 +3261,7 @@ static int fan_set_enable(void)
 		s &= 0x07;
 
 		/* Set fan to at least level 4 */
-		if (s < 4)
-			s = 4;
+		s |= 4;
 
 		if (!acpi_evalf(sfan_handle, NULL, NULL, "vd", s))
 			rc= -EIO;
-- 
1.5.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 8/9] ACPI: thinkpad-acpi: add sysfs support to the cmos command subdriver
       [not found] ` <11774261003184-git-send-email-hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
                     ` (2 preceding siblings ...)
  2007-04-24 14:48   ` [PATCH 5/9] ACPI: thinkpad-acpi: add sysfs support to the thermal subdriver Henrique de Moraes Holschuh
@ 2007-04-24 14:48   ` Henrique de Moraes Holschuh
  3 siblings, 0 replies; 14+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-04-24 14:48 UTC (permalink / raw)
  To: lenb-DgEjT+Ai2ygdnm+yROfE0A
  Cc: ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Henrique de Moraes Holschuh, linux-acpi-u79uwXL29TY76Z2rM5mHXA

Add sysfs attributes to send ThinkPad CMOS commands.

Signed-off-by: Henrique de Moraes Holschuh <hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
---
 Documentation/thinkpad-acpi.txt |   23 +++++++++++------------
 drivers/misc/thinkpad_acpi.c    |   33 +++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/Documentation/thinkpad-acpi.txt b/Documentation/thinkpad-acpi.txt
index 339ce21..352e8ae 100644
--- a/Documentation/thinkpad-acpi.txt
+++ b/Documentation/thinkpad-acpi.txt
@@ -378,23 +378,19 @@ supported. Use "eject2" instead of "eject" for the second bay.
 Note: the UltraBay eject support on the 600e/x, A22p and A3x is
 EXPERIMENTAL and may not work as expected. USE WITH CAUTION!
 
-CMOS control -- /proc/acpi/ibm/cmos
------------------------------------
+CMOS control
+------------
+
+procfs: /proc/acpi/ibm/cmos
+sysfs device attribute: cmos_command
 
 This feature is used internally by the ACPI firmware to control the
 ThinkLight on most newer ThinkPad models. It may also control LCD
 brightness, sounds volume and more, but only on some models.
 
-The commands are non-negative integer numbers:
-
-	echo 0 >/proc/acpi/ibm/cmos
-	echo 1 >/proc/acpi/ibm/cmos
-	echo 2 >/proc/acpi/ibm/cmos
-	...
-
-The range of valid numbers is 0 to 21, but not all have an effect and
-the behavior varies from model to model. Here is the behavior on the
-X40 (tpb is the ThinkPad Buttons utility):
+The range of valid cmos command numbers is 0 to 21, but not all have an
+effect and the behavior varies from model to model.  Here is the behavior
+on the X40 (tpb is the ThinkPad Buttons utility):
 
 	0 - no effect but tpb reports "Volume down"
 	1 - no effect but tpb reports "Volume up"
@@ -407,6 +403,9 @@ X40 (tpb is the ThinkPad Buttons utility):
 	13 - ThinkLight off
 	14 - no effect but tpb reports ThinkLight status change
 
+The cmos command interface is prone to firmware split-brain problems, as
+in newer ThinkPads it is just a compatibility layer.
+
 LED control -- /proc/acpi/ibm/led
 ---------------------------------
 
diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index 79abc68..ba749df 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
@@ -1743,8 +1743,30 @@ static struct ibm_struct bay_driver_data = {
  * CMOS subdriver
  */
 
+/* sysfs cmos_command -------------------------------------------------- */
+static ssize_t cmos_command_store(struct device *dev,
+			    struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	unsigned long cmos_cmd;
+	int res;
+
+	if (parse_strtoul(buf, 21, &cmos_cmd))
+		return -EINVAL;
+
+	res = issue_thinkpad_cmos_command(cmos_cmd);
+	return (res)? res : count;
+}
+
+static struct device_attribute dev_attr_cmos_command =
+	__ATTR(cmos_command, S_IWUSR, NULL, cmos_command_store);
+
+/* --------------------------------------------------------------------- */
+
 static int __init cmos_init(struct ibm_init_struct *iibm)
 {
+	int res;
+
 	vdbg_printk(TPACPI_DBG_INIT,
 		"initializing cmos commands subdriver\n");
 
@@ -1752,9 +1774,19 @@ static int __init cmos_init(struct ibm_init_struct *iibm)
 
 	vdbg_printk(TPACPI_DBG_INIT, "cmos commands are %s\n",
 		str_supported(cmos_handle != NULL));
+
+	res = device_create_file(&tpacpi_pdev->dev, &dev_attr_cmos_command);
+	if (res)
+		return res;
+
 	return (cmos_handle)? 0 : 1;
 }
 
+static void cmos_exit(void)
+{
+	device_remove_file(&tpacpi_pdev->dev, &dev_attr_cmos_command);
+}
+
 static int cmos_read(char *p)
 {
 	int len = 0;
@@ -1795,6 +1827,7 @@ static struct ibm_struct cmos_driver_data = {
 	.name = "cmos",
 	.read = cmos_read,
 	.write = cmos_write,
+	.exit = cmos_exit,
 };
 
 /*************************************************************************
-- 
1.5.1


-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 9/9] ACPI: thinkpad-acpi: update brightness sysfs interface support
  2007-04-24 14:48 [GIT PULL] thinkpad-acpi sysfs support part 1 Henrique de Moraes Holschuh
                   ` (4 preceding siblings ...)
       [not found] ` <11774261003184-git-send-email-hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
@ 2007-04-24 14:48 ` Henrique de Moraes Holschuh
  2007-04-25  6:04 ` [GIT PULL] thinkpad-acpi sysfs support part 1 Len Brown
  6 siblings, 0 replies; 14+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-04-24 14:48 UTC (permalink / raw)
  To: lenb; +Cc: ibm-acpi-devel, linux-acpi, Henrique de Moraes Holschuh

Update the brightness sysfs interface (done through the backlight class) to
be in line with the rest of the thinkpad-acpi driver.

This renames the incorrect, un-obvious, and clash-prone name of "ibm" for
the backlight device to a much more fitting and descriptive
"thinkpad_screen".  This is something I wanted to do for quite a while...

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
---
 Documentation/thinkpad-acpi.txt |   52 ++++++++++++++++++++++++++++++++++----
 drivers/misc/thinkpad_acpi.c    |    5 ++-
 drivers/misc/thinkpad_acpi.h    |    2 +
 3 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/Documentation/thinkpad-acpi.txt b/Documentation/thinkpad-acpi.txt
index 352e8ae..eab4997 100644
--- a/Documentation/thinkpad-acpi.txt
+++ b/Documentation/thinkpad-acpi.txt
@@ -611,19 +611,59 @@ registers contain the current battery capacity, etc. If you experiment
 with this, do send me your results (including some complete dumps with
 a description of the conditions when they were taken.)
 
-LCD brightness control -- /proc/acpi/ibm/brightness
----------------------------------------------------
+LCD brightness control
+----------------------
+
+procfs: /proc/acpi/ibm/brightness
+sysfs backlight device "thinkpad_screen"
 
 This feature allows software control of the LCD brightness on ThinkPad
-models which don't have a hardware brightness slider. The available
-commands are:
+models which don't have a hardware brightness slider.
+
+It has some limitations: the LCD backlight cannot be actually turned on or off
+by this interface, and in many ThinkPad models, the "dim while on battery"
+functionality will be enabled by the BIOS when this interface is used, and
+cannot be controlled.
+
+The backlight control has eight levels, ranging from 0 to 7.  Some of the
+levels may not be distinct.
+
+Procfs notes:
+
+	The available commands are:
 
 	echo up   >/proc/acpi/ibm/brightness
 	echo down >/proc/acpi/ibm/brightness
 	echo 'level <level>' >/proc/acpi/ibm/brightness
 
-The <level> number range is 0 to 7, although not all of them may be
-distinct. The current brightness level is shown in the file.
+Sysfs notes:
+
+The interface is implemented through the backlight sysfs class, which is poorly
+documented at this time.
+
+Locate the thinkpad_screen device under /sys/class/backlight, and inside it
+there will be the following attributes:
+
+	max_brightness:
+		Reads the maximum brightness the hardware can be set to.
+		The minimum is always zero.
+
+	actual_brightness:
+		Reads what brightness the screen is set to at this instant.
+
+	brightness:
+		Writes request the driver to change brightness to the given
+		value.  Reads will tell you what brightness the driver is trying
+		to set the display to when "power" is set to zero and the display
+		has not been dimmed by a kernel power management event.
+
+	power:
+		power management mode, where 0 is "display on", and 1 to 3 will
+		dim the display backlight to brightness level 0 because
+		thinkpad-acpi cannot really turn the backlight off.  Kernel
+		power management events can temporarily increase the current
+		power management level, i.e. they can dim the display.
+
 
 Volume control -- /proc/acpi/ibm/volume
 ---------------------------------------
diff --git a/drivers/misc/thinkpad_acpi.c b/drivers/misc/thinkpad_acpi.c
index ba749df..c0a023c 100644
--- a/drivers/misc/thinkpad_acpi.c
+++ b/drivers/misc/thinkpad_acpi.c
@@ -2414,8 +2414,9 @@ static int __init brightness_init(struct ibm_init_struct *iibm)
 	if (b < 0)
 		return b;
 
-	ibm_backlight_device = backlight_device_register("ibm", NULL, NULL,
-							 &ibm_backlight_data);
+	ibm_backlight_device = backlight_device_register(
+					TPACPI_BACKLIGHT_DEV_NAME, NULL, NULL,
+					&ibm_backlight_data);
 	if (IS_ERR(ibm_backlight_device)) {
 		printk(IBM_ERR "Could not register backlight device\n");
 		return PTR_ERR(ibm_backlight_device);
diff --git a/drivers/misc/thinkpad_acpi.h b/drivers/misc/thinkpad_acpi.h
index 2fe4d61..8348fc6 100644
--- a/drivers/misc/thinkpad_acpi.h
+++ b/drivers/misc/thinkpad_acpi.h
@@ -296,6 +296,8 @@ static int bluetooth_write(char *buf);
  * Brightness (backlight) subdriver
  */
 
+#define TPACPI_BACKLIGHT_DEV_NAME "thinkpad_screen"
+
 static struct backlight_device *ibm_backlight_device;
 static int brightness_offset = 0x31;
 
-- 
1.5.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 6/9] ACPI: thinkpad-acpi: add sysfs support to fan subdriver
  2007-04-24 14:48 ` [PATCH 6/9] ACPI: thinkpad-acpi: add sysfs support to fan subdriver Henrique de Moraes Holschuh
@ 2007-04-25  5:58   ` Len Brown
  2007-04-25 12:49     ` Henrique de Moraes Holschuh
       [not found]     ` <200704250158.18297.lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
  0 siblings, 2 replies; 14+ messages in thread
From: Len Brown @ 2007-04-25  5:58 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh; +Cc: ibm-acpi-devel, linux-acpi

I'm glad to see the proc->sysfs conversion.
I'm glad to see compatibility with hwmon.
I think it will be handy to read fan speed.

I'm doubtful about the utility of setting fan speed.
It seems like it is enough rope for users to hang themselves,
and the need for a "safety net" seems to confirm that.

-Len

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [GIT PULL] thinkpad-acpi sysfs support part 1
  2007-04-24 14:48 [GIT PULL] thinkpad-acpi sysfs support part 1 Henrique de Moraes Holschuh
                   ` (5 preceding siblings ...)
  2007-04-24 14:48 ` [PATCH 9/9] ACPI: thinkpad-acpi: update brightness sysfs interface support Henrique de Moraes Holschuh
@ 2007-04-25  6:04 ` Len Brown
  6 siblings, 0 replies; 14+ messages in thread
From: Len Brown @ 2007-04-25  6:04 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh; +Cc: ibm-acpi-devel, linux-acpi

On Tuesday 24 April 2007 10:48, Henrique de Moraes Holschuh wrote:
>>the following *new* patches:
> 
>       ACPI: thinkpad-acpi: register with the device model
>       ACPI: thinkpad-acpi: driver sysfs conversion
>       ACPI: thinkpad-acpi: add infrastructure for the sysfs device attributes
>       ACPI: thinkpad-acpi: protect fan and hotkey data structures
>       ACPI: thinkpad-acpi: add sysfs support to the thermal subdriver
>       ACPI: thinkpad-acpi: add sysfs support to fan subdriver
>       ACPI: thinkpad-acpi: add a safety net for TPEC fan control mode
>       ACPI: thinkpad-acpi: add sysfs support to the cmos command subdriver
>       ACPI: thinkpad-acpi: update brightness sysfs interface support

applied to acpi-test

thanks,
-Len

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 6/9] ACPI: thinkpad-acpi: add sysfs support to fan subdriver
  2007-04-25  5:58   ` Len Brown
@ 2007-04-25 12:49     ` Henrique de Moraes Holschuh
       [not found]     ` <200704250158.18297.lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
  1 sibling, 0 replies; 14+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-04-25 12:49 UTC (permalink / raw)
  To: Len Brown; +Cc: ibm-acpi-devel, linux-acpi

On Wed, 25 Apr 2007, Len Brown wrote:
> I'm glad to see the proc->sysfs conversion.
> I'm glad to see compatibility with hwmon.
> I think it will be handy to read fan speed.
> 
> I'm doubtful about the utility of setting fan speed.
> It seems like it is enough rope for users to hang themselves,
> and the need for a "safety net" seems to confirm that.

The safety net was added out of sheer paranoia in the last few days, nobody
*ever* reported any problems with this functionality, and I have never seen
a ThinkPad DSDT that does not support the AUTO mode in HFSP fan control
mode.

I was doing a final review before sending the patches to you, looked at that
code, and though "hey, wait.  There *is* a possible worst case scenario
here.  Nobody ever reported it, but what the heck...".

Fan speed control is a feature thinkpad owners want, it is in use for about
two years now.  Before I started maintaining ibm-acpi / thinkpad-acpi, it
already supported basic fan control, and a lot of people that wanted more
used some unwise tricks to do it (like playing directly with the EC).

What we have now is at least *known safe* for the A31, T2x, T3x, T4x, T6x,
X3x, X4x, X6x, R4x, R5x, R6x, Z6x in most commonly used BIOS/EC firmware
revisions.  All of those models are known *not* to need any safety nets,
either.  And earlier models don't support the codepath we are talking about
anyway, so the driver doesn't enable it.

Besides, unless the user decides to tell it to do something, the driver
changes exactly *nothing* on the EC (no writes at all), so the ThinkPad will
just continue doing whatever its firmware default operating mode and DSDT
told it to.

I won't say fan speed control is 100% safe, it would be foolhardy to do so,
especially if you factor in firmware bugs.  But it is safe enough that
nobody ever reported any problems with it in two years, so it is good enough
for the population that would want to use it.  And the driver is intelligent
enough to not change EC state unless it is actually commanded to do so.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 6/9] ACPI: thinkpad-acpi: add sysfs support to fan subdriver
       [not found]     ` <200704250158.18297.lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2007-04-25 12:59       ` Henrique de Moraes Holschuh
  0 siblings, 0 replies; 14+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-04-25 12:59 UTC (permalink / raw)
  To: Len Brown
  Cc: ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA

On Wed, 25 Apr 2007, Len Brown wrote:
> It seems like it is enough rope for users to hang themselves,
> and the need for a "safety net" seems to confirm that.

I will cook up a patch that removes the experimental status of fan control,
and adds in its place an explicit toggle to enable fan control (as opposed
to fan monitoring, i.e. read-only operations) as a module parameter.  And it
will default to "fan control is forbidden".

That way, the user has to effectively tell the kernel, through the kernel
command line or a module parameter, that he wants thinkpad-acpi to be
capable of writing fan control information to the EC.  Otherwise the driver
will return -EPERM on all fan control operations, and just allow one to read
fan status.

What do you think?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2007-04-25 12:59 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-04-24 14:48 [GIT PULL] thinkpad-acpi sysfs support part 1 Henrique de Moraes Holschuh
2007-04-24 14:48 ` [PATCH 1/9] ACPI: thinkpad-acpi: register with the device model Henrique de Moraes Holschuh
2007-04-24 14:48 ` [PATCH 3/9] ACPI: thinkpad-acpi: add infrastructure for the sysfs device attributes Henrique de Moraes Holschuh
2007-04-24 14:48 ` [PATCH 6/9] ACPI: thinkpad-acpi: add sysfs support to fan subdriver Henrique de Moraes Holschuh
2007-04-25  5:58   ` Len Brown
2007-04-25 12:49     ` Henrique de Moraes Holschuh
     [not found]     ` <200704250158.18297.lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2007-04-25 12:59       ` Henrique de Moraes Holschuh
2007-04-24 14:48 ` [PATCH 7/9] ACPI: thinkpad-acpi: add a safety net for TPEC fan control mode Henrique de Moraes Holschuh
     [not found] ` <11774261003184-git-send-email-hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
2007-04-24 14:48   ` [PATCH 2/9] ACPI: thinkpad-acpi: driver sysfs conversion Henrique de Moraes Holschuh
2007-04-24 14:48   ` [PATCH 4/9] ACPI: thinkpad-acpi: protect fan and hotkey data structures Henrique de Moraes Holschuh
2007-04-24 14:48   ` [PATCH 5/9] ACPI: thinkpad-acpi: add sysfs support to the thermal subdriver Henrique de Moraes Holschuh
2007-04-24 14:48   ` [PATCH 8/9] ACPI: thinkpad-acpi: add sysfs support to the cmos command subdriver Henrique de Moraes Holschuh
2007-04-24 14:48 ` [PATCH 9/9] ACPI: thinkpad-acpi: update brightness sysfs interface support Henrique de Moraes Holschuh
2007-04-25  6:04 ` [GIT PULL] thinkpad-acpi sysfs support part 1 Len Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox