Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32590
Change subject: mb/intel/kunimitsu: Refactor to get rid of `pei_data` ......................................................................
mb/intel/kunimitsu: 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: Ibc013ccea9f83ef29f22fe2da4c0d12096308636 Signed-off-by: Nico Huber nico.h@gmx.de --- M src/mainboard/intel/kunimitsu/Makefile.inc D src/mainboard/intel/kunimitsu/pei_data.c M src/mainboard/intel/kunimitsu/romstage.c M src/mainboard/intel/kunimitsu/spd/spd.c M src/mainboard/intel/kunimitsu/spd/spd.h M src/mainboard/intel/kunimitsu/spd/spd_util.c 6 files changed, 17 insertions(+), 69 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/32590/1
diff --git a/src/mainboard/intel/kunimitsu/Makefile.inc b/src/mainboard/intel/kunimitsu/Makefile.inc index dc4b83c..3330a0a 100644 --- a/src/mainboard/intel/kunimitsu/Makefile.inc +++ b/src/mainboard/intel/kunimitsu/Makefile.inc @@ -18,8 +18,6 @@
bootblock-y += bootblock_mainboard.c
-romstage-y += pei_data.c - bootblock-$(CONFIG_CHROMEOS) += chromeos.c verstage-$(CONFIG_CHROMEOS) += chromeos.c romstage-$(CONFIG_CHROMEOS) += chromeos.c @@ -28,7 +26,6 @@ ramstage-$(CONFIG_EC_GOOGLE_CHROMEEC) += ec.c
ramstage-y += mainboard.c -ramstage-y += pei_data.c ramstage-y += ramstage.c
smm-$(CONFIG_HAVE_SMI_HANDLER) += smihandler.c diff --git a/src/mainboard/intel/kunimitsu/pei_data.c b/src/mainboard/intel/kunimitsu/pei_data.c deleted file mode 100644 index bfc40c2..0000000 --- a/src/mainboard/intel/kunimitsu/pei_data.c +++ /dev/null @@ -1,29 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2014 Google Inc. - * Copyright (C) 2015 Intel Corporation. - * - * 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. - */ - -#include <stdint.h> -#include <soc/pei_data.h> -#include <soc/pei_wrapper.h> -#include "boardid.h" -#include "spd/spd.h" - -void mainboard_fill_pei_data(struct pei_data *pei_data) -{ - mainboard_fill_dq_map_data(&pei_data->dq_map); - mainboard_fill_dqs_map_data(&pei_data->dqs_map); - mainboard_fill_rcomp_res_data(&pei_data->RcompResistor); - mainboard_fill_rcomp_strength_data(&pei_data->RcompTarget); -} diff --git a/src/mainboard/intel/kunimitsu/romstage.c b/src/mainboard/intel/kunimitsu/romstage.c index 1c6f5a2..72eea6d 100644 --- a/src/mainboard/intel/kunimitsu/romstage.c +++ b/src/mainboard/intel/kunimitsu/romstage.c @@ -15,20 +15,13 @@ * GNU General Public License for more details. */
-#include <string.h> #include <gpio.h> -#include <soc/pei_data.h> -#include <soc/pei_wrapper.h> #include <soc/romstage.h> #include "gpio.h" #include "spd/spd.h"
void mainboard_romstage_entry(struct romstage_params *params) { - params->pei_data->mem_cfg_id = get_spd_index(); - /* Fill out PEI DATA */ - mainboard_fill_pei_data(params->pei_data); - mainboard_fill_spd_data(params->pei_data); /* Initialize memory */ romstage_common(params); } @@ -36,24 +29,11 @@ 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)); + mainboard_fill_spd_data(memory_params); + mainboard_fill_dq_map_data(&memory_params->DqByteMapCh0); + mainboard_fill_dqs_map_data(&memory_params->DqsMapCpu2DramCh0); + mainboard_fill_rcomp_res_data(&memory_params->RcompResistor); + mainboard_fill_rcomp_strength_data(&memory_params->RcompTarget); memory_params->MemorySpdDataLen = SPD_LEN; memory_params->DqPinsInterleaved = FALSE; } diff --git a/src/mainboard/intel/kunimitsu/spd/spd.c b/src/mainboard/intel/kunimitsu/spd/spd.c index 8656d4b..9ed8fff 100644 --- a/src/mainboard/intel/kunimitsu/spd/spd.c +++ b/src/mainboard/intel/kunimitsu/spd/spd.c @@ -16,8 +16,9 @@
#include <arch/byteorder.h> #include <console/console.h> -#include <soc/pei_data.h> +#include <fsp/soc_binding.h> #include <soc/romstage.h> +#include <stdint.h> #include <string.h>
#include "spd.h" @@ -73,20 +74,19 @@ } }
-/* Copy SPD data for on-board memory */ -void mainboard_fill_spd_data(struct pei_data *pei_data) +/* Fill SPD pointers for on-board memory */ +void mainboard_fill_spd_data(MEMORY_INIT_UPD *memory_params) { uintptr_t spd_data; spd_data = mainboard_get_spd_data();
- memcpy(pei_data->spd_data[0][0], (void *)spd_data, SPD_LEN); - - if (mainboard_has_dual_channel_mem()) - memcpy(pei_data->spd_data[1][0], (void *)spd_data, SPD_LEN); - /* Make sure a valid SPD was found */ - if (pei_data->spd_data[0][0][0] == 0) + if (*(uint8_t *)spd_data == 0) die("Invalid SPD data.");
- mainboard_print_spd_info(pei_data->spd_data[0][0]); + memory_params->MemorySpdPtr00 = spd_data; + if (mainboard_has_dual_channel_mem()) + memory_params->MemorySpdPtr10 = spd_data; + + mainboard_print_spd_info((uint8_t *)spd_data); } diff --git a/src/mainboard/intel/kunimitsu/spd/spd.h b/src/mainboard/intel/kunimitsu/spd/spd.h index f53c9ec..8807e1b 100644 --- a/src/mainboard/intel/kunimitsu/spd/spd.h +++ b/src/mainboard/intel/kunimitsu/spd/spd.h @@ -16,6 +16,7 @@
#ifndef MAINBOARD_SPD_H
+#include <fsp/soc_binding.h> #include <gpio.h> #include "../gpio.h"
@@ -53,6 +54,7 @@ }; return (gpio_base2_value(spd_gpios, ARRAY_SIZE(spd_gpios))); } +void mainboard_fill_spd_data(MEMORY_INIT_UPD *memory_params); void mainboard_fill_dq_map_data(void *dq_map_ptr); void mainboard_fill_dqs_map_data(void *dqs_map_ptr); void mainboard_fill_rcomp_res_data(void *rcomp_ptr); diff --git a/src/mainboard/intel/kunimitsu/spd/spd_util.c b/src/mainboard/intel/kunimitsu/spd/spd_util.c index fc0581c..b173628 100644 --- a/src/mainboard/intel/kunimitsu/spd/spd_util.c +++ b/src/mainboard/intel/kunimitsu/spd/spd_util.c @@ -17,8 +17,6 @@ #include <console/console.h> #include <stdint.h> #include <string.h> -#include <soc/pei_data.h> -#include <soc/pei_wrapper.h> #include "boardid.h" #include "spd.h"
Hello build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32590
to look at the new patch set (#2).
Change subject: mb/intel/kunimitsu: Refactor to get rid of `pei_data` ......................................................................
mb/intel/kunimitsu: 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: Ibc013ccea9f83ef29f22fe2da4c0d12096308636 Signed-off-by: Nico Huber nico.h@gmx.de --- M src/mainboard/intel/kunimitsu/Makefile.inc D src/mainboard/intel/kunimitsu/pei_data.c M src/mainboard/intel/kunimitsu/romstage.c M src/mainboard/intel/kunimitsu/spd/spd.c M src/mainboard/intel/kunimitsu/spd/spd.h M src/mainboard/intel/kunimitsu/spd/spd_util.c 6 files changed, 17 insertions(+), 69 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/90/32590/2
Nico Huber has removed Frans Hendriks from this change. ( https://review.coreboot.org/c/coreboot/+/32590 )
Change subject: mb/intel/kunimitsu: Refactor to get rid of `pei_data` ......................................................................
Removed reviewer Frans Hendriks.
Nico Huber has removed Michał Żygowski from this change. ( https://review.coreboot.org/c/coreboot/+/32590 )
Change subject: mb/intel/kunimitsu: Refactor to get rid of `pei_data` ......................................................................
Removed reviewer Michał Żygowski.
Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32590 )
Change subject: mb/intel/kunimitsu: Refactor to get rid of `pei_data` ......................................................................
Patch Set 2: Code-Review+1
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32590 )
Change subject: mb/intel/kunimitsu: Refactor to get rid of `pei_data` ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32590 )
Change subject: mb/intel/kunimitsu: Refactor to get rid of `pei_data` ......................................................................
mb/intel/kunimitsu: 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: Ibc013ccea9f83ef29f22fe2da4c0d12096308636 Signed-off-by: Nico Huber nico.h@gmx.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/32590 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/intel/kunimitsu/Makefile.inc D src/mainboard/intel/kunimitsu/pei_data.c M src/mainboard/intel/kunimitsu/romstage.c M src/mainboard/intel/kunimitsu/spd/spd.c M src/mainboard/intel/kunimitsu/spd/spd.h M src/mainboard/intel/kunimitsu/spd/spd_util.c 6 files changed, 17 insertions(+), 69 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/intel/kunimitsu/Makefile.inc b/src/mainboard/intel/kunimitsu/Makefile.inc index dc4b83c..3330a0a 100644 --- a/src/mainboard/intel/kunimitsu/Makefile.inc +++ b/src/mainboard/intel/kunimitsu/Makefile.inc @@ -18,8 +18,6 @@
bootblock-y += bootblock_mainboard.c
-romstage-y += pei_data.c - bootblock-$(CONFIG_CHROMEOS) += chromeos.c verstage-$(CONFIG_CHROMEOS) += chromeos.c romstage-$(CONFIG_CHROMEOS) += chromeos.c @@ -28,7 +26,6 @@ ramstage-$(CONFIG_EC_GOOGLE_CHROMEEC) += ec.c
ramstage-y += mainboard.c -ramstage-y += pei_data.c ramstage-y += ramstage.c
smm-$(CONFIG_HAVE_SMI_HANDLER) += smihandler.c diff --git a/src/mainboard/intel/kunimitsu/pei_data.c b/src/mainboard/intel/kunimitsu/pei_data.c deleted file mode 100644 index bfc40c2..0000000 --- a/src/mainboard/intel/kunimitsu/pei_data.c +++ /dev/null @@ -1,29 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2014 Google Inc. - * Copyright (C) 2015 Intel Corporation. - * - * 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. - */ - -#include <stdint.h> -#include <soc/pei_data.h> -#include <soc/pei_wrapper.h> -#include "boardid.h" -#include "spd/spd.h" - -void mainboard_fill_pei_data(struct pei_data *pei_data) -{ - mainboard_fill_dq_map_data(&pei_data->dq_map); - mainboard_fill_dqs_map_data(&pei_data->dqs_map); - mainboard_fill_rcomp_res_data(&pei_data->RcompResistor); - mainboard_fill_rcomp_strength_data(&pei_data->RcompTarget); -} diff --git a/src/mainboard/intel/kunimitsu/romstage.c b/src/mainboard/intel/kunimitsu/romstage.c index 1c6f5a2..f25f88b 100644 --- a/src/mainboard/intel/kunimitsu/romstage.c +++ b/src/mainboard/intel/kunimitsu/romstage.c @@ -15,20 +15,13 @@ * GNU General Public License for more details. */
-#include <string.h> #include <gpio.h> -#include <soc/pei_data.h> -#include <soc/pei_wrapper.h> #include <soc/romstage.h> #include "gpio.h" #include "spd/spd.h"
void mainboard_romstage_entry(struct romstage_params *params) { - params->pei_data->mem_cfg_id = get_spd_index(); - /* Fill out PEI DATA */ - mainboard_fill_pei_data(params->pei_data); - mainboard_fill_spd_data(params->pei_data); /* Initialize memory */ romstage_common(params); } @@ -36,24 +29,11 @@ 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)); + spd_memory_init_params(memory_params); + mainboard_fill_dq_map_data(&memory_params->DqByteMapCh0); + mainboard_fill_dqs_map_data(&memory_params->DqsMapCpu2DramCh0); + mainboard_fill_rcomp_res_data(&memory_params->RcompResistor); + mainboard_fill_rcomp_strength_data(&memory_params->RcompTarget); memory_params->MemorySpdDataLen = SPD_LEN; memory_params->DqPinsInterleaved = FALSE; } diff --git a/src/mainboard/intel/kunimitsu/spd/spd.c b/src/mainboard/intel/kunimitsu/spd/spd.c index 8656d4b..bebb544 100644 --- a/src/mainboard/intel/kunimitsu/spd/spd.c +++ b/src/mainboard/intel/kunimitsu/spd/spd.c @@ -16,8 +16,9 @@
#include <arch/byteorder.h> #include <console/console.h> -#include <soc/pei_data.h> +#include <fsp/soc_binding.h> #include <soc/romstage.h> +#include <stdint.h> #include <string.h>
#include "spd.h" @@ -73,20 +74,19 @@ } }
-/* Copy SPD data for on-board memory */ -void mainboard_fill_spd_data(struct pei_data *pei_data) +/* Fill SPD pointers for on-board memory */ +void spd_memory_init_params(MEMORY_INIT_UPD *memory_params) { uintptr_t spd_data; spd_data = mainboard_get_spd_data();
- memcpy(pei_data->spd_data[0][0], (void *)spd_data, SPD_LEN); - - if (mainboard_has_dual_channel_mem()) - memcpy(pei_data->spd_data[1][0], (void *)spd_data, SPD_LEN); - /* Make sure a valid SPD was found */ - if (pei_data->spd_data[0][0][0] == 0) + if (*(uint8_t *)spd_data == 0) die("Invalid SPD data.");
- mainboard_print_spd_info(pei_data->spd_data[0][0]); + memory_params->MemorySpdPtr00 = spd_data; + if (mainboard_has_dual_channel_mem()) + memory_params->MemorySpdPtr10 = spd_data; + + mainboard_print_spd_info((uint8_t *)spd_data); } diff --git a/src/mainboard/intel/kunimitsu/spd/spd.h b/src/mainboard/intel/kunimitsu/spd/spd.h index f53c9ec..22d371f 100644 --- a/src/mainboard/intel/kunimitsu/spd/spd.h +++ b/src/mainboard/intel/kunimitsu/spd/spd.h @@ -16,6 +16,7 @@
#ifndef MAINBOARD_SPD_H
+#include <fsp/soc_binding.h> #include <gpio.h> #include "../gpio.h"
@@ -53,6 +54,7 @@ }; return (gpio_base2_value(spd_gpios, ARRAY_SIZE(spd_gpios))); } +void spd_memory_init_params(MEMORY_INIT_UPD *memory_params); void mainboard_fill_dq_map_data(void *dq_map_ptr); void mainboard_fill_dqs_map_data(void *dqs_map_ptr); void mainboard_fill_rcomp_res_data(void *rcomp_ptr); diff --git a/src/mainboard/intel/kunimitsu/spd/spd_util.c b/src/mainboard/intel/kunimitsu/spd/spd_util.c index fc0581c..b173628 100644 --- a/src/mainboard/intel/kunimitsu/spd/spd_util.c +++ b/src/mainboard/intel/kunimitsu/spd/spd_util.c @@ -17,8 +17,6 @@ #include <console/console.h> #include <stdint.h> #include <string.h> -#include <soc/pei_data.h> -#include <soc/pei_wrapper.h> #include "boardid.h" #include "spd.h"
-
Matt DeVillier (Code Review) -
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)