Rajat Jain has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40030 )
Change subject: ec/google/chromeec: New host command EC_CMD_GET_KEYBD_TOP_ROW_LAYOUT ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40030/1/src/ec/google/chromeec/ec_c... File src/ec/google/chromeec/ec_commands.h:
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. This entry is supposed to provide information *only* about action keys. Adding IDs for function keys is not required.
Will do
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. This is because ssdt generator will be added to either arch/x86 or vendorcode/google/chromeos (since this still has some assumptions about Chrome OS). That file cannot directly include a chromeec header. Instead chromeec header will contain the command/response that are handled by EC driver in coreboot.
Thus, you can directly include the members in the above structure here.
Please see in the new review I have out there. That version does not include this header in acpigen_* file. I have still kept the top_row structure because that way atleast I do not have to duplicate another enum in the EC code.