I tried to give it a quick look, but the first files (ACPI) are
incredibly hard to read. Didn't come very far.
14 comments:
File src/mainboard/acer/aspire_vn7_572g/acpi/ac.asl:
Patch Set #122, Line 11: Name (ACST, One)
Where is this consumed? Why is it needed to cache this?
Patch Set #122, Line 14: // _HID: Hardware ID
Automated comments make it look like you borrowed the code from somebody else.
If (EACS)
{
ACST = One
}
Else
{
ACST = Zero
}
Isn't it easier to just write
ACST = EACS
? I would expect it to compile.
Method (_STA, 0, NotSerialized) // _STA: Status
{
Return (0x0F)
}
This is the default, are you sure it's needed?
File src/mainboard/acer/aspire_vn7_572g/acpi/battery.asl:
EB0A, 1,
, 2,
EB0R, 1,
EB0L, 1,
EB0F, 1,
EB0N, 1
These seem to describe the bits in `EB0S` below. Why not use them instead
of the manual masking? If it's to reduce the number of EC RAM reads, defined
macro names for the masks would be nice.
Patch Set #122, Line 41: Offset (0xE0),
Same offset as above. So the EC provides different data on the same addresses
in a predefined sequence, I assume? Can we have a comment what the expected
sequence is?
As far as I can see:
EBDC
EBFC
EBDV /* new value after reading EBFC */
EBSN
EBDN /* entire new value */
EBCH /* entire new value */
EBMN /* entire new value */
Arg1 [One] = 0xFFFFFFFF
Arg1 [0x02] = 0xFFFFFFFF
Arg1 [0x04] = 0xFFFFFFFF
Arg1 [0x05] = Zero
Arg1 [0x06] = Zero
Again, looks like borrowed code. Wouldn't
Arg1[1] = ...
Arg1[2] = ...
...
Arg1[6] = ...
be easier to read?
Patch Set #122, Line 78: (Local0 ^ One)
isn't this just `!Local0`?
Are the parentheses needed?
0?
If (Local0)
{
Local1 = (EBDC * 0x0A)
}
Else
{
Local1 = EBDC
}
Arg1 [One] = Local1 // Design capacity
If (Local0)
{
Local2 = (EBFC * 0x0A)
}
Else
{
Local2 = EBFC
}
Can this be written with one `if` block?
Patch Set #122, Line 100: 0x64
% calculations are easier to follow if you use decimal numbers.
Patch Set #122, Line 100: Divide (Local2, 0x64, Local7, Local6) // Warning capacity
Why Divide() instead of `Local6 = Local2 / 100`? Would it hurt to write
Arg1[5] = (Local2 / 100) * 7
Arg1[6] = ((Local2 / 100) * 11) / 2
?
Patch Set #122, Line 102: Arg1 [0x05] = Local3
/* 7% */
Patch Set #122, Line 107: Arg1 [0x06] = Local4
/* 5.5% */
To view, visit change 35523. To unsubscribe, or for help writing mail filters, visit settings.