Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38541 )
Change subject: ec/google/chromeec: Add SSDT generator for ChromeOS EC ......................................................................
Patch Set 16:
(7 comments)
https://review.coreboot.org/c/coreboot/+/38541/16/src/ec/google/chromeec/ec_... File src/ec/google/chromeec/ec_chip.c:
https://review.coreboot.org/c/coreboot/+/38541/16/src/ec/google/chromeec/ec_... PS16, Line 45: EC_PD_TRY_POWER_ROLE_NONE
Just noting: This case makes sense for completeness. […]
That's true; I can add a comment indicating so.
https://review.coreboot.org/c/coreboot/+/38541/16/src/ec/google/chromeec/ec_... PS16, Line 131: so malloc() : * and strdup() are used for that.
No problem with doing this. But this can also be handled by: […]
That is probably cleaner, thanks for the suggestion.
https://review.coreboot.org/c/coreboot/+/38541/16/src/ec/google/chromeec/ec_... PS16, Line 138: if (port->enabled && port->path.type == DEVICE_PATH_USB &&
Just a nit. […]
Done
https://review.coreboot.org/c/coreboot/+/38541/16/src/ec/google/chromeec/ec_... PS16, Line 150: config->type != UPC_TYPE_INTERNAL
Shouldn't this check ensure that config->type is one of the following: […]
I waffled back and forth on that... but I think you're right.
https://review.coreboot.org/c/coreboot/+/38541/16/src/ec/google/chromeec/ec_... PS16, Line 154: 10
sizeof(usb_port)?
Not needed anymore w/ your suggestion above.
https://review.coreboot.org/c/coreboot/+/38541/16/src/ec/google/chromeec/ec_... PS16, Line 159: 10
sizeof(usb_port)?
Not needed anymore w/ your suggestion above.
https://review.coreboot.org/c/coreboot/+/38541/16/src/ec/google/chromeec/ec_... PS16, Line 185: return
If you return here, then device and scope acpigen_pop_len() would be skipped. […]
Ah yes, good catch.