Below test cases shows that current ACPI CA has bug in passing argument by reference. Name (SXX0, Buffer (0x0100) {}) Method (SXX5, 2, NotSerialized) { Store ("sizeof arg0", Debug) Store (SizeOf (Arg0), Debug) Store ("arg1 = ", Debug) Store (Arg1, Debug) If (LLess (Arg1, SizeOf (Arg0))) { CreateByteField (Arg0, Arg1, SX20) Store(6, SX20) Store("SX20", Debug) Store(SX20, Debug) } } Method (_STA, 0, NotSerialized) { Name (SX24, Buffer (0x07) {}) Store ("1111111", Debug) Store (Zero, Local0) Store (0x06, SXX2) CreateByteField (SXX0, 0, SX80) Store ("SXX5", Debug) SXX5 (SXX0, Local0) Store ("SX80", Debug) Store (SX80, Debug) [ACPI Debug] String: 1111111 [ACPI Debug] String: SXX5 [ACPI Debug] String: sizeof arg0 [ACPI Debug] Integer: 0000000000000100 [ACPI Debug] String: arg1 = [ACPI Debug] Integer: 0000000000000000 [ACPI Debug] String: SX20 [ACPI Debug] Integer: 0000000000000006 [ACPI Debug] String: SX80 [ACPI Debug] Integer: 0000000000000000 Name (SXX0, Buffer (0x0100) {}) Method (SXX5, 2, NotSerialized) { Store ("sizeof arg0", Debug) Store (SizeOf (Derefof(Arg0)), Debug) Store ("arg1 = ", Debug) Store (Arg1, Debug) If (LLess (Arg1, SizeOf (Derefof(Arg0)))) { CreateByteField (Derefof(Arg0), Arg1, SX20) Store(6, SX20) Store("SX20", Debug) Store(SX20, Debug) } } Method (_STA, 0, NotSerialized) { Name (SX24, Buffer (0x07) {}) Store ("1111111", Debug) Store (Zero, Local0) Store (0x06, SXX2) CreateByteField (SXX0, 0, SX80) Store ("SXX5", Debug) SXX5 (Refof(SXX0), Local0) Store ("SX80", Debug) Store (SX80, Debug) [ACPI Debug] String: 1111111 [ACPI Debug] String: SXX5 [ACPI Debug] String: sizeof arg0 [ACPI Debug] Integer: 0000000000000100 [ACPI Debug] String: arg1 = [ACPI Debug] Integer: 0000000000000000 [ACPI Debug] String: SX20 [ACPI Debug] Integer: 0000000000000006 [ACPI Debug] String: SX80 [ACPI Debug] Integer: 0000000000000006
Created attachment 1774 [details] a proposal Since reference of buffer could increase due to buffer filed . (Please reference acpi/dispatcher/dsopcode.c : 546). But this patch is not good, just a proposal.
Created attachment 1777 [details] ACPI-CA formatted version of the same patch The attached patch fixes the problem for several FreeBSD Dell users and is simply a reformatted version of Luming's patch.
Are you sure this patch is the final version that will be in a future ACPI-CA dist? It seems like a workaround, not a fix.
This patch breaks battery evaluation on my IBM T23. It doesn't get the reference count right for my _BST method (actually GBST). Please see the ASL I will attach next to see how GBST works. Here is the console prints: utobject-0537 [39] UtGetSimpleObjectSize : Unsupported type=ca in object 0xc38b0f28 acpi_cmbat0: error fetching current battery status -- AE_TYPE utobject-0537 [42] UtGetSimpleObjectSize : Unsupported type=ca in object 0xc38b0ca8 acpi_cmbat0: error fetching current battery status -- AE_TYPE utobject-0537 [44] UtGetSimpleObjectSize : Unsupported type=ca in object 0xc38b0f28 acpi_cmbat0: error fetching current battery status -- AE_TYPE utobject-0537 [46] UtGetSimpleObjectSize : Unsupported type=ca in object 0xc38b0f28 acpi_cmbat0: error fetching current battery status -- AE_TYPE
Add Bob as CC since he did the original fix for GBST for thinkpads.
Created attachment 1794 [details] IBM T23 ASL (see _BST and GBST for ref count problems)
I'm not sure your problem is caused by this patch. But from you DSDT, I DID find out some error. Please reference http://bugzilla.kernel.org/show_bug.cgi?id=1791
*** Bug 1759 has been marked as a duplicate of this bug. ***
*** Bug 1776 has been marked as a duplicate of this bug. ***
I think that this may be the correct fix for these problems. It directly addresses the call-by-value versus call-by-reference issue by not copying the method arguments to new objects. ACPI_STATUS AcpiDsMethodDataInitArgs ( ACPI_OPERAND_OBJECT **Params, UINT32 MaxParamCount, ACPI_WALK_STATE *WalkState) { ACPI_STATUS Status; UINT32 Index = 0; ACPI_FUNCTION_TRACE_PTR ("DsMethodDataInitArgs", Params); if (!Params) { ACPI_DEBUG_PRINT ((ACPI_DB_EXEC, "No param list passed to method\n")); return_ACPI_STATUS (AE_OK); } /* Copy passed parameters into the new method stack frame */ while ((Index < ACPI_METHOD_NUM_ARGS) && (Index < MaxParamCount) && Params [Index]) { /* * A valid parameter. * Store the argument in the method/walk descriptor. * Do not copy the arg in order to implement call by reference */ Status = AcpiDsMethodDataSetValue (AML_ARG_OP, Index, Params[Index], WalkState); if (ACPI_FAILURE (Status)) { return_ACPI_STATUS (Status); } Index++; } ACPI_DEBUG_PRINT ((ACPI_DB_EXEC, "%d args passed to method\n", Index)); return_ACPI_STATUS (AE_OK); }
Created attachment 1964 [details] Bob's suggested fix as a diff vs. ACPICA 20040116
Created attachment 1965 [details] Bob's suggested fix as a diff vs. ACPICA 20040116 (Linux)
Created attachment 1971 [details] Revert Luming's workaround and Bob's patch against acpi-ca
Bob's patch doesn't work, unfortunately. Here is the error dump: acpi_cmbat0: error fetching current battery status -- AE_TYPE **** Exception AE_AML_INTERNAL during execution of method [\\_SB_.PCI0.LPC_.EC__.GBST] (Node 0xc37ad028) Method Execution Stack: Method [GBST] executing: af #%4.4hX Store (Local0, -Return Value- ()) Method [_BST] executing: Call to method [\\_SB_.PCI0.LPC_.EC__.GBST] (Node 0xc37ad028) Local Variables for method [GBST]: Local0: 0xc37f49a8 <Obj> Integer 0 1 Local1: 0xc38b0ea8 <Obj> Integer 0 41cc Local2: 0xc37a8428 <Obj> Integer 0 3e1c Local3: 0xc38b1028 <Obj> Integer 0 2cda Local4: 0 <NullObj> Local5: 0 <NullObj> Local6: 0 <NullObj> Local7: 0xc38b0e28 <Obj> Integer 0 5e Arguments for Method [GBST]: (4 arguments defined, max concurrency = ff) Arg0: 0xc38b0d28 <Obj> Integer 0 0 Arg1: 0xc38b0da8 <Obj> Integer 0 42 Arg2: 0xc37f45a8 <Obj> Integer 0 1 Arg3: 0xc37da528 <Obj> Package 0xc37da528 Arg4: 0 <NullObj> Arg5: 0 <NullObj> Arg6: 0 <NullObj> psparse-1287: *** Error: Method execution failed [\\_SB_.PCI0.LPC_.EC__.GBST] (Node 0xc37ad028), AE_AML_INTERNAL psparse-1287: *** Error: Method execution failed [\\_SB_.PCI0.LPC_.EC__.BAT0._BST] (Node 0xc37ac928), AE_AML_INTERNAL
The problem above is a problem with doing an Index on a package that is an Arg. I don't think that we've gotten this far before, so this is a new issue. Bob
Nate, the exstore.c part of your revert patch is actually from bug 1791, not this one.
As per Len's request, the 2004-01-29 @ 22:35 patch successfully fixes my oops on boot with a div-by-zero error in cpufreq code; I'm now running that patch on top of 2.6.2-rc2-bk4. For reference, I originally notified the lkml with this message: http://www.ussg.iu.edu/hypermail/linux/kernel/0312.3/0442.html Thanks !
The patch in http://bugzilla.kernel.org/attachment.cgi?id=1774&action=view is sufficient to fix the incorrect values for batteries on my Dell Latitude C840 (BIOS A12, if it matters).
I tried this patch on 2.6.2-rc3 as requested to resolve my problem originally reported under #1759. This did not fix my problem, or change the error message I am recieving under bootup. I tried patch number 5 in the above list (Bobs suggested fix (linux)).
Patch (http://bugzilla.kernel.org/attachment.cgi?id=1965&action=view) works to make 2.6.2-rc3 recognize my battery status (Dell Inspiron 8600). Thought posting should go here too (has been sent to LKML).
Len, you are correct. The workaround in this bug proposed by Luming is for Dells. However, that workaround broke GBST on my Thinkpad. So I thought that Bob's fix would work on both Dells and Thinkpads without needing the workaround in bug 1791. However, I was not correct. It appears Bob's fix still needs the workaround in bug 1791 to work on Thinkpads. I will forward this bug to some Dell users and see if it works for them.
Patch (http://bugzilla.kernel.org/attachment.cgi?id=1965&action=view) works for 2.4.25-pre8 on a Dell Inspiron 8600 also. Whithout it you get battery and processor files full of 0's.
From: Valdis.Kletnieks@vt.edu > Any chance you can test the later patch in this bug? > http://bugzilla.kernel.org/attachment.cgi?id=1965&action=view The patch in 1965 produces sane results in /proc/acpi/battery/BAT?/* as well. I looked around in /proc/acpi/* and the dmesg output and found no major regressions.
Patch integrated into CA version 20040211