Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47958 )
Change subject: mb/prodrive/hermes: Encapsulate GPIO setup ......................................................................
mb/prodrive/hermes: Encapsulate GPIO setup
Having variants' gpio.c call the `gpio_configure_pads` function results in an API that does not need to pass data around, which is much simpler.
Change-Id: I1064dc6258561bcf83f0e249d65b823368cf0d31 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/prodrive/hermes/bootblock.c M src/mainboard/prodrive/hermes/ramstage.c M src/mainboard/prodrive/hermes/variants/baseboard/gpio.c M src/mainboard/prodrive/hermes/variants/baseboard/include/variant/gpio.h 4 files changed, 13 insertions(+), 27 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/47958/1
diff --git a/src/mainboard/prodrive/hermes/bootblock.c b/src/mainboard/prodrive/hermes/bootblock.c index 1426a55..40fd0b4 100644 --- a/src/mainboard/prodrive/hermes/bootblock.c +++ b/src/mainboard/prodrive/hermes/bootblock.c @@ -6,20 +6,12 @@ #include <variant/gpio.h> #include "gpio.h"
-static void early_config_gpio(void) -{ - /* This is a hack for FSP because it does things in MemoryInit() - * which it shouldn't do. We have to prepare certain gpios here - * because of the brokenness in FSP. */ - size_t num = 0; - const struct pad_config *early_gpio_table = get_early_gpio_table(&num); - - gpio_configure_pads(early_gpio_table, num); -} - void bootblock_mainboard_early_init(void) { - early_config_gpio(); + /* This is a hack for FSP because it does things in MemoryInit() + which it shouldn't do. We have to prepare certain gpios here + because of the brokenness in FSP. */ + program_early_gpio_pads(); }
void bootblock_mainboard_init(void) diff --git a/src/mainboard/prodrive/hermes/ramstage.c b/src/mainboard/prodrive/hermes/ramstage.c index 135d775..e3dfffc 100644 --- a/src/mainboard/prodrive/hermes/ramstage.c +++ b/src/mainboard/prodrive/hermes/ramstage.c @@ -10,12 +10,9 @@
void mainboard_silicon_init_params(FSP_S_CONFIG *params) { - size_t num = 0; - const struct pad_config *gpio_table = get_gpio_table(&num); - /* Configure pads prior to SiliconInit() in case there's any dependencies during hardware initialization. */ - gpio_configure_pads(gpio_table, num); + program_gpio_pads();
params->SataLedEnable = 1;
diff --git a/src/mainboard/prodrive/hermes/variants/baseboard/gpio.c b/src/mainboard/prodrive/hermes/variants/baseboard/gpio.c index 8735a9e..096dc35 100644 --- a/src/mainboard/prodrive/hermes/variants/baseboard/gpio.c +++ b/src/mainboard/prodrive/hermes/variants/baseboard/gpio.c @@ -2,6 +2,8 @@
#include "include/variant/gpio.h" #include <commonlib/helpers.h> +#include <soc/gpio.h> +#include <intelblocks/gpio_defs.h>
/* Pad configuration in ramstage */ static const struct pad_config gpio_table[] = { @@ -389,14 +391,12 @@ PAD_CFG_GPO(GPP_H5, 0, DEEP), /* PCH_HBLED_n */ };
-const struct pad_config *get_gpio_table(size_t *num) +void program_gpio_pads(void) { - *num = ARRAY_SIZE(gpio_table); - return gpio_table; + gpio_configure_pads(gpio_table, ARRAY_SIZE(gpio_table)); }
-const struct pad_config *get_early_gpio_table(size_t *num) +void program_early_gpio_pads(void) { - *num = ARRAY_SIZE(early_gpio_table); - return early_gpio_table; + gpio_configure_pads(early_gpio_table, ARRAY_SIZE(early_gpio_table)); } diff --git a/src/mainboard/prodrive/hermes/variants/baseboard/include/variant/gpio.h b/src/mainboard/prodrive/hermes/variants/baseboard/include/variant/gpio.h index 50d1801..8fce3c8 100644 --- a/src/mainboard/prodrive/hermes/variants/baseboard/include/variant/gpio.h +++ b/src/mainboard/prodrive/hermes/variants/baseboard/include/variant/gpio.h @@ -3,10 +3,7 @@ #ifndef PCH_GPIO_H #define PCH_GPIO_H
-#include <soc/gpio.h> -#include <intelblocks/gpio_defs.h> - -const struct pad_config *get_gpio_table(size_t *num); -const struct pad_config *get_early_gpio_table(size_t *num); +void program_gpio_pads(void); +void program_early_gpio_pads(void);
#endif /* PCH_GPIO_H */
Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47958 )
Change subject: mb/prodrive/hermes: Encapsulate GPIO setup ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47958 )
Change subject: mb/prodrive/hermes: Encapsulate GPIO setup ......................................................................
mb/prodrive/hermes: Encapsulate GPIO setup
Having variants' gpio.c call the `gpio_configure_pads` function results in an API that does not need to pass data around, which is much simpler.
Change-Id: I1064dc6258561bcf83f0e249d65b823368cf0d31 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/47958 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Felix Singer felixsinger@posteo.net --- M src/mainboard/prodrive/hermes/bootblock.c M src/mainboard/prodrive/hermes/ramstage.c M src/mainboard/prodrive/hermes/variants/baseboard/gpio.c M src/mainboard/prodrive/hermes/variants/baseboard/include/variant/gpio.h 4 files changed, 13 insertions(+), 27 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved
diff --git a/src/mainboard/prodrive/hermes/bootblock.c b/src/mainboard/prodrive/hermes/bootblock.c index 1426a55..40fd0b4 100644 --- a/src/mainboard/prodrive/hermes/bootblock.c +++ b/src/mainboard/prodrive/hermes/bootblock.c @@ -6,20 +6,12 @@ #include <variant/gpio.h> #include "gpio.h"
-static void early_config_gpio(void) -{ - /* This is a hack for FSP because it does things in MemoryInit() - * which it shouldn't do. We have to prepare certain gpios here - * because of the brokenness in FSP. */ - size_t num = 0; - const struct pad_config *early_gpio_table = get_early_gpio_table(&num); - - gpio_configure_pads(early_gpio_table, num); -} - void bootblock_mainboard_early_init(void) { - early_config_gpio(); + /* This is a hack for FSP because it does things in MemoryInit() + which it shouldn't do. We have to prepare certain gpios here + because of the brokenness in FSP. */ + program_early_gpio_pads(); }
void bootblock_mainboard_init(void) diff --git a/src/mainboard/prodrive/hermes/ramstage.c b/src/mainboard/prodrive/hermes/ramstage.c index 135d775..e3dfffc 100644 --- a/src/mainboard/prodrive/hermes/ramstage.c +++ b/src/mainboard/prodrive/hermes/ramstage.c @@ -10,12 +10,9 @@
void mainboard_silicon_init_params(FSP_S_CONFIG *params) { - size_t num = 0; - const struct pad_config *gpio_table = get_gpio_table(&num); - /* Configure pads prior to SiliconInit() in case there's any dependencies during hardware initialization. */ - gpio_configure_pads(gpio_table, num); + program_gpio_pads();
params->SataLedEnable = 1;
diff --git a/src/mainboard/prodrive/hermes/variants/baseboard/gpio.c b/src/mainboard/prodrive/hermes/variants/baseboard/gpio.c index 8735a9e..096dc35 100644 --- a/src/mainboard/prodrive/hermes/variants/baseboard/gpio.c +++ b/src/mainboard/prodrive/hermes/variants/baseboard/gpio.c @@ -2,6 +2,8 @@
#include "include/variant/gpio.h" #include <commonlib/helpers.h> +#include <soc/gpio.h> +#include <intelblocks/gpio_defs.h>
/* Pad configuration in ramstage */ static const struct pad_config gpio_table[] = { @@ -389,14 +391,12 @@ PAD_CFG_GPO(GPP_H5, 0, DEEP), /* PCH_HBLED_n */ };
-const struct pad_config *get_gpio_table(size_t *num) +void program_gpio_pads(void) { - *num = ARRAY_SIZE(gpio_table); - return gpio_table; + gpio_configure_pads(gpio_table, ARRAY_SIZE(gpio_table)); }
-const struct pad_config *get_early_gpio_table(size_t *num) +void program_early_gpio_pads(void) { - *num = ARRAY_SIZE(early_gpio_table); - return early_gpio_table; + gpio_configure_pads(early_gpio_table, ARRAY_SIZE(early_gpio_table)); } diff --git a/src/mainboard/prodrive/hermes/variants/baseboard/include/variant/gpio.h b/src/mainboard/prodrive/hermes/variants/baseboard/include/variant/gpio.h index 50d1801..8fce3c8 100644 --- a/src/mainboard/prodrive/hermes/variants/baseboard/include/variant/gpio.h +++ b/src/mainboard/prodrive/hermes/variants/baseboard/include/variant/gpio.h @@ -3,10 +3,7 @@ #ifndef PCH_GPIO_H #define PCH_GPIO_H
-#include <soc/gpio.h> -#include <intelblocks/gpio_defs.h> - -const struct pad_config *get_gpio_table(size_t *num); -const struct pad_config *get_early_gpio_table(size_t *num); +void program_gpio_pads(void); +void program_early_gpio_pads(void);
#endif /* PCH_GPIO_H */
-
Angel Pons (Code Review) -
Felix Singer (Code Review)
-
Patrick Georgi (Code Review)