All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [Devel] Reserved method (_PLD) is a buffer instead of a package
@ 2010-07-07 19:52 Moore, Robert
  0 siblings, 0 replies; 4+ messages in thread
From: Moore, Robert @ 2010-07-07 19:52 UTC (permalink / raw)
  To: devel

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



>-----Original Message-----
>From: devel-bounces(a)acpica.org [mailto:devel-bounces(a)acpica.org] On Behalf
>Of Thomas Renninger
>Sent: Monday, July 05, 2010 6:10 AM
>To: devel(a)acpica.org
>Subject: [Devel] Reserved method (_PLD) is a buffer instead of a package
>
>Hi,
>
>I found this when compiling the DSDT from this dump:
>https://bugzilla.kernel.org/attachment.cgi?id=27016
>from this bug:
>https://bugzilla.kernel.org/show_bug.cgi?id=16072
>
>The iasl error is:
>DSDT.modif.dsl  6635:                         Name (_PLD, Buffer (0x10)
>Error    4080 -       Invalid object type for reserved name, must be ^
>(Package)
>
>
>ACPI spec says (3.0b):
>====================
>6.1.6 _PLD (Physical Device Location)
>
>The _PLD method returns data...
>
>This method returns a package containing, a single
>or multiple buffer entries. At least one buffer entry
>must be returned using the bit definitions below.
>====================
>
>So I expect the BIOS (see code snippet below) is wrong, because it
>returns a plain buffer, instead of a Package of buffer(s)?
>

Yes.


_PLD is defined to be a variable-length Package of Buffer objects.





>I could get rid of the error by explicitly declaring the buffer and
>returning it (which should still be wrong, because a Package of
>buffer(s) should get returned?).
>
>iasl should either:
>  - complain about both (a named buffer is declared and the named
>    object is returned or if simply a (unnamed) buffer is returned)
>  - or if in case returning a plain buffer is accepted/workarounded,
>    it should accept both versions.
>

Newer versions of iASL will in fact complain about a named buffer that is returned, I think.



>Argl, it's hard to get this into words, best you just have a look at below
>code snippets...
>
>Be aware that this seem to be a rather new machine/BIOS.
>It could be that neither Linux nor Windows make use of _PLD (don't know)
>and these guys may have to fix that up anyway (return a package...).
>
>It would be great to get a statement should fix this and if this is the
>case,
>whether my patch at the very end of the mail is the correct way to do that.
>

Yes, your change at the end is correct.


The ACPICA auto-repair code should be able to repair this problem. If a buffer is returned, it will wrap a package object around it.

That doesn't change the fact that the original ASL is incorrect.



>Thanks,
>
>      Thomas
>
>The affected ASL code:
>=========================
>Name (_ADR, Zero)
>                    Device (PRT1)
>                    {
>                        Name (_ADR, One)
>                        Name (_UPC, Package (0x04)
>                        {
>                            0xFF,
>                            Zero,
>                            Zero,
>                            Zero
>                        })
>                        Name (_PLD, Buffer (0x10)
>                        {
>                            /* 0000 */    0x81, 0x00, 0x00, 0x00, 0x00,
>0x00, 0x00, 0x00,
>                            /* 0008 */    0x31, 0x1C, 0x00, 0x00, 0x00,
>0x00, 0x00, 0x00
>                        })
>                    }
>=========================
>
>The patch that fixes the error, but the code is still not correct (plain
>buffer is
>returned instead of a package containing the buffer):
>=========================
>--- DSDT.dsl	2010-07-05 13:14:20.945010000 +0200
>+++ DSDT.modif.dsl	2010-07-05 13:56:56.478132000 +0200
>@@ -6601,6 +6601,11 @@
>                 Device (HU01)
>                 {
>                     Name (_ADR, Zero)
>+		    Name (PLDB, Buffer (0x10)
>+                    {
>+                            /* 0000 */    0x81, 0x00, 0x00, 0x00, 0x00,
>0x00, 0x00, 0x00,
>+                            /* 0008 */    0x31, 0x1C, 0x00, 0x00, 0x00,
>0x00, 0x00, 0x00
>+                    })
>                     Device (PRT1)
>                     {
>                         Name (_ADR, One)
>@@ -6611,11 +6616,10 @@
>                             Zero,
>                             Zero
>                         })
>-                        Name (_PLD, Buffer (0x10)
>+                        Method (_PLD, 0, NotSerialized)
>                         {
>-                            /* 0000 */    0x81, 0x00, 0x00, 0x00, 0x00,
>0x00, 0x00, 0x00,
>-                            /* 0008 */    0x31, 0x1C, 0x00, 0x00, 0x00,
>0x00, 0x00, 0x00
>-                        })
>+			    return (PLDB)
>+                        }
>                     }
>
>                     Device (PRT2)
>=========================
>
>The patch which works and should be the correct fix:
>=========================
>--- DSDT.dsl	2010-07-05 13:14:20.945010000 +0200
>+++ DSDT.modif.dsl	2010-07-05 14:34:23.335580000 +0200
>
>@@ -6628,10 +6635,13 @@
>                             Zero,
>                             Zero
>                         })
>-                        Name (_PLD, Buffer (0x10)
>+                        Name (_PLD, Package (0x1)
>                         {
>-                            /* 0000 */    0x81, 0x00, 0x00, 0x00, 0x00,
>0x00, 0x00, 0x00,
>-                            /* 0008 */    0x30, 0x1C, 0x00, 0x00, 0x00,
>0x00, 0x00, 0x00
>+                            Buffer (0x10)
>+                            {
>+                                /* 0000 */    0x81, 0x00, 0x00, 0x00,
>0x00, 0x00, 0x00, 0x00,
>+                                /* 0008 */    0x30, 0x1C, 0x00, 0x00,
>0x00, 0x00, 0x00, 0x00
>+                            }
>                         })
>                     }
>                 }
>=========================
>_______________________________________________
>Devel mailing list
>Devel(a)acpica.org
>http://lists.acpica.org/listinfo/devel

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [Devel] Reserved method (_PLD) is a buffer instead of a package
@ 2010-07-08 15:36 Moore, Robert
  0 siblings, 0 replies; 4+ messages in thread
