Re: oops with asus_acpi on P30/P35

[Christian Aichinger <Greek0-hi6Y0CQ0nG0@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2594 bytes --]

On Wed, Sep 21, 2005 at 03:39:53PM +0200, Timo Hoenig wrote:
> On Wed, 2005-09-21 at 11:08 +0200, Christian Aichinger wrote:
> > If this fix still breaks other laptops I'd really be puzzled, since
> > currently the value is dereferenced later when doing the string
> > comparisons. So the patch at least shouldn't break it more than it
> > already is ;)
> 
> I can confirm that your patch works fine for the W5A which previously
> needed the mentioned workaround.  But this single case of success should
> not be taken as proof.  More testing would be reasonable.

The old patch was kind of a "Want to get this thing to work
for me" version. This one is more the "Let's get this thing finally
fixed in mainline" version now.

The patch only changes the behavior if the INIT method returns an
integer instead of a string, or a pointer to an integer or similar.

Now if an integer is returned this breaks the assumption in the
whole rest of the function, namely that model->xxx.pointer can be
dereferenced. This works for ACPI_TYPE_STRING and ACPI_TYPE_BUFFER.
Why the old patch broke you is probably because the INIT method
in your DSDT returns a _BUFFER instead of an _STRING.

In any other case (including ACPI_TYPE_INTEGER) dereferencing
model->pointer (or whatever's stored at that offset) WILL lead to an
oops.

Pointer magic isn't possible in AML AFAIK and it would be sheer luck
(and still a bug) if dereferencing some random integer gotten from
some AML function would actually work.

That's what I ment with the "if this breaks something it was at
least as broken before". If this patch changes the behavior for you,
you got a kernel oops without it.

It would probably be nice to add a check for
(model->type == string || buffer) for defensive-programming's sake.
However that can (and should probably) be done later on.

Cheers,
Christian Aichinger

PS: Would be nice if you could test my assumption that model->type
is really ACPI_TYPE_BUFFER on your laptop, by pasting a dmesg
snippet with that patch applied (should apply to vanilla -rc2):

diff --git a/drivers/acpi/asus_acpi.c b/drivers/acpi/asus_acpi.c
--- a/drivers/acpi/asus_acpi.c
+++ b/drivers/acpi/asus_acpi.c
@@ -1006,6 +1006,7 @@ static int __init asus_hotk_get_info(voi
        }
 
        model = (union acpi_object *)buffer.pointer;
+       printk(KERN_NOTICE "  DEBUG: model->type == %d\n", model->type);
        if (model->type == ACPI_TYPE_STRING) {
                printk(KERN_NOTICE "  %s model detected, ",
                       model->string.pointer);

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]