Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47501 )
Change subject: mb/google/volteer/v/volteer2: Config for passive USB-C DB on C1 ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/c/coreboot/+/47501/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47501/3//COMMIT_MSG@10 PS3, Line 10: volteer
Nit: Should fit in the line above (75 characters per line).
75 doesn't look right in gerrit.
https://review.coreboot.org/c/coreboot/+/47501/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/Kconfig:
https://review.coreboot.org/c/coreboot/+/47501/3/src/mainboard/google/voltee... PS3, Line 153: config VARIANT_HAS_PASSIVE_USB_DB
I wonder why this is board specific, and not put into some common code, or at least the chipset code […]
at this point, this is a volteer specific feature.
https://review.coreboot.org/c/coreboot/+/47501/3/src/mainboard/google/voltee... PS3, Line 154: bool
Some description/help would be nice.
Ack
https://review.coreboot.org/c/coreboot/+/47501/3/src/mainboard/google/voltee... File src/mainboard/google/volteer/mainboard.c:
https://review.coreboot.org/c/coreboot/+/47501/3/src/mainboard/google/voltee... PS3, Line 128: #if CONFIG(VARIANT_HAS_PASSIVE_USB_DB)
Please use the C if, and not the preprocessor one, if possible.
GPIO_ID_TCP1_AUXx_DC isn't necessarily defined, so this needs to be conditionally compiled.