All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] ibm-acpi changes acpi-test/2.6.22
@ 2007-03-12  0:20 Henrique de Moraes Holschuh
  2007-03-12  0:20 ` [PATCH 1/6] ACPI: ibm-acpi: kill trailing whitespace Henrique de Moraes Holschuh
  2007-03-12  0:54 ` [GIT PULL] ibm-acpi changes acpi-test/2.6.22 Len Brown
  0 siblings, 2 replies; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-12  0:20 UTC (permalink / raw)
  To: lenb-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

Len,

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

to receive the following patches:
      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

Please merge them into acpi-test, targetting 2.6.22.



-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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

* [PATCH 1/6] ACPI: ibm-acpi: kill trailing whitespace
  2007-03-12  0:20 [GIT PULL] ibm-acpi changes acpi-test/2.6.22 Henrique de Moraes Holschuh
@ 2007-03-12  0:20 ` Henrique de Moraes Holschuh
       [not found]   ` <11736588493322-git-send-email-hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
  2007-03-12  0:54 ` [GIT PULL] ibm-acpi changes acpi-test/2.6.22 Len Brown
  1 sibling, 1 reply; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-12  0:20 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, ibm-acpi-devel, Henrique de Moraes Holschuh

I shall protect the ibm-acpi city against the invasion of the barbarian
blanks!  To the unforgiving jaws of sed s/[[:blank:]]\+$// they go!

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
---
 Documentation/ibm-acpi.txt |    6 +++---
 drivers/acpi/ibm_acpi.c    |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/ibm-acpi.txt b/Documentation/ibm-acpi.txt
index 0132d36..cdcef01 100644
--- a/Documentation/ibm-acpi.txt
+++ b/Documentation/ibm-acpi.txt
@@ -21,7 +21,7 @@ detailed description):
 
 	- Fn key combinations
 	- Bluetooth enable and disable
-	- video output switching, expansion control	
+	- video output switching, expansion control
 	- ThinkLight on and off
 	- limited docking and undocking
 	- UltraBay eject
@@ -472,7 +472,7 @@ This feature dumps the values of 256 embedded controller
 registers. Values which have changed since the last time the registers
 were dumped are marked with a star:
 
-[root@x40 ibm-acpi]# cat /proc/acpi/ibm/ecdump 
+[root@x40 ibm-acpi]# cat /proc/acpi/ibm/ecdump
 EC       +00 +01 +02 +03 +04 +05 +06 +07 +08 +09 +0a +0b +0c +0d +0e +0f
 EC 0x00:  a7  47  87  01  fe  96  00  08  01  00  cb  00  00  00  40  00
 EC 0x10:  00  00  ff  ff  f4  3c  87  09  01  ff  42  01  ff  ff  0d  00
@@ -503,7 +503,7 @@ vary. The second ensures that the fan-related values do vary, since
 the fan speed fluctuates a bit. The third will (hopefully) mark the
 fan register with a star:
 
-[root@x40 ibm-acpi]# cat /proc/acpi/ibm/ecdump 
+[root@x40 ibm-acpi]# cat /proc/acpi/ibm/ecdump
 EC       +00 +01 +02 +03 +04 +05 +06 +07 +08 +09 +0a +0b +0c +0d +0e +0f
 EC 0x00:  a7  47  87  01  fe  96  00  08  01  00  cb  00  00  00  40  00
 EC 0x10:  00  00  ff  ff  f4  3c  87  09  01  ff  42  01  ff  ff  0d  00
diff --git a/drivers/acpi/ibm_acpi.c b/drivers/acpi/ibm_acpi.c
index 3690136..a889dc5 100644
--- a/drivers/acpi/ibm_acpi.c
+++ b/drivers/acpi/ibm_acpi.c
@@ -28,7 +28,7 @@
  *  2006-11-22	0.13	new maintainer
  *  			changelog now lives in git commit history, and will
  *  			not be updated further in-file.
- *  
+ *
  *  2005-08-17  0.12	fix compilation on 2.6.13-rc kernels
  *  2005-03-17	0.11	support for 600e, 770x
  *			    thanks to Jamie Lentin <lentinj@dial.pipex.com>
@@ -38,7 +38,7 @@
  *			experimental brightness control
  *			experimental volume control
  *			experimental fan enable/disable
- *  2005-01-16	0.10	fix module loading on R30, R31 
+ *  2005-01-16	0.10	fix module loading on R30, R31
  *  2005-01-16	0.9	support for 570, R30, R31
  *			ultrabay support on A22p, A3x
  *			limit arg for cmos, led, beep, drop experimental status
@@ -161,7 +161,7 @@ IBM_HANDLE(dock, root, "\\_SB.GDCK",	/* X30, X31, X40 */
 #ifdef CONFIG_ACPI_IBM_BAY
 IBM_HANDLE(bay, root, "\\_SB.PCI.IDE.SECN.MAST",	/* 570 */
 	   "\\_SB.PCI0.IDE0.IDES.IDSM",	/* 600e/x, 770e, 770x */
-	   "\\_SB.PCI0.SATA.SCND.MSTR",	/* T60, X60, Z60 */ 
+	   "\\_SB.PCI0.SATA.SCND.MSTR",	/* T60, X60, Z60 */
 	   "\\_SB.PCI0.IDE0.SCND.MSTR",	/* all others */
     );				/* A21e, R30, R31 */
 
-- 
1.5.0.3


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

* [PATCH 2/6] ACPI: ibm-acpi: rename some identifiers
       [not found]   ` <11736588493322-git-send-email-hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
@ 2007-03-12  0:20     ` Henrique de Moraes Holschuh
       [not found]       ` <11736588491476-git-send-email-hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
  0 siblings, 1 reply; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-12  0:20 UTC (permalink / raw)
  To: lenb-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA, Henrique de Moraes Holschuh,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

Rename some identifiers so that they are more in tune with the rest of the
driver code, or less generic.

Signed-off-by: Henrique de Moraes Holschuh <hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
---
 drivers/acpi/ibm_acpi.c |   43 ++++++++++++++++++++++++-------------------
 1 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/acpi/ibm_acpi.c b/drivers/acpi/ibm_acpi.c
index a889dc5..3363eda 100644
--- a/drivers/acpi/ibm_acpi.c
+++ b/drivers/acpi/ibm_acpi.c
@@ -506,7 +506,7 @@ static int ibm_acpi_driver_init(void)
 	return 0;
 }
 
