Rajat Jain has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40030 )
Change subject: ec/google/chromeec: Add host command EC_CMD_GET_KEYBD_CONFIG ......................................................................
Patch Set 3:
(7 comments)
https://review.coreboot.org/c/coreboot/+/40030/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40030/1//COMMIT_MSG@7 PS1, Line 7: ec/google/chromeec: New host command EC_CMD_GET_KEYBD_TOP_ROW_LAYOUT
Will do.
Done
https://review.coreboot.org/c/coreboot/+/40030/1/src/ec/google/chromeec/ec_c... File src/ec/google/chromeec/ec_commands.h:
PS1:
src/platform/ec master branch should be the place where changes to ec_commands.h to take place […]
Done
https://review.coreboot.org/c/coreboot/+/40030/1/src/ec/google/chromeec/ec_c... PS1, Line 2976: 0x002E
I think we are actually on on 0x0130 for the next available host command
Will do
I realized that actually 0x129A is the next available host command, so used that.
https://review.coreboot.org/c/coreboot/+/40030/1/src/ec/google/chromeec/ec_c... PS1, Line 3028: action keys
As discussed, we should only be adding action key information to ec_commands.h. […]
Done
https://review.coreboot.org/c/coreboot/+/40030/1/src/ec/google/chromeec/ec_c... PS1, Line 3035:
nit: No space? […]
Done
https://review.coreboot.org/c/coreboot/+/40030/1/src/ec/google/chromeec/ec_c... PS1, Line 3043: struct top_row_layout top_row;
Like discussed, we cannot share the structure in ec_commands.h and ssdt generator. […]
Done
https://review.coreboot.org/c/coreboot/+/40030/1/src/ec/google/chromeec/ec_c... PS1, Line 3044: ;
You'll need __ec_align1 (align to 1-byte boundary) […]
Done