Patchwork [commit] r6127 - in trunk/src: mainboard/asus/p2b northbridge/intel/i440bx/acpi southbridge/intel/i82371eb southbridge/intel/i82371eb/acpi

login
register
about
Submitter Tobias Diedrich
Date 2010-11-28 11:06:56
Message ID <20101128110656.GE32428@yumi.tdiedrich.de>
Download mbox | patch
Permalink /patch/2366/
State Accepted
Commit r6132
Headers show

Comments

Tobias Diedrich - 2010-11-28 11:06:56
Tobias Diedrich wrote:
> Stefan Reinauer wrote:
> > The specified IO port is most likely wrong. As the comment mentions, the
> > SSDT is a good place for that. A preprocessor define used both in the
> > CPU init code and in the asl would solve the problem without an SSDT.
> > For some info on CPU SSDT creation on intel check out
> > src/cpu/intel/speedstep/acpi.c
> 
> The IO port is ok (and I wrote the comment myself ;)):
> DEFAULT_PMBASE is 0xe400
> PCNTRL reg offset is 0x10
> 
> Using the preprocessor will probably work too if iasl can do simple
> arithmetic (likely yes), I'll look into that.

BTW, my first idea was to use an acpi method that looks up pmbase in
the pci cfg space, but when I define a method like this:

        Method(TEST, 2)
        {
                Return (Add(Arg0, Arg1))
        }

I get:
|build/mainboard/asus/p2b/dsdt.ramstage.asl     9:   Processor (CPU0,
|0x01, TEST(0xe400, 0x10), 0x06) {}
|Error    4096 -       syntax error, unexpected PARSEOP_NAMESEG,
|expecting ')' ^ 

While using the builtin Add() directly works:

Signed-off-by: Tobias Diedrich <ranma+coreboot@tdiedrich.de>
Uwe Hermann - 2010-11-28 20:26:53
On Sun, Nov 28, 2010 at 12:06:56PM +0100, Tobias Diedrich wrote:
> BTW, my first idea was to use an acpi method that looks up pmbase in
> the pci cfg space, but when I define a method like this:
> 
>         Method(TEST, 2)
>         {
>                 Return (Add(Arg0, Arg1))
>         }
> 
> I get:
> |build/mainboard/asus/p2b/dsdt.ramstage.asl     9:   Processor (CPU0,
> |0x01, TEST(0xe400, 0x10), 0x06) {}
> |Error    4096 -       syntax error, unexpected PARSEOP_NAMESEG,
> |expecting ')' ^ 

Not sure if or how this can be fixes, but..

 
> While using the builtin Add() directly works:
> 
> Signed-off-by: Tobias Diedrich <ranma+coreboot@tdiedrich.de>

...this solution looks nice and readable too.

Acked-by: Uwe Hermann <uwe@hermann-uwe.de>

 
Uwe.
Tobias Diedrich - 2010-11-29 20:42:40
Uwe Hermann wrote:
> > While using the builtin Add() directly works:
> > 
> > Signed-off-by: Tobias Diedrich <ranma+coreboot@tdiedrich.de>
> 
> ...this solution looks nice and readable too.
> 
> Acked-by: Uwe Hermann <uwe@hermann-uwe.de>

Thanks, committed as r6132.
Tobias Diedrich - 2010-11-30 00:58:55
Tobias Diedrich wrote:
> Uwe Hermann wrote:
> > > While using the builtin Add() directly works:
> > > 
> > > Signed-off-by: Tobias Diedrich <ranma+coreboot@tdiedrich.de>
> > 
> > ...this solution looks nice and readable too.
> > 
> > Acked-by: Uwe Hermann <uwe@hermann-uwe.de>
> 
> Thanks, committed as r6132.

Maybe a bit too quick.  I didn't test-boot this because iasl did not
complain, but now that I did boot for another reason I get:
[    0.111215] ACPI: Core revision 20100428
[    0.115970] ACPI Error: Found unknown opcode 0xE4 at AML address e07ee578 offset 0x14, ignoring (20100428/psloop-141)
[    0.130114] ACPI Error: Found unknown opcode 0xE4 at AML address e07ee578 offset 0x14, ignoring (20100428/psloop-141)
[    0.140090] ACPI Exception: AE_CTRL_PENDING, While creating Arg 1 (20100428/dsutils-763)
[    0.148689] ACPI Error: Found unknown opcode 0xC3 at AML address 00000008 offset 0x1F811AA4, ignoring (20100428/psloop-141)
[    0.160027] ACPI Error: Found unknown opcode 0xE2 at AML address 00000009 offset 0x1F811AA5, ignoring (20100428/psloop-141)
[    0.172338] ACPI Error: Found unknown opcode 0xF0 at AML address 0000000b offset 0x1F811AA7, ignoring (20100428/psloop-141)
[    0.182325] ACPI Error: Found unknown opcode 0x39 at AML address 00000015 offset 0x1F811AB1, ignoring (20100428/psloop-141)
[    0.192354] ACPI Error: Found unknown opcode 0xC0 at AML address 00000017 offset 0x1F811AB3, ignoring (20100428/psloop-141)
[    0.202386] ACPI Error: Found unknown opcode 0xFE at AML address 00000021 offset 0x1F811ABD, ignoring (20100428/psloop-141)
[    0.219243] ACPI Error: Found unknown opcode 0xF0 at AML address 00000023 offset 0x1F811ABF, ignoring (20100428/psloop-141)

