James Ye has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31067 )
Change subject: mb/lenovo/x131e: function key support ......................................................................
Patch Set 1:
(2 comments)
(5 comments)
What’s the difference to `src/ec/quanta/it8518/`?
quanta/it8518 is used in google/stout
The EC is an IT8518 but (from what I can tell) is compatible with Lenovo interfaces.
While quanta/it8518 could be used, lenovo/h8 has support for ThinkPad ACPI interfaces that are used or useful on this board (function keys, thinkpad_acpi). X131e Chromebook (google/stout) doesn't have function keys, so doesn't have any of the relevant ACPI methods.
So H8 is the most appropriate EC code to use. However, there are a few differences like this that need to be taken into account.
https://review.coreboot.org/#/c/31067/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31067/1//COMMIT_MSG@11 PS1, Line 11: The IT8518e EC of this board uses some different ACPI methods compared to the : regular Lenovo H8.
Did you have documentation for that, or did you reverse engineer this?
The ACPI code for the vendor BIOS differs from the coreboot H8 code. I experimentally verified this. So a bit of both?
https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/ec.h File src/mainboard/lenovo/x131e/ec.h:
PS1:
Shouldn’t this go under `src/ec/…/it…/` like with `src/ec/quanta/it8518/ec. […]
I'm not sure. Is it more appropriate for it to be `src/ec/lenovo/it8518/ec.h`?