[coreboot-gerrit] Change in coreboot[master]: mb/google/bip: Add GPIO configuration settings

[Furquan Shaikh (Code Review)]

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/variants/bip/gpio.c
File src/mainboard/google/octopus/variants/bip/gpio.c:

https://review.coreboot.org/#/c/25741/1/src/mainboard/google/octopus/variants/bip/gpio.c@257
PS1, Line 257: __attribute__((weak)) 
Why weak in variant?


https://review.coreboot.org/#/c/25741/1/src/mainboard/google/octopus/variants/bip/gpio.c@283
PS1, Line 283: __attribute__((weak))
Why weak in variant?


https://review.coreboot.org/#/c/25741/1/src/mainboard/google/octopus/variants/bip/gpio.c@290
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/variants/bip/gpio.c@301
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.



-- 
To view, visit https://review.coreboot.org/25741
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icc205f576691427d78c9159dfdbced87e41a0517
Gerrit-Change-Number: 25741
Gerrit-PatchSet: 1
Gerrit-Owner: Shamile Khan <shamile.khan at intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams at intel.com>
Gerrit-Reviewer: Justin TerAvest <teravest at chromium.org>
Gerrit-Reviewer: Shaunak Saha <shaunak.saha at intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Comment-Date: Thu, 19 Apr 2018 20:33:54 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180419/74b5ce70/attachment.html>