From: Moore, Robert @ 2010-07-08 15:36 UTC (permalink / raw)
  To: devel

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



>-----Original Message-----
>From: Thomas Renninger [mailto:trenn(a)suse.de]
>Sent: Thursday, July 08, 2010 2:12 AM
>To: Moore, Robert
>Cc: devel(a)acpica.org
>Subject: Re: [Devel] Reserved method (_PLD) is a buffer instead of a
>package
>
>On Wednesday 07 July 2010 21:52:02 Moore, Robert wrote:
>...
>> Newer versions of iASL will in fact complain about a named buffer that
>> is returned, I think.
>I checked with version 20100702 and it does not.


Maybe only in very limited cases, because a named object can be changed at runtime.


>Never mind, I just want to get a picture about this problem.
>It would be a feature/enhancement to get iasl behave equal, but probably
>with a rather low prio...
>
>> Yes, your change at the end is correct.
>Thanks!
>
>> The ACPICA auto-repair code should be able to repair this problem. If
>> a buffer is returned, it will wrap a package object around it.
>Oh dear. If the world would be perfect, then...
>
>> That doesn't change the fact that the original ASL is incorrect.
>So the policy is to workaround/"runtime fix" the buggy ASL code in the
>kernel/interpreter (silently?) and complain about it in the compiler.
>Sounds reasonable.

Yes, that is the behavior.

1) The goal of iASL is to catch as many errors at compile time as possible.

2) The goal of ACPICA is to detect and repair as many errors at runtime as possible, and to always provide the drivers with valid data (from the evaluation of a predefined ACPI name) as long as AE_OK has been returned.

>
>Thanks,
>
>    Thomas

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [Devel] Reserved method (_PLD) is a buffer instead of a package
@ 2010-07-08  9:11 Thomas Renninger
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Renninger @ 2010-07-08  9:11 UTC (permalink / raw)
  To: devel

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

On Wednesday 07 July 2010 21:52:02 Moore, Robert wrote:
... 
> Newer versions of iASL will in fact complain about a named buffer that
> is returned, I think.
I checked with version 20100702 and it does not.
Never mind, I just want to get a picture about this problem.
It would be a feature/enhancement to get iasl behave equal, but probably
with a rather low prio...
 
> Yes, your change at the end is correct.
Thanks!

> The ACPICA auto-repair code should be able to repair this problem. If
> a buffer is returned, it will wrap a package object around it.
Oh dear. If the world would be perfect, then...

> That doesn't change the fact that the original ASL is incorrect.
So the policy is to workaround/"runtime fix" the buggy ASL code in the 
kernel/interpreter (silently?) and complain about it in the compiler.
Sounds reasonable.

