Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36452 )
Change subject: hatch: Create puff variant ......................................................................
Patch Set 7: Code-Review+2
(3 comments)
https://review.coreboot.org/c/coreboot/+/36452/4/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/puff/gpio.c:
https://review.coreboot.org/c/coreboot/+/36452/4/src/mainboard/google/hatch/... PS4, Line 41: /* E23 : TP1 */ : PAD_NC(GPP_E23, NONE), : /* H17 : TP2 */ : PAD_NC(GPP_H17, NONE),
I wasn't sure so I just included them however I removed them now which seems to be your advice here […]
That's right. In general, early_gpio_table[] is used to configure any pads that are: 1. Used by stages before getting to ramstage(which initializes everything including not-connected pads) 2. Controlling device power/reset which needs to be enabled/deasserted early on in the boot process.
https://review.coreboot.org/c/coreboot/+/36452/4/src/mainboard/google/hatch/... File src/mainboard/google/hatch/variants/puff/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/36452/4/src/mainboard/google/hatch/... PS4, Line 47: .rise_time_ns = 0, : .fall_time_ns = 0,
Seems harmless to be explicit and serves as a clear reminder how and were these need to be adjusted […]
Okay. SG.
https://review.coreboot.org/c/coreboot/+/36452/4/src/mainboard/google/hatch/... PS4, Line 76: chip drivers/i2c/generic
Yep ok, I commented out these blocks for now. […]
nit: You can just remove it and add it when really required. Upto you.