Attention is currently required from: Subrata Banik, Caveh Jalali, Nick Vaccaro, Boris Mittelberg.
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73325 )
Change subject: ec/google/chromeec: Use host command API ......................................................................
Patch Set 1:
(3 comments)
File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/c/coreboot/+/73325/comment/477a13b9_30646dba PS1, Line 20: (h) == NULL How about we define a macro for `NULL`
``` /* Required for ec_cmd_api.h but unused in coreboot */ #define H ((CROS_EC_COMMAND_INFO *)NULL) ```
and use it in API calls below? Then I think we can simply ignore `h` here.
https://review.coreboot.org/c/coreboot/+/73325/comment/f6475d95_78a13363 PS1, Line 469: _v1 How do we expect to update the cmd version? Before this patch we use macros such as `EC_VER_FLASH_PROTECT` defined in ec_commands.h. Therefore whenever the version is updated in ec_commands.h (for example CL:2929340), the version coreboot uses gets updated automatically (by syncing ec_commands.h in coreboot).
Now if we hardcode the version in ec.c, the process of syncing ec_commands.h will become much more complicated and error-prone. I wonder if we can make `_CROS_EC_C0_F_PF_RF` auto-decide the version from the `EC_VER_*` macros.
https://review.coreboot.org/c/coreboot/+/73325/comment/58c50d69_59b2dcdc PS1, Line 523: struct chromeec_command cmd = { Why isn't there an API for this command? I'd expect with this patch every chromeec_command declaration would be replaced.
-
Yu-Ping Wu (Code Review)