Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32594 )
Change subject: mb/google/glados: Refactor to get rid of `pei_data` ......................................................................
mb/google/glados: Refactor to get rid of `pei_data`
The SoC specific `struct pei_data` was filled with values that were later only consumed by the mainboard code again. Avoid jumping through this hoop and fill FSP UPDs directly.
Change-Id: I040f4a55b4f4bad3f6072920e5e2eceded4cb9bb Signed-off-by: Nico Huber nico.h@gmx.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/32594 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Matt DeVillier matt.devillier@gmail.com Reviewed-by: Patrick Rudolph siro@das-labor.org --- M src/mainboard/google/glados/romstage.c M src/mainboard/google/glados/spd/spd.c A src/mainboard/google/glados/spd/spd_util.h M src/mainboard/google/glados/variants/asuka/variant.c M src/mainboard/google/glados/variants/baseboard/include/baseboard/variant.h M src/mainboard/google/glados/variants/caroline/variant.c M src/mainboard/google/glados/variants/cave/variant.c M src/mainboard/google/glados/variants/chell/variant.c M src/mainboard/google/glados/variants/glados/variant.c M src/mainboard/google/glados/variants/lars/variant.c M src/mainboard/google/glados/variants/sentry/variant.c 11 files changed, 132 insertions(+), 116 deletions(-)
Approvals: build bot (Jenkins): Verified Matt DeVillier: Looks good to me, but someone else must approve Patrick Rudolph: Looks good to me, approved
diff --git a/src/mainboard/google/glados/romstage.c b/src/mainboard/google/glados/romstage.c index 81f0866..47524c2 100644 --- a/src/mainboard/google/glados/romstage.c +++ b/src/mainboard/google/glados/romstage.c @@ -15,16 +15,16 @@ * GNU General Public License for more details. */
-#include <string.h> +#include <baseboard/variant.h> #include <ec/google/chromeec/ec.h> #include <gpio.h> -#include <soc/pei_data.h> -#include <soc/pei_wrapper.h> #include <soc/romstage.h> -#include "spd/spd.h" #include <variant/ec.h> #include <variant/gpio.h>
+#include "spd/spd_util.h" +#include "spd/spd.h" + void mainboard_romstage_entry(struct romstage_params *params) { #ifdef EC_ENABLE_KEYBOARD_BACKLIGHT @@ -32,18 +32,6 @@ if (params->power_state->prev_sleep_state != ACPI_S3) google_chromeec_kbbacklight(25); #endif - /* Get SPD index */ - gpio_t spd_gpios[] = { - GPIO_MEM_CONFIG_0, - GPIO_MEM_CONFIG_1, - GPIO_MEM_CONFIG_2, - GPIO_MEM_CONFIG_3, - }; - params->pei_data->mem_cfg_id = - gpio_base2_value(spd_gpios, ARRAY_SIZE(spd_gpios)); - /* Fill out PEI DATA */ - mainboard_fill_pei_data(params->pei_data); - mainboard_fill_spd_data(params->pei_data); /* Initialize memory */ romstage_common(params); } @@ -51,26 +39,18 @@ void mainboard_memory_init_params(struct romstage_params *params, MEMORY_INIT_UPD *memory_params) { - if (params->pei_data->spd_data[0][0][0] != 0) { - memory_params->MemorySpdPtr00 = - (UINT32)(params->pei_data->spd_data[0][0]); - memory_params->MemorySpdPtr10 = - (UINT32)(params->pei_data->spd_data[1][0]); - } - memcpy(memory_params->DqByteMapCh0, params->pei_data->dq_map[0], - sizeof(params->pei_data->dq_map[0])); - memcpy(memory_params->DqByteMapCh1, params->pei_data->dq_map[1], - sizeof(params->pei_data->dq_map[1])); - memcpy(memory_params->DqsMapCpu2DramCh0, params->pei_data->dqs_map[0], - sizeof(params->pei_data->dqs_map[0])); - memcpy(memory_params->DqsMapCpu2DramCh1, params->pei_data->dqs_map[1], - sizeof(params->pei_data->dqs_map[1])); - memcpy(memory_params->RcompResistor, params->pei_data->RcompResistor, - sizeof(params->pei_data->RcompResistor)); - memcpy(memory_params->RcompTarget, params->pei_data->RcompTarget, - sizeof(params->pei_data->RcompTarget)); + /* Get SPD index */ + const gpio_t spd_gpios[] = { + GPIO_MEM_CONFIG_0, + GPIO_MEM_CONFIG_1, + GPIO_MEM_CONFIG_2, + GPIO_MEM_CONFIG_3, + }; + const int spd_idx = gpio_base2_value(spd_gpios, ARRAY_SIZE(spd_gpios)); + memory_params->MemorySpdDataLen = SPD_LEN; memory_params->DqPinsInterleaved = FALSE; - if (CONFIG(BOARD_GOOGLE_CAROLINE)) - memory_params->DdrFreqLimit = 1600; + + spd_memory_init_params(memory_params, spd_idx); + variant_memory_init_params(memory_params, spd_idx); } diff --git a/src/mainboard/google/glados/spd/spd.c b/src/mainboard/google/glados/spd/spd.c index 391b702..b3cf3f9 100644 --- a/src/mainboard/google/glados/spd/spd.c +++ b/src/mainboard/google/glados/spd/spd.c @@ -19,10 +19,11 @@ #include <console/console.h> #include <gpio.h> #include <soc/gpio.h> -#include <soc/pei_data.h> #include <soc/romstage.h> #include <string.h> #include <baseboard/variant.h> + +#include "spd_util.h" #include "spd.h"
static void mainboard_print_spd_info(uint8_t spd[]) @@ -83,13 +84,11 @@ }
/* Copy SPD data for on-board memory */ -void mainboard_fill_spd_data(struct pei_data *pei_data) +void spd_memory_init_params(MEMORY_INIT_UPD *const memory_params, int spd_index) { - char *spd_file; + uint8_t *spd_file; size_t spd_file_len; - int spd_index;
- spd_index = pei_data->mem_cfg_id; printk(BIOS_INFO, "SPD index %d\n", spd_index);
/* Load SPD data from CBFS */ @@ -108,15 +107,15 @@ spd_index = 1; }
- /* Assume same memory in both channels */ - spd_index *= SPD_LEN; - memcpy(pei_data->spd_data[0][0], spd_file + spd_index, SPD_LEN); - if (is_dual_channel(spd_index)) - memcpy(pei_data->spd_data[1][0], spd_file + spd_index, SPD_LEN); - + const size_t spd_offset = spd_index * SPD_LEN; /* Make sure a valid SPD was found */ - if (pei_data->spd_data[0][0][0] == 0) + if (spd_file[spd_offset] == 0) die("Invalid SPD data.");
- mainboard_print_spd_info(pei_data->spd_data[0][0]); + /* Assume same memory in both channels */ + memory_params->MemorySpdPtr00 = (uintptr_t)spd_file + spd_offset; + if (is_dual_channel(spd_index)) + memory_params->MemorySpdPtr10 = memory_params->MemorySpdPtr00; + + mainboard_print_spd_info(spd_file + spd_offset); } diff --git a/src/mainboard/google/glados/spd/spd_util.h b/src/mainboard/google/glados/spd/spd_util.h new file mode 100644 index 0000000..90dbd5f --- /dev/null +++ b/src/mainboard/google/glados/spd/spd_util.h @@ -0,0 +1,21 @@ +/* + * This file is part of the coreboot project. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef SPD_UTIL_H +#define SPD_UTIL_H + +#include <fsp/soc_binding.h> + +void spd_memory_init_params(MEMORY_INIT_UPD *, int spd_index); + +#endif /* SPD_UTIL_H */ diff --git a/src/mainboard/google/glados/variants/asuka/variant.c b/src/mainboard/google/glados/variants/asuka/variant.c index 13cfe4f..4c77897 100644 --- a/src/mainboard/google/glados/variants/asuka/variant.c +++ b/src/mainboard/google/glados/variants/asuka/variant.c @@ -17,10 +17,10 @@ #include <stdint.h> #include <string.h> #include <baseboard/variant.h> -#include <soc/pei_data.h> -#include <soc/pei_wrapper.h> +#include <fsp/soc_binding.h>
-void mainboard_fill_pei_data(struct pei_data *pei_data) +void variant_memory_init_params( + MEMORY_INIT_UPD *const memory_params, const int spd_index) { /* DQ byte map */ const u8 dq_map[2][12] = { @@ -39,12 +39,14 @@ /* Rcomp target */ const u16 RcompTarget[5] = { 100, 40, 40, 23, 40 };
- memcpy(pei_data->dq_map, dq_map, sizeof(dq_map)); - memcpy(pei_data->dqs_map, dqs_map, sizeof(dqs_map)); - memcpy(pei_data->RcompResistor, RcompResistor, - sizeof(RcompResistor)); - memcpy(pei_data->RcompTarget, RcompTarget, - sizeof(RcompTarget)); + memcpy(memory_params->DqByteMapCh0, dq_map, + sizeof(memory_params->DqByteMapCh0) * 2); + memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map, + sizeof(memory_params->DqsMapCpu2DramCh0) * 2); + memcpy(memory_params->RcompResistor, RcompResistor, + sizeof(memory_params->RcompResistor)); + memcpy(memory_params->RcompTarget, RcompTarget, + sizeof(memory_params->RcompTarget)); }
int is_dual_channel(const int spd_index) diff --git a/src/mainboard/google/glados/variants/baseboard/include/baseboard/variant.h b/src/mainboard/google/glados/variants/baseboard/include/baseboard/variant.h index bbab7fc..72eef68 100644 --- a/src/mainboard/google/glados/variants/baseboard/include/baseboard/variant.h +++ b/src/mainboard/google/glados/variants/baseboard/include/baseboard/variant.h @@ -15,7 +15,10 @@ #ifndef GLADOS_VARIANT_H #define GLADOS_VARIANT_H
+#include <fsp/soc_binding.h> + int is_dual_channel(const int spd_index); void mainboard_gpio_smi_sleep(void); +void variant_memory_init_params(MEMORY_INIT_UPD *memory_params, int spd_index);
#endif /* GLADOS_VARIANT_H */ diff --git a/src/mainboard/google/glados/variants/caroline/variant.c b/src/mainboard/google/glados/variants/caroline/variant.c index a00eacf..d61a538 100644 --- a/src/mainboard/google/glados/variants/caroline/variant.c +++ b/src/mainboard/google/glados/variants/caroline/variant.c @@ -15,14 +15,14 @@ */
#include <baseboard/variant.h> +#include <fsp/soc_binding.h> #include <gpio.h> #include <stdint.h> #include <string.h> -#include <soc/pei_data.h> -#include <soc/pei_wrapper.h> #include <variant/gpio.h>
-void mainboard_fill_pei_data(struct pei_data *pei_data) +void variant_memory_init_params( + MEMORY_INIT_UPD *const memory_params, const int spd_index) { /* DQ byte map */ const u8 dq_map[2][12] = { @@ -41,12 +41,15 @@ /* Rcomp target */ const u16 RcompTarget[5] = { 100, 40, 40, 23, 40 };
- memcpy(pei_data->dq_map, dq_map, sizeof(dq_map)); - memcpy(pei_data->dqs_map, dqs_map, sizeof(dqs_map)); - memcpy(pei_data->RcompResistor, RcompResistor, - sizeof(RcompResistor)); - memcpy(pei_data->RcompTarget, RcompTarget, - sizeof(RcompTarget)); + memcpy(memory_params->DqByteMapCh0, dq_map, + sizeof(memory_params->DqByteMapCh0) * 2); + memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map, + sizeof(memory_params->DqsMapCpu2DramCh0) * 2); + memcpy(memory_params->RcompResistor, RcompResistor, + sizeof(memory_params->RcompResistor)); + memcpy(memory_params->RcompTarget, RcompTarget, + sizeof(memory_params->RcompTarget)); + memory_params->DdrFreqLimit = 1600; }
void mainboard_gpio_smi_sleep(void) diff --git a/src/mainboard/google/glados/variants/cave/variant.c b/src/mainboard/google/glados/variants/cave/variant.c index 2ce0a900..fc27fb4 100644 --- a/src/mainboard/google/glados/variants/cave/variant.c +++ b/src/mainboard/google/glados/variants/cave/variant.c @@ -15,14 +15,14 @@ */
#include <baseboard/variant.h> +#include <fsp/soc_binding.h> #include <gpio.h> #include <stdint.h> #include <string.h> -#include <soc/pei_data.h> -#include <soc/pei_wrapper.h> #include <variant/gpio.h>
-void mainboard_fill_pei_data(struct pei_data *pei_data) +void variant_memory_init_params( + MEMORY_INIT_UPD *const memory_params, const int spd_index) { /* DQ byte map */ const u8 dq_map[2][12] = { @@ -41,12 +41,14 @@ /* Rcomp target */ const u16 RcompTarget[5] = { 100, 40, 40, 23, 40 };
- memcpy(pei_data->dq_map, dq_map, sizeof(dq_map)); - memcpy(pei_data->dqs_map, dqs_map, sizeof(dqs_map)); - memcpy(pei_data->RcompResistor, RcompResistor, - sizeof(RcompResistor)); - memcpy(pei_data->RcompTarget, RcompTarget, - sizeof(RcompTarget)); + memcpy(memory_params->DqByteMapCh0, dq_map, + sizeof(memory_params->DqByteMapCh0) * 2); + memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map, + sizeof(memory_params->DqsMapCpu2DramCh0) * 2); + memcpy(memory_params->RcompResistor, RcompResistor, + sizeof(memory_params->RcompResistor)); + memcpy(memory_params->RcompTarget, RcompTarget, + sizeof(memory_params->RcompTarget)); }
void mainboard_gpio_smi_sleep(void) diff --git a/src/mainboard/google/glados/variants/chell/variant.c b/src/mainboard/google/glados/variants/chell/variant.c index da83ed0..2d1b363 100644 --- a/src/mainboard/google/glados/variants/chell/variant.c +++ b/src/mainboard/google/glados/variants/chell/variant.c @@ -15,14 +15,14 @@ */
#include <baseboard/variant.h> +#include <fsp/soc_binding.h> #include <gpio.h> #include <stdint.h> #include <string.h> -#include <soc/pei_data.h> -#include <soc/pei_wrapper.h> #include <variant/gpio.h>
-void mainboard_fill_pei_data(struct pei_data *pei_data) +void variant_memory_init_params( + MEMORY_INIT_UPD *const memory_params, const int spd_index) { /* DQ byte map */ const u8 dq_map[2][12] = { @@ -41,12 +41,14 @@ /* Rcomp target */ const u16 RcompTarget[5] = { 100, 40, 40, 23, 40 };
- memcpy(pei_data->dq_map, dq_map, sizeof(dq_map)); - memcpy(pei_data->dqs_map, dqs_map, sizeof(dqs_map)); - memcpy(pei_data->RcompResistor, RcompResistor, - sizeof(RcompResistor)); - memcpy(pei_data->RcompTarget, RcompTarget, - sizeof(RcompTarget)); + memcpy(memory_params->DqByteMapCh0, dq_map, + sizeof(memory_params->DqByteMapCh0) * 2); + memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map, + sizeof(memory_params->DqsMapCpu2DramCh0) * 2); + memcpy(memory_params->RcompResistor, RcompResistor, + sizeof(memory_params->RcompResistor)); + memcpy(memory_params->RcompTarget, RcompTarget, + sizeof(memory_params->RcompTarget)); }
void mainboard_gpio_smi_sleep(void) diff --git a/src/mainboard/google/glados/variants/glados/variant.c b/src/mainboard/google/glados/variants/glados/variant.c index 2ce0a900..fc27fb4 100644 --- a/src/mainboard/google/glados/variants/glados/variant.c +++ b/src/mainboard/google/glados/variants/glados/variant.c @@ -15,14 +15,14 @@ */
#include <baseboard/variant.h> +#include <fsp/soc_binding.h> #include <gpio.h> #include <stdint.h> #include <string.h> -#include <soc/pei_data.h> -#include <soc/pei_wrapper.h> #include <variant/gpio.h>
-void mainboard_fill_pei_data(struct pei_data *pei_data) +void variant_memory_init_params( + MEMORY_INIT_UPD *const memory_params, const int spd_index) { /* DQ byte map */ const u8 dq_map[2][12] = { @@ -41,12 +41,14 @@ /* Rcomp target */ const u16 RcompTarget[5] = { 100, 40, 40, 23, 40 };
- memcpy(pei_data->dq_map, dq_map, sizeof(dq_map)); - memcpy(pei_data->dqs_map, dqs_map, sizeof(dqs_map)); - memcpy(pei_data->RcompResistor, RcompResistor, - sizeof(RcompResistor)); - memcpy(pei_data->RcompTarget, RcompTarget, - sizeof(RcompTarget)); + memcpy(memory_params->DqByteMapCh0, dq_map, + sizeof(memory_params->DqByteMapCh0) * 2); + memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map, + sizeof(memory_params->DqsMapCpu2DramCh0) * 2); + memcpy(memory_params->RcompResistor, RcompResistor, + sizeof(memory_params->RcompResistor)); + memcpy(memory_params->RcompTarget, RcompTarget, + sizeof(memory_params->RcompTarget)); }
void mainboard_gpio_smi_sleep(void) diff --git a/src/mainboard/google/glados/variants/lars/variant.c b/src/mainboard/google/glados/variants/lars/variant.c index 4fe88ef..cff0096 100644 --- a/src/mainboard/google/glados/variants/lars/variant.c +++ b/src/mainboard/google/glados/variants/lars/variant.c @@ -17,8 +17,7 @@ #include <stdint.h> #include <string.h> #include <baseboard/variant.h> -#include <soc/pei_data.h> -#include <soc/pei_wrapper.h> +#include <fsp/soc_binding.h>
#define K4E6E304EB_MEM_ID 0x5
@@ -29,7 +28,8 @@ #define MEM_SINGLE_CHANB 0xb #define MEM_SINGLE_CHANC 0xc
-void mainboard_fill_pei_data(struct pei_data *pei_data) +void variant_memory_init_params( + MEMORY_INIT_UPD *const params, const int spd_index) { /* DQ byte map */ const u8 dq_map[2][12] = { @@ -54,17 +54,18 @@ /* Default Rcomp Target assignment */ const u16 *targeted_rcomp = RcompTarget;
- memcpy(pei_data->dq_map, dq_map, sizeof(dq_map)); - memcpy(pei_data->dqs_map, dqs_map, sizeof(dqs_map)); - memcpy(pei_data->RcompResistor, RcompResistor, - sizeof(RcompResistor)); - /* Override Rcomp Target assignment for specific SKU(s) */ - if (pei_data->mem_cfg_id == K4E6E304EB_MEM_ID) + if (spd_index == K4E6E304EB_MEM_ID) targeted_rcomp = StrengthendRcompTarget;
- memcpy(pei_data->RcompTarget, targeted_rcomp, - sizeof(pei_data->RcompTarget)); + memcpy(params->DqByteMapCh0, dq_map, + sizeof(params->DqByteMapCh0) * 2); + memcpy(params->DqsMapCpu2DramCh0, dqs_map, + sizeof(params->DqsMapCpu2DramCh0) * 2); + memcpy(params->RcompResistor, RcompResistor, + sizeof(params->RcompResistor)); + memcpy(params->RcompTarget, targeted_rcomp, + sizeof(params->RcompTarget)); }
int is_dual_channel(const int spd_index) diff --git a/src/mainboard/google/glados/variants/sentry/variant.c b/src/mainboard/google/glados/variants/sentry/variant.c index 00f49fe..4c7fa23 100644 --- a/src/mainboard/google/glados/variants/sentry/variant.c +++ b/src/mainboard/google/glados/variants/sentry/variant.c @@ -17,12 +17,12 @@ #include <stdint.h> #include <string.h> #include <baseboard/variant.h> -#include <soc/pei_data.h> -#include <soc/pei_wrapper.h> +#include <fsp/soc_binding.h>
#define K4E6E304EE_MEM_ID 0x3
-void mainboard_fill_pei_data(struct pei_data *pei_data) +void variant_memory_init_params( + MEMORY_INIT_UPD *const memory_params, const int spd_index) { /* DQ byte map */ const u8 dq_map[2][12] = { @@ -47,15 +47,16 @@ /* Default Rcomp Target assignment */ const u16 *targeted_rcomp = RcompTarget;
- memcpy(pei_data->dq_map, dq_map, sizeof(dq_map)); - memcpy(pei_data->dqs_map, dqs_map, sizeof(dqs_map)); - memcpy(pei_data->RcompResistor, RcompResistor, - sizeof(RcompResistor)); - /* Override Rcomp Target assignment for specific SKU(s) */ - if (pei_data->mem_cfg_id == K4E6E304EE_MEM_ID) + if (spd_index == K4E6E304EE_MEM_ID) targeted_rcomp = StrengthendRcompTarget;
- memcpy(pei_data->RcompTarget, targeted_rcomp, - sizeof(pei_data->RcompTarget)); + memcpy(memory_params->DqByteMapCh0, dq_map, + sizeof(memory_params->DqByteMapCh0) * 2); + memcpy(memory_params->DqsMapCpu2DramCh0, dqs_map, + sizeof(memory_params->DqsMapCpu2DramCh0) * 2); + memcpy(memory_params->RcompResistor, RcompResistor, + sizeof(memory_params->RcompResistor)); + memcpy(memory_params->RcompTarget, targeted_rcomp, + sizeof(memory_params->RcompTarget)); }