And changing the Add(DEFAULT_PMBASE, PCNTRL) back to 0xe410 fixes it.
Either iasl is buggy or the linux kernel... :/
Rudolf Marek - 2010-11-30 08:13:11
Hi,

Maybe DSDT version is bad?

Thanks,
Rudolf
Tobias Diedrich - 2010-11-30 09:46:09
Rudolf Marek wrote:
> Maybe DSDT version is bad?

Test dsdt:
DefinitionBlock ("DSDT.aml", "DSDT", 2, "CORE  ", "COREBOOT", 1)
{
        Scope (\_PR)
        {
                Processor (CPU0, 1, Add(2, 3), 4) {}
        }
}

iasl dsdt.asl:
|ASL Input:  dsdt.asl - 9 lines, 128 bytes, 3 keywords
|AML Output: DSDT.aml - 60 bytes, 2 named objects, 1 executable opcodes
|
|Compilation complete. 0 Errors, 0 Warnings, 0 Remarks, 1 Optimizations

iasl -d DSDT.aml:
|Loading Acpi table from file DSDT.aml
|Acpi table [DSDT] successfully installed and loaded
|Pass 1 parse of [DSDT]
|Pass 2 parse of [DSDT]
|Parsing Deferred Opcodes (Methods/Buffers/Packages/Regions)
|
|Parsing completed
|Disassembly completed, written to "DSDT.dsl"

So far so good, but the output doesn't match the original input:
|ACPI Error: Found unknown opcode 0x2 at AML address 0x1d14af7 offset 0x13, ignoring (20100528/psloop-232)
|ACPI Error: Found unknown opcode 0x4 at AML address 0x1d14afb offset 0x17, ignoring (20100528/psloop-232)
|ACPI Error: Found unknown opcode 0x2 at AML address 0x1d14af7 offset 0x13, ignoring (20100528/psloop-232)
|ACPI Error: Found unknown opcode 0x4 at AML address 0x1d14afb offset 0x17, ignoring (20100528/psloop-232)
|/*
| * Intel ACPI Component Architecture
| * AML Disassembler version 20100528
[...]
|DefinitionBlock ("DSDT.aml", "DSDT", 2, "CORE  ", "COREBOOT", 0x00000001)
|{
|    Scope (_PR)
|    {
|        Processor (CPU0, 0x01, 0x00000000, 0x0A)
|        {
|            0x03
|            Zero
|        }
|    }
|}

Definitively a iasl problem, it can't even disassemble it's own
output back to something equivalent to the input file.
It seems to be generating Bytecode for the Add where it shouldn't.

Patch

Index: src/southbridge/intel/i82371eb/i82371eb.h
===================================================================
--- src/southbridge/intel/i82371eb/i82371eb.h	(revision 6128)
+++ src/southbridge/intel/i82371eb/i82371eb.h	(working copy)
@@ -23,6 +23,7 @@ 
 
 #if !defined(ASSEMBLY)
 #if !defined(__PRE_RAM__)
+#if !defined(__ACPI__) /* dsdt include */
 
 #include <arch/io.h>
 #include <device/device.h>
@@ -33,6 +34,7 @@ 
 
 #endif
 #endif
+#endif
 
 /* If 'cond' is true this macro sets the bit(s) specified by 'bits' in the
  * 'reg' variable, otherwise it clears those bits.
Index: src/mainboard/asus/p2b/dsdt.asl
===================================================================
--- src/mainboard/asus/p2b/dsdt.asl	(revision 6128)
+++ src/mainboard/asus/p2b/dsdt.asl	(working copy)
@@ -17,14 +17,17 @@ 
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
  */
 
+#include "southbridge/intel/i82371eb/i82371eb.h"
+
 DefinitionBlock ("DSDT.aml", "DSDT", 2, "CORE  ", "COREBOOT", 1)
 {
 	/* Define the main processor.*/
 	Scope (\_PR)
 	{
-		/* Looks like the P_CNT field can't be a method or name
-		 * and has to be hardcoded to 0xe410 or generated in SSDT */
-		Processor (CPU0, 0x01, 0xe410, 0x06) {}
+		/* Looks like the P_CNT field can't be a name or method (except
+		 * builtins like Add()) and has to be hardcoded or generated
+		 * into SSDT */
+		Processor (CPU0, 0x01, Add(DEFAULT_PMBASE, PCNTRL), 0x06) {}
 	}
 
 	/* For now only define 2 power states: