Hello, it appears the problems I mailed about earlier are due to a bug in the ACPI code. Reading from multi-byte fields in a ByteAccess-IndexField seems to be broken. When I replace IndexField (EIND, EDAT, ByteAcc, NoLock, Preserve) { Offset (0x10), IKFG, 8, FRPN, 16, RAMB, 32, AVOL, 8, ... } Method (CRMB, 0, NotSerialized) { Return (RAMB) } with IndexField (EIND, EDAT, ByteAcc, NoLock, Preserve) { Offset (0x10), IKFG, 8, FRPN, 16, RAM0, 8, RAM1, 8, RAM2, 8, RAM3, 8, AVOL, 8, ... } Method (CRMB, 0, NotSerialized) { Store (RAM3, Local0) ShiftLeft (Local0, 0x8, Local0) Add (RAM2, Local0, Local0) ShiftLeft (Local0, 0x8, Local0) Add (RAM1, Local0, Local0) ShiftLeft (Local0, 0x8, Local0) Add (RAM0, Local0, Local0) Return (Local0) } the first version of CRMB computes 0x64646464, while the second version computes the correct 0x0F750064. The least byte seems to just be copied to the other positions. Cheers Robert
After some testing, I found that this bug happens with word access as well, and only when the field is not aligned with 8 (in case of dword), which do not make sence as a buggy ASL due to the ByteAcc. I consider that bug being critical. I'm not sure if that happens for other types other than Index one (that piece of code being shared between 4 kind of OR). Anyway, most of use done by IndexField is to access CMOS data. I prefer not to think what happens if a write is done, and if the write is buggy as well. -- Ducrot Bruno
In my test, if access width of an index field is less than the index field 's width, the problem occurs.
On Mon, Sep 22, 2003 at 07:36:15PM +0200, Ducrot Bruno wrote: > On Mon, Sep 22, 2003 at 07:18:47PM +0200, Ducrot Bruno wrote: > > Hi Andy, > > > > There is a funny bug in acpi/executer/exfldio.c::acpi_ex_field_datum_io > > > > When the field come from an indexed one, we read all the time from the > > address obj_desc->index_field.value > > Unfortunately, this is wrong, since the correct address is > > obviously &obj_desc->index_field.value + field_datum_byte_offset > > Ok, that one should be OK, now. My apologize for the first one. > Definitely, I need to go to sleep.. Take the one from Robert Vollmert and that should be OK. > > --- linux-2.6.0-test5/drivers/acpi/executer/exfldio.c 2003/09/22 17:09:20 1.1 > +++ linux-2.6.0-test5/drivers/acpi/executer/exfldio.c 2003/09/22 17:31:55 > @@ -413,6 +413,8 @@ > > > case ACPI_TYPE_LOCAL_INDEX_FIELD: > + { > + u32 old_addr = 0; > > > /* Ensure that the index_value is not beyond the capacity of the register */ > @@ -424,9 +426,12 @@ > > /* Write the index value to the index_register (itself a region_field) */ > > + old_addr = obj_desc->index_field.value; > + obj_desc->index_field.value += field_datum_byte_offset; > status = acpi_ex_insert_into_field (obj_desc- >index_field.index_obj, > - &obj_desc->index_field.value, > + &obj_desc->index_field.value + field_datum_byte_offset, ^^^^^^^^^^^^^^^^^^^^^^^^^ WEIRD! > sizeof (obj_desc->index_field.value)); > + obj_desc->index_field.value = old_address; > if (ACPI_FAILURE (status)) { > return_ACPI_STATUS (status); > } > @@ -443,6 +448,7 @@ > status = acpi_ex_insert_into_field (obj_desc- >index_field.data_obj, > value, obj_desc- >common_field.access_byte_width); > } > + } > break; > > -- Ducrot Bruno
Access BankField uses the similar code with IndexField, but BankField seems to have not such bug.
God, my test method is wrong. I use memory space other than IO space to test the problem. So the result above I reported is not true. I need a piece of ASL code to investigate
Created attachment 930 [details] DSDT of ASUS M2400N that is affected by this it's the original, minus a few 'If (SSx) { }' that caused compile failures.
Created attachment 931 [details] fix for this bug This fixes the bug for me, and some other M2N users. It's against 2.4.23 + ACPI 20030916.
The patch has been merged in kernel.