Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31067 )
Change subject: mb/lenovo/x131e: function key support ......................................................................
Patch Set 1:
(5 comments)
What’s the difference to `src/ec/quanta/it8518/`?
https://review.coreboot.org/#/c/31067/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31067/1//COMMIT_MSG@7 PS1, Line 7: mb/lenovo/x131e: function key support Please use a statement:
Add function key support
Support function key
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?
https://review.coreboot.org/#/c/31067/1/src/ec/lenovo/h8/Kconfig File src/ec/lenovo/h8/Kconfig:
https://review.coreboot.org/#/c/31067/1/src/ec/lenovo/h8/Kconfig@35 PS1, Line 35: Use alternative EC query methods for X131e Please also mention the EC.
… using an IT8518e EC.
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.h`?
https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/mainboard... File src/mainboard/lenovo/x131e/mainboard.c:
https://review.coreboot.org/#/c/31067/1/src/mainboard/lenovo/x131e/mainboard... PS1, Line 25: send_ec_command(EC_CMD_NOTIFY_ACPI_ENTER); Please add a comment, why this is needed.