Iru Cai (vimacs) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44975 )
Change subject: ec: Add support for MEC5055 for Dell laptops ......................................................................
Patch Set 5:
(8 comments)
https://review.coreboot.org/c/coreboot/+/44975/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/44975/1//COMMIT_MSG@10 PS1, Line 10: powered on
I’d say: *being powered on* or *power-on*
Done
https://review.coreboot.org/c/coreboot/+/44975/1/src/ec/dell/mec5055/early_i... File src/ec/dell/mec5055/early_init.c:
https://review.coreboot.org/c/coreboot/+/44975/1/src/ec/dell/mec5055/early_i... PS1, Line 2: /* This file is part of the coreboot project. */
Please remove
Done
https://review.coreboot.org/c/coreboot/+/44975/1/src/ec/dell/mec5055/mec5055... File src/ec/dell/mec5055/mec5055.h:
https://review.coreboot.org/c/coreboot/+/44975/1/src/ec/dell/mec5055/mec5055... PS1, Line 2: /* This file is part of the coreboot project. */
please remove
Done
https://review.coreboot.org/c/coreboot/+/44975/1/src/ec/dell/mec5055/mec5055... File src/ec/dell/mec5055/mec5055.c:
https://review.coreboot.org/c/coreboot/+/44975/1/src/ec/dell/mec5055/mec5055... PS1, Line 2: /* This file is part of the coreboot project. */
please remove
Done
https://review.coreboot.org/c/coreboot/+/44975/1/src/ec/dell/mec5055/mec5055... PS1, Line 19: int
Use CB_SUCCESS and friends?
Done
https://review.coreboot.org/c/coreboot/+/44975/1/src/ec/dell/mec5055/mec5055... PS1, Line 55: /* Steps to communicate with the EC:
Please add a blank comment line: […]
Done
https://review.coreboot.org/c/coreboot/+/44975/1/src/ec/dell/mec5055/mec5055... PS1, Line 59: * 2. wait EC
Maybe: wait for EC
Done
https://review.coreboot.org/c/coreboot/+/44975/1/src/ec/dell/mec5055/mec5055... PS1, Line 70: result = -1;
Print a debug message?
Done