Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/25741 )
Change subject: mb/google/bip: Add GPIO configuration settings ......................................................................
Patch Set 1:
(5 comments)
https://review.coreboot.org/#/c/25741/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/25741/1//COMMIT_MSG@9 PS1, Line 9: copy of yorp settings Did you compare yorp and bip GPIOs to see if there are any differences at all? If not, it might make sense to reuse the baseboard gpio table. We can fork off if there are differences.
https://review.coreboot.org/#/c/25741/1/src/mainboard/google/octopus/variant... File src/mainboard/google/octopus/variants/bip/gpio.c:
https://review.coreboot.org/#/c/25741/1/src/mainboard/google/octopus/variant... PS1, Line 257: __attribute__((weak)) Why weak in variant?
https://review.coreboot.org/#/c/25741/1/src/mainboard/google/octopus/variant... PS1, Line 283: __attribute__((weak)) Why weak in variant?
https://review.coreboot.org/#/c/25741/1/src/mainboard/google/octopus/variant... PS1, Line 290: /* GPIO settings before entering sleep. */ : static const struct pad_config sleep_gpio_table[] = { : }; : : const struct pad_config *__attribute__((weak)) : variant_sleep_gpio_table(size_t *num) : { : *num = ARRAY_SIZE(sleep_gpio_table); : return sleep_gpio_table; : } No need to add this right now. It can be done when required.
https://review.coreboot.org/#/c/25741/1/src/mainboard/google/octopus/variant... PS1, Line 301: static const struct cros_gpio cros_gpios[] = { : CROS_GPIO_WP_AH(PAD_SCC(GPIO_PCH_WP), GPIO_COMM_SCC_NAME), : }; : : const struct cros_gpio *__attribute__((weak)) variant_cros_gpios(size_t *num) : { : *num = ARRAY_SIZE(cros_gpios); : return cros_gpios; : } This will most likely be same across variants. We can add it later if there are differences.