-static int driver_read(char *p)
+static int ibm_acpi_driver_read(char *p)
 {
 	int len = 0;
 
@@ -1310,9 +1310,9 @@ static const int led_exp_hlbl[] = { 0, 0, 1 };	/* led# * */
 static const int led_exp_hlcl[] = { 0, 1, 1 };	/* led# * */
 static const int led_led_arg1[] = { 0, 0x80, 0xc0 };
 
-#define EC_HLCL 0x0c
-#define EC_HLBL 0x0d
-#define EC_HLMS 0x0e
+#define IBMACPI_LED_EC_HLCL 0x0c
+#define IBMACPI_LED_EC_HLBL 0x0d
+#define IBMACPI_LED_EC_HLMS 0x0e
 
 static int led_write(char *buf)
 {
@@ -1344,13 +1344,15 @@ static int led_write(char *buf)
 		} else if (led_supported == IBMACPI_LED_OLD) {
 			/* 600e/x, 770e, 770x, A21e, A2xm/p, T20-22, X20 */
 			led = 1 << led;
-			ret = ec_write(EC_HLMS, led);
+			ret = ec_write(IBMACPI_LED_EC_HLMS, led);
 			if (ret >= 0)
 				ret =
-				    ec_write(EC_HLBL, led * led_exp_hlbl[ind]);
+				    ec_write(IBMACPI_LED_EC_HLBL,
+				    	     led * led_exp_hlbl[ind]);
 			if (ret >= 0)
 				ret =
-				    ec_write(EC_HLCL, led * led_exp_hlcl[ind]);
+				    ec_write(IBMACPI_LED_EC_HLCL,
+				    	     led * led_exp_hlcl[ind]);
 			if (ret < 0)
 				return ret;
 		} else {
@@ -1657,8 +1659,8 @@ static int brightness_read(char *p)
 	return len;
 }
 
-#define BRIGHTNESS_UP	4
-#define BRIGHTNESS_DOWN	5
+#define TP_CMOS_BRIGHTNESS_UP	4
+#define TP_CMOS_BRIGHTNESS_DOWN	5
 
 static int brightness_set(int value)
 {
@@ -1667,7 +1669,8 @@ static int brightness_set(int value)
 
 	value &= 7;
 
-	cmos_cmd = value > current_value ? BRIGHTNESS_UP : BRIGHTNESS_DOWN;
+	cmos_cmd = value > current_value ?
+		   TP_CMOS_BRIGHTNESS_UP : TP_CMOS_BRIGHTNESS_DOWN;
 	inc = value > current_value ? 1 : -1;
 	for (i = current_value; i != value; i += inc) {
 		if (!cmos_eval(cmos_cmd))
@@ -1769,9 +1772,9 @@ static int volume_read(char *p)
 	return len;
 }
 
-#define VOLUME_DOWN	0
-#define VOLUME_UP	1
-#define VOLUME_MUTE	2
+#define TP_CMOS_VOLUME_DOWN	0
+#define TP_CMOS_VOLUME_UP	1
+#define TP_CMOS_VOLUME_MUTE	2
 
 static int volume_write(char *buf)
 {
@@ -1805,7 +1808,8 @@ static int volume_write(char *buf)
 			return -EINVAL;
 
 		if (new_level != level) {	/* mute doesn't change */
-			cmos_cmd = new_level > level ? VOLUME_UP : VOLUME_DOWN;
+			cmos_cmd = new_level > level ?
+					TP_CMOS_VOLUME_UP : TP_CMOS_VOLUME_DOWN;
 			inc = new_level > level ? 1 : -1;
 
 			if (mute && (!cmos_eval(cmos_cmd) ||
@@ -1817,14 +1821,15 @@ static int volume_write(char *buf)
 				    !acpi_ec_write(volume_offset, i + inc))
 					return -EIO;
 
-			if (mute && (!cmos_eval(VOLUME_MUTE) ||
+			if (mute && (!cmos_eval(TP_CMOS_VOLUME_MUTE) ||
 				     !acpi_ec_write(volume_offset,
 						    new_level + mute)))
 				return -EIO;
 		}
 
 		if (new_mute != mute) {	/* level doesn't change */
-			cmos_cmd = new_mute ? VOLUME_MUTE : VOLUME_UP;
+			cmos_cmd = new_mute ?
+				   TP_CMOS_VOLUME_MUTE : TP_CMOS_VOLUME_UP;
 
 			if (!cmos_eval(cmos_cmd) ||
 			    !acpi_ec_write(volume_offset, level + new_mute))
@@ -2315,7 +2320,7 @@ static struct ibm_struct ibms[] = {
 	{
 	 .name = "driver",
 	 .init = ibm_acpi_driver_init,
-	 .read = driver_read,
+	 .read = ibm_acpi_driver_read,
 	 },
 	{
 	 .name = "hotkey",
@@ -2529,7 +2534,7 @@ static int __init ibm_device_add(struct acpi_device *device)
 	return 0;
 }
 
-static int __init register_driver(struct ibm_struct *ibm)
+static int __init register_ibmacpi_subdriver(struct ibm_struct *ibm)
 {
 	int ret;
 
@@ -2562,7 +2567,7 @@ static int __init ibm_init(struct ibm_struct *ibm)
 		return 0;
 
 	if (ibm->hid) {
-		ret = register_driver(ibm);
+		ret = register_ibmacpi_subdriver(ibm);
 		if (ret < 0)
 			return ret;
 		ibm->driver_registered = 1;
-- 
1.5.0.3


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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

* [PATCH 3/6] ACPI: ibm-acpi: add header file
       [not found]       ` <11736588491476-git-send-email-hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
@ 2007-03-12  0:20         ` Henrique de Moraes Holschuh
       [not found]           ` <11736588493502-git-send-email-hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
  0 siblings, 1 reply; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-12  0:20 UTC (permalink / raw)
  To: lenb-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA, Henrique de Moraes Holschuh,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

Add a (private) header file for ibm-acpi, and move type definitions and
ThinkPad driver constants to the new header file.

This patch has no functional changes.

Signed-off-by: Henrique de Moraes Holschuh <hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
---
 drivers/acpi/ibm_acpi.c |  137 +---------------
 drivers/acpi/ibm_acpi.h |  437 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 438 insertions(+), 136 deletions(-)
 create mode 100644 drivers/acpi/ibm_acpi.h

diff --git a/drivers/acpi/ibm_acpi.c b/drivers/acpi/ibm_acpi.c
index 3363eda..59cbafc 100644
--- a/drivers/acpi/ibm_acpi.c
+++ b/drivers/acpi/ibm_acpi.c
@@ -78,44 +78,13 @@
  *  2004-08-09	0.1	initial release, support for X series
  */
 
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/types.h>
-#include <linux/string.h>
-
-#include <linux/proc_fs.h>
-#include <linux/backlight.h>
-#include <linux/fb.h>
-#include <asm/uaccess.h>
-
-#include <linux/dmi.h>
-#include <linux/jiffies.h>
-#include <linux/workqueue.h>
-
-#include <acpi/acpi_drivers.h>
-#include <acpi/acnamesp.h>
-
-#define IBM_NAME "ibm"
-#define IBM_DESC "IBM ThinkPad ACPI Extras"
-#define IBM_FILE "ibm_acpi"
-#define IBM_URL "http://ibm-acpi.sf.net/"
+#include "ibm_acpi.h"
 
 MODULE_AUTHOR("Borislav Deianov, Henrique de Moraes Holschuh");
 MODULE_DESCRIPTION(IBM_DESC);
 MODULE_VERSION(IBM_VERSION);
 MODULE_LICENSE("GPL");
 
-#define IBM_DIR IBM_NAME
-
-#define IBM_LOG IBM_FILE ": "
-#define IBM_ERR	   KERN_ERR    IBM_LOG
-#define IBM_NOTICE KERN_NOTICE IBM_LOG
-#define IBM_INFO   KERN_INFO   IBM_LOG
-#define IBM_DEBUG  KERN_DEBUG  IBM_LOG
-
-#define IBM_MAX_ACPI_ARGS 3
-
 #define __unused __attribute__ ((unused))
 
 static int experimental;
@@ -207,22 +176,6 @@ IBM_HANDLE(sfan, ec, "SFAN",	/* 570 */
 	   "JFNS",		/* 770x-JL */
     );				/* all others */
 
-#define IBM_HKEY_HID	"IBM0068"
-#define IBM_PCI_HID	"PNP0A03"
-
-enum thermal_access_mode {
-	IBMACPI_THERMAL_NONE = 0,	/* No thermal support */
-	IBMACPI_THERMAL_ACPI_TMP07,	/* Use ACPI TMP0-7 */
-	IBMACPI_THERMAL_ACPI_UPDT,	/* Use ACPI TMP0-7 with UPDT */
-	IBMACPI_THERMAL_TPEC_8,		/* Use ACPI EC regs, 8 sensors */
-	IBMACPI_THERMAL_TPEC_16,	/* Use ACPI EC regs, 16 sensors */
-};
-
-#define IBMACPI_MAX_THERMAL_SENSORS 16	/* Max thermal sensors supported */
-struct ibm_thermal_sensors_struct {
-	s32 temp[IBMACPI_MAX_THERMAL_SENSORS];
-};
-
 /*
  * FAN ACCESS MODES
  *
@@ -323,72 +276,12 @@ struct ibm_thermal_sensors_struct {
  * 	but the ACPI tables just mention level 7.
  */
 
-enum fan_status_access_mode {
-	IBMACPI_FAN_NONE = 0,		/* No fan status or control */
-	IBMACPI_FAN_RD_ACPI_GFAN,	/* Use ACPI GFAN */
-	IBMACPI_FAN_RD_TPEC,		/* Use ACPI EC regs 0x2f, 0x84-0x85 */
-};
-
-enum fan_control_access_mode {
-	IBMACPI_FAN_WR_NONE = 0,	/* No fan control */
-	IBMACPI_FAN_WR_ACPI_SFAN,	/* Use ACPI SFAN */
-	IBMACPI_FAN_WR_TPEC,		/* Use ACPI EC reg 0x2f */
-	IBMACPI_FAN_WR_ACPI_FANS,	/* Use ACPI FANS and EC reg 0x2f */
-};
-
-enum fan_control_commands {
-	IBMACPI_FAN_CMD_SPEED 	= 0x0001,	/* speed command */
-	IBMACPI_FAN_CMD_LEVEL 	= 0x0002,	/* level command  */
-	IBMACPI_FAN_CMD_ENABLE	= 0x0004,	/* enable/disable cmd,
-						 * and also watchdog cmd */
-};
-
-enum {					/* Fan control constants */
-	fan_status_offset = 0x2f,	/* EC register 0x2f */
-	fan_rpm_offset = 0x84,		/* EC register 0x84: LSB, 0x85 MSB (RPM)
-					 * 0x84 must be read before 0x85 */
-
-	IBMACPI_FAN_EC_DISENGAGED 	= 0x40,	/* EC mode: tachometer
-						 * disengaged */
-	IBMACPI_FAN_EC_AUTO		= 0x80, /* EC mode: auto fan
-						 * control */
-};
-
 static char *ibm_thinkpad_ec_found = NULL;
 
-struct ibm_struct {
-	char *name;
-	char param[32];
-
-	char *hid;
-	struct acpi_driver *driver;
-
-	int (*init) (void);
-	int (*read) (char *);
-	int (*write) (char *);
-	void (*exit) (void);
-
-	void (*notify) (struct ibm_struct *, u32);
-	acpi_handle *handle;
-	int type;
-	struct acpi_device *device;
-
-	int driver_registered;
-	int proc_created;
-	int init_called;
-	int notify_installed;
-
-	int experimental;
-};
-
 static struct proc_dir_entry *proc_dir = NULL;
 
 static struct backlight_device *ibm_backlight_device = NULL;
 
-#define onoff(status,bit) ((status) & (1 << (bit)) ? "on" : "off")
-#define enabled(status,bit) ((status) & (1 << (bit)) ? "enabled" : "disabled")
-#define strlencmp(a,b) (strncmp((a), (b), strlen(b)))
-
 static int acpi_evalf(acpi_handle handle,
 		      void *res, char *method, char *fmt, ...)
 {
@@ -775,13 +668,6 @@ static int wan_write(char *buf)
 	return 0;
 }
 
-enum video_access_mode {
-	IBMACPI_VIDEO_NONE = 0,
-	IBMACPI_VIDEO_570,	/* 570 */
-	IBMACPI_VIDEO_770,	/* 600e/x, 770e, 770x */
-	IBMACPI_VIDEO_NEW,	/* all others */
-};
-
 static enum video_access_mode video_supported;
 static int video_orig_autosw;
 
@@ -1248,12 +1134,6 @@ static int cmos_write(char *buf)
 	return 0;
 }
 
-enum led_access_mode {
-	IBMACPI_LED_NONE = 0,
-	IBMACPI_LED_570,	/* 570 */
-	IBMACPI_LED_OLD,	/* 600e/x, 770e, 770x, A21e, A2xm/p, T20-22, X20-21 */
-	IBMACPI_LED_NEW,	/* all others */
-};
 static enum led_access_mode led_supported;
 
 static int led_init(void)
@@ -1310,10 +1190,6 @@ static const int led_exp_hlbl[] = { 0, 0, 1 };	/* led# * */
 static const int led_exp_hlcl[] = { 0, 1, 1 };	/* led# * */
 static const int led_led_arg1[] = { 0, 0x80, 0xc0 };
 
-#define IBMACPI_LED_EC_HLCL 0x0c
-#define IBMACPI_LED_EC_HLBL 0x0d
-#define IBMACPI_LED_EC_HLMS 0x0e
-
 static int led_write(char *buf)
 {
 	char *cmd;
@@ -1629,8 +1505,6 @@ static int ecdump_write(char *buf)
 	return 0;
 }
 
-static int brightness_offset = 0x31;
-
 static int brightness_get(struct backlight_device *bd)
 {
 	u8 level;
@@ -1659,9 +1533,6 @@ static int brightness_read(char *p)
 	return len;
 }
 
-#define TP_CMOS_BRIGHTNESS_UP	4
-#define TP_CMOS_BRIGHTNESS_DOWN	5
-
 static int brightness_set(int value)
 {
 	int cmos_cmd, inc, i;
@@ -1752,8 +1623,6 @@ static void brightness_exit(void)
 	}
 }
 
-static int volume_offset = 0x30;
-
 static int volume_read(char *p)
 {
 	int len = 0;
@@ -1772,10 +1641,6 @@ static int volume_read(char *p)
 	return len;
 }
 
-#define TP_CMOS_VOLUME_DOWN	0
-#define TP_CMOS_VOLUME_UP	1
-#define TP_CMOS_VOLUME_MUTE	2
-
 static int volume_write(char *buf)
 {
 	int cmos_cmd, inc, i;
diff --git a/drivers/acpi/ibm_acpi.h b/drivers/acpi/ibm_acpi.h
new file mode 100644
index 0000000..7ebaaa4
--- /dev/null
+++ b/drivers/acpi/ibm_acpi.h
@@ -0,0 +1,437 @@
+/*
+ *  ibm_acpi.h - IBM ThinkPad ACPI Extras
+ *
+ *
+ *  Copyright (C) 2004-2005 Borislav Deianov <borislav-iA+eEnwkJgzk1uMJSBkQmQ@public.gmane.org>
+ *  Copyright (C) 2006-2007 Henrique de Moraes Holschuh <hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ *  02110-1301, USA.
+ */
+
+#ifndef __IBM_ACPI_H__
+#define __IBM_ACPI_H__
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/string.h>
+
+#include <linux/proc_fs.h>
+#include <linux/backlight.h>
+#include <linux/fb.h>
+#include <asm/uaccess.h>
+
+#include <linux/dmi.h>
+#include <linux/jiffies.h>
+#include <linux/workqueue.h>
+
+#include <acpi/acpi_drivers.h>
+#include <acpi/acnamesp.h>
+
+
+/****************************************************************************
+ * Main driver
+ */
+
+#define IBM_NAME "ibm"
+#define IBM_DESC "IBM ThinkPad ACPI Extras"
+#define IBM_FILE "ibm_acpi"
+#define IBM_URL "http://ibm-acpi.sf.net/"
+
+#define IBM_DIR IBM_NAME
+
+#define IBM_LOG IBM_FILE ": "
+#define IBM_ERR	   KERN_ERR    IBM_LOG
+#define IBM_NOTICE KERN_NOTICE IBM_LOG
+#define IBM_INFO   KERN_INFO   IBM_LOG
+#define IBM_DEBUG  KERN_DEBUG  IBM_LOG
+
+#define IBM_MAX_ACPI_ARGS 3
+
+/* ThinkPad CMOS commands */
+#define TP_CMOS_VOLUME_DOWN	0
+#define TP_CMOS_VOLUME_UP	1
+#define TP_CMOS_VOLUME_MUTE	2
+#define TP_CMOS_BRIGHTNESS_UP	4
+#define TP_CMOS_BRIGHTNESS_DOWN	5
+
+#define onoff(status,bit) ((status) & (1 << (bit)) ? "on" : "off")
+#define enabled(status,bit) ((status) & (1 << (bit)) ? "enabled" : "disabled")
+#define strlencmp(a,b) (strncmp((a), (b), strlen(b)))
+
+/* ACPI HIDs */
+#define IBM_HKEY_HID    "IBM0068"
+#define IBM_PCI_HID     "PNP0A03"
+
+/* ACPI helpers */
+static int acpi_evalf(acpi_handle handle,
+		      void *res, char *method, char *fmt, ...);
+static int acpi_ec_read(int i, u8 * p);
+static int acpi_ec_write(int i, u8 v);
+static int _sta(acpi_handle handle);
+
+/* ACPI handles */
+static acpi_handle root_handle;			/* root namespace */
+static acpi_handle ec_handle;			/* EC */
+static acpi_handle ecrd_handle, ecwr_handle;	/* 570 EC access */
+static acpi_handle cmos_handle, hkey_handle;	/* basic thinkpad handles */
+
+static void ibm_handle_init(char *name,
+		   acpi_handle * handle, acpi_handle parent,
+		   char **paths, int num_paths, char **path);
+#define IBM_HANDLE_INIT(object)						\
+	ibm_handle_init(#object, &object##_handle, *object##_parent,	\
+		object##_paths, ARRAY_SIZE(object##_paths), &object##_path)
+
+/* procfs support */
+static struct proc_dir_entry *proc_dir;
+static int ibm_acpi_driver_init(void);
+static int ibm_acpi_driver_read(char *p);
+
+/* procfs helpers */
+static int dispatch_read(char *page, char **start, off_t off, int count,
+		int *eof, void *data);
+static int dispatch_write(struct file *file, const char __user * userbuf,
+		unsigned long count, void *data);
+static char *next_cmd(char **cmds);
+
+/* Module */
+static int experimental;
+static char *ibm_thinkpad_ec_found;
+
+static char* check_dmi_for_ec(void);
+static int acpi_ibm_init(void);
+static void acpi_ibm_exit(void);
+
+
+/****************************************************************************
+ * Subdrivers
+ */
+
+struct ibm_struct {
+	char *name;
+	char param[32];
+
+	char *hid;
+	struct acpi_driver *driver;
+
+	int (*init) (void);
+	int (*read) (char *);
+	int (*write) (char *);
+	void (*exit) (void);
+
+	void (*notify) (struct ibm_struct *, u32);
+	acpi_handle *handle;
+	int type;
+	struct acpi_device *device;
+
+	int driver_registered;
+	int proc_created;
+	int init_called;
+	int notify_installed;
+
+	int experimental;
+};
+
+static struct ibm_struct ibms[];
+static int set_ibm_param(const char *val, struct kernel_param *kp);
+static int ibm_init(struct ibm_struct *ibm);
+static void ibm_exit(struct ibm_struct *ibm);
+
+/* ACPI devices */
+static void dispatch_notify(acpi_handle handle, u32 event, void *data);
+static int setup_notify(struct ibm_struct *ibm);
+static int ibm_device_add(struct acpi_device *device);
+static int register_ibmacpi_subdriver(struct ibm_struct *ibm);
+
+
+/*
+ * Bay subdriver
+ */
+
+#ifdef CONFIG_ACPI_IBM_BAY
+static int bay_status_supported, bay_eject_supported;
+static int bay_status2_supported, bay_eject2_supported;
+
+static acpi_handle bay_handle, bay_ej_handle;
+static acpi_handle bay2_handle, bay2_ej_handle;
+
+static int bay_init(void);
+static void bay_notify(struct ibm_struct *ibm, u32 event);
+static int bay_read(char *p);
+static int bay_write(char *buf);
+#endif /* CONFIG_ACPI_IBM_BAY */
+
+
+/*
+ * Beep subdriver
+ */
+
+static acpi_handle beep_handle;
+
+static int beep_read(char *p);
+static int beep_write(char *buf);
+
+
+/*
+ * Bluetooth subdriver
+ */
+
+static int bluetooth_supported;
+
+static int bluetooth_init(void);
+static int bluetooth_status(void);
+static int bluetooth_read(char *p);
+static int bluetooth_write(char *buf);
+
+
+/*
+ * Brightness (backlight) subdriver
+ */
+
+static struct backlight_device *ibm_backlight_device;
+static int brightness_offset = 0x31;
+
+static int brightness_init(void);
+static void brightness_exit(void);
+static int brightness_get(struct backlight_device *bd);
+static int brightness_set(int value);
+static int brightness_update_status(struct backlight_device *bd);
+static int brightness_read(char *p);
+static int brightness_write(char *buf);
+
+
+/*
+ * CMOS subdriver
+ */
+
+static int cmos_eval(int cmos_cmd);
+static int cmos_read(char *p);
+static int cmos_write(char *buf);
+
+
+/*
+ * Dock subdriver
+ */
+
+static acpi_handle pci_handle;
+#ifdef CONFIG_ACPI_IBM_DOCK
+static acpi_handle dock_handle;
+
+static void dock_notify(struct ibm_struct *ibm, u32 event);
+static int dock_read(char *p);
+static int dock_write(char *buf);
+#endif /* CONFIG_ACPI_IBM_DOCK */
+
+
+/*
+ * EC dump subdriver
+ */
+
+static int ecdump_read(char *p) ;
+static int ecdump_write(char *buf);
+
+
+/*
+ * Fan subdriver
+ */
+
+enum {					/* Fan control constants */
+	fan_status_offset = 0x2f,	/* EC register 0x2f */
+	fan_rpm_offset = 0x84,		/* EC register 0x84: LSB, 0x85 MSB (RPM)
+					 * 0x84 must be read before 0x85 */
+
+	IBMACPI_FAN_EC_DISENGAGED 	= 0x40,	/* EC mode: tachometer
+						 * disengaged */
+	IBMACPI_FAN_EC_AUTO		= 0x80, /* EC mode: auto fan
+						 * control */
+};
+
+enum fan_status_access_mode {
+	IBMACPI_FAN_NONE = 0,		/* No fan status or control */
+	IBMACPI_FAN_RD_ACPI_GFAN,	/* Use ACPI GFAN */
+	IBMACPI_FAN_RD_TPEC,		/* Use ACPI EC regs 0x2f, 0x84-0x85 */
+};
+
+enum fan_control_access_mode {
+	IBMACPI_FAN_WR_NONE = 0,	/* No fan control */
+	IBMACPI_FAN_WR_ACPI_SFAN,	/* Use ACPI SFAN */
+	IBMACPI_FAN_WR_TPEC,		/* Use ACPI EC reg 0x2f */
+	IBMACPI_FAN_WR_ACPI_FANS,	/* Use ACPI FANS and EC reg 0x2f */
+};
+
+enum fan_control_commands {
+	IBMACPI_FAN_CMD_SPEED 	= 0x0001,	/* speed command */
+	IBMACPI_FAN_CMD_LEVEL 	= 0x0002,	/* level command  */
+	IBMACPI_FAN_CMD_ENABLE	= 0x0004,	/* enable/disable cmd,
+						 * and also watchdog cmd */
+};
+
+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 int fan_control_status_known;
+static u8 fan_control_initial_status;
+static int fan_watchdog_maxinterval;
+
+static acpi_handle fans_handle, gfan_handle, sfan_handle;
+
+static int fan_init(void);
+static void fan_exit(void);
+static int fan_get_status(u8 *status);
+static int fan_get_speed(unsigned int *speed);
+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_enable(void);
+static int fan_set_disable(void);
+static int fan_set_speed(int speed);
+static int fan_read(char *p);
+static int fan_write(char *buf);
+static int fan_write_cmd_level(const char *cmd, int *rc);
+static int fan_write_cmd_enable(const char *cmd, int *rc);
+static int fan_write_cmd_disable(const char *cmd, int *rc);
+static int fan_write_cmd_speed(const char *cmd, int *rc);
+static int fan_write_cmd_watchdog(const char *cmd, int *rc);
+
+
+/*
+ * Hotkey subdriver
+ */
+
+static int hotkey_supported;
+static int hotkey_mask_supported;
+static int hotkey_orig_status;
+static int hotkey_orig_mask;
+
+static int hotkey_init(void);
+static void hotkey_exit(void);
+static int hotkey_get(int *status, int *mask);
+static int hotkey_set(int status, int mask);
+static void hotkey_notify(struct ibm_struct *ibm, u32 event);
+static int hotkey_read(char *p);
+static int hotkey_write(char *buf);
+
+
+/*
+ * LED subdriver
+ */
+
+enum led_access_mode {
+	IBMACPI_LED_NONE = 0,
+	IBMACPI_LED_570,	/* 570 */
+	IBMACPI_LED_OLD,	/* 600e/x, 770e, 770x, A21e, A2xm/p, T20-22, X20-21 */
+	IBMACPI_LED_NEW,	/* all others */
+};
+
+enum {	/* For IBMACPI_LED_OLD */
+	IBMACPI_LED_EC_HLCL = 0x0c,	/* EC reg to get led to power on */
+	IBMACPI_LED_EC_HLBL = 0x0d,	/* EC reg to blink a lit led */
+	IBMACPI_LED_EC_HLMS = 0x0e,	/* EC reg to select led to command */
+};
+
+static enum led_access_mode led_supported;
+static acpi_handle led_handle;
+
+static int led_init(void);
+static int led_read(char *p);
+static int led_write(char *buf);
+
+/*
+ * Light (thinklight) subdriver
+ */
+
+static int light_supported;
+static int light_status_supported;
+static acpi_handle lght_handle, ledb_handle;
+
+static int light_init(void);
+static int light_read(char *p);
+static int light_write(char *buf);
+
+
+/*
+ * Thermal subdriver
+ */
+
+enum thermal_access_mode {
+	IBMACPI_THERMAL_NONE = 0,	/* No thermal support */
+	IBMACPI_THERMAL_ACPI_TMP07,	/* Use ACPI TMP0-7 */
+	IBMACPI_THERMAL_ACPI_UPDT,	/* Use ACPI TMP0-7 with UPDT */
+	IBMACPI_THERMAL_TPEC_8,		/* Use ACPI EC regs, 8 sensors */
+	IBMACPI_THERMAL_TPEC_16,	/* Use ACPI EC regs, 16 sensors */
+};
+
+#define IBMACPI_MAX_THERMAL_SENSORS 16	/* Max thermal sensors supported */
+struct ibm_thermal_sensors_struct {
+	s32 temp[IBMACPI_MAX_THERMAL_SENSORS];
+};
+
+static int thermal_init(void);
+static int thermal_get_sensors(struct ibm_thermal_sensors_struct *s);
+static int thermal_read(char *p);
+
+
+/*
+ * Video subdriver
+ */
+
+enum video_access_mode {
+	IBMACPI_VIDEO_NONE = 0,
+	IBMACPI_VIDEO_570,	/* 570 */
+	IBMACPI_VIDEO_770,	/* 600e/x, 770e, 770x */
+	IBMACPI_VIDEO_NEW,	/* all others */
+};
+
+static enum video_access_mode video_supported;
+static int video_orig_autosw;
+static acpi_handle vid_handle, vid2_handle;
+
+static int video_init(void);
+static void video_exit(void);
+static int video_status(void);
+static int video_autosw(void);
+static int video_switch(void);
+static int video_switch2(int status);
+static int video_expand(void);
+static int video_read(char *p);
+static int video_write(char *buf);
+
+
+/*
+ * Volume subdriver
+ */
+
+static int volume_offset = 0x30;
+
+static int volume_read(char *p);
+static int volume_write(char *buf);
+
+
+/*
+ * Wan subdriver
+ */
+
+static int wan_supported;
+
+static int wan_init(void);
+static int wan_status(void);
+static int wan_read(char *p);
+static int wan_write(char *buf);
+
+
+#endif /* __IBM_ACPI_H */
-- 
1.5.0.3


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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

* [PATCH 4/6] ACPI: ibm-acpi: organize code
       [not found]           ` <11736588493502-git-send-email-hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
@ 2007-03-12  0:20             ` Henrique de Moraes Holschuh
  2007-03-12  0:20               ` [PATCH 5/6] ACPI: ibm-acpi: update copyright notice Henrique de Moraes Holschuh
  0 siblings, 1 reply; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-12  0:20 UTC (permalink / raw)
  To: lenb-DgEjT+Ai2ygdnm+yROfE0A
  Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA, Henrique de Moraes Holschuh,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

Shuffle code around to better organize the driver code inside the
ibm-acpi.c file.

This patch adds no functional changes.  It is pure fluff that will make me
a bit more productive.

Signed-off-by: Henrique de Moraes Holschuh <hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
---
 drivers/acpi/ibm_acpi.c | 1388 +++++++++++++++++++++++++----------------------
 1 files changed, 752 insertions(+), 636 deletions(-)

diff --git a/drivers/acpi/ibm_acpi.c b/drivers/acpi/ibm_acpi.c
index 59cbafc..7cd2888 100644
--- a/drivers/acpi/ibm_acpi.c
+++ b/drivers/acpi/ibm_acpi.c
@@ -87,8 +87,17 @@ MODULE_LICENSE("GPL");
 
 #define __unused __attribute__ ((unused))
 
-static int experimental;
-module_param(experimental, int, 0);
+/****************************************************************************
+ ****************************************************************************
+ *
+ * ACPI Helpers and device model
+ *
+ ****************************************************************************
+ ****************************************************************************/
+
+/*************************************************************************
+ * ACPI basic handles
+ */
 
 static acpi_handle root_handle = NULL;
 
@@ -105,183 +114,31 @@ IBM_HANDLE(ec, root, "\\_SB.PCI0.ISA.EC0",	/* 240, 240x */
 	   "\\_SB.PCI0.AD4S.EC0",	/* i1400, R30 */
 	   "\\_SB.PCI0.ICH3.EC0",	/* R31 */
 	   "\\_SB.PCI0.LPC.EC",	/* all others */
-    );
+	   );
 
-IBM_HANDLE(vid, root, "\\_SB.PCI.AGP.VGA",	/* 570 */
-	   "\\_SB.PCI0.AGP0.VID0",	/* 600e/x, 770x */
-	   "\\_SB.PCI0.VID0",	/* 770e */
-	   "\\_SB.PCI0.VID",	/* A21e, G4x, R50e, X30, X40 */
-	   "\\_SB.PCI0.AGP.VID",	/* all others */
-    );				/* R30, R31 */
+IBM_HANDLE(ecrd, ec, "ECRD");	/* 570 */
+IBM_HANDLE(ecwr, ec, "ECWR");	/* 570 */
 
-IBM_HANDLE(vid2, root, "\\_SB.PCI0.AGPB.VID");	/* G41 */
+
+/*************************************************************************
+ * Misc ACPI handles
+ */
 
 IBM_HANDLE(cmos, root, "\\UCMS",	/* R50, R50e, R50p, R51, T4x, X31, X40 */
 	   "\\CMOS",		/* A3x, G4x, R32, T23, T30, X22-24, X30 */
 	   "\\CMS",		/* R40, R40e */
-    );				/* all others */
-#ifdef CONFIG_ACPI_IBM_DOCK
-IBM_HANDLE(dock, root, "\\_SB.GDCK",	/* X30, X31, X40 */
-	   "\\_SB.PCI0.DOCK",	/* 600e/x,770e,770x,A2xm/p,T20-22,X20-21 */
-	   "\\_SB.PCI0.PCI1.DOCK",	/* all others */
-	   "\\_SB.PCI.ISA.SLCE",	/* 570 */
-    );				/* A21e,G4x,R30,R31,R32,R40,R40e,R50e */
-#endif
-#ifdef CONFIG_ACPI_IBM_BAY
-IBM_HANDLE(bay, root, "\\_SB.PCI.IDE.SECN.MAST",	/* 570 */
-	   "\\_SB.PCI0.IDE0.IDES.IDSM",	/* 600e/x, 770e, 770x */
-	   "\\_SB.PCI0.SATA.SCND.MSTR",	/* T60, X60, Z60 */
-	   "\\_SB.PCI0.IDE0.SCND.MSTR",	/* all others */
-    );				/* A21e, R30, R31 */
-
-IBM_HANDLE(bay_ej, bay, "_EJ3",	/* 600e/x, A2xm/p, A3x */
-	   "_EJ0",		/* all others */
-    );				/* 570,A21e,G4x,R30,R31,R32,R40e,R50e */
-
-IBM_HANDLE(bay2, root, "\\_SB.PCI0.IDE0.PRIM.SLAV",	/* A3x, R32 */
-	   "\\_SB.PCI0.IDE0.IDEP.IDPS",	/* 600e/x, 770e, 770x */
-    );				/* all others */
-
-IBM_HANDLE(bay2_ej, bay2, "_EJ3",	/* 600e/x, 770e, A3x */
-	   "_EJ0",		/* 770x */
-    );				/* all others */
-#endif /* CONFIG_ACPI_IBM_BAY */
-
-/* don't list other alternatives as we install a notify handler on the 570 */
-IBM_HANDLE(pci, root, "\\_SB.PCI");	/* 570 */
+	   );			/* all others */
 
 IBM_HANDLE(hkey, ec, "\\_SB.HKEY",	/* 600e/x, 770e, 770x */
 	   "^HKEY",		/* R30, R31 */
 	   "HKEY",		/* all others */
-    );				/* 570 */
+	   );			/* 570 */
 
-IBM_HANDLE(lght, root, "\\LGHT");	/* A21e, A2xm/p, T20-22, X20-21 */
-IBM_HANDLE(ledb, ec, "LEDB");	/* G4x */
 
-IBM_HANDLE(led, ec, "SLED",	/* 570 */
-	   "SYSL",		/* 600e/x, 770e, 770x, A21e, A2xm/p, T20-22, X20-21 */
-	   "LED",		/* all others */
-    );				/* R30, R31 */
-
-IBM_HANDLE(beep, ec, "BEEP");	/* all except R30, R31 */
-IBM_HANDLE(ecrd, ec, "ECRD");	/* 570 */
-IBM_HANDLE(ecwr, ec, "ECWR");	/* 570 */
-IBM_HANDLE(fans, ec, "FANS");	/* X31, X40, X41 */
-
-IBM_HANDLE(gfan, ec, "GFAN",	/* 570 */
-	   "\\FSPD",		/* 600e/x, 770e, 770x */
-    );				/* all others */
-
-IBM_HANDLE(sfan, ec, "SFAN",	/* 570 */
-	   "JFNS",		/* 770x-JL */
-    );				/* all others */
-
-/*
- * FAN ACCESS MODES
- *
- * IBMACPI_FAN_RD_ACPI_GFAN:
- * 	ACPI GFAN method: returns fan level
- *
- * 	see IBMACPI_FAN_WR_ACPI_SFAN
- * 	EC 0x2f not available if GFAN exists
- *
- * IBMACPI_FAN_WR_ACPI_SFAN:
- * 	ACPI SFAN method: sets fan level, 0 (stop) to 7 (max)
- *
- * 	EC 0x2f might be available *for reading*, but never for writing.
- *
- * IBMACPI_FAN_WR_TPEC:
- * 	ThinkPad EC register 0x2f (HFSP): fan control loop mode Supported
- * 	on almost all ThinkPads
- *
- * 	Fan speed changes of any sort (including those caused by the
- * 	disengaged mode) are usually done slowly by the firmware as the
- * 	maximum ammount of fan duty cycle change per second seems to be
- * 	limited.
- *
- * 	Reading is not available if GFAN exists.
- * 	Writing is not available if SFAN exists.
- *
- * 	Bits
- *	 7	automatic mode engaged;
- *  		(default operation mode of the ThinkPad)
- * 		fan level is ignored in this mode.
- *	 6	disengage mode (takes precedence over bit 7);
- *		not available on all thinkpads.  May disable
- *		the tachometer, and speeds up fan to 100% duty-cycle,
- *		which speeds it up far above the standard RPM
- *		levels.  It is not impossible that it could cause
- *		hardware damage.
- *	5-3	unused in some models.  Extra bits for fan level
- *		in others, but still useless as all values above
- *		7 map to the same speed as level 7 in these models.
- *	2-0	fan level (0..7 usually)
- *			0x00 = stop
- * 			0x07 = max (set when temperatures critical)
- * 		Some ThinkPads may have other levels, see
- * 		IBMACPI_FAN_WR_ACPI_FANS (X31/X40/X41)
- *
- *	FIRMWARE BUG: on some models, EC 0x2f might not be initialized at
- *	boot. Apparently the EC does not intialize it, so unless ACPI DSDT
- *	does so, its initial value is meaningless (0x07).
- *
- *	For firmware bugs, refer to:
- *	http://thinkwiki.org/wiki/Embedded_Controller_Firmware#Firmware_Issues
- *
- * 	----
- *
- *	ThinkPad EC register 0x84 (LSB), 0x85 (MSB):
- *	Main fan tachometer reading (in RPM)
- *
- *	This register is present on all ThinkPads with a new-style EC, and
- *	it is known not to be present on the A21m/e, and T22, as there is
- *	something else in offset 0x84 according to the ACPI DSDT.  Other
- *	ThinkPads from this same time period (and earlier) probably lack the
- *	tachometer as well.
- *
- *	Unfortunately a lot of ThinkPads with new-style ECs but whose firwmare
- *	was never fixed by IBM to report the EC firmware version string
- *	probably support the tachometer (like the early X models), so
- *	detecting it is quite hard.  We need more data to know for sure.
- *
- *	FIRMWARE BUG: always read 0x84 first, otherwise incorrect readings
- *	might result.
- *
- *	FIRMWARE BUG: when EC 0x2f bit 6 is set (disengaged mode), this
- *	register is not invalidated in ThinkPads that disable tachometer
- *	readings.  Thus, the tachometer readings go stale.
- *
- *	For firmware bugs, refer to:
- *	http://thinkwiki.org/wiki/Embedded_Controller_Firmware#Firmware_Issues
- *
- * IBMACPI_FAN_WR_ACPI_FANS:
- *	ThinkPad X31, X40, X41.  Not available in the X60.
- *
- *	FANS ACPI handle: takes three arguments: low speed, medium speed,
- *	high speed.  ACPI DSDT seems to map these three speeds to levels
- *	as follows: STOP LOW LOW MED MED HIGH HIGH HIGH HIGH
- *	(this map is stored on FAN0..FAN8 as "0,1,1,2,2,3,3,3,3")
- *
- * 	The speeds are stored on handles
- * 	(FANA:FAN9), (FANC:FANB), (FANE:FAND).
- *
- * 	There are three default speed sets, acessible as handles:
- * 	FS1L,FS1M,FS1H; FS2L,FS2M,FS2H; FS3L,FS3M,FS3H
- *
- * 	ACPI DSDT switches which set is in use depending on various
- * 	factors.
- *
- * 	IBMACPI_FAN_WR_TPEC is also available and should be used to
- * 	command the fan.  The X31/X40/X41 seems to have 8 fan levels,
- * 	but the ACPI tables just mention level 7.
+/*************************************************************************
+ * ACPI helpers
  */
 
-static char *ibm_thinkpad_ec_found = NULL;
-
-static struct proc_dir_entry *proc_dir = NULL;
-
-static struct backlight_device *ibm_backlight_device = NULL;
-
 static int acpi_evalf(acpi_handle handle,
 		      void *res, char *method, char *fmt, ...)
 {
@@ -371,6 +228,198 @@ static void __unused acpi_print_int(acpi_handle handle, char *method)
 		printk(IBM_ERR "error calling %s\n", method);
 }
 
+static int acpi_ec_read(int i, u8 * p)
+{
+	int v;
+
+	if (ecrd_handle) {
+		if (!acpi_evalf(ecrd_handle, &v, NULL, "dd", i))
+			return 0;
+		*p = v;
+	} else {
+		if (ec_read(i, p) < 0)
+			return 0;
+	}
+
+	return 1;
+}
+
+static int acpi_ec_write(int i, u8 v)
+{
+	if (ecwr_handle) {
+		if (!acpi_evalf(ecwr_handle, NULL, NULL, "vdd", i, v))
+			return 0;
+	} else {
+		if (ec_write(i, v) < 0)
+			return 0;
+	}
+
+	return 1;
+}
+
+static int _sta(acpi_handle handle)
+{
+	int status;
+
+	if (!handle || !acpi_evalf(handle, &status, "_STA", "d"))
+		status = 0;
+
+	return status;
+}
+
+/*************************************************************************
+ * ACPI device model
+ */
+
+static void __init ibm_handle_init(char *name,
+				   acpi_handle * handle, acpi_handle parent,
+				   char **paths, int num_paths, char **path)
+{
+	int i;
+	acpi_status status;
+
+	for (i = 0; i < num_paths; i++) {
+		status = acpi_get_handle(parent, paths[i], handle);
+		if (ACPI_SUCCESS(status)) {
+			*path = paths[i];
+			return;
+		}
+	}
+
+	*handle = NULL;
+}
+
+static void dispatch_notify(acpi_handle handle, u32 event, void *data)
+{
+	struct ibm_struct *ibm = data;
+
+	if (!ibm || !ibm->notify)
+		return;
+
+	ibm->notify(ibm, event);
+}
+
+static int __init setup_notify(struct ibm_struct *ibm)
+{
+	acpi_status status;
+	int ret;
+
+	if (!*ibm->handle)
+		return 0;
+
+	ret = acpi_bus_get_device(*ibm->handle, &ibm->device);
+	if (ret < 0) {
+		printk(IBM_ERR "%s device not present\n", ibm->name);
+		return 0;
+	}
+
+	acpi_driver_data(ibm->device) = ibm;
+	sprintf(acpi_device_class(ibm->device), "%s/%s", IBM_NAME, ibm->name);
+
+	status = acpi_install_notify_handler(*ibm->handle, ibm->type,
+					     dispatch_notify, ibm);
+	if (ACPI_FAILURE(status)) {
+		printk(IBM_ERR "acpi_install_notify_handler(%s) failed: %d\n",
+		       ibm->name, status);
+		return -ENODEV;
+	}
+	ibm->notify_installed = 1;
+	return 0;
+}
+
+static int __init ibm_device_add(struct acpi_device *device)
+{
+	return 0;
+}
+
+static int __init register_ibmacpi_subdriver(struct ibm_struct *ibm)
+{
+	int ret;
+
+	ibm->driver = kzalloc(sizeof(struct acpi_driver), GFP_KERNEL);
+	if (!ibm->driver) {
+		printk(IBM_ERR "kmalloc(ibm->driver) failed\n");
+		return -1;
+	}
+
+	sprintf(ibm->driver->name, "%s_%s", IBM_NAME, ibm->name);
+	ibm->driver->ids = ibm->hid;
+	ibm->driver->ops.add = &ibm_device_add;
+
+	ret = acpi_bus_register_driver(ibm->driver);
+	if (ret < 0) {
+		printk(IBM_ERR "acpi_bus_register_driver(%s) failed: %d\n",
+		       ibm->hid, ret);
+		kfree(ibm->driver);
+	}
+
+	return ret;
+}
+
+
+/****************************************************************************
+ ****************************************************************************
+ *
+ * Procfs Helpers
+ *
+ ****************************************************************************
+ ****************************************************************************/
+
+static int dispatch_read(char *page, char **start, off_t off, int count,
+			 int *eof, void *data)
+{
+	struct ibm_struct *ibm = data;
+	int len;
+
+	if (!ibm || !ibm->read)
+		return -EINVAL;
+
+	len = ibm->read(page);
+	if (len < 0)
+		return len;
+
+	if (len <= off + count)
+		*eof = 1;
+	*start = page + off;
+	len -= off;
+	if (len > count)
+		len = count;
+	if (len < 0)
+		len = 0;
+
+	return len;
+}
+
+static int dispatch_write(struct file *file, const char __user * userbuf,
+			  unsigned long count, void *data)
+{
+	struct ibm_struct *ibm = data;
+	char *kernbuf;
+	int ret;
+
+	if (!ibm || !ibm->write)
+		return -EINVAL;
+
+	kernbuf = kmalloc(count + 2, GFP_KERNEL);
+	if (!kernbuf)
+		return -ENOMEM;
+
+	if (copy_from_user(kernbuf, userbuf, count)) {
+		kfree(kernbuf);
+		return -EFAULT;
+	}
+
+	kernbuf[count] = 0;
+	strcat(kernbuf, ",");
+	ret = ibm->write(kernbuf);
+	if (ret == 0)
+		ret = count;
+
+	kfree(kernbuf);
+
+	return ret;
+}
+
 static char *next_cmd(char **cmds)
 {
 	char *start = *cmds;
@@ -387,6 +436,19 @@ static char *next_cmd(char **cmds)
 	return start;
 }
 
+
+/****************************************************************************
+ ****************************************************************************
+ *
+ * Subdrivers
+ *
+ ****************************************************************************
+ ****************************************************************************/
+
+/*************************************************************************
+ * ibm-acpi init subdriver
+ */
+
 static int ibm_acpi_driver_init(void)
 {
 	printk(IBM_INFO "%s v%s\n", IBM_DESC, IBM_VERSION);
@@ -409,11 +471,51 @@ static int ibm_acpi_driver_read(char *p)
 	return len;
 }
 
+/*************************************************************************
+ * Hotkey subdriver
+ */
+
 static int hotkey_supported;
 static int hotkey_mask_supported;
 static int hotkey_orig_status;
 static int hotkey_orig_mask;
 
+static int hotkey_init(void)
+{
+	/* hotkey not supported on 570 */
+	hotkey_supported = hkey_handle != NULL;
+
+	if (hotkey_supported) {
+		/* mask not supported on 570, 600e/x, 770e, 770x, A21e, A2xm/p,
+		   A30, R30, R31, T20-22, X20-21, X22-24 */
+		hotkey_mask_supported =
+		    acpi_evalf(hkey_handle, NULL, "DHKN", "qv");
+
+		if (!hotkey_get(&hotkey_orig_status, &hotkey_orig_mask))
+			return -ENODEV;
+	}
+
+	return 0;
+}
+
+static void hotkey_exit(void)
+{
+	if (hotkey_supported)
+		hotkey_set(hotkey_orig_status, hotkey_orig_mask);
+}
+
+static void hotkey_notify(struct ibm_struct *ibm, u32 event)
+{
+	int hkey;
+
+	if (acpi_evalf(hkey_handle, &hkey, "MHKP", "d"))
+		acpi_bus_generate_event(ibm->device, event, hkey);
+	else {
+		printk(IBM_ERR "unknown hotkey event %d\n", event);
+		acpi_bus_generate_event(ibm->device, event, 0);
+	}
+}
+
 static int hotkey_get(int *status, int *mask)
 {
 	if (!acpi_evalf(hkey_handle, status, "DHKC", "d"))
@@ -444,24 +546,6 @@ static int hotkey_set(int status, int mask)
 	return 1;
 }
 
-static int hotkey_init(void)
-{
-	/* hotkey not supported on 570 */
-	hotkey_supported = hkey_handle != NULL;
-
-	if (hotkey_supported) {
-		/* mask not supported on 570, 600e/x, 770e, 770x, A21e, A2xm/p,
-		   A30, R30, R31, T20-22, X20-21, X22-24 */
-		hotkey_mask_supported =
-		    acpi_evalf(hkey_handle, NULL, "DHKN", "qv");
-
-		if (!hotkey_get(&hotkey_orig_status, &hotkey_orig_mask))
-			return -ENODEV;
-	}
-
-	return 0;
-}
-
 static int hotkey_read(char *p)
 {
 	int status, mask;
@@ -523,23 +607,9 @@ static int hotkey_write(char *buf)
 	return 0;
 }
 
-static void hotkey_exit(void)
-{
-	if (hotkey_supported)
-		hotkey_set(hotkey_orig_status, hotkey_orig_mask);
-}
-
-static void hotkey_notify(struct ibm_struct *ibm, u32 event)
-{
-	int hkey;
-
-	if (acpi_evalf(hkey_handle, &hkey, "MHKP", "d"))
-		acpi_bus_generate_event(ibm->device, event, hkey);
-	else {
-		printk(IBM_ERR "unknown hotkey event %d\n", event);
-		acpi_bus_generate_event(ibm->device, event, 0);
-	}
-}
+/*************************************************************************
+ * Bluetooth subdriver
+ */
 
 static int bluetooth_supported;
 
@@ -606,6 +676,10 @@ static int bluetooth_write(char *buf)
 	return 0;
 }
 
+/*************************************************************************
+ * Wan subdriver
+ */
+
 static int wan_supported;
 
 static int wan_init(void)
@@ -668,9 +742,22 @@ static int wan_write(char *buf)
 	return 0;
 }
 
+/*************************************************************************
+ * Video subdriver
+ */
+
 static enum video_access_mode video_supported;
 static int video_orig_autosw;
 
+IBM_HANDLE(vid, root, "\\_SB.PCI.AGP.VGA",	/* 570 */
+	   "\\_SB.PCI0.AGP0.VID0",	/* 600e/x, 770x */
+	   "\\_SB.PCI0.VID0",	/* 770e */
+	   "\\_SB.PCI0.VID",	/* A21e, G4x, R50e, X30, X40 */
+	   "\\_SB.PCI0.AGP.VID",	/* all others */
+	   );				/* R30, R31 */
+
+IBM_HANDLE(vid2, root, "\\_SB.PCI0.AGPB.VID");	/* G41 */
+
 static int video_init(void)
 {
 	int ivga;
@@ -695,6 +782,11 @@ static int video_init(void)
 	return 0;
 }
 
+static void video_exit(void)
+{
+	acpi_evalf(vid_handle, NULL, "_DOS", "vd", video_orig_autosw);
+}
+
 static int video_status(void)
 {
 	int status = 0;
@@ -736,33 +828,6 @@ static int video_autosw(void)
 	return autosw & 1;
 }
 
-static int video_read(char *p)
-{
-	int status = video_status();
-	int autosw = video_autosw();
-	int len = 0;
-
-	if (!video_supported) {
-		len += sprintf(p + len, "status:\t\tnot supported\n");
-		return len;
-	}
-
-	len += sprintf(p + len, "status:\t\tsupported\n");
-	len += sprintf(p + len, "lcd:\t\t%s\n", enabled(status, 0));
-	len += sprintf(p + len, "crt:\t\t%s\n", enabled(status, 1));
-	if (video_supported == IBMACPI_VIDEO_NEW)
-		len += sprintf(p + len, "dvi:\t\t%s\n", enabled(status, 3));
-	len += sprintf(p + len, "auto:\t\t%s\n", enabled(autosw, 0));
-	len += sprintf(p + len, "commands:\tlcd_enable, lcd_disable\n");
-	len += sprintf(p + len, "commands:\tcrt_enable, crt_disable\n");
-	if (video_supported == IBMACPI_VIDEO_NEW)
-		len += sprintf(p + len, "commands:\tdvi_enable, dvi_disable\n");
-	len += sprintf(p + len, "commands:\tauto_enable, auto_disable\n");
-	len += sprintf(p + len, "commands:\tvideo_switch, expand_toggle\n");
-
-	return len;
-}
-
 static int video_switch(void)
 {
 	int autosw = video_autosw();
@@ -812,6 +877,33 @@ static int video_switch2(int status)
 	return ret;
 }
 
+static int video_read(char *p)
+{
+	int status = video_status();
+	int autosw = video_autosw();
+	int len = 0;
+
+	if (!video_supported) {
+		len += sprintf(p + len, "status:\t\tnot supported\n");
+		return len;
+	}
+
+	len += sprintf(p + len, "status:\t\tsupported\n");
+	len += sprintf(p + len, "lcd:\t\t%s\n", enabled(status, 0));
+	len += sprintf(p + len, "crt:\t\t%s\n", enabled(status, 1));
+	if (video_supported == IBMACPI_VIDEO_NEW)
+		len += sprintf(p + len, "dvi:\t\t%s\n", enabled(status, 3));
+	len += sprintf(p + len, "auto:\t\t%s\n", enabled(autosw, 0));
+	len += sprintf(p + len, "commands:\tlcd_enable, lcd_disable\n");
+	len += sprintf(p + len, "commands:\tcrt_enable, crt_disable\n");
+	if (video_supported == IBMACPI_VIDEO_NEW)
+		len += sprintf(p + len, "commands:\tdvi_enable, dvi_disable\n");
+	len += sprintf(p + len, "commands:\tauto_enable, auto_disable\n");
+	len += sprintf(p + len, "commands:\tvideo_switch, expand_toggle\n");
+
+	return len;
+}
+
 static int video_write(char *buf)
 {
 	char *cmd;
@@ -862,14 +954,16 @@ static int video_write(char *buf)
 	return 0;
 }
 
-static void video_exit(void)
-{
-	acpi_evalf(vid_handle, NULL, "_DOS", "vd", video_orig_autosw);
-}
+/*************************************************************************
+ * Light (thinklight) subdriver
+ */
 
 static int light_supported;
 static int light_status_supported;
 
+IBM_HANDLE(lght, root, "\\LGHT");	/* A21e, A2xm/p, T20-22, X20-21 */
+IBM_HANDLE(ledb, ec, "LEDB");		/* G4x */
+
 static int light_init(void)
 {
 	/* light not supported on 570, 600e/x, 770e, 770x, G4x, R30, R31 */
@@ -933,21 +1027,45 @@ static int light_write(char *buf)
 	return 0;
 }
 
-#if defined(CONFIG_ACPI_IBM_DOCK) || defined(CONFIG_ACPI_IBM_BAY)
-static int _sta(acpi_handle handle)
-{
-	int status;
-
-	if (!handle || !acpi_evalf(handle, &status, "_STA", "d"))
-		status = 0;
+/*************************************************************************
+ * Dock subdriver
+ */
 
-	return status;
-}
-#endif
+/* don't list other alternatives as we install a notify handler on the 570 */
+IBM_HANDLE(pci, root, "\\_SB.PCI");	/* 570 */
 
 #ifdef CONFIG_ACPI_IBM_DOCK
+
+IBM_HANDLE(dock, root, "\\_SB.GDCK",	/* X30, X31, X40 */
+	   "\\_SB.PCI0.DOCK",	/* 600e/x,770e,770x,A2xm/p,T20-22,X20-21 */
+	   "\\_SB.PCI0.PCI1.DOCK",	/* all others */
+	   "\\_SB.PCI.ISA.SLCE",	/* 570 */
+    );				/* A21e,G4x,R30,R31,R32,R40,R40e,R50e */
+
 #define dock_docked() (_sta(dock_handle) & 1)
 
+static void dock_notify(struct ibm_struct *ibm, u32 event)
+{
+	int docked = dock_docked();
+	int pci = ibm->hid && strstr(ibm->hid, IBM_PCI_HID);
+
+	if (event == 1 && !pci)	/* 570 */
+		acpi_bus_generate_event(ibm->device, event, 1);	/* button */
+	else if (event == 1 && pci)	/* 570 */
+		acpi_bus_generate_event(ibm->device, event, 3);	/* dock */
+	else if (event == 3 && docked)
+		acpi_bus_generate_event(ibm->device, event, 1);	/* button */
+	else if (event == 3 && !docked)
+		acpi_bus_generate_event(ibm->device, event, 2);	/* undock */
+	else if (event == 0 && docked)
+		acpi_bus_generate_event(ibm->device, event, 3);	/* dock */
+	else {
+		printk(IBM_ERR "unknown dock event %d, status %d\n",
+		       event, _sta(dock_handle));
+		acpi_bus_generate_event(ibm->device, event, 0);	/* unknown */
+	}
+}
+
 static int dock_read(char *p)
 {
 	int len = 0;
@@ -987,28 +1105,11 @@ static int dock_write(char *buf)
 	return 0;
 }
 
-static void dock_notify(struct ibm_struct *ibm, u32 event)
-{
-	int docked = dock_docked();
-	int pci = ibm->hid && strstr(ibm->hid, IBM_PCI_HID);
+#endif /* CONFIG_ACPI_IBM_DOCK */
 
-	if (event == 1 && !pci)	/* 570 */
-		acpi_bus_generate_event(ibm->device, event, 1);	/* button */
-	else if (event == 1 && pci)	/* 570 */
-		acpi_bus_generate_event(ibm->device, event, 3);	/* dock */
-	else if (event == 3 && docked)
-		acpi_bus_generate_event(ibm->device, event, 1);	/* button */
-	else if (event == 3 && !docked)
-		acpi_bus_generate_event(ibm->device, event, 2);	/* undock */
-	else if (event == 0 && docked)
-		acpi_bus_generate_event(ibm->device, event, 3);	/* dock */
-	else {
-		printk(IBM_ERR "unknown dock event %d, status %d\n",
-		       event, _sta(dock_handle));
-		acpi_bus_generate_event(ibm->device, event, 0);	/* unknown */
-	}
-}
-#endif
+/*************************************************************************
+ * Bay subdriver
+ */
 
 #ifdef CONFIG_ACPI_IBM_BAY
 static int bay_status_supported;
@@ -1016,6 +1117,21 @@ static int bay_status2_supported;
 static int bay_eject_supported;
 static int bay_eject2_supported;
 
+IBM_HANDLE(bay, root, "\\_SB.PCI.IDE.SECN.MAST",	/* 570 */
+	   "\\_SB.PCI0.IDE0.IDES.IDSM",	/* 600e/x, 770e, 770x */
+	   "\\_SB.PCI0.SATA.SCND.MSTR",	/* T60, X60, Z60 */
+	   "\\_SB.PCI0.IDE0.SCND.MSTR",	/* all others */
+	   );				/* A21e, R30, R31 */
+IBM_HANDLE(bay_ej, bay, "_EJ3",	/* 600e/x, A2xm/p, A3x */
+	   "_EJ0",		/* all others */
+	   );			/* 570,A21e,G4x,R30,R31,R32,R40e,R50e */
+IBM_HANDLE(bay2, root, "\\_SB.PCI0.IDE0.PRIM.SLAV",	/* A3x, R32 */
+	   "\\_SB.PCI0.IDE0.IDEP.IDPS",	/* 600e/x, 770e, 770x */
+	   );				/* all others */
+IBM_HANDLE(bay2_ej, bay2, "_EJ3",	/* 600e/x, 770e, A3x */
+	   "_EJ0",			/* 770x */
+	   );				/* all others */
+
 static int bay_init(void)
 {
 	bay_status_supported = bay_handle &&
@@ -1031,6 +1147,11 @@ static int bay_init(void)
 	return 0;
 }
 
+static void bay_notify(struct ibm_struct *ibm, u32 event)
+{
+	acpi_bus_generate_event(ibm->device, event, 0);
+}
+
 #define bay_occupied(b) (_sta(b##_handle) & 1)
 
 static int bay_read(char *p)
@@ -1081,12 +1202,19 @@ static int bay_write(char *buf)
 
 	return 0;
 }
+#endif /* CONFIG_ACPI_IBM_BAY */
 
-static void bay_notify(struct ibm_struct *ibm, u32 event)
+/*************************************************************************
+ * CMOS subdriver
+ */
+
+static int cmos_eval(int cmos_cmd)
 {
-	acpi_bus_generate_event(ibm->device, event, 0);
+	if (cmos_handle)
+		return acpi_evalf(cmos_handle, NULL, NULL, "vd", cmos_cmd);
+	else
+		return 1;
 }
-#endif /* CONFIG_ACPI_IBM_BAY */
 
 static int cmos_read(char *p)
 {
@@ -1104,14 +1232,6 @@ static int cmos_read(char *p)
 	return len;
 }
 
-static int cmos_eval(int cmos_cmd)
-{
-	if (cmos_handle)
-		return acpi_evalf(cmos_handle, NULL, NULL, "vd", cmos_cmd);
-	else
-		return 1;
-}
-
 static int cmos_write(char *buf)
 {
 	char *cmd;
@@ -1134,8 +1254,18 @@ static int cmos_write(char *buf)
 	return 0;
 }
 
+
+/*************************************************************************
+ * LED subdriver
+ */
+
 static enum led_access_mode led_supported;
 
+IBM_HANDLE(led, ec, "SLED",	/* 570 */
+	   "SYSL",		/* 600e/x, 770e, 770x, A21e, A2xm/p, T20-22, X20-21 */
+	   "LED",		/* all others */
+	   );			/* R30, R31 */
+
 static int led_init(void)
 {
 	if (!led_handle)
@@ -1242,6 +1372,12 @@ static int led_write(char *buf)
 	return 0;
 }
 
+/*************************************************************************
+ * Beep subdriver
+ */
+
+IBM_HANDLE(beep, ec, "BEEP");	/* all except R30, R31 */
+
 static int beep_read(char *p)
 {
 	int len = 0;
@@ -1277,34 +1413,9 @@ static int beep_write(char *buf)
 	return 0;
 }
 
-static int acpi_ec_read(int i, u8 * p)
-{
-	int v;
-
-	if (ecrd_handle) {
-		if (!acpi_evalf(ecrd_handle, &v, NULL, "dd", i))
-			return 0;
-		*p = v;
-	} else {
-		if (ec_read(i, p) < 0)
-			return 0;
-	}
-
-	return 1;
-}
-
-static int acpi_ec_write(int i, u8 v)
-{
-	if (ecwr_handle) {
-		if (!acpi_evalf(ecwr_handle, NULL, NULL, "vdd", i, v))
-			return 0;
-	} else {
-		if (ec_write(i, v) < 0)
-			return 0;
-	}
-
-	return 1;
-}
+/*************************************************************************
+ * Thermal subdriver
+ */
 
 static enum thermal_access_mode thermal_read_mode;
 
@@ -1446,6 +1557,10 @@ static int thermal_read(char *p)
 	return len;
 }
 
+/*************************************************************************
+ * EC Dump subdriver
+ */
+
 static u8 ecdump_regs[256];
 
 static int ecdump_read(char *p)
@@ -1505,6 +1620,55 @@ static int ecdump_write(char *buf)
 	return 0;
 }
 
+/*************************************************************************
+ * Backlight/brightness subdriver
+ */
+
+static struct backlight_device *ibm_backlight_device = NULL;
+
+static struct backlight_ops ibm_backlight_data = {
+        .get_brightness = brightness_get,
+        .update_status  = brightness_update_status,
+};
+
+static int brightness_init(void)
+{
+	int b;
+
+	b = brightness_get(NULL);
+	if (b < 0)
+		return b;
+
+	ibm_backlight_device = backlight_device_register("ibm", 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);
+	}
+
+	ibm_backlight_device->props.max_brightness = 7;
+	ibm_backlight_device->props.brightness = b;
+	backlight_update_status(ibm_backlight_device);
+
+	return 0;
+}
+
+static void brightness_exit(void)
+{
+	if (ibm_backlight_device) {
+		backlight_device_unregister(ibm_backlight_device);
+		ibm_backlight_device = NULL;
+	}
+}
+
+static int brightness_update_status(struct backlight_device *bd)
+{
+	return brightness_set(
+		(bd->props.fb_blank == FB_BLANK_UNBLANK &&
+		 bd->props.power == FB_BLANK_UNBLANK) ?
+				bd->props.brightness : 0);
+}
+
 static int brightness_get(struct backlight_device *bd)
 {
 	u8 level;
@@ -1516,23 +1680,6 @@ static int brightness_get(struct backlight_device *bd)
 	return level;
 }
 
-static int brightness_read(char *p)
-{
-	int len = 0;
-	int level;
-
-	if ((level = brightness_get(NULL)) < 0) {
-		len += sprintf(p + len, "level:\t\tunreadable\n");
-	} else {
-		len += sprintf(p + len, "level:\t\t%d\n", level & 0x7);
-		len += sprintf(p + len, "commands:\tup, down\n");
-		len += sprintf(p + len, "commands:\tlevel <level>"
-			       " (<level> is 0-7)\n");
-	}
-
-	return len;
-}
-
 static int brightness_set(int value)
 {
 	int cmos_cmd, inc, i;
@@ -1540,8 +1687,7 @@ static int brightness_set(int value)
 
 	value &= 7;
 
-	cmos_cmd = value > current_value ?
-		   TP_CMOS_BRIGHTNESS_UP : TP_CMOS_BRIGHTNESS_DOWN;
+	cmos_cmd = value > current_value ? TP_CMOS_BRIGHTNESS_UP : TP_CMOS_BRIGHTNESS_DOWN;
 	inc = value > current_value ? 1 : -1;
 	for (i = current_value; i != value; i += inc) {
 		if (!cmos_eval(cmos_cmd))
@@ -1553,6 +1699,23 @@ static int brightness_set(int value)
 	return 0;
 }
 
+static int brightness_read(char *p)
+{
+	int len = 0;
+	int level;
+
+	if ((level = brightness_get(NULL)) < 0) {
+		len += sprintf(p + len, "level:\t\tunreadable\n");
+	} else {
+		len += sprintf(p + len, "level:\t\t%d\n", level & 0x7);
+		len += sprintf(p + len, "commands:\tup, down\n");
+		len += sprintf(p + len, "commands:\tlevel <level>"
+			       " (<level> is 0-7)\n");
+	}
+
+	return len;
+}
+
 static int brightness_write(char *buf)
 {
 	int level;
@@ -1580,48 +1743,9 @@ static int brightness_write(char *buf)
 	return 0;
 }
 
-static int brightness_update_status(struct backlight_device *bd)
-{
-	return brightness_set(
-		(bd->props.fb_blank == FB_BLANK_UNBLANK &&
-		 bd->props.power == FB_BLANK_UNBLANK) ?
-				bd->props.brightness : 0);
-}
-
-static struct backlight_ops ibm_backlight_data = {
-        .get_brightness = brightness_get,
-        .update_status  = brightness_update_status,
-};
-
-static int brightness_init(void)
-{
-	int b;
-
-	b = brightness_get(NULL);
-	if (b < 0)
-		return b;
-
-	ibm_backlight_device = backlight_device_register("ibm", 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);
-	}
-
-	ibm_backlight_device->props.max_brightness = 7;
-	ibm_backlight_device->props.brightness = b;
-	backlight_update_status(ibm_backlight_device);
-
-	return 0;
-}
-
-static void brightness_exit(void)
-{
-	if (ibm_backlight_device) {
-		backlight_device_unregister(ibm_backlight_device);
-		ibm_backlight_device = NULL;
-	}
-}
+/*************************************************************************
+ * Volume subdriver
+ */
 
 static int volume_read(char *p)
 {
@@ -1673,8 +1797,7 @@ static int volume_write(char *buf)
 			return -EINVAL;
 
 		if (new_level != level) {	/* mute doesn't change */
-			cmos_cmd = new_level > level ?
-					TP_CMOS_VOLUME_UP : TP_CMOS_VOLUME_DOWN;
+			cmos_cmd = new_level > level ? TP_CMOS_VOLUME_UP : TP_CMOS_VOLUME_DOWN;
 			inc = new_level > level ? 1 : -1;
 
 			if (mute && (!cmos_eval(cmos_cmd) ||
@@ -1693,8 +1816,7 @@ static int volume_write(char *buf)
 		}
 
 		if (new_mute != mute) {	/* level doesn't change */
-			cmos_cmd = new_mute ?
-				   TP_CMOS_VOLUME_MUTE : TP_CMOS_VOLUME_UP;
+			cmos_cmd = new_mute ? TP_CMOS_VOLUME_MUTE : TP_CMOS_VOLUME_UP;
 
 			if (!cmos_eval(cmos_cmd) ||
 			    !acpi_ec_write(volume_offset, level + new_mute))
@@ -1705,6 +1827,111 @@ static int volume_write(char *buf)
 	return 0;
 }
 
+
+/*************************************************************************
+ * Fan subdriver
+ */
+
+/*
+ * FAN ACCESS MODES
+ *
+ * IBMACPI_FAN_RD_ACPI_GFAN:
+ * 	ACPI GFAN method: returns fan level
+ *
+ * 	see IBMACPI_FAN_WR_ACPI_SFAN
+ * 	EC 0x2f not available if GFAN exists
+ *
+ * IBMACPI_FAN_WR_ACPI_SFAN:
+ * 	ACPI SFAN method: sets fan level, 0 (stop) to 7 (max)
+ *
+ * 	EC 0x2f might be available *for reading*, but never for writing.
+ *
+ * IBMACPI_FAN_WR_TPEC:
+ * 	ThinkPad EC register 0x2f (HFSP): fan control loop mode Supported
+ * 	on almost all ThinkPads
+ *
+ * 	Fan speed changes of any sort (including those caused by the
+ * 	disengaged mode) are usually done slowly by the firmware as the
+ * 	maximum ammount of fan duty cycle change per second seems to be
+ * 	limited.
+ *
+ * 	Reading is not available if GFAN exists.
+ * 	Writing is not available if SFAN exists.
+ *
+ * 	Bits
+ *	 7	automatic mode engaged;
+ *  		(default operation mode of the ThinkPad)
+ * 		fan level is ignored in this mode.
+ *	 6	disengage mode (takes precedence over bit 7);
+ *		not available on all thinkpads.  May disable
+ *		the tachometer, and speeds up fan to 100% duty-cycle,
+ *		which speeds it up far above the standard RPM
+ *		levels.  It is not impossible that it could cause
+ *		hardware damage.
+ *	5-3	unused in some models.  Extra bits for fan level
+ *		in others, but still useless as all values above
+ *		7 map to the same speed as level 7 in these models.
+ *	2-0	fan level (0..7 usually)
+ *			0x00 = stop
+ * 			0x07 = max (set when temperatures critical)
+ * 		Some ThinkPads may have other levels, see
+ * 		IBMACPI_FAN_WR_ACPI_FANS (X31/X40/X41)
+ *
+ *	FIRMWARE BUG: on some models, EC 0x2f might not be initialized at
+ *	boot. Apparently the EC does not intialize it, so unless ACPI DSDT
+ *	does so, its initial value is meaningless (0x07).
+ *
+ *	For firmware bugs, refer to:
+ *	http://thinkwiki.org/wiki/Embedded_Controller_Firmware#Firmware_Issues
+ *
+ * 	----
+ *
+ *	ThinkPad EC register 0x84 (LSB), 0x85 (MSB):
+ *	Main fan tachometer reading (in RPM)
+ *
+ *	This register is present on all ThinkPads with a new-style EC, and
+ *	it is known not to be present on the A21m/e, and T22, as there is
+ *	something else in offset 0x84 according to the ACPI DSDT.  Other
+ *	ThinkPads from this same time period (and earlier) probably lack the
+ *	tachometer as well.
+ *
+ *	Unfortunately a lot of ThinkPads with new-style ECs but whose firwmare
+ *	was never fixed by IBM to report the EC firmware version string
+ *	probably support the tachometer (like the early X models), so
+ *	detecting it is quite hard.  We need more data to know for sure.
+ *
+ *	FIRMWARE BUG: always read 0x84 first, otherwise incorrect readings
+ *	might result.
+ *
+ *	FIRMWARE BUG: when EC 0x2f bit 6 is set (disengaged mode), this
+ *	register is not invalidated in ThinkPads that disable tachometer
+ *	readings.  Thus, the tachometer readings go stale.
+ *
+ *	For firmware bugs, refer to:
+ *	http://thinkwiki.org/wiki/Embedded_Controller_Firmware#Firmware_Issues
+ *
+ * IBMACPI_FAN_WR_ACPI_FANS:
+ *	ThinkPad X31, X40, X41.  Not available in the X60.
+ *
+ *	FANS ACPI handle: takes three arguments: low speed, medium speed,
+ *	high speed.  ACPI DSDT seems to map these three speeds to levels
+ *	as follows: STOP LOW LOW MED MED HIGH HIGH HIGH HIGH
+ *	(this map is stored on FAN0..FAN8 as "0,1,1,2,2,3,3,3,3")
+ *
+ * 	The speeds are stored on handles
+ * 	(FANA:FAN9), (FANC:FANB), (FANE:FAND).
+ *
+ * 	There are three default speed sets, acessible as handles:
+ * 	FS1L,FS1M,FS1H; FS2L,FS2M,FS2H; FS3L,FS3M,FS3H
+ *
+ * 	ACPI DSDT switches which set is in use depending on various
+ * 	factors.
+ *
+ * 	IBMACPI_FAN_WR_TPEC is also available and should be used to
+ * 	command the fan.  The X31/X40/X41 seems to have 8 fan levels,
+ * 	but the ACPI tables just mention level 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;
@@ -1716,6 +1943,14 @@ static void fan_watchdog_fire(struct work_struct *ignored);
 static int fan_watchdog_maxinterval;
 static DECLARE_DELAYED_WORK(fan_watchdog_task, fan_watchdog_fire);
 
+IBM_HANDLE(fans, ec, "FANS");	/* X31, X40, X41 */
+IBM_HANDLE(gfan, ec, "GFAN",	/* 570 */
+	   "\\FSPD",		/* 600e/x, 770e, 770x */
+	   );			/* all others */
+IBM_HANDLE(sfan, ec, "SFAN",	/* 570 */
+	   "JFNS",		/* 770x-JL */
+	   );			/* all others */
+
 static int fan_init(void)
 {
 	fan_status_access_mode = IBMACPI_FAN_NONE;
@@ -1831,6 +2066,12 @@ static int fan_get_status(u8 *status)
 	return 0;
 }
 
+static void fan_exit(void)
+{
+	cancel_delayed_work(&fan_watchdog_task);
+	flush_scheduled_work();
+}
+
 static int fan_get_speed(unsigned int *speed)
 {
 	u8 hi, lo;
@@ -1854,10 +2095,14 @@ static int fan_get_speed(unsigned int *speed)
 	return 0;
 }
 
-static void fan_exit(void)
+static void fan_watchdog_fire(struct work_struct *ignored)
 {
-	cancel_delayed_work(&fan_watchdog_task);
-	flush_scheduled_work();
+	printk(IBM_NOTICE "fan watchdog: enabling fan\n");
+	if (fan_set_enable()) {
+		printk(IBM_ERR "fan watchdog: error while enabling fan\n");
+		/* reschedule for later */
+		fan_watchdog_reset();
+	}
 }
 
 static void fan_watchdog_reset(void)
@@ -1879,90 +2124,6 @@ static void fan_watchdog_reset(void)
 		fan_watchdog_active = 0;
 }
 
-static int fan_read(char *p)
-{
-	int len = 0;
-	int rc;
-	u8 status;
-	unsigned int speed = 0;
-
-	switch (fan_status_access_mode) {
-	case IBMACPI_FAN_RD_ACPI_GFAN:
-		/* 570, 600e/x, 770e, 770x */
-		if ((rc = fan_get_status(&status)) < 0)
-			return rc;
-
-		len += sprintf(p + len, "status:\t\t%s\n"
-			       "level:\t\t%d\n",
-			       (status != 0) ? "enabled" : "disabled", status);
-		break;
-
-	case IBMACPI_FAN_RD_TPEC:
-		/* all except 570, 600e/x, 770e, 770x */
-		if ((rc = fan_get_status(&status)) < 0)
-			return rc;
-
-		if (unlikely(!fan_control_status_known)) {
-			if (status != fan_control_initial_status)
-				fan_control_status_known = 1;
-			else
-				/* Return most likely status. In fact, it
-				 * might be the only possible status */
-				status = IBMACPI_FAN_EC_AUTO;
-		}
-
-		len += sprintf(p + len, "status:\t\t%s\n",
-			       (status != 0) ? "enabled" : "disabled");
-
-		/* No ThinkPad boots on disengaged mode, we can safely
-		 * assume the tachometer is online if fan control status
-		 * was unknown */
-		if ((rc = fan_get_speed(&speed)) < 0)
-			return rc;
-
-		len += sprintf(p + len, "speed:\t\t%d\n", speed);
-
-		if (status & IBMACPI_FAN_EC_DISENGAGED)
-			/* Disengaged mode takes precedence */
-			len += sprintf(p + len, "level:\t\tdisengaged\n");
-		else if (status & IBMACPI_FAN_EC_AUTO)
-			len += sprintf(p + len, "level:\t\tauto\n");
-		else
-			len += sprintf(p + len, "level:\t\t%d\n", status);
-		break;
-
-	case IBMACPI_FAN_NONE:
-	default:
-		len += sprintf(p + len, "status:\t\tnot supported\n");
-	}
-
-	if (fan_control_commands & IBMACPI_FAN_CMD_LEVEL) {
-		len += sprintf(p + len, "commands:\tlevel <level>");
-
-		switch (fan_control_access_mode) {
-		case IBMACPI_FAN_WR_ACPI_SFAN:
-			len += sprintf(p + len, " (<level> is 0-7)\n");
-			break;
-
-		default:
-			len += sprintf(p + len, " (<level> is 0-7, "
-				       "auto, disengaged)\n");
-			break;
-		}
-	}
-
-	if (fan_control_commands & IBMACPI_FAN_CMD_ENABLE)
-		len += sprintf(p + len, "commands:\tenable, disable\n"
-			       "commands:\twatchdog <timeout> (<timeout> is 0 (off), "
-			       "1-120 (seconds))\n");
-
-	if (fan_control_commands & IBMACPI_FAN_CMD_SPEED)
-		len += sprintf(p + len, "commands:\tspeed <speed>"
-			       " (<speed> is 0-65535)\n");
-
-	return len;
-}
-
 static int fan_set_level(int level)
 {
 	switch (fan_control_access_mode) {
@@ -2074,6 +2235,90 @@ static int fan_set_speed(int speed)
 	return 0;
 }
 
+static int fan_read(char *p)
+{
+	int len = 0;
+	int rc;
+	u8 status;
+	unsigned int speed = 0;
+
+	switch (fan_status_access_mode) {
+	case IBMACPI_FAN_RD_ACPI_GFAN:
+		/* 570, 600e/x, 770e, 770x */
+		if ((rc = fan_get_status(&status)) < 0)
+			return rc;
+
+		len += sprintf(p + len, "status:\t\t%s\n"
+			       "level:\t\t%d\n",
+			       (status != 0) ? "enabled" : "disabled", status);
+		break;
+
+	case IBMACPI_FAN_RD_TPEC:
+		/* all except 570, 600e/x, 770e, 770x */
+		if ((rc = fan_get_status(&status)) < 0)
+			return rc;
+
+		if (unlikely(!fan_control_status_known)) {
+			if (status != fan_control_initial_status)
+				fan_control_status_known = 1;
+			else
+				/* Return most likely status. In fact, it
+				 * might be the only possible status */
+				status = IBMACPI_FAN_EC_AUTO;
+		}
+
+		len += sprintf(p + len, "status:\t\t%s\n",
+			       (status != 0) ? "enabled" : "disabled");
+
+		/* No ThinkPad boots on disengaged mode, we can safely
+		 * assume the tachometer is online if fan control status
+		 * was unknown */
+		if ((rc = fan_get_speed(&speed)) < 0)
+			return rc;
+
+		len += sprintf(p + len, "speed:\t\t%d\n", speed);
+
+		if (status & IBMACPI_FAN_EC_DISENGAGED)
+			/* Disengaged mode takes precedence */
+			len += sprintf(p + len, "level:\t\tdisengaged\n");
+		else if (status & IBMACPI_FAN_EC_AUTO)
+			len += sprintf(p + len, "level:\t\tauto\n");
+		else
+			len += sprintf(p + len, "level:\t\t%d\n", status);
+		break;
+
+	case IBMACPI_FAN_NONE:
+	default:
+		len += sprintf(p + len, "status:\t\tnot supported\n");
+	}
+
+	if (fan_control_commands & IBMACPI_FAN_CMD_LEVEL) {
+		len += sprintf(p + len, "commands:\tlevel <level>");
+
+		switch (fan_control_access_mode) {
+		case IBMACPI_FAN_WR_ACPI_SFAN:
+			len += sprintf(p + len, " (<level> is 0-7)\n");
+			break;
+
+		default:
+			len += sprintf(p + len, " (<level> is 0-7, "
+				       "auto, disengaged)\n");
+			break;
+		}
+	}
+
+	if (fan_control_commands & IBMACPI_FAN_CMD_ENABLE)
+		len += sprintf(p + len, "commands:\tenable, disable\n"
+			       "commands:\twatchdog <timeout> (<timeout> is 0 (off), "
+			       "1-120 (seconds))\n");
+
+	if (fan_control_commands & IBMACPI_FAN_CMD_SPEED)
+		len += sprintf(p + len, "commands:\tspeed <speed>"
+			       " (<speed> is 0-65535)\n");
+
+	return len;
+}
+
 static int fan_write_cmd_level(const char *cmd, int *rc)
 {
 	int level;
@@ -2171,16 +2416,18 @@ static int fan_write(char *buf)
 	return rc;
 }
 
-static void fan_watchdog_fire(struct work_struct *ignored)
-{
-	printk(IBM_NOTICE "fan watchdog: enabling fan\n");
-	if (fan_set_enable()) {
-		printk(IBM_ERR "fan watchdog: error while enabling fan\n");
-		/* reschedule for later */
-		fan_watchdog_reset();
-	}
-}
+/****************************************************************************
+ ****************************************************************************
+ *
+ * Infrastructure
+ *
+ ****************************************************************************
+ ****************************************************************************/
 
+/* /proc support */
+static struct proc_dir_entry *proc_dir = NULL;
+
+/* Subdriver registry */
 static struct ibm_struct ibms[] = {
 	{
 	 .name = "driver",
@@ -2301,127 +2548,9 @@ static struct ibm_struct ibms[] = {
 	 },
 };
 
-static int dispatch_read(char *page, char **start, off_t off, int count,
-			 int *eof, void *data)
-{
-	struct ibm_struct *ibm = data;
-	int len;
-
-	if (!ibm || !ibm->read)
-		return -EINVAL;
-
-	len = ibm->read(page);
-	if (len < 0)
-		return len;
-
-	if (len <= off + count)
-		*eof = 1;
-	*start = page + off;
-	len -= off;
-	if (len > count)
-		len = count;
-	if (len < 0)
-		len = 0;
-
-	return len;
-}
-
-static int dispatch_write(struct file *file, const char __user * userbuf,
-			  unsigned long count, void *data)
-{
-	struct ibm_struct *ibm = data;
-	char *kernbuf;
-	int ret;
-
-	if (!ibm || !ibm->write)
-		return -EINVAL;
-
-	kernbuf = kmalloc(count + 2, GFP_KERNEL);
-	if (!kernbuf)
-		return -ENOMEM;
-
-	if (copy_from_user(kernbuf, userbuf, count)) {
-		kfree(kernbuf);
-		return -EFAULT;
-	}
-
-	kernbuf[count] = 0;
-	strcat(kernbuf, ",");
-	ret = ibm->write(kernbuf);
-	if (ret == 0)
-		ret = count;
-
-	kfree(kernbuf);
-
-	return ret;
-}
-
-static void dispatch_notify(acpi_handle handle, u32 event, void *data)
-{
-	struct ibm_struct *ibm = data;
-
-	if (!ibm || !ibm->notify)
-		return;
-
-	ibm->notify(ibm, event);
-}
-
-static int __init setup_notify(struct ibm_struct *ibm)
-{
-	acpi_status status;
-	int ret;
-
-	if (!*ibm->handle)
-		return 0;
-
-	ret = acpi_bus_get_device(*ibm->handle, &ibm->device);
-	if (ret < 0) {
-		printk(IBM_ERR "%s device not present\n", ibm->name);
-		return 0;
-	}
-
-	acpi_driver_data(ibm->device) = ibm;
-	sprintf(acpi_device_class(ibm->device), "%s/%s", IBM_NAME, ibm->name);
-
-	status = acpi_install_notify_handler(*ibm->handle, ibm->type,
-					     dispatch_notify, ibm);
-	if (ACPI_FAILURE(status)) {
-		printk(IBM_ERR "acpi_install_notify_handler(%s) failed: %d\n",
-		       ibm->name, status);
-		return -ENODEV;
-	}
-	ibm->notify_installed = 1;
-	return 0;
-}
-
-static int __init ibm_device_add(struct acpi_device *device)
-{
-	return 0;
-}
-
-static int __init register_ibmacpi_subdriver(struct ibm_struct *ibm)
-{
-	int ret;
-
-	ibm->driver = kzalloc(sizeof(struct acpi_driver), GFP_KERNEL);
-	if (!ibm->driver) {
-		printk(IBM_ERR "kmalloc(ibm->driver) failed\n");
-		return -1;
-	}
-
-	sprintf(ibm->driver->name, "%s_%s", IBM_NAME, ibm->name);
-	ibm->driver->ids = ibm->hid;
-	ibm->driver->ops.add = &ibm_device_add;
-
-	ret = acpi_bus_register_driver(ibm->driver);
-	if (ret < 0) {
-		printk(IBM_ERR "acpi_bus_register_driver(%s) failed: %d\n",
-		       ibm->hid, ret);
-		kfree(ibm->driver);
-	}
-
-	return ret;
-}
+/*
+ * Module and infrastructure proble, init and exit handling
+ */
 
 static int __init ibm_init(struct ibm_struct *ibm)
 {
@@ -2489,27 +2618,35 @@ static void ibm_exit(struct ibm_struct *ibm)
 	}
 }
 
-static void __init ibm_handle_init(char *name,
-				   acpi_handle * handle, acpi_handle parent,
-				   char **paths, int num_paths, char **path)
+/* Probing */
+
+static char *ibm_thinkpad_ec_found = NULL;
+
+static char* __init check_dmi_for_ec(void)
 {
-	int i;
-	acpi_status status;
+	struct dmi_device *dev = NULL;
+	char ec_fw_string[18];
 
-	for (i = 0; i < num_paths; i++) {
-		status = acpi_get_handle(parent, paths[i], handle);
-		if (ACPI_SUCCESS(status)) {
-			*path = paths[i];
-			return;
+	/*
+	 * ThinkPad T23 or newer, A31 or newer, R50e or newer,
+	 * X32 or newer, all Z series;  Some models must have an
+	 * up-to-date BIOS or they will not be detected.
+	 *
+	 * See http://thinkwiki.org/wiki/List_of_DMI_IDs
+	 */
+	while ((dev = dmi_find_device(DMI_DEV_TYPE_OEM_STRING, NULL, dev))) {
+		if (sscanf(dev->name,
+			   "IBM ThinkPad Embedded Controller -[%17c",
+			   ec_fw_string) == 1) {
+			ec_fw_string[sizeof(ec_fw_string) - 1] = 0;
+			ec_fw_string[strcspn(ec_fw_string, " ]")] = 0;
+			return kstrdup(ec_fw_string, GFP_KERNEL);
 		}
 	}
-
-	*handle = NULL;
+	return NULL;
 }
 
-#define IBM_HANDLE_INIT(object)						\
-	ibm_handle_init(#object, &object##_handle, *object##_parent,	\
-		object##_paths, ARRAY_SIZE(object##_paths), &object##_path)
+/* Module init, exit, parameters */
 
 static int __init set_ibm_param(const char *val, struct kernel_param *kp)
 {
@@ -2527,6 +2664,9 @@ static int __init set_ibm_param(const char *val, struct kernel_param *kp)
 	return -EINVAL;
 }
 
+static int experimental;
+module_param(experimental, int, 0);
+
 #define IBM_PARAM(feature) \
 	module_param_call(feature, set_ibm_param, NULL, NULL, 0)
 
@@ -2548,44 +2688,6 @@ IBM_PARAM(brightness);
 IBM_PARAM(volume);
 IBM_PARAM(fan);
 
-static void acpi_ibm_exit(void)
-{
-	int i;
-
-	for (i = ARRAY_SIZE(ibms) - 1; i >= 0; i--)
-		ibm_exit(&ibms[i]);
-
-	if (proc_dir)
-		remove_proc_entry(IBM_DIR, acpi_root_dir);
-
-	if (ibm_thinkpad_ec_found)
-		kfree(ibm_thinkpad_ec_found);
-}
-
-static char* __init check_dmi_for_ec(void)
-{
-	struct dmi_device *dev = NULL;
-	char ec_fw_string[18];
-
-	/*
-	 * ThinkPad T23 or newer, A31 or newer, R50e or newer,
-	 * X32 or newer, all Z series;  Some models must have an
-	 * up-to-date BIOS or they will not be detected.
-	 *
-	 * See http://thinkwiki.org/wiki/List_of_DMI_IDs
-	 */
-	while ((dev = dmi_find_device(DMI_DEV_TYPE_OEM_STRING, NULL, dev))) {
-		if (sscanf(dev->name,
-			   "IBM ThinkPad Embedded Controller -[%17c",
-			   ec_fw_string) == 1) {
-			ec_fw_string[sizeof(ec_fw_string) - 1] = 0;
-			ec_fw_string[strcspn(ec_fw_string, " ]")] = 0;
-			return kstrdup(ec_fw_string, GFP_KERNEL);
-		}
-	}
-	return NULL;
-}
-
 static int __init acpi_ibm_init(void)
 {
 	int ret, i;
@@ -2651,5 +2753,19 @@ static int __init acpi_ibm_init(void)
 	return 0;
 }
 
+static void acpi_ibm_exit(void)
+{
+	int i;
+
+	for (i = ARRAY_SIZE(ibms) - 1; i >= 0; i--)
+		ibm_exit(&ibms[i]);
+
+	if (proc_dir)
+		remove_proc_entry(IBM_DIR, acpi_root_dir);
+
+	if (ibm_thinkpad_ec_found)
+		kfree(ibm_thinkpad_ec_found);
+}
+
 module_init(acpi_ibm_init);
 module_exit(acpi_ibm_exit);
-- 
1.5.0.3


-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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

* [PATCH 5/6] ACPI: ibm-acpi: update copyright notice
  2007-03-12  0:20             ` [PATCH 4/6] ACPI: ibm-acpi: organize code Henrique de Moraes Holschuh
@ 2007-03-12  0:20               ` Henrique de Moraes Holschuh
  2007-03-12  0:20                 ` [PATCH 6/6] ACPI: ibm-acpi: update documentation Henrique de Moraes Holschuh
  0 siblings, 1 reply; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-12  0:20 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, ibm-acpi-devel, Henrique de Moraes Holschuh

Update copyright and license info on the source code comments.  No
functional changes.

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
---
 drivers/acpi/ibm_acpi.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/ibm_acpi.c b/drivers/acpi/ibm_acpi.c
index 7cd2888..85116c2 100644
--- a/drivers/acpi/ibm_acpi.c
+++ b/drivers/acpi/ibm_acpi.c
@@ -3,7 +3,7 @@
  *
  *
  *  Copyright (C) 2004-2005 Borislav Deianov <borislav@users.sf.net>
- *  Copyright (C) 2006 Henrique de Moraes Holschuh <hmh@hmh.eng.br>
+ *  Copyright (C) 2006-2007 Henrique de Moraes Holschuh <hmh@hmh.eng.br>
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -17,7 +17,8 @@
  *
  *  You should have received a copy of the GNU General Public License
  *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ *  02110-1301, USA.
  */
 
 #define IBM_VERSION "0.13"
-- 
1.5.0.3


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

* [PATCH 6/6] ACPI: ibm-acpi: update documentation
  2007-03-12  0:20               ` [PATCH 5/6] ACPI: ibm-acpi: update copyright notice Henrique de Moraes Holschuh
@ 2007-03-12  0:20                 ` Henrique de Moraes Holschuh
  0 siblings, 0 replies; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-12  0:20 UTC (permalink / raw)
  To: lenb; +Cc: linux-acpi, ibm-acpi-devel, Henrique de Moraes Holschuh

Update documentation header, and relocate a hunk of text that was missplaced.

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
---
 Documentation/ibm-acpi.txt |   85 +++++++++++++-------------------------------
 1 files changed, 25 insertions(+), 60 deletions(-)

diff --git a/Documentation/ibm-acpi.txt b/Documentation/ibm-acpi.txt
index cdcef01..f409f4b 100644
--- a/Documentation/ibm-acpi.txt
+++ b/Documentation/ibm-acpi.txt
@@ -1,16 +1,17 @@
 		    IBM ThinkPad ACPI Extras Driver
 
-                            Version 0.12
-                           17 August 2005
+                            Version 0.13
+                           31 December 2006
 
                Borislav Deianov <borislav@users.sf.net>
+	     Henrique de Moraes Holschuh <hmh@hmh.eng.br>
 		      http://ibm-acpi.sf.net/
 
 
 This is a Linux ACPI driver for the IBM ThinkPad laptops. It supports
 various features of these laptops which are accessible through the
-ACPI framework but not otherwise supported by the generic Linux ACPI
-drivers.
+ACPI framework but not otherwise fully supported by the generic Linux
+ACPI drivers.
 
 
 Status
@@ -638,6 +639,26 @@ 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 ibm-acpi.
 
+The ibm-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
 ---------------------------------------
 
@@ -670,59 +691,3 @@ example:
 
 	modprobe ibm_acpi hotkey=enable,0xffff video=auto_disable
 
-The ibm-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.
-
-
-Example Configuration
----------------------
-
-The ACPI support in the kernel is intended to be used in conjunction
-with a user-space daemon, acpid. The configuration files for this
-daemon control what actions are taken in response to various ACPI
-events. An example set of configuration files are included in the
-config/ directory of the tarball package available on the web
-site. Note that these are provided for illustration purposes only and
-may need to be adapted to your particular setup.
-
-The following utility scripts are used by the example action
-scripts (included with ibm-acpi for completeness):
-
-	/usr/local/sbin/idectl -- from the hdparm source distribution,
-		see http://www.ibiblio.org/pub/Linux/system/hardware
-	/usr/local/sbin/laptop_mode -- from the Linux kernel source
-		distribution, see Documentation/laptop-mode.txt
-	/sbin/service -- comes with Redhat/Fedora distributions
-	/usr/sbin/hibernate -- from the Software Suspend 2 distribution,
-		see http://softwaresuspend.berlios.de/
-
-Toan T Nguyen <ntt@physics.ucla.edu> notes that Suse uses the
-powersave program to suspend ('powersave --suspend-to-ram') or
-hibernate ('powersave --suspend-to-disk'). This means that the
-hibernate script is not needed on that distribution.
-
-Henrik Brix Andersen <brix@gentoo.org> has written a Gentoo ACPI event
-handler script for the X31. You can get the latest version from
-http://dev.gentoo.org/~brix/files/x31.sh
-
-David Schweikert <dws@ee.eth.ch> has written an alternative blank.sh
-script which works on Debian systems. This scripts has now been
-extended to also work on Fedora systems and included as the default
-blank.sh in the distribution.
-- 
1.5.0.3


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

* Re: [GIT PULL] ibm-acpi changes acpi-test/2.6.22
  2007-03-12  0:20 [GIT PULL] ibm-acpi changes acpi-test/2.6.22 Henrique de Moraes Holschuh
  2007-03-12  0:20 ` [PATCH 1/6] ACPI: ibm-acpi: kill trailing whitespace Henrique de Moraes Holschuh
@ 2007-03-12  0:54 ` Len Brown
  2007-03-12  1:27   ` Henrique de Moraes Holschuh
  1 sibling, 1 reply; 60+ messages in thread
From: Len Brown @ 2007-03-12  0:54 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh; +Cc: linux-acpi, ibm-acpi-devel

On Sunday 11 March 2007 20:20, Henrique de Moraes Holschuh wrote:
> Len,
> 
> Please pull from:
> git://repo.or.cz/linux-2.6/linux-acpi-2.6/ibm-acpi-2.6.git
> branch for-upstream/acpi-test
> 
> to receive the following patches:
>       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
> 
> Please merge them into acpi-test, targetting 2.6.22.

series applied to acpi-test.

BTW. do you have a plan for moving this driver to drivers/misc?

thanks,
-Len

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

* Re: [GIT PULL] ibm-acpi changes acpi-test/2.6.22
  2007-03-12  0:54 ` [GIT PULL] ibm-acpi changes acpi-test/2.6.22 Len Brown
@ 2007-03-12  1:27   ` Henrique de Moraes Holschuh
  2007-03-15  3:10     ` [GIT PULL] ibm-acpi move to drivers/misc Henrique de Moraes Holschuh
  0 siblings, 1 reply; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-12  1:27 UTC (permalink / raw)
  To: Len Brown; +Cc: linux-acpi, ibm-acpi-devel

On Sun, 11 Mar 2007, Len Brown wrote:
> > Please merge them into acpi-test, targetting 2.6.22.
> 
> series applied to acpi-test.

Thanks.

> BTW. do you have a plan for moving this driver to drivers/misc?

Whenever you want to.  Would you like me to prepare patches for acpi-test,
targetting 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] 60+ messages in thread

* [GIT PULL] ibm-acpi move to drivers/misc
  2007-03-12  1:27   ` Henrique de Moraes Holschuh
@ 2007-03-15  3:10     ` Henrique de Moraes Holschuh
  2007-03-15  3:12       ` [PATCH] ACPI: ibm-acpi: move driver to drivers/misc hierarchy Henrique de Moraes Holschuh
  0 siblings, 1 reply; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-15  3:10 UTC (permalink / raw)
  To: Len Brown; +Cc: linux-acpi, ibm-acpi-devel

Len,

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

to receive the following patch, targetted at 2.6.22:
	ACPI: ibm-acpi: move driver to drivers/misc hierarchy

Note: the branch I am asking you to pull from is the same branch I asked you
to pull for acpi-test before, rebased to your current linus branch.  So, it
contains the other patches you have already accepted, whitespace-stripped by
git rebase.

I hope this doesn't cause you trouble for the pull, if it does, please tell
me how you'd rather I prepare such branches in the future, when it is
impossible to have them against Linus' tree.

The patch follows this message in email format, using git format-patch -M
to reduce the noise tremendously.

-- 
  "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] 60+ messages in thread

* [PATCH] ACPI: ibm-acpi: move driver to drivers/misc hierarchy
  2007-03-15  3:10     ` [GIT PULL] ibm-acpi move to drivers/misc Henrique de Moraes Holschuh
@ 2007-03-15  3:12       ` Henrique de Moraes Holschuh
  2007-03-15  6:06         ` CONFIG_IBM_BAY Len Brown
  0 siblings, 1 reply; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-15  3:12 UTC (permalink / raw)
  To: Len Brown; +Cc: linux-acpi, ibm-acpi-devel

ibm-acpi is not an ACPICA driver, so move it to drivers/misc as per Len
Brown's request.

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
---
 drivers/acpi/Kconfig              |   37 -------------------------------------
 drivers/acpi/Makefile             |    1 -
 drivers/misc/Kconfig              |   37 +++++++++++++++++++++++++++++++++++++
 drivers/misc/Makefile             |    1 +
 drivers/{acpi => misc}/ibm_acpi.c |    0 
 drivers/{acpi => misc}/ibm_acpi.h |    0 
 6 files changed, 38 insertions(+), 38 deletions(-)
 rename drivers/{acpi => misc}/ibm_acpi.c (100%)
 rename drivers/{acpi => misc}/ibm_acpi.h (100%)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index e2ce4a9..45c4315 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -218,43 +218,6 @@ config ACPI_ASUS
 	  NOTE: This driver is deprecated and will probably be removed soon,
 	  use asus-laptop instead.
 
-config ACPI_IBM
-	tristate "IBM ThinkPad Laptop Extras"
-	depends on X86
-	select BACKLIGHT_CLASS_DEVICE
-	---help---
-	  This is a Linux ACPI driver for the IBM ThinkPad laptops. It adds
-	  support for Fn-Fx key combinations, Bluetooth control, video
-	  output switching, ThinkLight control, UltraBay eject and more.
-	  For more information about this driver see <file:Documentation/ibm-acpi.txt>
-	  and <http://ibm-acpi.sf.net/> .
-
-	  If you have an IBM ThinkPad laptop, say Y or M here.
-
-config ACPI_IBM_DOCK
-	bool "Legacy Docking Station Support"
-	depends on ACPI_IBM
-	depends on ACPI_DOCK=n
-	default n
-	---help---
-	  Allows the ibm_acpi driver to handle docking station events.
-	  This support is obsoleted by CONFIG_HOTPLUG_PCI_ACPI.  It will
-	  allow locking and removing the laptop from the docking station,
-	  but will not properly connect PCI devices.
-
-	  If you are not sure, say N here.
-
-config ACPI_IBM_BAY
-	bool "Legacy Removable Bay Support"
-	depends on ACPI_IBM
-	default y
-	---help---
-	  Allows the ibm_acpi driver to handle removable bays.  It will allow
-	  disabling the device in the bay, and also generate notifications when
-	  the bay lever is ejected or inserted.
-
-	  If you are not sure, say Y here.
-
 config ACPI_TOSHIBA
 	tristate "Toshiba Laptop Extras"
 	depends on X86
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 5956e9f..92642ab 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -55,7 +55,6 @@ obj-$(CONFIG_ACPI_SYSTEM)	+= system.o event.o
 obj-$(CONFIG_ACPI_DEBUG)	+= debug.o
 obj-$(CONFIG_ACPI_NUMA)		+= numa.o
 obj-$(CONFIG_ACPI_ASUS)		+= asus_acpi.o
-obj-$(CONFIG_ACPI_IBM)		+= ibm_acpi.o
 obj-$(CONFIG_ACPI_TOSHIBA)	+= toshiba_acpi.o
 obj-$(CONFIG_ACPI_HOTPLUG_MEMORY)	+= acpi_memhotplug.o
 obj-y				+= cm_sbs.o
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 80b199f..5d2bcbf 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -122,4 +122,41 @@ config SONY_LAPTOP
 
 	  Read <file:Documentation/sony-laptop.txt> for more information.
 
+config ACPI_IBM
+	tristate "IBM ThinkPad Laptop Extras"
+	depends on X86 && ACPI
+	select BACKLIGHT_CLASS_DEVICE
+	---help---
+	  This is a Linux ACPI driver for the IBM ThinkPad laptops. It adds
+	  support for Fn-Fx key combinations, Bluetooth control, video
+	  output switching, ThinkLight control, UltraBay eject and more.
+	  For more information about this driver see <file:Documentation/ibm-acpi.txt>
+	  and <http://ibm-acpi.sf.net/> .
+
+	  If you have an IBM ThinkPad laptop, say Y or M here.
+
+config ACPI_IBM_DOCK
+	bool "Legacy Docking Station Support"
+	depends on ACPI_IBM
+	depends on ACPI_DOCK=n
+	default n
+	---help---
+	  Allows the ibm_acpi driver to handle docking station events.
+	  This support is obsoleted by CONFIG_HOTPLUG_PCI_ACPI.  It will
+	  allow locking and removing the laptop from the docking station,
+	  but will not properly connect PCI devices.
+
+	  If you are not sure, say N here.
+
+config ACPI_IBM_BAY
+	bool "Legacy Removable Bay Support"
+	depends on ACPI_IBM
+	default y
+	---help---
+	  Allows the ibm_acpi driver to handle removable bays.  It will allow
+	  disabling the device in the bay, and also generate notifications when
+	  the bay lever is ejected or inserted.
+
+	  If you are not sure, say Y here.
+
 endmenu
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 7793ccd..848b398 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -12,3 +12,4 @@ obj-$(CONFIG_TIFM_CORE)       	+= tifm_core.o
 obj-$(CONFIG_TIFM_7XX1)       	+= tifm_7xx1.o
 obj-$(CONFIG_SGI_IOC4)		+= ioc4.o
 obj-$(CONFIG_SONY_LAPTOP)	+= sony-laptop.o
+obj-$(CONFIG_ACPI_IBM)		+= ibm_acpi.o
diff --git a/drivers/acpi/ibm_acpi.c b/drivers/misc/ibm_acpi.c
similarity index 100%
rename from drivers/acpi/ibm_acpi.c
rename to drivers/misc/ibm_acpi.c
diff --git a/drivers/acpi/ibm_acpi.h b/drivers/misc/ibm_acpi.h
similarity index 100%
rename from drivers/acpi/ibm_acpi.h
rename to drivers/misc/ibm_acpi.h
-- 
1.5.0.3

-- 
  "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 related	[flat|nested] 60+ messages in thread

* CONFIG_IBM_BAY
  2007-03-15  3:12       ` [PATCH] ACPI: ibm-acpi: move driver to drivers/misc hierarchy Henrique de Moraes Holschuh
@ 2007-03-15  6:06         ` Len Brown
  2007-03-15 13:39           ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
  0 siblings, 1 reply; 60+ messages in thread
From: Len Brown @ 2007-03-15  6:06 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh, Kristen Carlson Accardi
  Cc: linux-acpi, ibm-acpi-devel


> +config ACPI_IBM_BAY

Should this depend on ACPI_BAY=n?

> +	bool "Legacy Removable Bay Support"
> +	depends on ACPI_IBM
> +	default y
> +	---help---
> +	  Allows the ibm_acpi driver to handle removable bays.  It will allow
> +	  disabling the device in the bay, and also generate notifications when
> +	  the bay lever is ejected or inserted.
> +
> +	  If you are not sure, say Y here.
>

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

* Re: CONFIG_IBM_BAY
  2007-03-15  6:06         ` CONFIG_IBM_BAY Len Brown
@ 2007-03-15 13:39           ` Henrique de Moraes Holschuh
  2007-03-15 15:06             ` CONFIG_IBM_BAY Kristen Carlson Accardi
  0 siblings, 1 reply; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-15 13:39 UTC (permalink / raw)
  To: Len Brown; +Cc: Kristen Carlson Accardi, linux-acpi, ibm-acpi-devel

On Thu, 15 Mar 2007, Len Brown wrote:
> > +config ACPI_IBM_BAY
> 
> Should this depend on ACPI_BAY=n?

It should allow both as modules, so that the user can choose which one to
run.

-- 
  "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] 60+ messages in thread

* Re: CONFIG_IBM_BAY
  2007-03-15 13:39           ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
@ 2007-03-15 15:06             ` Kristen Carlson Accardi
  2007-03-15 17:55               ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
  0 siblings, 1 reply; 60+ messages in thread
From: Kristen Carlson Accardi @ 2007-03-15 15:06 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh; +Cc: Len Brown, linux-acpi, ibm-acpi-devel

On Thu, 15 Mar 2007 10:39:00 -0300
Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:

> On Thu, 15 Mar 2007, Len Brown wrote:
> > > +config ACPI_IBM_BAY
> > 
> > Should this depend on ACPI_BAY=n?
> 
> It should allow both as modules, so that the user can choose which one to
> run.

Doesn't this option enable support for controlling the bay device via ibm_acpi?
If so - if you compile this one, then the user who chooses to use bay.ko instead
to control the bay can not use ibm_acpi at all to control the remaining
functionality.  I feel this should be dependent on ACPI_BAY=n.
 

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

* Re: CONFIG_IBM_BAY
  2007-03-15 15:06             ` CONFIG_IBM_BAY Kristen Carlson Accardi
@ 2007-03-15 17:55               ` Henrique de Moraes Holschuh
  2007-03-15 18:01                 ` CONFIG_IBM_BAY Kristen Carlson Accardi
  2007-03-15 18:22                 ` CONFIG_IBM_BAY Kristen Carlson Accardi
  0 siblings, 2 replies; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-15 17:55 UTC (permalink / raw)
  To: Kristen Carlson Accardi; +Cc: Len Brown, linux-acpi, ibm-acpi-devel

On Thu, 15 Mar 2007, Kristen Carlson Accardi wrote:
> Doesn't this option enable support for controlling the bay device via ibm_acpi?

Yes, but the problem is that ACPI_BAY is not nearly good enough in
ThinkPads... yet.

> If so - if you compile this one, then the user who chooses to use bay.ko instead
> to control the bay can not use ibm_acpi at all to control the remaining
> functionality.  I feel this should be dependent on ACPI_BAY=n.

Please look at the patch I just sent.  It allows ibm-acpi to load if
ACPI_BAY is loaded.  It will, of course, cause ACPI_BAY to refuse to load if
ibm-acpi is already loaded, but that's probably fine as ACPI_BAY has just a
single function.

Note that these are all just stop-gap measures.  We either need to have
ACPI_BAY do everything needed in ThinkPads (like handling bay batteries), or
to make both drivers work together, or I'll just pull all the ACPI_BAY
functionality into ibm-acpi (and then I will definately conflict with
ACPI_BAY).

-- 
  "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] 60+ messages in thread

* Re: CONFIG_IBM_BAY
  2007-03-15 17:55               ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
@ 2007-03-15 18:01                 ` Kristen Carlson Accardi
       [not found]                   ` <20070315110156.09e80b99.kristen.c.accardi-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  2007-03-15 18:22                 ` CONFIG_IBM_BAY Kristen Carlson Accardi
  1 sibling, 1 reply; 60+ messages in thread
From: Kristen Carlson Accardi @ 2007-03-15 18:01 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh; +Cc: Len Brown, linux-acpi, ibm-acpi-devel

On Thu, 15 Mar 2007 14:55:06 -0300
Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:

> Note that these are all just stop-gap measures.  We either need to have
> ACPI_BAY do everything needed in ThinkPads (like handling bay batteries), or
> to make both drivers work together, or I'll just pull all the ACPI_BAY
> functionality into ibm-acpi (and then I will definately conflict with
> ACPI_BAY).

I think we should focus our efforts on a generic (non platform specific) 
solution that can work across many laptops (i.e. ACPI_BAY).  Let's make
the bay driver do what it should - I've got a stack of modules for Dell
module bays we can check against too - and I believe that Dell also 
supports charging the battery in the module bay.  I am happy to take 
patches to the bay driver, and am also happy to help develop them.  
I don't have handling content of bay changes on my radar at the moment, 
so if you do have time to work on this, that would be great.  I am happy 
to test on the set of laptops I've got.

Thanks,
Kristen

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

* Re: CONFIG_IBM_BAY
  2007-03-15 17:55               ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
  2007-03-15 18:01                 ` CONFIG_IBM_BAY Kristen Carlson Accardi
@ 2007-03-15 18:22                 ` Kristen Carlson Accardi
  2007-03-15 19:33                   ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
  1 sibling, 1 reply; 60+ messages in thread
From: Kristen Carlson Accardi @ 2007-03-15 18:22 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh; +Cc: Len Brown, linux-acpi, ibm-acpi-devel

On Thu, 15 Mar 2007 14:55:06 -0300
Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:

> Please look at the patch I just sent.  It allows ibm-acpi to load if
> ACPI_BAY is loaded.  It will, of course, cause ACPI_BAY to refuse to load if
> ibm-acpi is already loaded, but that's probably fine as ACPI_BAY has just a
> single function.

The patch you sent seems fine - however, the bay driver will need a patch
to correctly refuse to load when ibm_acpi is loaded and handling the bay.
I'm testing this out now - will check it with your patch.

Thanks,
Kristen

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

* Re: CONFIG_IBM_BAY
       [not found]                   ` <20070315110156.09e80b99.kristen.c.accardi-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2007-03-15 19:31                     ` Henrique de Moraes Holschuh
  2007-03-15 19:50                       ` CONFIG_IBM_BAY Kristen Carlson Accardi
                                         ` (2 more replies)
  0 siblings, 3 replies; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-15 19:31 UTC (permalink / raw)
  To: Kristen Carlson Accardi
  Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Len Brown

On Thu, 15 Mar 2007, Kristen Carlson Accardi wrote:
> I think we should focus our efforts on a generic (non platform specific) 
> solution that can work across many laptops (i.e. ACPI_BAY).  Let's make

Agreed, as long as that doesn't mean we have to pollute ACPI_BAY with a
truckload of crap required to deal with vendor/model-specific idioticities.
I hope ThinkPads are not like that, but I am not sure, and I'd feel bad
about adding a lot of quirks to ACPI_BAY because of ThinkPad weirdness :p

Otherwise, it would be best to allow model-specific modules to provide extra
functionality for ACPI_BAY, that way we have generic code, and we can
contain the braindamage to where it belongs (e.g. in ibm-acpi).

> so if you do have time to work on this, that would be great.  I am happy 
> to test on the set of laptops I've got.

I will see what I can do.  My "first approach" at a requirements list for a
proper bay module are as follows:

1. It will handle all device types (ATAPI, PATA, SATA, batteries);

2. It will do the right thing on plug and unplug.  This means telling the
rest of the kernel to disable the device in the bay, for example.  Right now
we shutdown one end of the PATA/SATA link on ThinkPads eletrically, and
leave libata to scream blood murder until it disables its end due to too
many retries, for example;

3. It will allow model-specific drivers (like ibm-acpi) to be notified of
bay events (callbacks or a notifier chain, since ACPICA doesn't allow for
more than one driver notifier per handle) and to request an immediate
attempt of ejection or hotplug/bay device change verification.  Some
thinkpads benefit from an eject before S3 sleep -- as long as what is inside
is not a battery...

I don't see why the generic driver could not do all of the above.  But it
will take a while to get there :-)  And no, ibm-acpi can't do much more than
(1).

-- 
  "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

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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

* Re: CONFIG_IBM_BAY
  2007-03-15 18:22                 ` CONFIG_IBM_BAY Kristen Carlson Accardi
@ 2007-03-15 19:33                   ` Henrique de Moraes Holschuh
  0 siblings, 0 replies; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-15 19:33 UTC (permalink / raw)
  To: Kristen Carlson Accardi; +Cc: Len Brown, linux-acpi, ibm-acpi-devel

On Thu, 15 Mar 2007, Kristen Carlson Accardi wrote:
> On Thu, 15 Mar 2007 14:55:06 -0300
> Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:
> 
> > Please look at the patch I just sent.  It allows ibm-acpi to load if
> > ACPI_BAY is loaded.  It will, of course, cause ACPI_BAY to refuse to load if
> > ibm-acpi is already loaded, but that's probably fine as ACPI_BAY has just a
> > single function.
> 
> The patch you sent seems fine - however, the bay driver will need a patch
> to correctly refuse to load when ibm_acpi is loaded and handling the bay.
> I'm testing this out now - will check it with your patch.

Will it? On 2.6.20 (I backported it) it bangs out with an error and refuses
to load just fine...  The only reason that's not quite acceptable in
ibm-acpi is because ibm-acpi can do a lot more than just handle bay
devices.

-- 
  "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] 60+ messages in thread

* Re: CONFIG_IBM_BAY
  2007-03-15 19:31                     ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
@ 2007-03-15 19:50                       ` Kristen Carlson Accardi
  2007-03-18  5:38                         ` CONFIG_IBM_BAY Tejun Heo
  2007-03-15 20:31                       ` [ibm-acpi-devel] CONFIG_IBM_BAY Shem Multinymous
  2007-03-19 12:04                       ` Theodore Tso
  2 siblings, 1 reply; 60+ messages in thread
From: Kristen Carlson Accardi @ 2007-03-15 19:50 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Len Brown, linux-acpi, ibm-acpi-devel, linux-ide

On Thu, 15 Mar 2007 16:31:24 -0300
Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:

> On Thu, 15 Mar 2007, Kristen Carlson Accardi wrote:
> > I think we should focus our efforts on a generic (non platform specific) 
> > solution that can work across many laptops (i.e. ACPI_BAY).  Let's make
> 
> Agreed, as long as that doesn't mean we have to pollute ACPI_BAY with a
> truckload of crap required to deal with vendor/model-specific idioticities.
> I hope ThinkPads are not like that, but I am not sure, and I'd feel bad
> about adding a lot of quirks to ACPI_BAY because of ThinkPad weirdness :p

So for the dock driver I supplied an interface to allow interested subsystems
to register handlers to be called by the dock driver during dock events.
This allowed me to let individual subsystems do whatever unique things they
needed to do when the dock was either inserted or removed.  For the bay
driver, we could actually provide a mechanism to allow platform specific
drivers to take advantage of the generic stuff, but then have the bay driver
call a registered handler to do some weird specific thing - this way at
least we are sharing the bulk of our code and weird stuff can be contained
somewhere else.

> 
> Otherwise, it would be best to allow model-specific modules to provide extra
> functionality for ACPI_BAY, that way we have generic code, and we can
> contain the braindamage to where it belongs (e.g. in ibm-acpi).
> 
> > so if you do have time to work on this, that would be great.  I am happy 
> > to test on the set of laptops I've got.
> 
> I will see what I can do.  My "first approach" at a requirements list for a
> proper bay module are as follows:
> 
> 1. It will handle all device types (ATAPI, PATA, SATA, batteries);
> 
> 2. It will do the right thing on plug and unplug.  This means telling the
> rest of the kernel to disable the device in the bay, for example.  Right now
> we shutdown one end of the PATA/SATA link on ThinkPads eletrically, and
> leave libata to scream blood murder until it disables its end due to too
> many retries, for example;

Personally - I think tighter integration to libata here would be beneficial.
Once libata acpi support is straightened out, if we can store acpi handles
associated with libata devices, we can perhaps have a mechanism of obtaining
ata_device structs so that we can have a nice way of telling libata to 
delete devices etc.  I am hoping libata-acpi will be straightened out for
2.6.22.

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

* Re: [ibm-acpi-devel] CONFIG_IBM_BAY
  2007-03-15 19:31                     ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
  2007-03-15 19:50                       ` CONFIG_IBM_BAY Kristen Carlson Accardi
@ 2007-03-15 20:31                       ` Shem Multinymous
  2007-03-15 20:49                         ` Henrique de Moraes Holschuh
  2007-03-19 12:04                       ` Theodore Tso
  2 siblings, 1 reply; 60+ messages in thread
From: Shem Multinymous @ 2007-03-15 20:31 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Kristen Carlson Accardi, linux-acpi, ibm-acpi-devel, Len Brown

On 3/15/07, Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:
> 2. It will do the right thing on plug and unplug.  This means telling the
> rest of the kernel to disable the device in the bay, for example.  Right now
> we shutdown one end of the PATA/SATA link on ThinkPads eletrically, and
> leave libata to scream blood murder until it disables its end due to too
> many retries, for example;

I'm not sure which stage of the unplug  you're referring to, but after
a "lever released" event on ThinkPads there must remain a way for
userspace code to run before the kernel does anything irreversible to
the drivers or hardware (e.g., in order to try unmounting filesystems
and, if that failed, tell the user to not pull the lever). The
two-stage mechanical eject mechanisms on ThinkPad is pretty useful in
this respect.

  Shem

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

* Re: [ibm-acpi-devel] CONFIG_IBM_BAY
  2007-03-15 20:31                       ` [ibm-acpi-devel] CONFIG_IBM_BAY Shem Multinymous
@ 2007-03-15 20:49                         ` Henrique de Moraes Holschuh
  0 siblings, 0 replies; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-15 20:49 UTC (permalink / raw)
  To: Shem Multinymous
  Cc: Kristen Carlson Accardi, linux-acpi, ibm-acpi-devel, Len Brown

On Thu, 15 Mar 2007, Shem Multinymous wrote:
> I'm not sure which stage of the unplug  you're referring to, but after
> a "lever released" event on ThinkPads there must remain a way for
> userspace code to run before the kernel does anything irreversible to

I mean when one echo 1 >eject, so it won't clash with what you speak of.

-- 
  "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] 60+ messages in thread

* Re: CONFIG_IBM_BAY
  2007-03-15 19:50                       ` CONFIG_IBM_BAY Kristen Carlson Accardi
@ 2007-03-18  5:38                         ` Tejun Heo
  2007-03-18  8:27                           ` CONFIG_IBM_BAY Holger Macht
       [not found]                           ` <45FCD062.3050001-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  0 siblings, 2 replies; 60+ messages in thread
From: Tejun Heo @ 2007-03-18  5:38 UTC (permalink / raw)
  To: Kristen Carlson Accardi
  Cc: Henrique de Moraes Holschuh, Len Brown, linux-acpi,
	ibm-acpi-devel, linux-ide

Hello,

Kristen Carlson Accardi wrote:
>> 1. It will handle all device types (ATAPI, PATA, SATA, batteries);
>>
>> 2. It will do the right thing on plug and unplug.  This means telling the
>> rest of the kernel to disable the device in the bay, for example.  Right now
>> we shutdown one end of the PATA/SATA link on ThinkPads eletrically, and
>> leave libata to scream blood murder until it disables its end due to too
>> many retries, for example;

:-)

> Personally - I think tighter integration to libata here would be beneficial.
> Once libata acpi support is straightened out, if we can store acpi handles
> associated with libata devices, we can perhaps have a mechanism of obtaining
> ata_device structs so that we can have a nice way of telling libata to 
> delete devices etc.  I am hoping libata-acpi will be straightened out for
> 2.6.22.

I dunno what the thread is really about but can't this be dealt within
acpid?  Finding out the correct scsi host node can be tricky but I think
it can be done reliably by jumping through some sysfs whoops.  Once
you're there, telling libata to kill or probe is really easy.

  http://www.thinkwiki.org/wiki/How_to_hotswap_UltraBay_devices

Thanks.

-- 
tejun

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

* Re: CONFIG_IBM_BAY
  2007-03-18  5:38                         ` CONFIG_IBM_BAY Tejun Heo
@ 2007-03-18  8:27                           ` Holger Macht
  2007-03-18 18:36                             ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
  2007-03-19 13:22                             ` CONFIG_IBM_BAY Matthew Garrett
       [not found]                           ` <45FCD062.3050001-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  1 sibling, 2 replies; 60+ messages in thread
From: Holger Macht @ 2007-03-18  8:27 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Kristen Carlson Accardi, Henrique de Moraes Holschuh, Len Brown,
	linux-acpi, ibm-acpi-devel, linux-ide

On Sun 18. Mar - 14:38:42, Tejun Heo wrote:
> Hello,
> 
> Kristen Carlson Accardi wrote:
> >> 1. It will handle all device types (ATAPI, PATA, SATA, batteries);
> >>
> >> 2. It will do the right thing on plug and unplug.  This means telling the
> >> rest of the kernel to disable the device in the bay, for example.  Right now
> >> we shutdown one end of the PATA/SATA link on ThinkPads eletrically, and
> >> leave libata to scream blood murder until it disables its end due to too
> >> many retries, for example;
> 
> :-)
> 
> > Personally - I think tighter integration to libata here would be beneficial.
> > Once libata acpi support is straightened out, if we can store acpi handles
> > associated with libata devices, we can perhaps have a mechanism of obtaining
> > ata_device structs so that we can have a nice way of telling libata to 
> > delete devices etc.  I am hoping libata-acpi will be straightened out for
> > 2.6.22.
> 
> I dunno what the thread is really about but can't this be dealt within
> acpid?  Finding out the correct scsi host node can be tricky but I think
> it can be done reliably by jumping through some sysfs whoops.  Once
> you're there, telling libata to kill or probe is really easy.

That's actually what I've created dockutils [1] for. What it basically
does is 'echo 1 > /sys/devices/.../delete' on undock, and 'echo "- - -" >
/sys/class/scsi_host/*/scan' on dock to get the bay device back to life on
those ThinkPads where it is needed. Afterwards it does the corresponding
dock/undock request on ibm_acpi. And this works reliably good what I can
see from the feedback I already got. But for this to work, userspace would
actually need an event on docking and preferably would need to do the ACPI
undock itself. Both things are not possible with the generic dock driver
currently. And I don't think it matters much if libata's hotplug
capabilities are improved in this case, userspace just needs some time to
interact with the user in case there is more to do like unmounting
filesystems inside the bay etc...

I don't prefer any solution, whether doing it inside the kernel, or doing
it in userspace. What would be good would be to know what's the 'right'
way to go, or at least what both kernel people and userspace people can
agree on so that we can find a solution across distributions, whatever.
I'm currently just looking how to integrate the generic dock and bay
driver into the openSUSE distribution, and this seems to be quite hard,
especially because of the above mentioned already working solution ;-)

Just my 2 cents,
     Holger

[1] http://en.opensuse.org/Dockutils

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

* Re: CONFIG_IBM_BAY
       [not found]                           ` <45FCD062.3050001-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2007-03-18 18:26                             ` Henrique de Moraes Holschuh
  0 siblings, 0 replies; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-18 18:26 UTC (permalink / raw)
  To: Tejun Heo
  Cc: linux-ide-u79uwXL29TY76Z2rM5mHXA,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Kristen Carlson Accardi, Len Brown

On Sun, 18 Mar 2007, Tejun Heo wrote:
> Kristen Carlson Accardi wrote:
> >> 1. It will handle all device types (ATAPI, PATA, SATA, batteries);
> >>
> >> 2. It will do the right thing on plug and unplug.  This means telling the
> >> rest of the kernel to disable the device in the bay, for example.  Right now
> >> we shutdown one end of the PATA/SATA link on ThinkPads eletrically, and
> >> leave libata to scream blood murder until it disables its end due to too
> >> many retries, for example;
> 
> :-)

Yeah :)  Try it with old ide, and the results are not... pretty.  libata new
EH *rules*.

> > Personally - I think tighter integration to libata here would be beneficial.
> > Once libata acpi support is straightened out, if we can store acpi handles
> > associated with libata devices, we can perhaps have a mechanism of obtaining
> > ata_device structs so that we can have a nice way of telling libata to 
> > delete devices etc.  I am hoping libata-acpi will be straightened out for
> > 2.6.22.
> 
> I dunno what the thread is really about but can't this be dealt within
> acpid?  Finding out the correct scsi host node can be tricky but I think

It could, yes.

But to me the kernel looks like the proper place for this one.  Not to
mention that we *really* should know the ACPI namespace -> kernel device
mapping in kernel, instead of doing half-assed things in userspace to call
back into kernel land later... it may come in handy for other things (proper
ACPI/driver exclusion of access to devices, for one), not just libata
matters and dock/bay ejects.

It is not like it is a policy thing.  The only possible correct path to
follow when doing an eject is to disable the devices and buses in the "it
will be powered off" side of the ejection hardware.  And depending on
userspace for this is just (IMHO) ugly and error-prone.  The kernel really
should know how to do it, and doing it is *easy* as long as you know the
mapping (which you should know for other reasons).

-- 
  "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

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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

* Re: CONFIG_IBM_BAY
  2007-03-18  8:27                           ` CONFIG_IBM_BAY Holger Macht
@ 2007-03-18 18:36                             ` Henrique de Moraes Holschuh
  2007-03-18 18:55                               ` CONFIG_IBM_BAY Holger Macht
  2007-03-19 18:17                               ` CONFIG_IBM_BAY Kristen Carlson Accardi
  2007-03-19 13:22                             ` CONFIG_IBM_BAY Matthew Garrett
  1 sibling, 2 replies; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-18 18:36 UTC (permalink / raw)
  To: Kristen Carlson Accardi, Len Brown,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-ide-u79uwXL29TY76Z2rM5mHXA

On Sun, 18 Mar 2007, Holger Macht wrote:
> those ThinkPads where it is needed. Afterwards it does the corresponding
> dock/undock request on ibm_acpi. And this works reliably good what I can
> see from the feedback I already got. But for this to work, userspace would

It should work with the generic bay device too, but I have no ideas about
dock.  But you'll need to deal with udev with the new bay device, something
I am not too happy about.  These things are ACPI events, they should remain
so unless all other ACPI events are going to become uevents.

-- 
  "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

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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

* Re: CONFIG_IBM_BAY
  2007-03-18 18:36                             ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
@ 2007-03-18 18:55                               ` Holger Macht
  2007-03-18 19:00                                 ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
  2007-03-19 18:04                                 ` CONFIG_IBM_BAY Kristen Carlson Accardi
  2007-03-19 18:17                               ` CONFIG_IBM_BAY Kristen Carlson Accardi
  1 sibling, 2 replies; 60+ messages in thread
From: Holger Macht @ 2007-03-18 18:55 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Kristen Carlson Accardi, Len Brown, linux-acpi, ibm-acpi-devel,
	linux-ide

On Sun 18. Mar - 15:36:52, Henrique de Moraes Holschuh wrote:
> On Sun, 18 Mar 2007, Holger Macht wrote:
> > those ThinkPads where it is needed. Afterwards it does the corresponding
> > dock/undock request on ibm_acpi. And this works reliably good what I can
> > see from the feedback I already got. But for this to work, userspace would
> 
> It should work with the generic bay device too, but I have no ideas about
> dock.  But you'll need to deal with udev with the new bay device, something
> I am not too happy about.  These things are ACPI events, they should remain
> so unless all other ACPI events are going to become uevents.

It doesn't work, I've already tried. The bay driver only emits an event if
you really try to remove the bay, but not on docking/undocking.

Regards,
	Holger

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

* Re: CONFIG_IBM_BAY
  2007-03-18 18:55                               ` CONFIG_IBM_BAY Holger Macht
@ 2007-03-18 19:00                                 ` Henrique de Moraes Holschuh
  2007-03-19 18:04                                 ` CONFIG_IBM_BAY Kristen Carlson Accardi
  1 sibling, 0 replies; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-18 19:00 UTC (permalink / raw)
  To: Kristen Carlson Accardi, Len Brown, linux-acpi, ibm-acpi-devel,
	linux-ide

On Sun, 18 Mar 2007, Holger Macht wrote:
> It doesn't work, I've already tried. The bay driver only emits an event if
> you really try to remove the bay, but not on docking/undocking.

Ah, now I understand what you mean.

Looks like we really need the dock driver to sort of like act as a bus.

-- 
  "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] 60+ messages in thread

* Re: [ibm-acpi-devel] CONFIG_IBM_BAY
  2007-03-15 19:31                     ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
  2007-03-15 19:50                       ` CONFIG_IBM_BAY Kristen Carlson Accardi
  2007-03-15 20:31                       ` [ibm-acpi-devel] CONFIG_IBM_BAY Shem Multinymous
@ 2007-03-19 12:04                       ` Theodore Tso
  2007-03-19 23:24                         ` Henrique de Moraes Holschuh
  2 siblings, 1 reply; 60+ messages in thread
From: Theodore Tso @ 2007-03-19 12:04 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Kristen Carlson Accardi, linux-acpi, ibm-acpi-devel, Len Brown

On Thu, Mar 15, 2007 at 04:31:24PM -0300, Henrique de Moraes Holschuh wrote:
> I will see what I can do.  My "first approach" at a requirements list for a
> proper bay module are as follows:
> 
> 1. It will handle all device types (ATAPI, PATA, SATA, batteries);
> 
> 2. It will do the right thing on plug and unplug.  This means telling the
> rest of the kernel to disable the device in the bay, for example.  Right now
> we shutdown one end of the PATA/SATA link on ThinkPads eletrically, and
> leave libata to scream blood murder until it disables its end due to too
> many retries, for example;
> 
> 3. It will allow model-specific drivers (like ibm-acpi) to be notified of
> bay events (callbacks or a notifier chain, since ACPICA doesn't allow for
> more than one driver notifier per handle) and to request an immediate
> attempt of ejection or hotplug/bay device change verification.  Some
> thinkpads benefit from an eject before S3 sleep -- as long as what is inside
> is not a battery...
> 
> I don't see why the generic driver could not do all of the above.  But it
> will take a while to get there :-)  And no, ibm-acpi can't do much more than
> (1).

One more thing I would add from a power management perspective.  It
should be able to powerdown the bay cleanly on suspend-to-ram, without
necessarily disconnecting libata or unmounting the filesystem.  

It should also be possible for the user to be able to powerdown the
bay (after first unmounting the filesystem and telling libata to let
go of the device) if the user wishes to power down the bay for battery
life reasons --- *without* requiring the user to eject the lever or
remove the bay as a prerequisite to powering down the bay.

(The T60 burns enough power as it is, without any additional help from
the bay.  :-)

						- Ted

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

* Re: CONFIG_IBM_BAY
  2007-03-18  8:27                           ` CONFIG_IBM_BAY Holger Macht
  2007-03-18 18:36                             ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
@ 2007-03-19 13:22                             ` Matthew Garrett
       [not found]                               ` <20070319132243.GA2869-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org>
  2007-03-19 18:00                               ` CONFIG_IBM_BAY Kristen Carlson Accardi
  1 sibling, 2 replies; 60+ messages in thread
From: Matthew Garrett @ 2007-03-19 13:22 UTC (permalink / raw)
  To: Tejun Heo, Kristen Carlson Accardi, Henrique de Moraes Holschuh,
	Len Brown, linux-acpi, ibm-acpi-devel, linux-ide

On Sun, Mar 18, 2007 at 09:27:51AM +0100, Holger Macht wrote:

> I don't prefer any solution, whether doing it inside the kernel, or doing
> it in userspace. What would be good would be to know what's the 'right'
> way to go, or at least what both kernel people and userspace people can
> agree on so that we can find a solution across distributions, whatever.
> I'm currently just looking how to integrate the generic dock and bay
> driver into the openSUSE distribution, and this seems to be quite hard,
> especially because of the above mentioned already working solution ;-)

If the kernel knows that a bay device has just been added or removed, it 
makes sense for the device removal to take place in the kernel rather 
than bouncing it out to userspace and then back into the kernel. Pulling 
out a cardbus card doesn't require us to run a userspace helper to 
detach the hardware.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

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

* Re: CONFIG_IBM_BAY
       [not found]                               ` <20070319132243.GA2869-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org>
@ 2007-03-19 13:48                                 ` Holger Macht
  2007-03-19 14:04                                   ` CONFIG_IBM_BAY Matthew Garrett
  0 siblings, 1 reply; 60+ messages in thread
From: Holger Macht @ 2007-03-19 13:48 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: linux-ide-u79uwXL29TY76Z2rM5mHXA, Tejun Heo,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA, Henrique de Moraes Holschuh,
	Kristen Carlson Accardi, Len Brown

On Mon 19. Mar - 13:22:43, Matthew Garrett wrote:
> On Sun, Mar 18, 2007 at 09:27:51AM +0100, Holger Macht wrote:
> 
> > I don't prefer any solution, whether doing it inside the kernel, or doing
> > it in userspace. What would be good would be to know what's the 'right'
> > way to go, or at least what both kernel people and userspace people can
> > agree on so that we can find a solution across distributions, whatever.
> > I'm currently just looking how to integrate the generic dock and bay
> > driver into the openSUSE distribution, and this seems to be quite hard,
> > especially because of the above mentioned already working solution ;-)
> 
> If the kernel knows that a bay device has just been added or removed, it 
> makes sense for the device removal to take place in the kernel rather 
> than bouncing it out to userspace and then back into the kernel. Pulling 
> out a cardbus card doesn't require us to run a userspace helper to 
> detach the hardware.

Yes, makes sense to me. So if this is the way to go, we would need two
things:

  1. libata integration into the bay driver
  2. The dock station driver has to inform the bay driver that an undock
     event took place, right?

But you still have to deal with mounted filesystems, no matter if it a
cardbus or a cdrom. Wouldn't we need something like 'save removal'
triggered from userspace like you maybe know from 'the other' operating
system?

Regards,
	Holger

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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

* Re: CONFIG_IBM_BAY
  2007-03-19 13:48                                 ` CONFIG_IBM_BAY Holger Macht
@ 2007-03-19 14:04                                   ` Matthew Garrett
  2007-03-19 14:37                                     ` CONFIG_IBM_BAY Holger Macht
       [not found]                                     ` <20070319140403.GA3697-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org>
  0 siblings, 2 replies; 60+ messages in thread
From: Matthew Garrett @ 2007-03-19 14:04 UTC (permalink / raw)
  To: Tejun Heo, Kristen Carlson Accardi, Henrique de Moraes Holschuh,
	Len Brown, linux-acpi, ibm-acpi-devel, linux-ide

On Mon, Mar 19, 2007 at 02:48:22PM +0100, Holger Macht wrote:

>   1. libata integration into the bay driver

It actually makes sense to tie all PATA/SATA devices to their ACPI 
handles where appropriate - there's already code to do that in 
libata-acpi.c, but it would be nicer if it was integrated in the same 
way that PCI devices are.

>   2. The dock station driver has to inform the bay driver that an undock
>      event took place, right?

Yeah.

> But you still have to deal with mounted filesystems, no matter if it a
> cardbus or a cdrom. Wouldn't we need something like 'save removal'
> triggered from userspace like you maybe know from 'the other' operating
> system?

Yes, there's a need for a mechanism to deal with all of this safely, but 
the same is true of any storage device that can be hotplugged (USB, 
firewire, anything in a hotplug bay...)

-- 
Matthew Garrett | mjg59@srcf.ucam.org

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

* Re: CONFIG_IBM_BAY
  2007-03-19 14:04                                   ` CONFIG_IBM_BAY Matthew Garrett
@ 2007-03-19 14:37                                     ` Holger Macht
       [not found]                                       ` <20070319143746.GC4885-tB0uoaAy/Z1bpigZmTR7Iw@public.gmane.org>
       [not found]                                     ` <20070319140403.GA3697-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org>
  1 sibling, 1 reply; 60+ messages in thread
From: Holger Macht @ 2007-03-19 14:37 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Tejun Heo, Kristen Carlson Accardi, Henrique de Moraes Holschuh,
	Len Brown, linux-acpi, ibm-acpi-devel, linux-ide

On Mon 19. Mar - 14:04:03, Matthew Garrett wrote:
> On Mon, Mar 19, 2007 at 02:48:22PM +0100, Holger Macht wrote:
> 
> >   1. libata integration into the bay driver
> 
> It actually makes sense to tie all PATA/SATA devices to their ACPI 
> handles where appropriate - there's already code to do that in 
> libata-acpi.c, but it would be nicer if it was integrated in the same 
> way that PCI devices are.
> 
> >   2. The dock station driver has to inform the bay driver that an undock
> >      event took place, right?
> 
> Yeah.
> 
> > But you still have to deal with mounted filesystems, no matter if it a
> > cardbus or a cdrom. Wouldn't we need something like 'save removal'
> > triggered from userspace like you maybe know from 'the other' operating
> > system?
> 
> Yes, there's a need for a mechanism to deal with all of this safely, but 
> the same is true of any storage device that can be hotplugged (USB, 
> firewire, anything in a hotplug bay...)

Sure. What actually bothers me is that in its current state, the dock
station driver signals 'green' on the dock station as soon as the user
presses the hardware undock button, but regardlessly of anything else. I
think it would be ok to let the ACPI undock event up to userspace because
in many situations it knows best when it is save to undock.

Regards,
	Holger


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

* Re: CONFIG_IBM_BAY
       [not found]                                     ` <20070319140403.GA3697-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org>
@ 2007-03-19 15:36                                       ` Henrique de Moraes Holschuh
  2007-03-19 15:47                                         ` CONFIG_IBM_BAY Holger Macht
       [not found]                                         ` <20070319153659.GB15942-ZGHd14iZgfaRjzvQDGKj+xxZW9W5cXbT@public.gmane.org>
  0 siblings, 2 replies; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-19 15:36 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Tejun Heo, linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	linux-ide-u79uwXL29TY76Z2rM5mHXA,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Kristen Carlson Accardi, Len Brown

On Mon, 19 Mar 2007, Matthew Garrett wrote:
> > But you still have to deal with mounted filesystems, no matter if it a
> > cardbus or a cdrom. Wouldn't we need something like 'save removal'
> > triggered from userspace like you maybe know from 'the other' operating
> > system?
> 
> Yes, there's a need for a mechanism to deal with all of this safely, but 
> the same is true of any storage device that can be hotplugged (USB, 
> firewire, anything in a hotplug bay...)

True.  The best available procedure to do this for ACPI bay/dock currently
is, AFAIK:

1. Send event when bay/dock "please release" button/lever is pressed. Do
*nothing* else.  I know bay does this right, maybe dock doesn't.

2. Wait for something to echo 1 >eject or to call the appropriate routine
directly, or (if this exists) for an notification from firmware that the
user IS disconnecting the device for real.

3. delete the device.  This means force-umount, force-close, etc.

4. Tell the hardware to eject.


Note that currently (2) and (3) are swapped, as (3) is being done by
userspace request, instead of by the kernel.  This is something I *don't*
like.  IMO should do as PCMCIA and hotplug PCI does, and do that in kernel
space.

The time window where one can ask the user something or notify it is not a
good idea to eject is between (1) and (2).  If the eject command is issued,
delete devices (includes sync, of course) and eject.

-- 
  "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

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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

* Re: CONFIG_IBM_BAY
  2007-03-19 15:36                                       ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
@ 2007-03-19 15:47                                         ` Holger Macht
       [not found]                                         ` <20070319153659.GB15942-ZGHd14iZgfaRjzvQDGKj+xxZW9W5cXbT@public.gmane.org>
  1 sibling, 0 replies; 60+ messages in thread
From: Holger Macht @ 2007-03-19 15:47 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Matthew Garrett, Tejun Heo, Kristen Carlson Accardi, Len Brown,
	linux-acpi, ibm-acpi-devel, linux-ide

On Mon 19. Mar - 12:36:59, Henrique de Moraes Holschuh wrote:
> On Mon, 19 Mar 2007, Matthew Garrett wrote:
> > > But you still have to deal with mounted filesystems, no matter if it a
> > > cardbus or a cdrom. Wouldn't we need something like 'save removal'
> > > triggered from userspace like you maybe know from 'the other' operating
> > > system?
> > 
> > Yes, there's a need for a mechanism to deal with all of this safely, but 
> > the same is true of any storage device that can be hotplugged (USB, 
> > firewire, anything in a hotplug bay...)
> 
> True.  The best available procedure to do this for ACPI bay/dock currently
> is, AFAIK:
> 
> 1. Send event when bay/dock "please release" button/lever is pressed. Do
> *nothing* else.  I know bay does this right, maybe dock doesn't.

Yes, the dock driver just does the undock, without any event.

As you know, the "please release" mechanism is how it works with
ibm_acpi. ACPI event comes through proc and userspace does echo undock >
....That would also be my favourite because it already works quite
well. Just without generic docking.

> 2. Wait for something to echo 1 >eject or to call the appropriate routine
> directly, or (if this exists) for an notification from firmware that the
> user IS disconnecting the device for real.
> 
> 3. delete the device.  This means force-umount, force-close, etc.
> 
> 4. Tell the hardware to eject.
> 
> 
> Note that currently (2) and (3) are swapped, as (3) is being done by
> userspace request, instead of by the kernel.  This is something I *don't*
> like.  IMO should do as PCMCIA and hotplug PCI does, and do that in kernel
> space.

Right.

> The time window where one can ask the user something or notify it is not a
> good idea to eject is between (1) and (2).  If the eject command is issued,
> delete devices (includes sync, of course) and eject.

Yes, at this point in time, userspace is required to have finished the
tasks it is responsible for.

Regards,
	Holger

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

* Re: CONFIG_IBM_BAY
       [not found]                                         ` <20070319153659.GB15942-ZGHd14iZgfaRjzvQDGKj+xxZW9W5cXbT@public.gmane.org>
@ 2007-03-19 16:41                                           ` Shem Multinymous
       [not found]                                             ` <41840b750703190941p77fc4a64k49361b678dd6aa90-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 60+ messages in thread
From: Shem Multinymous @ 2007-03-19 16:41 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Matthew Garrett, Tejun Heo, linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-ide-u79uwXL29TY76Z2rM5mHXA, Kristen Carlson Accardi,
	Len Brown

On 3/19/07, Henrique de Moraes Holschuh <hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org> wrote:
> True.  The best available procedure to do this for ACPI bay/dock currently
> is, AFAIK:
>
> 1. Send event when bay/dock "please release" button/lever is pressed. Do
> *nothing* else.  I know bay does this right, maybe dock doesn't.
>
> 2. Wait for something to echo 1 >eject or to call the appropriate routine
> directly, or (if this exists) for an notification from firmware that the
> user IS disconnecting the device for real.
>
> 3. delete the device.  This means force-umount, force-close, etc.
>
> 4. Tell the hardware to eject.
>
>
> Note that currently (2) and (3) are swapped, as (3) is being done by
> userspace request, instead of by the kernel.  This is something I *don't*
> like.

Userspace wants to (non-force-)-unmount by itself after (1), so it can
stop  the eject process if the filesystems cannot be cleanly
unmounted. So the force-unmount at (3) ends up being a redundant
safety measure at best.

  Shem

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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

* Re: CONFIG_IBM_BAY
  2007-03-19 13:22                             ` CONFIG_IBM_BAY Matthew Garrett
       [not found]                               ` <20070319132243.GA2869-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org>
@ 2007-03-19 18:00                               ` Kristen Carlson Accardi
  1 sibling, 0 replies; 60+ messages in thread
From: Kristen Carlson Accardi @ 2007-03-19 18:00 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Tejun Heo, Henrique de Moraes Holschuh, Len Brown, linux-acpi,
	ibm-acpi-devel, linux-ide

On Mon, 19 Mar 2007 13:22:43 +0000
Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> On Sun, Mar 18, 2007 at 09:27:51AM +0100, Holger Macht wrote:
> 
> > I don't prefer any solution, whether doing it inside the kernel, or doing
> > it in userspace. What would be good would be to know what's the 'right'
> > way to go, or at least what both kernel people and userspace people can
> > agree on so that we can find a solution across distributions, whatever.
> > I'm currently just looking how to integrate the generic dock and bay
> > driver into the openSUSE distribution, and this seems to be quite hard,
> > especially because of the above mentioned already working solution ;-)
> 
> If the kernel knows that a bay device has just been added or removed, it 
> makes sense for the device removal to take place in the kernel rather 
> than bouncing it out to userspace and then back into the kernel. Pulling 
> out a cardbus card doesn't require us to run a userspace helper to 
> detach the hardware.
> 
> -- 
> Matthew Garrett | mjg59@srcf.ucam.org
> 

Not to mention, with dock stations some laptops don't actually lock the
laptop into the dock station, so when the user decides to press the button
and undock, it just does it instantly.  And same with some bay devices - 
they don't actually give you an event until the bay is physically removed.
Because of this, I think we should handle things in kernel space.

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

* Re: CONFIG_IBM_BAY
  2007-03-18 18:55                               ` CONFIG_IBM_BAY Holger Macht
  2007-03-18 19:00                                 ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
@ 2007-03-19 18:04                                 ` Kristen Carlson Accardi
  2007-03-20 15:53                                   ` CONFIG_IBM_BAY Holger Macht
  1 sibling, 1 reply; 60+ messages in thread
From: Kristen Carlson Accardi @ 2007-03-19 18:04 UTC (permalink / raw)
  To: Holger Macht
  Cc: Henrique de Moraes Holschuh, Len Brown, linux-acpi,
	ibm-acpi-devel, linux-ide

On Sun, 18 Mar 2007 19:55:30 +0100
Holger Macht <hmacht@suse.de> wrote:

> On Sun 18. Mar - 15:36:52, Henrique de Moraes Holschuh wrote:
> > On Sun, 18 Mar 2007, Holger Macht wrote:
> > > those ThinkPads where it is needed. Afterwards it does the corresponding
> > > dock/undock request on ibm_acpi. And this works reliably good what I can
> > > see from the feedback I already got. But for this to work, userspace would
> > 
> > It should work with the generic bay device too, but I have no ideas about
> > dock.  But you'll need to deal with udev with the new bay device, something
> > I am not too happy about.  These things are ACPI events, they should remain
> > so unless all other ACPI events are going to become uevents.
> 
> It doesn't work, I've already tried. The bay driver only emits an event if
> you really try to remove the bay, but not on docking/undocking.
> 
> Regards,
> 	Holger
> 

this *should* work.  The Bay driver registers with the dock driver to get
dock events:

        /* if we are on a dock station, we should register for dock
         * notifications.
         */
        if (bay_is_dock_device(handle)) {
                bay_dprintk(handle, "Is dependent on dock\n");
                register_hotplug_dock_device(handle, bay_notify, new_bay);
        }
 

then, on undock the dock driver calls the bay_notify function and passes
it the EJECT request event.  This causes the bay driver to emit an event to
userspace.

        case ACPI_NOTIFY_EJECT_REQUEST:
                kobject_uevent(&dev->kobj, KOBJ_CHANGE);
                break;
        default:
 

an event to userspace.

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

* Re: CONFIG_IBM_BAY
       [not found]                                       ` <20070319143746.GC4885-tB0uoaAy/Z1bpigZmTR7Iw@public.gmane.org>
@ 2007-03-19 18:11                                         ` Kristen Carlson Accardi
  2007-03-19 23:11                                           ` [ibm-acpi-devel] CONFIG_IBM_BAY Henrique de Moraes Holschuh
  2007-03-20 16:07                                           ` CONFIG_IBM_BAY Holger Macht
  0 siblings, 2 replies; 60+ messages in thread
From: Kristen Carlson Accardi @ 2007-03-19 18:11 UTC (permalink / raw)
  To: Holger Macht
  Cc: linux-ide-u79uwXL29TY76Z2rM5mHXA, Matthew Garrett, Tejun Heo,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Henrique de Moraes Holschuh, Len Brown

On Mon, 19 Mar 2007 15:37:46 +0100
Holger Macht <hmacht-l3A5Bk7waGM@public.gmane.org> wrote:

> Sure. What actually bothers me is that in its current state, the dock
> station driver signals 'green' on the dock station as soon as the user
> presses the hardware undock button, but regardlessly of anything else. I
> think it would be ok to let the ACPI undock event up to userspace because
> in many situations it knows best when it is save to undock.

I disagree - as I mentioned, not all laptops actually let you (userspace)
control the undock process because they don't lock.  The dock driver
does notify user space of an undock, before it actually undocks:

        /*
         * here we need to generate the undock
         * event prior to actually doing the undock
         * so that the device struct still exists.
         */
        dock_event(ds, event, UNDOCK_EVENT);
        hotplug_dock_devices(ds, ACPI_NOTIFY_EJECT_REQUEST);
        undock(ds);
        eject_dock(ds);
 

We even notify other subsystems of our intent to undock prior to actually
doing it.  

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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

* Re: CONFIG_IBM_BAY
  2007-03-18 18:36                             ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
  2007-03-18 18:55                               ` CONFIG_IBM_BAY Holger Macht
@ 2007-03-19 18:17                               ` Kristen Carlson Accardi
  2007-03-20  0:07                                 ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
  1 sibling, 1 reply; 60+ messages in thread
From: Kristen Carlson Accardi @ 2007-03-19 18:17 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Len Brown, linux-acpi, ibm-acpi-devel, linux-ide

On Sun, 18 Mar 2007 15:36:52 -0300
Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:

> On Sun, 18 Mar 2007, Holger Macht wrote:
> > those ThinkPads where it is needed. Afterwards it does the corresponding
> > dock/undock request on ibm_acpi. And this works reliably good what I can
> > see from the feedback I already got. But for this to work, userspace would
> 
> It should work with the generic bay device too, but I have no ideas about
> dock.  But you'll need to deal with udev with the new bay device, something
> I am not too happy about.  These things are ACPI events, they should remain
> so unless all other ACPI events are going to become uevents.

So ACPI is just a mechanism - an implementation detail in the kernel.  I
don't think we should have special ACPI events to deal with bay/dock activity.
If for whatever reason we ever dock something without using ACPI, we can
make this transparent to userspace.  

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

* Re: CONFIG_IBM_BAY
       [not found]                                             ` <41840b750703190941p77fc4a64k49361b678dd6aa90-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2007-03-19 23:04                                               ` Henrique de Moraes Holschuh
  2007-03-20 14:18                                                 ` [ibm-acpi-devel] CONFIG_IBM_BAY Shem Multinymous
  0 siblings, 1 reply; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-19 23:04 UTC (permalink / raw)
  To: Shem Multinymous
  Cc: Matthew Garrett, Tejun Heo, linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-ide-u79uwXL29TY76Z2rM5mHXA, Kristen Carlson Accardi,
	Len Brown

On Mon, 19 Mar 2007, Shem Multinymous wrote:
> Userspace wants to (non-force-)-unmount by itself after (1), so it can
> stop  the eject process if the filesystems cannot be cleanly
> unmounted. So the force-unmount at (3) ends up being a redundant
> safety measure at best.

More like it is a "make sure we can actually eject, as we have been told
to".  We might return an error instead, but if we do, we need a way to
force-eject (e.g. echo 2 >eject).

-- 
  "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

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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

* Re: [ibm-acpi-devel] CONFIG_IBM_BAY
  2007-03-19 18:11                                         ` CONFIG_IBM_BAY Kristen Carlson Accardi
@ 2007-03-19 23:11                                           ` Henrique de Moraes Holschuh
  2007-03-19 23:56                                             ` Kristen Carlson Accardi
  2007-03-20 16:07                                           ` CONFIG_IBM_BAY Holger Macht
  1 sibling, 1 reply; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-19 23:11 UTC (permalink / raw)
  To: Kristen Carlson Accardi
  Cc: Holger Macht, Matthew Garrett, linux-acpi, ibm-acpi-devel, Len Brown

On Mon, 19 Mar 2007, Kristen Carlson Accardi wrote:
> On Mon, 19 Mar 2007 15:37:46 +0100
> Holger Macht <hmacht@suse.de> wrote:
> > Sure. What actually bothers me is that in its current state, the dock
> > station driver signals 'green' on the dock station as soon as the user
> > presses the hardware undock button, but regardlessly of anything else. I
> > think it would be ok to let the ACPI undock event up to userspace because
> > in many situations it knows best when it is save to undock.
> 
> I disagree - as I mentioned, not all laptops actually let you (userspace)
> control the undock process because they don't lock.  The dock driver
> does notify user space of an undock, before it actually undocks:
> 
>         /*
>          * here we need to generate the undock
>          * event prior to actually doing the undock
>          * so that the device struct still exists.
>          */
>         dock_event(ds, event, UNDOCK_EVENT);
>         hotplug_dock_devices(ds, ACPI_NOTIFY_EJECT_REQUEST);
>         undock(ds);
>         eject_dock(ds);
>  
> 
> We even notify other subsystems of our intent to undock prior to actually
> doing it.

The above does not support two-stage-capable (i.e. non-braindamaged)
hardware docks properly.  While a two-stage (notify always, but undock only
if userspace (or another kernel module) tells you to) can support both
two-stage and broken-as-designed single-stage docks.

You can either let a machine-specific module (e.g. ibm-acpi) generate the
"undock immediately" functionality if the machine has a single-stage dock,
or let userspace do it, or even use black/whitelists on the dock module if
you want... but please give us the two-stage undock support.

-- 
  "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] 60+ messages in thread

* Re: [ibm-acpi-devel] CONFIG_IBM_BAY
  2007-03-19 12:04                       ` Theodore Tso
@ 2007-03-19 23:24                         ` Henrique de Moraes Holschuh
  2007-03-20 14:57                           ` Theodore Tso
  0 siblings, 1 reply; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-19 23:24 UTC (permalink / raw)
  To: Theodore Tso
  Cc: Kristen Carlson Accardi, linux-acpi, ibm-acpi-devel, Len Brown

On Mon, 19 Mar 2007, Theodore Tso wrote:
> One more thing I would add from a power management perspective.  It
> should be able to powerdown the bay cleanly on suspend-to-ram, without
> necessarily disconnecting libata or unmounting the filesystem.  

The disconnect-or-not needs to be user-configurable, as the user may well
swap devices in the bay while the system is sleeping, AND this is supposed
to be a valid thing to do from a usercase PoV (earlier ThinkPad laptops only
had warm-swap bays, for example).  So must be the powerdown, I suppose, just
in case.

> It should also be possible for the user to be able to powerdown the
> bay (after first unmounting the filesystem and telling libata to let
> go of the device) if the user wishes to power down the bay for battery
> life reasons --- *without* requiring the user to eject the lever or
> remove the bay as a prerequisite to powering down the bay.

Currently the drivers do allow you to do just that, at least for the bay. I
don't know about docks.

> (The T60 burns enough power as it is, without any additional help from
> the bay.  :-)

Agreed :)  I want to add configurable powerdown-if-it-is-not-a-battery to
ibm-acpi, as it is acceptable to Linux.  Either Windows or some dumb
harddisk must dislike this for some stupid reason, as the DSDT doesn't do it
by itself and it could easily do it.

In fact, I suppose password-protected HDs might be a nuisance to power off
during S3, as they will be locked up when the system resumes...

-- 
  "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] 60+ messages in thread

* Re: [ibm-acpi-devel] CONFIG_IBM_BAY
  2007-03-19 23:11                                           ` [ibm-acpi-devel] CONFIG_IBM_BAY Henrique de Moraes Holschuh
@ 2007-03-19 23:56                                             ` Kristen Carlson Accardi
  2007-03-20  0:12                                               ` Henrique de Moraes Holschuh
  0 siblings, 1 reply; 60+ messages in thread
From: Kristen Carlson Accardi @ 2007-03-19 23:56 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Holger Macht, Matthew Garrett, linux-acpi, ibm-acpi-devel, Len Brown

On Mon, 19 Mar 2007 20:11:19 -0300
Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:

> On Mon, 19 Mar 2007, Kristen Carlson Accardi wrote:
> > On Mon, 19 Mar 2007 15:37:46 +0100
> > Holger Macht <hmacht@suse.de> wrote:
> > > Sure. What actually bothers me is that in its current state, the dock
> > > station driver signals 'green' on the dock station as soon as the user
> > > presses the hardware undock button, but regardlessly of anything else. I
> > > think it would be ok to let the ACPI undock event up to userspace because
> > > in many situations it knows best when it is save to undock.
> > 
> > I disagree - as I mentioned, not all laptops actually let you (userspace)
> > control the undock process because they don't lock.  The dock driver
> > does notify user space of an undock, before it actually undocks:
> > 
> >         /*
> >          * here we need to generate the undock
> >          * event prior to actually doing the undock
> >          * so that the device struct still exists.
> >          */
> >         dock_event(ds, event, UNDOCK_EVENT);
> >         hotplug_dock_devices(ds, ACPI_NOTIFY_EJECT_REQUEST);
> >         undock(ds);
> >         eject_dock(ds);
> >  
> > 
> > We even notify other subsystems of our intent to undock prior to actually
> > doing it.
> 
> The above does not support two-stage-capable (i.e. non-braindamaged)
> hardware docks properly.  While a two-stage (notify always, but undock only
> if userspace (or another kernel module) tells you to) can support both
> two-stage and broken-as-designed single-stage docks.
> 
> You can either let a machine-specific module (e.g. ibm-acpi) generate the
> "undock immediately" functionality if the machine has a single-stage dock,
> or let userspace do it, or even use black/whitelists on the dock module if
> you want... but please give us the two-stage undock support.
> 
> -- 

Is this what you had in mind?  (Warning - I didn't test this).


Allow the driver to be loaded with an option that will allow userspace to
control whether the laptop is ejected immediately when the user presses the
button, or only when the syfs undock file is written.  

if immediate_undock == 1, then when the user presses the undock button, the
laptop will send an event to userspace to notify userspace of the undock, but
then immediately undock without waiting for userspace.  This is the current
behavior, and I set this to be the default.

if immediate_undock == 0, then when the user presses the undock button, the
laptop will send an event to userspace and do nothing.  User space can query
the "flags" sysfs entry to determine if an undock request has been made by
the user (if bit 1 is set).  User space will then need to write the undock 
sysfs entry to complete the undocking process.

Signed-off-by: Kristen Carlson Accardi <kristen.c.accardi@intel.com>
Index: 2.6-git/drivers/acpi/dock.c
===================================================================
--- 2.6-git.orig/drivers/acpi/dock.c
+++ 2.6-git/drivers/acpi/dock.c
@@ -39,6 +39,9 @@ MODULE_AUTHOR("Kristen Carlson Accardi")
 MODULE_DESCRIPTION(ACPI_DOCK_DRIVER_DESCRIPTION);
 MODULE_LICENSE("GPL");
 
+static int immediate_undock;
+module_param(immediate_undock, int, 1);
+
 static struct atomic_notifier_head dock_notifier_list;
 static struct platform_device dock_device;
 static char dock_device_name[] = "dock";
@@ -62,6 +65,7 @@ struct dock_dependent_device {
 };
 
 #define DOCK_DOCKING	0x00000001
+#define DOCK_UNDOCKING  0x00000002
 #define DOCK_EVENT	3
 #define UNDOCK_EVENT	2
 
@@ -419,6 +423,16 @@ static inline void complete_dock(struct 
 	ds->last_dock_time = jiffies;
 }
 
+static inline void begin_undock(struct dock_station *ds)
+{
+	ds->flags |= DOCK_UNDOCKING;
+}
+
+static inline void complete_undock(struct dock_station *ds)
+{
+	ds->flags &= ~(DOCK_UNDOCKING);
+}
+
 /**
  * dock_in_progress - see if we are in the middle of handling a dock event
  * @ds: the dock station
@@ -549,7 +563,7 @@ static int handle_eject_request(struct d
 		printk(KERN_ERR PREFIX "Unable to undock!\n");
 		return -EBUSY;
 	}
-
+	complete_undock(ds);
 	return 0;
 }
 
@@ -593,7 +607,11 @@ static void dock_notify(acpi_handle hand
 	 * to the driver who wish to hotplug.
          */
 	case ACPI_NOTIFY_EJECT_REQUEST:
-		handle_eject_request(ds, event);
+		begin_undock(ds);
+		if (immediate_undock)
+			handle_eject_request(ds, event);
+		else
+			dock_event(ds, event, UNDOCK_EVENT);
 		break;
 	default:
 		printk(KERN_ERR PREFIX "Unknown dock event %d\n", event);
@@ -652,6 +670,17 @@ static ssize_t show_docked(struct device
 DEVICE_ATTR(docked, S_IRUGO, show_docked, NULL);
 
 /*
+ * show_flags - read method for flags file in sysfs
+ */
+static ssize_t show_flags(struct device *dev,
+			  struct device_attribute *attr, char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%d\n", dock_station->flags);
+
+}
+DEVICE_ATTR(flags, S_IRUGO, show_flags, NULL);
+
+/*
  * write_undock - write method for "undock" file in sysfs
  */
 static ssize_t write_undock(struct device *dev, struct device_attribute *attr,
@@ -667,6 +696,7 @@ static ssize_t write_undock(struct devic
 }
 DEVICE_ATTR(undock, S_IWUSR, NULL, write_undock);
 
+
 /**
  * dock_add - add a new dock station
  * @handle: the dock station handle
@@ -715,6 +745,15 @@ static int dock_add(acpi_handle handle)
 		kfree(dock_station);
 		return ret;
 	}
+	ret = device_create_file(&dock_device.dev, &dev_attr_flags);
+	if (ret) {
+		printk("Error %d adding sysfs file\n", ret);
+		device_remove_file(&dock_device.dev, &dev_attr_docked);
+		device_remove_file(&dock_device.dev, &dev_attr_undock);
+		platform_device_unregister(&dock_device);
+		kfree(dock_station);
+		return ret;
+	}
 
 	/* Find dependent devices */
 	acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
@@ -750,6 +789,7 @@ dock_add_err:
 dock_add_err_unregister:
 	device_remove_file(&dock_device.dev, &dev_attr_docked);
 	device_remove_file(&dock_device.dev, &dev_attr_undock);
+	device_remove_file(&dock_device.dev, &dev_attr_flags);
 	platform_device_unregister(&dock_device);
 	kfree(dock_station);
 	return ret;
@@ -781,6 +821,7 @@ static int dock_remove(void)
 	/* cleanup sysfs */
 	device_remove_file(&dock_device.dev, &dev_attr_docked);
 	device_remove_file(&dock_device.dev, &dev_attr_undock);
+	device_remove_file(&dock_device.dev, &dev_attr_flags);
 	platform_device_unregister(&dock_device);
 
 	/* free dock station memory */

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

* Re: CONFIG_IBM_BAY
  2007-03-19 18:17                               ` CONFIG_IBM_BAY Kristen Carlson Accardi
@ 2007-03-20  0:07                                 ` Henrique de Moraes Holschuh
       [not found]                                   ` <20070320000706.GE5932-ZGHd14iZgfaRjzvQDGKj+xxZW9W5cXbT@public.gmane.org>
  0 siblings, 1 reply; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-20  0:07 UTC (permalink / raw)
  To: Kristen Carlson Accardi; +Cc: Len Brown, linux-acpi, ibm-acpi-devel

On Mon, 19 Mar 2007, Kristen Carlson Accardi wrote:
> > It should work with the generic bay device too, but I have no ideas about
> > dock.  But you'll need to deal with udev with the new bay device, something
> > I am not too happy about.  These things are ACPI events, they should remain
> > so unless all other ACPI events are going to become uevents.
> 
> So ACPI is just a mechanism - an implementation detail in the kernel.  I
> don't think we should have special ACPI events to deal with bay/dock activity.

Well, that's how the userspace power management structure operates right
now.  We can walk away from that, of course, but if we are going to, then
let's do it for the entire set of ACPI events sent to userspace.

In the meanwhile, I would really appreciate a way to hook ibm-acpi into bay
so that I can provide the ACPI events ibm-acpi used to generate... we can
deprecate them and remove them in one year's time, or when the
/proc/acpi/events (and its sysfs counterpart) interface gets removed,
whichever happens first.

> If for whatever reason we ever dock something without using ACPI, we can
> make this transparent to userspace.  

Yes.  But this is also true for battery, thermal notifications,
suspend/resume, and pretty much every ACPI event :-)

-- 
  "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] 60+ messages in thread

* Re: [ibm-acpi-devel] CONFIG_IBM_BAY
  2007-03-19 23:56                                             ` Kristen Carlson Accardi
@ 2007-03-20  0:12                                               ` Henrique de Moraes Holschuh
  2007-03-20 16:11                                                 ` Kristen Carlson Accardi
  0 siblings, 1 reply; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-20  0:12 UTC (permalink / raw)
  To: Kristen Carlson Accardi
  Cc: Holger Macht, Matthew Garrett, linux-acpi, ibm-acpi-devel, Len Brown

On Mon, 19 Mar 2007, Kristen Carlson Accardi wrote:
> Is this what you had in mind?  (Warning - I didn't test this).
> 
> 
> Allow the driver to be loaded with an option that will allow userspace to
> control whether the laptop is ejected immediately when the user presses the
> button, or only when the syfs undock file is written.  
> 
> if immediate_undock == 1, then when the user presses the undock button, the
> laptop will send an event to userspace to notify userspace of the undock, but
> then immediately undock without waiting for userspace.  This is the current
> behavior, and I set this to be the default.
> 
> if immediate_undock == 0, then when the user presses the undock button, the
> laptop will send an event to userspace and do nothing.  User space can query
> the "flags" sysfs entry to determine if an undock request has been made by
> the user (if bit 1 is set).  User space will then need to write the undock 
> sysfs entry to complete the undocking process.

Yes, that's it exactly.  I didn't try to test it, as I don't have a dock,
but the above describes what I wanted, indeed.

-- 
  "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] 60+ messages in thread

* Re: [ibm-acpi-devel] CONFIG_IBM_BAY
  2007-03-19 23:04                                               ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
@ 2007-03-20 14:18                                                 ` Shem Multinymous
  2007-03-20 15:12                                                   ` Henrique de Moraes Holschuh
  0 siblings, 1 reply; 60+ messages in thread
From: Shem Multinymous @ 2007-03-20 14:18 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Matthew Garrett, Tejun Heo, linux-acpi, ibm-acpi-devel,
	linux-ide, Kristen Carlson Accardi, Len Brown

On 3/19/07, Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:
> On Mon, 19 Mar 2007, Shem Multinymous wrote:
> > Userspace wants to (non-force-)-unmount by itself after (1), so it can
> > stop  the eject process if the filesystems cannot be cleanly
> > unmounted. So the force-unmount at (3) ends up being a redundant
> > safety measure at best.
>
> More like it is a "make sure we can actually eject, as we have been told
> to".  We might return an error instead, but if we do, we need a way to
> force-eject (e.g. echo 2 >eject).

Which stage are you referring to?

Note that the only way to check if you can unmount is to actually do
so. Otherwise, you're very likely to run into race conditions with
someone accessing the filesystem between  the check and the actual
unmount.

  Shem

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

* Re: [ibm-acpi-devel] CONFIG_IBM_BAY
  2007-03-19 23:24                         ` Henrique de Moraes Holschuh
@ 2007-03-20 14:57                           ` Theodore Tso
  2007-03-20 15:14                             ` Henrique de Moraes Holschuh
  0 siblings, 1 reply; 60+ messages in thread
From: Theodore Tso @ 2007-03-20 14:57 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Kristen Carlson Accardi, linux-acpi, ibm-acpi-devel, Len Brown

On Mon, Mar 19, 2007 at 08:24:19PM -0300, Henrique de Moraes Holschuh wrote:
> On Mon, 19 Mar 2007, Theodore Tso wrote:
> > One more thing I would add from a power management perspective.  It
> > should be able to powerdown the bay cleanly on suspend-to-ram, without
> > necessarily disconnecting libata or unmounting the filesystem.  
> 
> The disconnect-or-not needs to be user-configurable, as the user may well
> swap devices in the bay while the system is sleeping, AND this is supposed
> to be a valid thing to do from a usercase PoV (earlier ThinkPad laptops only
> had warm-swap bays, for example).  So must be the powerdown, I suppose, just
> in case.

Maybe, but then the suspend mail have to fail if there are processes
that are keeping the filesystem from being unmounted.  Or the
processes hanging out on the hard drive or CD-ROM could get all of
their file descriptors revoked on resume, I suppose, if the bay has
been switched out.....

							- Ted

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

* Re: [ibm-acpi-devel] CONFIG_IBM_BAY
  2007-03-20 14:18                                                 ` [ibm-acpi-devel] CONFIG_IBM_BAY Shem Multinymous
@ 2007-03-20 15:12                                                   ` Henrique de Moraes Holschuh
  0 siblings, 0 replies; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-20 15:12 UTC (permalink / raw)
  To: Shem Multinymous
  Cc: Matthew Garrett, Tejun Heo, linux-acpi, ibm-acpi-devel,
	linux-ide, Kristen Carlson Accardi, Len Brown

On Tue, 20 Mar 2007, Shem Multinymous wrote:
> >More like it is a "make sure we can actually eject, as we have been told
> >to".  We might return an error instead, but if we do, we need a way to
> >force-eject (e.g. echo 2 >eject).
> 
> Which stage are you referring to?

Stage 2, of course.  It is useful to have a force_eject functionality that
**will** eject and not error out because of anything other than the eject
command itself failing.

-- 
  "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] 60+ messages in thread

* Re: [ibm-acpi-devel] CONFIG_IBM_BAY
  2007-03-20 14:57                           ` Theodore Tso
@ 2007-03-20 15:14                             ` Henrique de Moraes Holschuh
  2007-03-20 18:45                               ` Theodore Tso
  0 siblings, 1 reply; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-20 15:14 UTC (permalink / raw)
  To: Theodore Tso
  Cc: Kristen Carlson Accardi, linux-acpi, ibm-acpi-devel, Len Brown

On Tue, 20 Mar 2007, Theodore Tso wrote:
> > The disconnect-or-not needs to be user-configurable, as the user may well
> > swap devices in the bay while the system is sleeping, AND this is supposed
> > to be a valid thing to do from a usercase PoV (earlier ThinkPad laptops only
> > had warm-swap bays, for example).  So must be the powerdown, I suppose, just
> > in case.
> 
> Maybe, but then the suspend mail have to fail if there are processes
> that are keeping the filesystem from being unmounted.  Or the

Yes. But this is standard functionality on the suspend path, anyway.

> processes hanging out on the hard drive or CD-ROM could get all of
> their file descriptors revoked on resume, I suppose, if the bay has
> been switched out.....

Urk.  It is better to either fail the suspend, or to not power down the bay.
Or to give the user a choice of which he'd rather happen.  Hmm...

-- 
  "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] 60+ messages in thread

* Re: CONFIG_IBM_BAY
  2007-03-19 18:04                                 ` CONFIG_IBM_BAY Kristen Carlson Accardi
@ 2007-03-20 15:53                                   ` Holger Macht
  2007-03-20 16:19                                     ` CONFIG_IBM_BAY Kristen Carlson Accardi
  0 siblings, 1 reply; 60+ messages in thread
From: Holger Macht @ 2007-03-20 15:53 UTC (permalink / raw)
  To: Kristen Carlson Accardi
  Cc: Henrique de Moraes Holschuh, Len Brown, linux-acpi,
	ibm-acpi-devel, linux-ide

On Mon 19. Mar - 11:04:12, Kristen Carlson Accardi wrote:
> On Sun, 18 Mar 2007 19:55:30 +0100
> Holger Macht <hmacht@suse.de> wrote:
> 
> > On Sun 18. Mar - 15:36:52, Henrique de Moraes Holschuh wrote:
> > > On Sun, 18 Mar 2007, Holger Macht wrote:
> > > > those ThinkPads where it is needed. Afterwards it does the corresponding
> > > > dock/undock request on ibm_acpi. And this works reliably good what I can
> > > > see from the feedback I already got. But for this to work, userspace would
> > > 
> > > It should work with the generic bay device too, but I have no ideas about
> > > dock.  But you'll need to deal with udev with the new bay device, something
> > > I am not too happy about.  These things are ACPI events, they should remain
> > > so unless all other ACPI events are going to become uevents.
> > 
> > It doesn't work, I've already tried. The bay driver only emits an event if
> > you really try to remove the bay, but not on docking/undocking.
> > 
> > Regards,
> > 	Holger
> > 
> 
> this *should* work.  The Bay driver registers with the dock driver to get
> dock events:
> 
>         /* if we are on a dock station, we should register for dock
>          * notifications.
>          */
>         if (bay_is_dock_device(handle)) {
>                 bay_dprintk(handle, "Is dependent on dock\n");
>                 register_hotplug_dock_device(handle, bay_notify, new_bay);
>         }
>  

But is_dock_device(...) for both the bay and for the parent handle return
false. I'm using an X60 here, so bay_notify is never registered. I
couldn't find the reason in the short time I was looking at it, though.

Regards,
	Holger

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

* Re: CONFIG_IBM_BAY
  2007-03-19 18:11                                         ` CONFIG_IBM_BAY Kristen Carlson Accardi
  2007-03-19 23:11                                           ` [ibm-acpi-devel] CONFIG_IBM_BAY Henrique de Moraes Holschuh
@ 2007-03-20 16:07                                           ` Holger Macht
       [not found]                                             ` <20070320160707.GB6079-tB0uoaAy/Z1bpigZmTR7Iw@public.gmane.org>
  1 sibling, 1 reply; 60+ messages in thread
From: Holger Macht @ 2007-03-20 16:07 UTC (permalink / raw)
  To: Kristen Carlson Accardi
  Cc: Matthew Garrett, Tejun Heo, Henrique de Moraes Holschuh,
	Len Brown, linux-acpi, ibm-acpi-devel, linux-ide

On Mon 19. Mar - 11:11:09, Kristen Carlson Accardi wrote:
> On Mon, 19 Mar 2007 15:37:46 +0100
> Holger Macht <hmacht@suse.de> wrote:
> 
> > Sure. What actually bothers me is that in its current state, the dock
> > station driver signals 'green' on the dock station as soon as the user
> > presses the hardware undock button, but regardlessly of anything else. I
> > think it would be ok to let the ACPI undock event up to userspace because
> > in many situations it knows best when it is save to undock.
> 
> I disagree - as I mentioned, not all laptops actually let you (userspace)
> control the undock process because they don't lock.  The dock driver
> does notify user space of an undock, before it actually undocks:

Yes, I know that, but userspace won't have enough time to interact with
the user in this case. I just imagine having an external disk connected to
the usb port of the docking station. In the moment the user undocks, his
unsynced or unwritten data is lost because the usb port gets
disconnected. The user needs the knowledge to actually "save remove" the
external disk before he undocks. You are right, this won't help in case
the dock station has no indication when it is "safe" to undock. 

> 
>         /*
>          * here we need to generate the undock
>          * event prior to actually doing the undock
>          * so that the device struct still exists.
>          */
>         dock_event(ds, event, UNDOCK_EVENT);
>         hotplug_dock_devices(ds, ACPI_NOTIFY_EJECT_REQUEST);
>         undock(ds);
>         eject_dock(ds);
>  
> 
> We even notify other subsystems of our intent to undock prior to actually
> doing it.  

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

* Re: [ibm-acpi-devel] CONFIG_IBM_BAY
  2007-03-20  0:12                                               ` Henrique de Moraes Holschuh
@ 2007-03-20 16:11                                                 ` Kristen Carlson Accardi
  0 siblings, 0 replies; 60+ messages in thread
From: Kristen Carlson Accardi @ 2007-03-20 16:11 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Holger Macht, Matthew Garrett, linux-acpi, ibm-acpi-devel, Len Brown

On Mon, 19 Mar 2007 21:12:40 -0300
Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:

> On Mon, 19 Mar 2007, Kristen Carlson Accardi wrote:
> > Is this what you had in mind?  (Warning - I didn't test this).
> > 
> > 
> > Allow the driver to be loaded with an option that will allow userspace to
> > control whether the laptop is ejected immediately when the user presses the
> > button, or only when the syfs undock file is written.  
> > 
> > if immediate_undock == 1, then when the user presses the undock button, the
> > laptop will send an event to userspace to notify userspace of the undock, but
> > then immediately undock without waiting for userspace.  This is the current
> > behavior, and I set this to be the default.
> > 
> > if immediate_undock == 0, then when the user presses the undock button, the
> > laptop will send an event to userspace and do nothing.  User space can query
> > the "flags" sysfs entry to determine if an undock request has been made by
> > the user (if bit 1 is set).  User space will then need to write the undock 
> > sysfs entry to complete the undocking process.
> 
> Yes, that's it exactly.  I didn't try to test it, as I don't have a dock,
> but the above describes what I wanted, indeed.
> 

Ok - I will implement this for real later this week and try to get something
that is somewhat tested by friday.  I won't have access to my test laptops
till later in the week.

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

* Re: CONFIG_IBM_BAY
       [not found]                                   ` <20070320000706.GE5932-ZGHd14iZgfaRjzvQDGKj+xxZW9W5cXbT@public.gmane.org>
@ 2007-03-20 16:14                                     ` Kristen Carlson Accardi
  0 siblings, 0 replies; 60+ messages in thread
From: Kristen Carlson Accardi @ 2007-03-20 16:14 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Len Brown

On Mon, 19 Mar 2007 21:07:06 -0300
Henrique de Moraes Holschuh <hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org> wrote:

> In the meanwhile, I would really appreciate a way to hook ibm-acpi into bay
> so that I can provide the ACPI events ibm-acpi used to generate... we can
> deprecate them and remove them in one year's time, or when the
> /proc/acpi/events (and its sysfs counterpart) interface gets removed,
> whichever happens first.

Ok - I'll design an interface for the bay driver which will allow platform
specific drivers to have a way to do whatever special things they want
to do.  Hopefully will get something for review next week.

Kristen

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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

* Re: CONFIG_IBM_BAY
       [not found]                                             ` <20070320160707.GB6079-tB0uoaAy/Z1bpigZmTR7Iw@public.gmane.org>
@ 2007-03-20 16:16                                               ` Holger Macht
  0 siblings, 0 replies; 60+ messages in thread
From: Holger Macht @ 2007-03-20 16:16 UTC (permalink / raw)
  To: Kristen Carlson Accardi, Matthew Garrett, Tejun Heo,
	Henrique de Moraes Holschuh, Len Brown,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-ide-u79uwXL29TY76Z2rM5mHXA

On Tue 20. Mar - 17:07:07, Holger Macht wrote:
> On Mon 19. Mar - 11:11:09, Kristen Carlson Accardi wrote:
> > On Mon, 19 Mar 2007 15:37:46 +0100
> > Holger Macht <hmacht-l3A5Bk7waGM@public.gmane.org> wrote:
> > 
> > > Sure. What actually bothers me is that in its current state, the dock
> > > station driver signals 'green' on the dock station as soon as the user
> > > presses the hardware undock button, but regardlessly of anything else. I
> > > think it would be ok to let the ACPI undock event up to userspace because
> > > in many situations it knows best when it is save to undock.
> > 
> > I disagree - as I mentioned, not all laptops actually let you (userspace)
> > control the undock process because they don't lock.  The dock driver
> > does notify user space of an undock, before it actually undocks:
> 
> Yes, I know that, but userspace won't have enough time to interact with
> the user in this case. I just imagine having an external disk connected to
> the usb port of the docking station. In the moment the user undocks, his
> unsynced or unwritten data is lost because the usb port gets
> disconnected. The user needs the knowledge to actually "save remove" the
> external disk before he undocks. You are right, this won't help in case
> the dock station has no indication when it is "safe" to undock. 

This mail was accidentially sent a little bit too early ;-)

What I wanted to add is just that with letting it up to userspace, it
would also work in all cases, with or without lock, it would just give
userspace more possibilities and might be more convenient for those
systems where the dock station locks the machine. But I clearly see the
advantage of not needing a userspace helper and for getting this docking
thing to work in 99% of all cases ;-)

Regards,
	Holger

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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

* Re: CONFIG_IBM_BAY
  2007-03-20 15:53                                   ` CONFIG_IBM_BAY Holger Macht
@ 2007-03-20 16:19                                     ` Kristen Carlson Accardi
       [not found]                                       ` <20070320091932.f61bbdcc.kristen.c.accardi-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 60+ messages in thread
From: Kristen Carlson Accardi @ 2007-03-20 16:19 UTC (permalink / raw)
  To: Holger Macht
  Cc: Henrique de Moraes Holschuh, Len Brown, linux-acpi,
	ibm-acpi-devel, linux-ide

On Tue, 20 Mar 2007 16:53:21 +0100
Holger Macht <hmacht@suse.de> wrote:

> On Mon 19. Mar - 11:04:12, Kristen Carlson Accardi wrote:
> > On Sun, 18 Mar 2007 19:55:30 +0100
> > Holger Macht <hmacht@suse.de> wrote:
> > 
> > > On Sun 18. Mar - 15:36:52, Henrique de Moraes Holschuh wrote:
> > > > On Sun, 18 Mar 2007, Holger Macht wrote:
> > > > > those ThinkPads where it is needed. Afterwards it does the corresponding
> > > > > dock/undock request on ibm_acpi. And this works reliably good what I can
> > > > > see from the feedback I already got. But for this to work, userspace would
> > > > 
> > > > It should work with the generic bay device too, but I have no ideas about
> > > > dock.  But you'll need to deal with udev with the new bay device, something
> > > > I am not too happy about.  These things are ACPI events, they should remain
> > > > so unless all other ACPI events are going to become uevents.
> > > 
> > > It doesn't work, I've already tried. The bay driver only emits an event if
> > > you really try to remove the bay, but not on docking/undocking.
> > > 
> > > Regards,
> > > 	Holger
> > > 
> > 
> > this *should* work.  The Bay driver registers with the dock driver to get
> > dock events:
> > 
> >         /* if we are on a dock station, we should register for dock
> >          * notifications.
> >          */
> >         if (bay_is_dock_device(handle)) {
> >                 bay_dprintk(handle, "Is dependent on dock\n");
> >                 register_hotplug_dock_device(handle, bay_notify, new_bay);
> >         }
> >  
> 
> But is_dock_device(...) for both the bay and for the parent handle return
> false. I'm using an X60 here, so bay_notify is never registered. I
> couldn't find the reason in the short time I was looking at it, though.
> 
> Regards,
> 	Holger
> 

Works for me on my X60 - have you tested on the latest rc kernel?  Also,
let me know what your BIOS sets your SATA controller to.  I can try to 
duplicate your issue and get it to work.

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

* Re: CONFIG_IBM_BAY
       [not found]                                       ` <20070320091932.f61bbdcc.kristen.c.accardi-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
@ 2007-03-20 16:34                                         ` Holger Macht
  0 siblings, 0 replies; 60+ messages in thread
From: Holger Macht @ 2007-03-20 16:34 UTC (permalink / raw)
  To: Kristen Carlson Accardi
  Cc: linux-ide-u79uwXL29TY76Z2rM5mHXA,
	linux-acpi-u79uwXL29TY76Z2rM5mHXA,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	Henrique de Moraes Holschuh, Len Brown


On Tue 20. Mar - 09:19:32, Kristen Carlson Accardi wrote:
> On Tue, 20 Mar 2007 16:53:21 +0100
> Holger Macht <hmacht-l3A5Bk7waGM@public.gmane.org> wrote:
> 
> > On Mon 19. Mar - 11:04:12, Kristen Carlson Accardi wrote:
> > > On Sun, 18 Mar 2007 19:55:30 +0100
> > > Holger Macht <hmacht-l3A5Bk7waGM@public.gmane.org> wrote:
> > > 
> > > > On Sun 18. Mar - 15:36:52, Henrique de Moraes Holschuh wrote:
> > > > > On Sun, 18 Mar 2007, Holger Macht wrote:
> > > > > > those ThinkPads where it is needed. Afterwards it does the corresponding
> > > > > > dock/undock request on ibm_acpi. And this works reliably good what I can
> > > > > > see from the feedback I already got. But for this to work, userspace would
> > > > > 
> > > > > It should work with the generic bay device too, but I have no ideas about
> > > > > dock.  But you'll need to deal with udev with the new bay device, something
> > > > > I am not too happy about.  These things are ACPI events, they should remain
> > > > > so unless all other ACPI events are going to become uevents.
> > > > 
> > > > It doesn't work, I've already tried. The bay driver only emits an event if
> > > > you really try to remove the bay, but not on docking/undocking.
> > > > 
> > > > Regards,
> > > > 	Holger
> > > > 
> > > 
> > > this *should* work.  The Bay driver registers with the dock driver to get
> > > dock events:
> > > 
> > >         /* if we are on a dock station, we should register for dock
> > >          * notifications.
> > >          */
> > >         if (bay_is_dock_device(handle)) {
> > >                 bay_dprintk(handle, "Is dependent on dock\n");
> > >                 register_hotplug_dock_device(handle, bay_notify, new_bay);
> > >         }
> > >  
> > 
> > But is_dock_device(...) for both the bay and for the parent handle return
> > false. I'm using an X60 here, so bay_notify is never registered. I
> > couldn't find the reason in the short time I was looking at it, though.
> > 
> > Regards,
> > 	Holger
> > 
> 
> Works for me on my X60 - have you tested on the latest rc kernel?  Also,
> let me know what your BIOS sets your SATA controller to.  I can try to 
> duplicate your issue and get it to work.

Tried with Linus' git tree from today. SATA is set to AHCI if that's the
information you're asking for.

Thanks,
  Holger

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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

* Re: [ibm-acpi-devel] CONFIG_IBM_BAY
  2007-03-20 15:14                             ` Henrique de Moraes Holschuh
@ 2007-03-20 18:45                               ` Theodore Tso
  2007-03-20 20:25                                 ` Shem Multinymous
       [not found]                                 ` <20070320184541.GB29493-AKGzg7BKzIDYtjvyW6yDsg@public.gmane.org>
  0 siblings, 2 replies; 60+ messages in thread
From: Theodore Tso @ 2007-03-20 18:45 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Kristen Carlson Accardi, linux-acpi, ibm-acpi-devel, Len Brown

On Tue, Mar 20, 2007 at 12:14:43PM -0300, Henrique de Moraes Holschuh wrote:
> > Maybe, but then the suspend mail have to fail if there are processes
> > that are keeping the filesystem from being unmounted.  Or the
> 
> Yes. But this is standard functionality on the suspend path, anyway.

The problem with this is that some users will hit the suspend button,
close the lid, put the laptop into their laptop bag, and then
immediately head off to the next conference talk (or whatever).  If
the suspend is going to fail, they might not notice, and they might
not swap out the bay in any case.  

> > processes hanging out on the hard drive or CD-ROM could get all of
> > their file descriptors revoked on resume, I suppose, if the bay has
> > been switched out.....
> 
> Urk.  It is better to either fail the suspend, or to not power down the bay.
> Or to give the user a choice of which he'd rather happen.  Hmm...

Yeah, I think it needs to be configurable.  In many cases the right
thing to do is for the user to promise not to swap out bay, and in the
exception case where they do, all of the processes that have files
open on that bay will have to get their fd's revoked.  We can try to
make it less likely for there to be data loss, like asking filesystems
that support write_super_lockfs() to quiesce the filesystem before the
suspend, but the assumption should be that users aren't supposed to be
swapping out the bay if they request the suspend code not to
disconnect the filesystem.

						- Ted

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

* Re: [ibm-acpi-devel] CONFIG_IBM_BAY
  2007-03-20 18:45                               ` Theodore Tso
@ 2007-03-20 20:25                                 ` Shem Multinymous
       [not found]                                 ` <20070320184541.GB29493-AKGzg7BKzIDYtjvyW6yDsg@public.gmane.org>
  1 sibling, 0 replies; 60+ messages in thread
From: Shem Multinymous @ 2007-03-20 20:25 UTC (permalink / raw)
  To: Theodore Tso
  Cc: Henrique de Moraes Holschuh, Kristen Carlson Accardi, linux-acpi,
	ibm-acpi-devel, Len Brown

On 3/20/07, Theodore Tso <tytso@mit.edu> wrote:
> > Urk.  It is better to either fail the suspend, or to not power down the bay.
> > Or to give the user a choice of which he'd rather happen.  Hmm...
>
> Yeah, I think it needs to be configurable.  In many cases the right
> thing to do is for the user to promise not to swap out bay, and in the
> exception case where they do, all of the processes that have files
> open on that bay will have to get their fd's revoked.  We can try to
> make it less likely for there to be data loss, like asking filesystems
> that support write_super_lockfs() to quiesce the filesystem before the
> suspend, but the assumption should be that users aren't supposed to be
> swapping out the bay if they request the suspend code not to
> disconnect the filesystem.

This is sensical and useful for other devices as well (e.g., USB flash drives).
So the interface, and preferably the implementation too, should be generic.

  Shem

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

* Re: CONFIG_IBM_BAY
       [not found]                                 ` <20070320184541.GB29493-AKGzg7BKzIDYtjvyW6yDsg@public.gmane.org>
@ 2007-03-20 20:44                                   ` Henrique de Moraes Holschuh
  0 siblings, 0 replies; 60+ messages in thread
From: Henrique de Moraes Holschuh @ 2007-03-20 20:44 UTC (permalink / raw)
  To: Theodore Tso
  Cc: linux-acpi-u79uwXL29TY76Z2rM5mHXA, Len Brown,
	Kristen Carlson Accardi,
	ibm-acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f

On Tue, 20 Mar 2007, Theodore Tso wrote:
> On Tue, Mar 20, 2007 at 12:14:43PM -0300, Henrique de Moraes Holschuh wrote:
> > > Maybe, but then the suspend mail have to fail if there are processes
> > > that are keeping the filesystem from being unmounted.  Or the
> > 
> > Yes. But this is standard functionality on the suspend path, anyway.
> 
> The problem with this is that some users will hit the suspend button,
> close the lid, put the laptop into their laptop bag, and then
> immediately head off to the next conference talk (or whatever).  If
> the suspend is going to fail, they might not notice, and they might
> not swap out the bay in any case.  

Well, it needs to be *optional* anyway.  But the above isn't the reason,
though.  Suspend *can* already fail, and anyone who acts like the above
deserves to have to replace their laptop HD more often than those who
actually wait the laptop to be suspended before they toss it into the bag
:-)

