Hi,
Since several kernel-versions now the asus_acpi module is broken on several
Samsung notebooks, it causes an oops when loading and a kernelpanic when
compiled into the kernel.
This is known for ages. There was a patch by Karol Kozimor shortly after the
bug became public that was ignored.
The code was changed so the patch failed. Christian Aichinger again made a
patch. It was ignored as well.
Now, finally the patch is in the mm-source, I asked Andrew Morton to push it
to Linus so 2.6.15 will be fixed, Andrew said this is up to Len Brown. No
Reply from him.
Now it seems that 2.6.15 is going to be released soon, the patch still has not
made it into linus tree.
This is not "some minor issue", this completely breaks the usage of current
vanilla-kernels on certain Hardware. Can please, please, please anyone in the
position to do this take care that this patch get's accepted before 2.6.15?
The patch is available inside mm-sources or here:
http://www.int21.de/samsung/p30-2.6.14.diff
If I should send it to anyone else or if there's anything I can do to help
fixing this, I'm glad to help.
cu,
--
Hanno Böck Blog: http://www.hboeck.de/
GPG: 3DBD3B20 Jabber: [email protected]
On Wed, 21 Dec 2005, Hanno Böck wrote:
>
> This is not "some minor issue", this completely breaks the usage of current
> vanilla-kernels on certain Hardware. Can please, please, please anyone in the
> position to do this take care that this patch get's accepted before 2.6.15?
>
> The patch is available inside mm-sources or here:
> http://www.int21.de/samsung/p30-2.6.14.diff
>
> If I should send it to anyone else or if there's anything I can do to help
> fixing this, I'm glad to help.
Last I saw this patch, I wrote this reply (the patch above is still
broken). Nobody ever came back to me on it.
Linus
---
Date: Tue, 13 Dec 2005 21:15:56 -0800 (PST)
From: Linus Torvalds <[email protected]>
To: Carl-Daniel Hailfinger <[email protected]>
cc: Greg KH <[email protected]>,
Linux Kernel Mailing List <[email protected]>,
[email protected], acpi-devel <[email protected]>
Subject: Re: [PATCH] Fix oops in asus_acpi.c on Samsung P30/P35 Laptops
On Wed, 14 Dec 2005, Carl-Daniel Hailfinger wrote:
>
> The patch has been tested and verified, is shipped in the
> SUSE 10.0 kernel and does not cause any regressions.
I'd be _much_ happier if
- the patch wasn't totally whitespace-damaged (your mailer seems
to not only remove spaces at the end of lines, it _also_ adds them to
the beginning when there was another space there, as far as I can tell)
Being right "on average" thanks to having two different bugs does not a
good mailer make.
- you were to separate out the oops-fixing code from the code that adds
handling for that (strange?) model type logic.
It seems that the _oops_ is because the later paths just assume that
it's a ACPI_TYPE_STRING and will dereference "model->string.pointer"
regardless of whether that is true or not. And you add a test for
ACPI_TYPE_INTEGER, however, you do _not_ fix the oops for any other
type, so the exact _same_ bug is still waiting to happen if there is
some other strange ACPI table entry some day.
So I think the proper fix is to _first_ just do something like
if (model->type != ACPI_TYPE_STRING)
goto unknown;
which should fix the oops (no?), and then handling ACPI_TYPE_INTEGER above
that as one case would be a separate patch.
Linus
Karol,
Do you have an update of your asus driver in the pipeline
that addresses this?
thanks,
-Len
>-----Original Message-----
>From: Linus Torvalds [mailto:[email protected]]
>Sent: Wednesday, December 21, 2005 1:37 PM
>To: Hanno Böck
>Cc: Andrew Morton; Brown, Len;
>[email protected];
>[email protected]; Karol Kozimor; Christian Aichinger
>Subject: Re: asus_acpi still broken on Samsung P30/P35
>
>
>
>On Wed, 21 Dec 2005, Hanno Böck wrote:
>>
>> This is not "some minor issue", this completely breaks the
>usage of current
>> vanilla-kernels on certain Hardware. Can please, please,
>please anyone in the
>> position to do this take care that this patch get's accepted
>before 2.6.15?
>>
>> The patch is available inside mm-sources or here:
>> http://www.int21.de/samsung/p30-2.6.14.diff
>>
>> If I should send it to anyone else or if there's anything I
>can do to help
>> fixing this, I'm glad to help.
>
>Last I saw this patch, I wrote this reply (the patch above is still
>broken). Nobody ever came back to me on it.
>
> Linus
>
>---
>Date: Tue, 13 Dec 2005 21:15:56 -0800 (PST)
>From: Linus Torvalds <[email protected]>
>To: Carl-Daniel Hailfinger <[email protected]>
>cc: Greg KH <[email protected]>,
> Linux Kernel Mailing List <[email protected]>,
> [email protected], acpi-devel <[email protected]>
>Subject: Re: [PATCH] Fix oops in asus_acpi.c on Samsung P30/P35 Laptops
>
>On Wed, 14 Dec 2005, Carl-Daniel Hailfinger wrote:
>>
>> The patch has been tested and verified, is shipped in the
>> SUSE 10.0 kernel and does not cause any regressions.
>
>I'd be _much_ happier if
>
> - the patch wasn't totally whitespace-damaged (your mailer seems
> to not only remove spaces at the end of lines, it _also_
>adds them to
> the beginning when there was another space there, as far as
>I can tell)
>
> Being right "on average" thanks to having two different
>bugs does not a
> good mailer make.
>
> - you were to separate out the oops-fixing code from the code
>that adds
> handling for that (strange?) model type logic.
>
> It seems that the _oops_ is because the later paths just
>assume that
> it's a ACPI_TYPE_STRING and will dereference
>"model->string.pointer"
> regardless of whether that is true or not. And you add a test for
> ACPI_TYPE_INTEGER, however, you do _not_ fix the oops for any other
> type, so the exact _same_ bug is still waiting to happen if
>there is
> some other strange ACPI table entry some day.
>
>So I think the proper fix is to _first_ just do something like
>
> if (model->type != ACPI_TYPE_STRING)
> goto unknown;
>
>which should fix the oops (no?), and then handling
>ACPI_TYPE_INTEGER above
>that as one case would be a separate patch.
>
> Linus
>
Linus Torvalds schrieb:
>
> On Wed, 21 Dec 2005, Hanno Böck wrote:
>
>>This is not "some minor issue", this completely breaks the usage of current
>>vanilla-kernels on certain Hardware. Can please, please, please anyone in the
>>position to do this take care that this patch get's accepted before 2.6.15?
>>
>>The patch is available inside mm-sources or here:
>>http://www.int21.de/samsung/p30-2.6.14.diff
>>
>>If I should send it to anyone else or if there's anything I can do to help
>>fixing this, I'm glad to help.
>
>
> Last I saw this patch, I wrote this reply (the patch above is still
> broken). Nobody ever came back to me on it.
> [...]
I've been busy trying to gather all the different DSDTs to compare
them and find out if the logic can be simplified. Will try to come
up with a patch addressing all your and Andrew's concerns until friday.
Regards,
Carl-Daniel
Thus wrote Brown, Len:
> Karol,
> Do you have an update of your asus driver in the pipeline
> that addresses this?
I still believe the only _right_ *workaround* is
http://bugme.osdl.org/attachment.cgi?id=6006&action=view
I'll take a shot at rediffing it against recent kernels in a couple of
hours (unless someone beats me to it).
acpi=strict will work until a suitable patch is merged.
Note: it's still a workaround, to properly fix this we need to make ACPI
interpreter behave predictably, as written in
http://bugme.osdl.org/show_bug.cgi?id=5067#c6 -- I believe I still haven't
heard from Robert Moore on the feasibility of such a solution.
Please also see http://bugme.osdl.org/show_bug.cgi?id=5067 and
http://bugzilla.kernel.org/show_bug.cgi?id=5092 for more info.
Best regards,
--
Karol 'sziwan' Kozimor
[email protected]
Thus wrote Brown, Len:
> Karol,
> Do you have an update of your asus driver in the pipeline
> that addresses this?
Here it goes. Rediffed, also plugs a leak my previous patch introduced. I
believe it addresses Linus' comments. It's still not a proper fix (see
below), but I believe it's better than none.
Best regards,
--
Karol 'sziwan' Kozimor
[email protected]
Work around asus_acpi driver oopses on Samsung P30s and the like due to the
ACPI implicit return.
The code used to rely on a certain method to return a NULL buffer, which
is now hardly possible with the implicit return code on by default. This
sort of fixes bugs #5067 and #5092 for now.
Note: this patch makes the driver unusable on said machines (and on said
machines only) iff acpi=strict is specified, but it seems noone really uses
that.
Signed-off-by: Karol Kozimor <[email protected]>
--- a/drivers/acpi/asus_acpi.c 2005-12-22 18:08:56.000000000 +0100
+++ b/drivers/acpi/asus_acpi.c 2005-12-22 18:16:19.000000000 +0100
@@ -987,9 +987,21 @@ static int __init asus_hotk_get_info(voi
printk(KERN_NOTICE " BSTS called, 0x%02x returned\n",
bsts_result);
- /* Samsung P30 has a device with a valid _HID whose INIT does not
- * return anything. Catch this one and any similar here */
- if (buffer.pointer == NULL) {
+ /* This is unlikely with implicit return */
+ if (buffer.pointer == NULL)
+ return -EINVAL;
+
+ model = (union acpi_object *) buffer.pointer;
+ /*
+ * Samsung P30 has a device with a valid _HID whose INIT does not
+ * return anything. It used to be possible to catch this exception,
+ * but the implicit return code will now happily confuse the
+ * driver. We assume that every ACPI_TYPE_STRING is a valid model
+ * identifier but it's still possible to get completely bogus data.
+ */
+ if (model->type == ACPI_TYPE_STRING) {
+ printk(KERN_NOTICE " %s model detected, ", model->string.pointer);
+ } else {
if (asus_info && /* Samsung P30 */
strncmp(asus_info->oem_table_id, "ODEM", 4) == 0) {
hotk->model = P30;
@@ -1002,13 +1014,10 @@ static int __init asus_hotk_get_info(voi
"the developers with your DSDT\n");
}
hotk->methods = &model_conf[hotk->model];
- return AE_OK;
- }
+
+ acpi_os_free(model);
- model = (union acpi_object *)buffer.pointer;
- if (model->type == ACPI_TYPE_STRING) {
- printk(KERN_NOTICE " %s model detected, ",
- model->string.pointer);
+ return AE_OK;
}
hotk->model = END_MODEL;
On Thu, Dec 22, 2005 at 06:42:26PM +0100, Karol Kozimor wrote:
> Thus wrote Brown, Len:
> > Karol,
> > Do you have an update of your asus driver in the pipeline
> > that addresses this?
>
> Here it goes. Rediffed, also plugs a leak my previous patch introduced. I
> believe it addresses Linus' comments. It's still not a proper fix (see
> below), but I believe it's better than none.
> Best regards,
This will break other hardware as the P30/P35 as well, since there
are some buggy DSDT's out there that return an ACPI_TYPE_BUFFER.
That's the whole reason why I was testing exactly for
ACPI_TYPE_INTEGER in my patch.
My first version was pretty simmilar to yours, until I was told on
acpi-devel that this breaks someone elses hardware (causing it to be
considered as P30/P35, while it isn't). I can dig up the mails if
you want.
Cheers,
Christian
Thus wrote Christian Aichinger:
> > Here it goes. Rediffed, also plugs a leak my previous patch introduced. I
> > believe it addresses Linus' comments. It's still not a proper fix (see
> > below), but I believe it's better than none.
> > Best regards,
> This will break other hardware as the P30/P35 as well, since there
> are some buggy DSDT's out there that return an ACPI_TYPE_BUFFER.
I've never seen one, I'd like to look at those if you've got them. Are you
sure it's the actual machine code that returns buffers, and not an implicit
return of the interpreter?
> That's the whole reason why I was testing exactly for
> ACPI_TYPE_INTEGER in my patch.
I don't really seem to follow: my logic checks for ACPI_TYPE_STRING, and if
not found, looks at DSDT signatures. If something different than a string
is returned by INIT _and_ the machine isn't a P30/P35 (DSDT sigs) then the
defaults are set as in every other case a machine is not recognized by the
driver.
> My first version was pretty simmilar to yours, until I was told on
> acpi-devel that this breaks someone elses hardware (causing it to be
> considered as P30/P35, while it isn't). I can dig up the mails if
> you want.
The original driver behaviour was: ( if INIT returns NULL && DSDT sigs
match ) machine is a P30. My patch changes that to ( if INIT returns
!ACPI_TYPE_STRING && DSDT sigs match ) machine is a P30. I'd like to see
those reports please.
Best regards,
--
Karol 'sziwan' Kozimor
[email protected]
Any news on this?
--
Hanno Böck Blog: http://www.hboeck.de/
GPG: 3DBD3B20 Jabber: [email protected]
Thus wrote Hanno B?ck:
> Any news on this?
The fix is in acpi-test already, and a rework of this code is in the
acpi4asus CVS and will be sent to Len as soon as I rediff the patches
against the git tree.
As for -stable, my understanding is that Chris wants to wait for the fix to
boil in the mainline for a while.
Best regards,
--
Karol 'sziwan' Kozimor
[email protected]
FWIW, I need the patch below to stop ACPI freezing at boot on Asus S1300N.
There is a BIOS update from Asus, but no mention of any fixes in ACPI,
so as I have no means to backup the BIOS in case something goes wrong
I didn't do the update.
I found out (by putting printks in the initialization code) that a
call to INI (whatever it is) of VGA_ (whatever this is) immediately
freezes the notebook and the fan goes on shortly afterwards.
diff --git a/drivers/acpi/asus_acpi.c b/drivers/acpi/asus_acpi.c
index f4c8775..d415b30 100644
--- a/drivers/acpi/asus_acpi.c
+++ b/drivers/acpi/asus_acpi.c
@@ -352,8 +352,8 @@ static struct model_data model_conf[END_
.lcd_status = "\\BKLT",
.brightness_set = "SPLV",
.brightness_get = "GPLV",
- .display_set = "SDSP",
- .display_get = "\\ADVG"}
+ /* .display_set = "SDSP",
+ .display_get = "\\ADVG" */}
};
/* procdir we use */
diff --git a/drivers/acpi/namespace/nsinit.c b/drivers/acpi/namespace/nsinit.c
index 9f929e4..79fa2ec 100644
--- a/drivers/acpi/namespace/nsinit.c
+++ b/drivers/acpi/namespace/nsinit.c
@@ -384,7 +384,12 @@ acpi_ns_init_one_device(acpi_handle obj_
pinfo.parameters = NULL;
pinfo.parameter_type = ACPI_PARAM_ARGS;
- status = acpi_ut_execute_STA(pinfo.node, &flags);
+ /* workaround Asus S1300N freeze at INI */
+ if ( memcmp(pinfo.node->name.ascii, "VGA_",4)==0 ) {
+ printk(KERN_ERR "acpi: VGA_ ignored\n");
+ status = AE_NOT_FOUND;
+ } else
+ status = acpi_ut_execute_STA(pinfo.node, &flags);
if (ACPI_FAILURE(status)) {
/* Ignore error and move on to next device */
Alex Riesen <[email protected]> wrote:
>
> FWIW, I need the patch below to stop ACPI freezing at boot on Asus S1300N.
> There is a BIOS update from Asus, but no mention of any fixes in ACPI,
> so as I have no means to backup the BIOS in case something goes wrong
> I didn't do the update.
I think it'd be worth trying the update anyway please. Normally those
updating programs are pretty careful to not give you a dead box.
> I found out (by putting printks in the initialization code) that a
> call to INI (whatever it is) of VGA_ (whatever this is) immediately
> freezes the notebook and the fan goes on shortly afterwards.
>
Is this a recent problem, or did earlier 2.6.x kernels also fail?
> diff --git a/drivers/acpi/asus_acpi.c b/drivers/acpi/asus_acpi.c
> index f4c8775..d415b30 100644
> --- a/drivers/acpi/asus_acpi.c
> +++ b/drivers/acpi/asus_acpi.c
> @@ -352,8 +352,8 @@ static struct model_data model_conf[END_
> .lcd_status = "\\BKLT",
> .brightness_set = "SPLV",
> .brightness_get = "GPLV",
> - .display_set = "SDSP",
> - .display_get = "\\ADVG"}
> + /* .display_set = "SDSP",
> + .display_get = "\\ADVG" */}
> };
>
> /* procdir we use */
> diff --git a/drivers/acpi/namespace/nsinit.c b/drivers/acpi/namespace/nsinit.c
> index 9f929e4..79fa2ec 100644
> --- a/drivers/acpi/namespace/nsinit.c
> +++ b/drivers/acpi/namespace/nsinit.c
> @@ -384,7 +384,12 @@ acpi_ns_init_one_device(acpi_handle obj_
> pinfo.parameters = NULL;
> pinfo.parameter_type = ACPI_PARAM_ARGS;
>
> - status = acpi_ut_execute_STA(pinfo.node, &flags);
> + /* workaround Asus S1300N freeze at INI */
> + if ( memcmp(pinfo.node->name.ascii, "VGA_",4)==0 ) {
> + printk(KERN_ERR "acpi: VGA_ ignored\n");
> + status = AE_NOT_FOUND;
> + } else
> + status = acpi_ut_execute_STA(pinfo.node, &flags);
> if (ACPI_FAILURE(status)) {
> /* Ignore error and move on to next device */
>
On 2/20/06, Andrew Morton <[email protected]> wrote:
> Alex Riesen <[email protected]> wrote:
> >
> > FWIW, I need the patch below to stop ACPI freezing at boot on Asus S1300N.
> > There is a BIOS update from Asus, but no mention of any fixes in ACPI,
> > so as I have no means to backup the BIOS in case something goes wrong
> > I didn't do the update.
>
> I think it'd be worth trying the update anyway please. Normally those
> updating programs are pretty careful to not give you a dead box.
You think? ... Still, it'll have to wait till the next weekend. I really
cannot afford a "dead box" midweek.
> > I found out (by putting printks in the initialization code) that a
> > call to INI (whatever it is) of VGA_ (whatever this is) immediately
> > freezes the notebook and the fan goes on shortly afterwards.
> >
>
> Is this a recent problem, or did earlier 2.6.x kernels also fail?
Yes. I think I got the notebook around 2.6.9, and it already had the problem.
I don't remember what I did back than, probably run it with ACPI disabled,
or maybe I made that workaround immediately (there were some sound
problems without ACPI, I recall).