[ltp] [patch 08/12][2.6.18] ibm-acpi: extend fan status functions

Henrique de Moraes Holschuh linux-thinkpad@linux-thinkpad.org
Mon, 23 Oct 2006 12:17:53 -0300


This patch fixes fan_read to return correct values for all fan access
modes, taking into account some firmware bugs.  It also implements some
fan access mode status output that was missing, and normalizes the proc
fan abi to return consistent data across all fan read/write modes.

Userspace ABI changes and extensions:
	1. Return status: enable/disable for *all* modes
	   (this actually improves compatibility with userspace utils!)
	2. Return level: auto and level: disengaged for EC 2f access mode
	3. Return level: <number> for EC 0x2f access mode
	4. Add string "\t(stale)" to end of speed: reading in disengaged
	   mode
	5. Return level 0 as well as "disabled" in level-aware modes

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: borislav@users.sourceforge.net
---

While preparing this patch I was faced with a difficult decision,
regarding ABI changes breaking userspace programs.  Some widely-used
ibm-acpi userspace montiors are *already* broken, and I tried to minimize
futher breakage the best I could.

However, when faced with breaking the few of them that had at least
attempted to do some proper parsing of the ibm-acpi output (by looking for
a line that starts with the keyword it wants), and breaking the others who
cut corners and did "skip one line, read the integer after a few tabs"
style of parsing, I decided to break these later ones on the grounds that
we have to draw a line somewhere, and I feel the writers of better parsing
code deserve a lot more respect.

Therefore, this patch does this:
1. It respects the following line ordering (WHEN such lines would
   be output):

	fan status
	fan speed
	fan level
	(other fan data)
	commands
	
2. When some variable is not available, it is NOT output at all.  This is
   already what ibm-acpi used to do, and breaks the lame parsers.  Well,
   tough.  They should have done a better job in the first place, their
   crap is already broken with stock ibm-acpi anyway.

3. Output stale speed readings (for some fan control systems where
   disengaged mode is triggered on and off to confuse the internal EC
   algorithm).

I won't name the offender applets, but the good parsing kudos go to
the ibm-acpi applet, which gets things mostly right, and to the various
shell parsers that use grep ^keyword: to locate what they need.

 drivers/acpi/ibm_acpi.c |   33 +++++++++++++++++++++++++--------
 1 file changed, 25 insertions(+), 8 deletions(-)

Index: 2.6.18/drivers/acpi/ibm_acpi.c
===================================================================
--- 2.6.18.orig/drivers/acpi/ibm_acpi.c
+++ 2.6.18/drivers/acpi/ibm_acpi.c
@@ -345,6 +345,11 @@ 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 int ibm_thinkpad_ec_found;
@@ -1795,24 +1800,36 @@ static int fan_read(char *p)
 		if (unlikely(!acpi_evalf(gfan_handle, &status, NULL, "d")))
 			return -EIO;
 
-		len += sprintf(p + len, "level:\t\t%d\n", status);
-
+		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 (unlikely(!acpi_ec_read(fan_status_offset, &status)))
 			return -EIO;
-		else
-			len += sprintf(p + len, "status:\t\t%s\n",
-				       enabled(status, 7));
+
+		len += sprintf(p + len, "status:\t\t%s\n",
+				(status != 0) ? "enabled" : "disabled" );
 
 		if (unlikely(!acpi_ec_read(fan_rpm_offset, &lo) ||
-		    !acpi_ec_read(fan_rpm_offset + 1, &hi)))
+			  !acpi_ec_read(fan_rpm_offset + 1, &hi)))
 			return -EIO;
 		else
-			len += sprintf(p + len, "speed:\t\t%d\n",
-				       (hi << 8) + lo);
+			len += sprintf(p + len, "speed:\t\t%d%s\n",
+					(hi << 8) + lo,
+					(status & IBMACPI_FAN_EC_DISENGAGED) ?
+						"\t(stale)" : "" );
+
+		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;
 

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