> > > processes hanging out on the hard drive or CD-ROM could get all of
> > > their file descriptors revoked on resume, I suppose, if the bay has
> > > been switched out.....
> > 
> > Urk.  It is better to either fail the suspend, or to not power down the bay.
> > Or to give the user a choice of which he'd rather happen.  Hmm...
> 
> Yeah, I think it needs to be configurable.  In many cases the right
> thing to do is for the user to promise not to swap out bay, and in the
> exception case where they do, all of the processes that have files
> open on that bay will have to get their fd's revoked.  We can try to
> make it less likely for there to be data loss, like asking filesystems
> that support write_super_lockfs() to quiesce the filesystem before the
> suspend, but the assumption should be that users aren't supposed to be
> swapping out the bay if they request the suspend code not to
> disconnect the filesystem.

Agreed.

-- 
  "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

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV

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

end of thread, other threads:[~2007-03-20 20:44 UTC | newest]

Thread overview: 60+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-03-12  0:20 [GIT PULL] ibm-acpi changes acpi-test/2.6.22 Henrique de Moraes Holschuh
2007-03-12  0:20 ` [PATCH 1/6] ACPI: ibm-acpi: kill trailing whitespace Henrique de Moraes Holschuh
     [not found]   ` <11736588493322-git-send-email-hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
