Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35523 )
Change subject: mb/acer: Add Acer Aspire VN7-572G ......................................................................
Patch Set 126:
(1 comment)
Usually, I still feel confident about my signed-off-by when I copied a few lines (if it's merely boilerplate for instance). But what you have done here seems bold. Don't know if too bold ;)
Are you thinking of code quality issues or license-related problems? Because for one, it works, and for the other, isn't the recommended way to get started porting a board to copy configuration that works, etc?
Copyright actually, where I'm from (we have something similar to copyright) the basic concept is when you are not licensed to do something with somebody else's work, copyright applies. Worth to mention [1].
Yes it works, sometimes one gets lucky even with copied ASL code. If somebody told you it's recommended to start with copied code, they most likely had copies of free software in mind.
IANAL, so I can't give any real advice here. But I guess nobody really cares who copies ASL code. However, if the code is unrea- dable (often the result of copying vendor ASL code) then the code is not reviewable (for me). You can still hope that some- body rubber stamps it.
[1] https://www.coreboot.org/Development_Guidelines#Sign-off_Procedure
https://review.coreboot.org/c/coreboot/+/35523/122/src/mainboard/acer/aspire... File src/mainboard/acer/aspire_vn7_572g/acpi/battery.asl:
https://review.coreboot.org/c/coreboot/+/35523/122/src/mainboard/acer/aspire... PS122, Line 283: (Local1 & 0x08)
I'm not sure about endianness for the 8051 and if it matters to ACPI.
Well, endianness (byte order) only matters if there are multiple bytes. And both the `Field` with individual bits specified and the masking with `& 0x08` happen in the ACPI interpreter on the host CPU, so the 8051 has nothing to say about it.
Couldn't find anything in the ACPI spec about the bit order, but usually, the least significant bit is specified first and nothing else makes sense here. If we would specify the most significant bit first, we'd have to ask "the most significant out of how many bits?", i.e. we would need to group the bits into words of a specific size.
So counting from top (first) to bottom: EB0A is bit 0 and as bitmask 0x01, two bits skipped, EB0R bit 3 => 0x08, and EB0L bit 4 => 0x10 etc.
(If it were multiple bytes, btw., it would be little endian by ACPI spec, no matter on what hardware.)