Attention is currently required from: Raul Rangel, Jason Nien, Jon Murphy, Martin Roth, Tim Van Patten, Karthik Ramasubramanian, Mark Hasemeyer.
Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74607 )
Change subject: mb/google/skyrim: Add named GPIO's ......................................................................
Patch Set 2:
(5 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/74607/comment/a612e0db_18d99d15 PS2, Line 9: ' Nit: No apostrophe needed here.
File src/mainboard/google/skyrim/variants/baseboard/include/baseboard/gpio.h:
https://review.coreboot.org/c/coreboot/+/74607/comment/83093728_3d01fbc9 PS2, Line 8: GPIO_0 Any reason not to make these a regular column instead of just using the single space?
To me the single space makes it harder to read because the name and GPIO blend together.
https://review.coreboot.org/c/coreboot/+/74607/comment/049923a8_08f460f3 PS2, Line 69: SOC_I2C_1_SDA Most of these names aren't currently used anywhere. Do we need to define all of them?
File src/mainboard/google/skyrim/variants/frostflow/include/variant/gpio.h:
https://review.coreboot.org/c/coreboot/+/74607/comment/ed75d3da_9edcd314 PS2, Line 6: GPIO_8 should we undefine EN_PWR_WWAN_X?
File src/mainboard/google/skyrim/variants/markarth/include/variant/gpio.h:
https://review.coreboot.org/c/coreboot/+/74607/comment/f420fd32_d0a2cd32 PS2, Line 5: /* SOC_PEN_DETECT_ODL(GPIO_3) => Unused */ : /* EN_PWR_FP(GPIO_4) => Unused */ : /* EN_PWR_WWAN_X(GPIO_8) => Unused */ : /* SOC_FP_INT_L(GPIO_24) => Unused */ : /* SD_AUX_RST_SOC_L(GPIO_27) => Unused */ : /* WWAN_RST(GPIO_42) => Unused */ Should we undefine these GPIOs?
-
Martin L Roth (Code Review)