Attention is currently required from: Subrata Banik, Nick Vaccaro, Yu-Ping Wu, Boris Mittelberg.
Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73325 )
Change subject: ec/google/chromeec: Use host command API ......................................................................
Patch Set 2:
(2 comments)
File src/ec/google/chromeec/ec.c:
https://review.coreboot.org/c/coreboot/+/73325/comment/1a722d64_712cfdb0 PS1, Line 20: (h) == NULL
callers would simply pass H (maybe choose a better name) instead of NULL? […]
Done
https://review.coreboot.org/c/coreboot/+/73325/comment/b4c645a7_1dd957ba PS1, Line 469: _v1
doing so exposes the caller to semantic changes because simply updating ec_commands. […]
ya, `EC_VER_*` should not be used for aforementioned reasons.
i'm not sure how to enforce an update of ec_cmd_api.h to track ec_commands.h... at least not without adding some pretty complex presubmit checks. hopefully, reviewers will comment and authors will notice that functions they're about to implement will be provided by updating ec_cmd_api.h.
we do have some automated EC code coverage tests and metrics, so at least people will be reminded to update the unit tests when adding new code. it still won't ensure people update ec_cmd_api.h to track ec_commands.h, but there are many examples in the unit tests where ec_cmd_api.h is used as intended, so there should be plenty of good examples to follow.
-
Caveh Jalali (Code Review)