2007-03-12  0:20     ` [PATCH 2/6] ACPI: ibm-acpi: rename some identifiers Henrique de Moraes Holschuh
     [not found]       ` <11736588491476-git-send-email-hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
2007-03-12  0:20         ` [PATCH 3/6] ACPI: ibm-acpi: add header file Henrique de Moraes Holschuh
     [not found]           ` <11736588493502-git-send-email-hmh-N3TV7GIv+o9fyO9Q7EP/yw@public.gmane.org>
2007-03-12  0:20             ` [PATCH 4/6] ACPI: ibm-acpi: organize code Henrique de Moraes Holschuh
2007-03-12  0:20               ` [PATCH 5/6] ACPI: ibm-acpi: update copyright notice Henrique de Moraes Holschuh
2007-03-12  0:20                 ` [PATCH 6/6] ACPI: ibm-acpi: update documentation Henrique de Moraes Holschuh
2007-03-12  0:54 ` [GIT PULL] ibm-acpi changes acpi-test/2.6.22 Len Brown
2007-03-12  1:27   ` Henrique de Moraes Holschuh
2007-03-15  3:10     ` [GIT PULL] ibm-acpi move to drivers/misc Henrique de Moraes Holschuh
2007-03-15  3:12       ` [PATCH] ACPI: ibm-acpi: move driver to drivers/misc hierarchy Henrique de Moraes Holschuh
2007-03-15  6:06         ` CONFIG_IBM_BAY Len Brown
2007-03-15 13:39           ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
2007-03-15 15:06             ` CONFIG_IBM_BAY Kristen Carlson Accardi
2007-03-15 17:55               ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
2007-03-15 18:01                 ` CONFIG_IBM_BAY Kristen Carlson Accardi
     [not found]                   ` <20070315110156.09e80b99.kristen.c.accardi-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2007-03-15 19:31                     ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
2007-03-15 19:50                       ` CONFIG_IBM_BAY Kristen Carlson Accardi
2007-03-18  5:38                         ` CONFIG_IBM_BAY Tejun Heo
2007-03-18  8:27                           ` CONFIG_IBM_BAY Holger Macht
2007-03-18 18:36                             ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
2007-03-18 18:55                               ` CONFIG_IBM_BAY Holger Macht
2007-03-18 19:00                                 ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
2007-03-19 18:04                                 ` CONFIG_IBM_BAY Kristen Carlson Accardi
2007-03-20 15:53                                   ` CONFIG_IBM_BAY Holger Macht
2007-03-20 16:19                                     ` CONFIG_IBM_BAY Kristen Carlson Accardi
     [not found]                                       ` <20070320091932.f61bbdcc.kristen.c.accardi-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2007-03-20 16:34                                         ` CONFIG_IBM_BAY Holger Macht