Thanks,

    Thomas

^ permalink raw reply	[flat|nested] 4+ messages in thread
* [Devel] Reserved method (_PLD) is a buffer instead of a package
@ 2010-07-05 13:10 Thomas Renninger
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Renninger @ 2010-07-05 13:10 UTC (permalink / raw)
  To: devel

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

Hi,

I found this when compiling the DSDT from this dump:
https://bugzilla.kernel.org/attachment.cgi?id=27016
from this bug:
https://bugzilla.kernel.org/show_bug.cgi?id=16072

The iasl error is:
DSDT.modif.dsl  6635:                         Name (_PLD, Buffer (0x10)
Error    4080 -       Invalid object type for reserved name, must be ^  (Package)


ACPI spec says (3.0b):
====================
6.1.6 _PLD (Physical Device Location)

The _PLD method returns data...

This method returns a package containing, a single
or multiple buffer entries. At least one buffer entry
must be returned using the bit definitions below.
====================

So I expect the BIOS (see code snippet below) is wrong, because it
returns a plain buffer, instead of a Package of buffer(s)?

I could get rid of the error by explicitly declaring the buffer and
returning it (which should still be wrong, because a Package of
buffer(s) should get returned?).

iasl should either:
  - complain about both (a named buffer is declared and the named
    object is returned or if simply a (unnamed) buffer is returned)
  - or if in case returning a plain buffer is accepted/workarounded,
    it should accept both versions.

Argl, it's hard to get this into words, best you just have a look at below
code snippets...

Be aware that this seem to be a rather new machine/BIOS.
It could be that neither Linux nor Windows make use of _PLD (don't know)
and these guys may have to fix that up anyway (return a package...).

It would be great to get a statement should fix this and if this is the case,
whether my patch at the very end of the mail is the correct way to do that.

Thanks,

      Thomas

The affected ASL code:
=========================
Name (_ADR, Zero)
                    Device (PRT1)
                    {
                        Name (_ADR, One)
                        Name (_UPC, Package (0x04)
                        {
                            0xFF, 
                            Zero, 
                            Zero, 
                            Zero
                        })
                        Name (_PLD, Buffer (0x10)
                        {
                            /* 0000 */    0x81, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
                            /* 0008 */    0x31, 0x1C, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
                        })
                    }
=========================

The patch that fixes the error, but the code is still not correct (plain buffer is
returned instead of a package containing the buffer):
=========================
--- DSDT.dsl	2010-07-05 13:14:20.945010000 +0200
+++ DSDT.modif.dsl	2010-07-05 13:56:56.478132000 +0200
@@ -6601,6 +6601,11 @@
                 Device (HU01)
                 {
                     Name (_ADR, Zero)
+		    Name (PLDB, Buffer (0x10)
+                    {
+                            /* 0000 */    0x81, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
+                            /* 0008 */    0x31, 0x1C, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+                    })
                     Device (PRT1)
                     {
                         Name (_ADR, One)
@@ -6611,11 +6616,10 @@
                             Zero, 
                             Zero
                         })
-                        Name (_PLD, Buffer (0x10)
+                        Method (_PLD, 0, NotSerialized)
                         {
-                            /* 0000 */    0x81, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
-                            /* 0008 */    0x31, 0x1C, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
-                        })
+			    return (PLDB)
+                        }
                     }
 
                     Device (PRT2)
=========================

The patch which works and should be the correct fix:
=========================
--- DSDT.dsl	2010-07-05 13:14:20.945010000 +0200
+++ DSDT.modif.dsl	2010-07-05 14:34:23.335580000 +0200

@@ -6628,10 +6635,13 @@
                             Zero, 
                             Zero
                         })
-                        Name (_PLD, Buffer (0x10)
+                        Name (_PLD, Package (0x1)
                         {
-                            /* 0000 */    0x81, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
-                            /* 0008 */    0x30, 0x1C, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+                            Buffer (0x10)
+                            {
+                                /* 0000 */    0x81, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 
+                                /* 0008 */    0x30, 0x1C, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+                            }
                         })
                     }
                 }
=========================

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-07-08 15:36 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-07 19:52 [Devel] Reserved method (_PLD) is a buffer instead of a package Moore, Robert
  -- strict thread matches above, loose matches on Subject: below --
2010-07-08 15:36 Moore, Robert
2010-07-08  9:11 Thomas Renninger
2010-07-05 13:10 Thomas Renninger

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.