[ltp] [patch 11/12][2.6.18] ibm-acpi: workaround for EC 0x2f initialization bug

[Henrique de Moraes Holschuh]

Hi Shem!

On Mon, 23 Oct 2006, Shem Multinymous wrote:

> On 10/23/06, Henrique de Moraes Holschuh <hmh@hmh.eng.br> wrote:
> >A few ThinkPads fail to initialize EC register 0x2f both in the EC
> >firmware and ACPI DSDT.  If the BIOS and the ACPI DSDT also do not
> >initialize it, then the initial status of that register does not
> >correspond to reality.
> 
> Nice work wit this patch. But are you sure faking auto is  better than
> saying "unknown"?

I am unsure.  Return "unknown" is of course the 100% correct thing to do,
but the !#@$!$%$%#$!@# parsers in the userspace utilities don't expect
something other than a number and do the wrong thing.  Never mind ibm-acpi
could *always* have returned "unreadable" there, and thus those parsers were
buggy even before my patches changed the ABI under their feet.

Assuming it is "auto" has the following failure modes:

Its failure modes are:
	1. fan level set to 0x7 and ibm-acpi is reloaded
	2. thinkpad boots in 0x7 mode (might be impossible)
	3. fan level in unknown state, and DSDT or BIOS enables
	   fan level 0x07 -- this one is actually likely, as
	   the DSDT *does* have code to kick the fan to level 7,
	   and the fan level will be in unknown state unless the 
	   user is doing userland fan control.

What I fucking hate in this bug is that it is a trivial one that is
dangerous to fix in the more confortable way (actually kicking the fan into
auto mode if the status is undefined at module load...).

There is another option.  I can overengineer the thing.  I can read all
thermal sensors, and if all sensors are within some safe range (say, below
60 C), kick the fan to mode auto.  If they are outside that range, kick the
fan to mode 0x07.

What is your vote on the issue, as a T43 owner?

> >                if (unlikely(!acpi_ec_read(fan_rpm_offset, &lo) ||
> >                          !acpi_ec_read(fan_rpm_offset + 1, &hi)))
> >                        return -EIO;
> 
> Does this make sense?

unlikely (
	acpi_ec_read lo fails ||
	acpi_ec_read hi fails
)

It should, unless I stared at the code for too long, and what is written in
the code is not doing what I wanted it to, but I can't see it anymore.  If
it is broken, please pinpoint it to me :(

> >+               else
> >+                       fan_control_status_known = 1;
> >                break;
> 
> Maybe these "fan_control_status_known = 1;" should be factored out,
> since the watchdog needs hooks at the same points.

I am hooking the watchdog to the procfs (and later, sysfs) handlers and not
from the generic functions, so they won't clash.

Do you think the watchdog should be hooked from the generic functions
instead?  I could do that, as well... might be a better idea, actually, but
if I do that we lose the ability to call them without affecting the
watchdog.

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