2007-03-19 18:17                               ` CONFIG_IBM_BAY Kristen Carlson Accardi
2007-03-20  0:07                                 ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
     [not found]                                   ` <20070320000706.GE5932-ZGHd14iZgfaRjzvQDGKj+xxZW9W5cXbT@public.gmane.org>
2007-03-20 16:14                                     ` CONFIG_IBM_BAY Kristen Carlson Accardi
2007-03-19 13:22                             ` CONFIG_IBM_BAY Matthew Garrett
     [not found]                               ` <20070319132243.GA2869-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org>
2007-03-19 13:48                                 ` CONFIG_IBM_BAY Holger Macht
2007-03-19 14:04                                   ` CONFIG_IBM_BAY Matthew Garrett
2007-03-19 14:37                                     ` CONFIG_IBM_BAY Holger Macht
     [not found]                                       ` <20070319143746.GC4885-tB0uoaAy/Z1bpigZmTR7Iw@public.gmane.org>
2007-03-19 18:11                                         ` CONFIG_IBM_BAY Kristen Carlson Accardi
2007-03-19 23:11                                           ` [ibm-acpi-devel] CONFIG_IBM_BAY Henrique de Moraes Holschuh
2007-03-19 23:56                                             ` Kristen Carlson Accardi
2007-03-20  0:12                                               ` Henrique de Moraes Holschuh
2007-03-20 16:11                                                 ` Kristen Carlson Accardi
2007-03-20 16:07                                           ` CONFIG_IBM_BAY Holger Macht
     [not found]                                             ` <20070320160707.GB6079-tB0uoaAy/Z1bpigZmTR7Iw@public.gmane.org>
2007-03-20 16:16                                               ` CONFIG_IBM_BAY Holger Macht
     [not found]                                     ` <20070319140403.GA3697-1xO5oi07KQx4cg9Nei1l7Q@public.gmane.org>
2007-03-19 15:36                                       ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
2007-03-19 15:47                                         ` CONFIG_IBM_BAY Holger Macht
     [not found]                                         ` <20070319153659.GB15942-ZGHd14iZgfaRjzvQDGKj+xxZW9W5cXbT@public.gmane.org>
2007-03-19 16:41                                           ` CONFIG_IBM_BAY Shem Multinymous
     [not found]                                             ` <41840b750703190941p77fc4a64k49361b678dd6aa90-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2007-03-19 23:04                                               ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
2007-03-20 14:18                                                 ` [ibm-acpi-devel] CONFIG_IBM_BAY Shem Multinymous
2007-03-20 15:12                                                   ` Henrique de Moraes Holschuh
2007-03-19 18:00                               ` CONFIG_IBM_BAY Kristen Carlson Accardi
     [not found]                           ` <45FCD062.3050001-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2007-03-18 18:26                             ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
2007-03-15 20:31                       ` [ibm-acpi-devel] CONFIG_IBM_BAY Shem Multinymous
2007-03-15 20:49                         ` Henrique de Moraes Holschuh
2007-03-19 12:04                       ` Theodore Tso
2007-03-19 23:24                         ` Henrique de Moraes Holschuh
2007-03-20 14:57                           ` Theodore Tso
2007-03-20 15:14                             ` Henrique de Moraes Holschuh
2007-03-20 18:45                               ` Theodore Tso
2007-03-20 20:25                                 ` Shem Multinymous
     [not found]                                 ` <20070320184541.GB29493-AKGzg7BKzIDYtjvyW6yDsg@public.gmane.org>
2007-03-20 20:44                                   ` CONFIG_IBM_BAY Henrique de Moraes Holschuh
2007-03-15 18:22                 ` CONFIG_IBM_BAY Kristen Carlson Accardi
2007-03-15 19:33                   ` CONFIG_IBM_BAY Henrique de Moraes Holschuh

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.