Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43127 )
Change subject: haswell: Add function to retrieve SPD addresses ......................................................................
haswell: Add function to retrieve SPD addresses
And use it instead of directly writing to the MRC struct.
Change-Id: I7f04db29a08512c1a8b2b2300dba71cb3b84a5c5 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/asrock/b85m_pro4/romstage.c M src/mainboard/asrock/h81m-hds/romstage.c M src/mainboard/google/beltino/romstage.c M src/mainboard/google/slippy/romstage.c M src/mainboard/intel/baskingridge/romstage.c M src/mainboard/lenovo/t440p/romstage.c M src/mainboard/supermicro/x10slm-f/romstage.c M src/northbridge/intel/haswell/raminit.h M src/northbridge/intel/haswell/romstage.c 9 files changed, 61 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/43127/1
diff --git a/src/mainboard/asrock/b85m_pro4/romstage.c b/src/mainboard/asrock/b85m_pro4/romstage.c index f625824..6a71abb 100644 --- a/src/mainboard/asrock/b85m_pro4/romstage.c +++ b/src/mainboard/asrock/b85m_pro4/romstage.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <stdint.h> #include <northbridge/intel/haswell/haswell.h> #include <northbridge/intel/haswell/raminit.h> #include <southbridge/intel/lynxpoint/pch.h> @@ -16,12 +17,16 @@ RCBA16(D20IR) = DIR_ROUTE(PIRQA, PIRQB, PIRQC, PIRQD); }
+void mb_get_spd_map(uint8_t spd_map[4]) +{ + spd_map[0] = 0xa0; + spd_map[1] = 0xa2; + spd_map[2] = 0xa4; + spd_map[3] = 0xa6; +} + void mainboard_fill_pei_data(struct pei_data *pei_data) { - pei_data->spd_addresses[0] = 0xa0; - pei_data->spd_addresses[1] = 0xa2; - pei_data->spd_addresses[2] = 0xa4; - pei_data->spd_addresses[3] = 0xa6; pei_data->ec_present = 0; pei_data->gbe_enable = 1;
diff --git a/src/mainboard/asrock/h81m-hds/romstage.c b/src/mainboard/asrock/h81m-hds/romstage.c index cfefdda..0573b6b 100644 --- a/src/mainboard/asrock/h81m-hds/romstage.c +++ b/src/mainboard/asrock/h81m-hds/romstage.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <stdint.h> #include <northbridge/intel/haswell/haswell.h> #include <northbridge/intel/haswell/raminit.h> #include <southbridge/intel/lynxpoint/pch.h> @@ -16,10 +17,14 @@ RCBA16(D20IR) = DIR_ROUTE(PIRQA, PIRQB, PIRQC, PIRQD); }
+void mb_get_spd_map(uint8_t spd_map[4]) +{ + spd_map[0] = 0xa0; + spd_map[2] = 0xa4; +} + void mainboard_fill_pei_data(struct pei_data *pei_data) { - pei_data->spd_addresses[0] = 0xa0; - pei_data->spd_addresses[2] = 0xa4; pei_data->ec_present = 0;
struct usb2_port_setting usb2_ports[MAX_USB2_PORTS] = { diff --git a/src/mainboard/google/beltino/romstage.c b/src/mainboard/google/beltino/romstage.c index 839cd6c..8410d04 100644 --- a/src/mainboard/google/beltino/romstage.c +++ b/src/mainboard/google/beltino/romstage.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <stdint.h> #include <northbridge/intel/haswell/haswell.h> #include <northbridge/intel/haswell/raminit.h> #include <southbridge/intel/lynxpoint/pch.h> @@ -39,10 +40,14 @@ RCBA16(D23IR) = DIR_ROUTE(PIRQH, PIRQH, PIRQH, PIRQH); /* SDIO */ }
+void mb_get_spd_map(uint8_t spd_map[4]) +{ + spd_map[0] = 0xa0; + spd_map[2] = 0xa4; +} + void mainboard_fill_pei_data(struct pei_data *pei_data) { - pei_data->spd_addresses[0] = 0xa0; - pei_data->spd_addresses[2] = 0xa4; pei_data->ec_present = 0; pei_data->dq_pins_interleaved = 1; pei_data->usb_xhci_on_resume = 1; diff --git a/src/mainboard/google/slippy/romstage.c b/src/mainboard/google/slippy/romstage.c index e571944..47bcb47 100644 --- a/src/mainboard/google/slippy/romstage.c +++ b/src/mainboard/google/slippy/romstage.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <stdint.h> #include <northbridge/intel/haswell/haswell.h> #include <northbridge/intel/haswell/raminit.h> #include <southbridge/intel/lynxpoint/pch.h> @@ -40,10 +41,14 @@ RCBA16(D23IR) = DIR_ROUTE(PIRQH, PIRQH, PIRQH, PIRQH); /* SDIO */ }
+void mb_get_spd_map(uint8_t spd_map[4]) +{ + spd_map[0] = 0xff; + spd_map[2] = 0xff; +} + void mainboard_fill_pei_data(struct pei_data *pei_data) { - pei_data->spd_addresses[0] = 0xff; - pei_data->spd_addresses[2] = 0xff; pei_data->ec_present = 1; pei_data->usb_xhci_on_resume = 1;
diff --git a/src/mainboard/intel/baskingridge/romstage.c b/src/mainboard/intel/baskingridge/romstage.c index 73fc54e..5641775 100644 --- a/src/mainboard/intel/baskingridge/romstage.c +++ b/src/mainboard/intel/baskingridge/romstage.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <stdint.h> #include <northbridge/intel/haswell/haswell.h> #include <northbridge/intel/haswell/raminit.h> #include <southbridge/intel/lynxpoint/pch.h> @@ -40,12 +41,16 @@ RCBA16(D22IR) = DIR_ROUTE(PIRQA, PIRQB, PIRQC, PIRQD); }
+void mb_get_spd_map(uint8_t spd_map[4]) +{ + spd_map[0] = 0xa0; + spd_map[1] = 0xa2; + spd_map[2] = 0xa4; + spd_map[3] = 0xa6; +} + void mainboard_fill_pei_data(struct pei_data *pei_data) { - pei_data->spd_addresses[0] = 0xa0; - pei_data->spd_addresses[1] = 0xa2; - pei_data->spd_addresses[2] = 0xa4; - pei_data->spd_addresses[3] = 0xa6; pei_data->ec_present = 0;
struct usb2_port_setting usb2_ports[MAX_USB2_PORTS] = { diff --git a/src/mainboard/lenovo/t440p/romstage.c b/src/mainboard/lenovo/t440p/romstage.c index 9ce4486..c4baf7f 100644 --- a/src/mainboard/lenovo/t440p/romstage.c +++ b/src/mainboard/lenovo/t440p/romstage.c @@ -40,10 +40,14 @@ } }
+void mb_get_spd_map(uint8_t spd_map[4]) +{ + spd_map[0] = 0xa0; + spd_map[2] = 0xa2; +} + void mainboard_fill_pei_data(struct pei_data *pei_data) { - pei_data->spd_addresses[0] = 0xa0; - pei_data->spd_addresses[2] = 0xa2; pei_data->ec_present = 1; pei_data->gbe_enable = 1;
diff --git a/src/mainboard/supermicro/x10slm-f/romstage.c b/src/mainboard/supermicro/x10slm-f/romstage.c index 26f820a..036a89c 100644 --- a/src/mainboard/supermicro/x10slm-f/romstage.c +++ b/src/mainboard/supermicro/x10slm-f/romstage.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <stdint.h> #include <northbridge/intel/haswell/haswell.h> #include <northbridge/intel/haswell/raminit.h> #include <southbridge/intel/lynxpoint/pch.h> @@ -16,12 +17,16 @@ RCBA16(D20IR) = DIR_ROUTE(PIRQA, PIRQB, PIRQC, PIRQD); }
+void mb_get_spd_map(uint8_t spd_map[4]) +{ + spd_map[0] = 0xa0; + spd_map[1] = 0xa2; + spd_map[2] = 0xa4; + spd_map[3] = 0xa6; +} + void mainboard_fill_pei_data(struct pei_data *pei_data) { - pei_data->spd_addresses[0] = 0xa0; - pei_data->spd_addresses[1] = 0xa2; - pei_data->spd_addresses[2] = 0xa4; - pei_data->spd_addresses[3] = 0xa6; pei_data->ec_present = 0;
struct usb2_port_setting usb2_ports[MAX_USB2_PORTS] = { diff --git a/src/northbridge/intel/haswell/raminit.h b/src/northbridge/intel/haswell/raminit.h index 140ea20..adf53ea 100644 --- a/src/northbridge/intel/haswell/raminit.h +++ b/src/northbridge/intel/haswell/raminit.h @@ -3,11 +3,15 @@ #ifndef RAMINIT_H #define RAMINIT_H
+#include <stdint.h> #include "pei_data.h"
/* Optional function to copy SPD data for on-board memory */ void copy_spd(struct pei_data *peid);
+/* Mainboard callback to fill in the SPD addresses in MRC format */ +void mb_get_spd_map(uint8_t spd_map[4]); + /* Necessary function to initialize pei_data with mainboard-specific settings */ void mainboard_fill_pei_data(struct pei_data *pei_data);
diff --git a/src/northbridge/intel/haswell/romstage.c b/src/northbridge/intel/haswell/romstage.c index 57041e0..014306d 100644 --- a/src/northbridge/intel/haswell/romstage.c +++ b/src/northbridge/intel/haswell/romstage.c @@ -87,6 +87,9 @@ /* MRC has hardcoded assumptions of 2 meaning S3 wake. Normalize it here. */ pei_data.boot_mode = wake_from_s3 ? 2 : 0;
+ /* Obtain the SPD addresses from mainboard code */ + mb_get_spd_map(pei_data.spd_addresses); + /* Calculate unimplemented DIMM slots for each channel */ pei_data.dimm_channel0_disabled = make_channel_disabled_mask(&pei_data, 0); pei_data.dimm_channel1_disabled = make_channel_disabled_mask(&pei_data, 1);
Hello Tristan Corrick, Alexander Couzens, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43127
to look at the new patch set (#2).
Change subject: haswell: Add function to retrieve SPD addresses ......................................................................
haswell: Add function to retrieve SPD addresses
And use it instead of directly writing to the MRC struct.
Change-Id: I7f04db29a08512c1a8b2b2300dba71cb3b84a5c5 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/asrock/b85m_pro4/romstage.c M src/mainboard/asrock/h81m-hds/romstage.c M src/mainboard/google/beltino/romstage.c M src/mainboard/google/slippy/romstage.c M src/mainboard/intel/baskingridge/romstage.c M src/mainboard/lenovo/t440p/romstage.c M src/mainboard/supermicro/x10slm-f/romstage.c M src/northbridge/intel/haswell/raminit.h M src/northbridge/intel/haswell/romstage.c 9 files changed, 61 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/27/43127/2
Tristan Corrick has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43127 )
Change subject: haswell: Add function to retrieve SPD addresses ......................................................................
Patch Set 2: Code-Review+2
I'm not too keen on the argument being `uint8_t [4]`, since it will decay to `uint8_t *`. I know this is mirroring existing patterns in the codebase, though.
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43127 )
Change subject: haswell: Add function to retrieve SPD addresses ......................................................................
haswell: Add function to retrieve SPD addresses
And use it instead of directly writing to the MRC struct.
Change-Id: I7f04db29a08512c1a8b2b2300dba71cb3b84a5c5 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/43127 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Tristan Corrick tristan@corrick.kiwi --- M src/mainboard/asrock/b85m_pro4/romstage.c M src/mainboard/asrock/h81m-hds/romstage.c M src/mainboard/google/beltino/romstage.c M src/mainboard/google/slippy/romstage.c M src/mainboard/intel/baskingridge/romstage.c M src/mainboard/lenovo/t440p/romstage.c M src/mainboard/supermicro/x10slm-f/romstage.c M src/northbridge/intel/haswell/raminit.h M src/northbridge/intel/haswell/romstage.c 9 files changed, 61 insertions(+), 20 deletions(-)
Approvals: build bot (Jenkins): Verified Tristan Corrick: Looks good to me, approved
diff --git a/src/mainboard/asrock/b85m_pro4/romstage.c b/src/mainboard/asrock/b85m_pro4/romstage.c index f625824..6a71abb 100644 --- a/src/mainboard/asrock/b85m_pro4/romstage.c +++ b/src/mainboard/asrock/b85m_pro4/romstage.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <stdint.h> #include <northbridge/intel/haswell/haswell.h> #include <northbridge/intel/haswell/raminit.h> #include <southbridge/intel/lynxpoint/pch.h> @@ -16,12 +17,16 @@ RCBA16(D20IR) = DIR_ROUTE(PIRQA, PIRQB, PIRQC, PIRQD); }
+void mb_get_spd_map(uint8_t spd_map[4]) +{ + spd_map[0] = 0xa0; + spd_map[1] = 0xa2; + spd_map[2] = 0xa4; + spd_map[3] = 0xa6; +} + void mainboard_fill_pei_data(struct pei_data *pei_data) { - pei_data->spd_addresses[0] = 0xa0; - pei_data->spd_addresses[1] = 0xa2; - pei_data->spd_addresses[2] = 0xa4; - pei_data->spd_addresses[3] = 0xa6; pei_data->ec_present = 0; pei_data->gbe_enable = 1;
diff --git a/src/mainboard/asrock/h81m-hds/romstage.c b/src/mainboard/asrock/h81m-hds/romstage.c index cfefdda..0573b6b 100644 --- a/src/mainboard/asrock/h81m-hds/romstage.c +++ b/src/mainboard/asrock/h81m-hds/romstage.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <stdint.h> #include <northbridge/intel/haswell/haswell.h> #include <northbridge/intel/haswell/raminit.h> #include <southbridge/intel/lynxpoint/pch.h> @@ -16,10 +17,14 @@ RCBA16(D20IR) = DIR_ROUTE(PIRQA, PIRQB, PIRQC, PIRQD); }
+void mb_get_spd_map(uint8_t spd_map[4]) +{ + spd_map[0] = 0xa0; + spd_map[2] = 0xa4; +} + void mainboard_fill_pei_data(struct pei_data *pei_data) { - pei_data->spd_addresses[0] = 0xa0; - pei_data->spd_addresses[2] = 0xa4; pei_data->ec_present = 0;
struct usb2_port_setting usb2_ports[MAX_USB2_PORTS] = { diff --git a/src/mainboard/google/beltino/romstage.c b/src/mainboard/google/beltino/romstage.c index 839cd6c..8410d04 100644 --- a/src/mainboard/google/beltino/romstage.c +++ b/src/mainboard/google/beltino/romstage.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <stdint.h> #include <northbridge/intel/haswell/haswell.h> #include <northbridge/intel/haswell/raminit.h> #include <southbridge/intel/lynxpoint/pch.h> @@ -39,10 +40,14 @@ RCBA16(D23IR) = DIR_ROUTE(PIRQH, PIRQH, PIRQH, PIRQH); /* SDIO */ }
+void mb_get_spd_map(uint8_t spd_map[4]) +{ + spd_map[0] = 0xa0; + spd_map[2] = 0xa4; +} + void mainboard_fill_pei_data(struct pei_data *pei_data) { - pei_data->spd_addresses[0] = 0xa0; - pei_data->spd_addresses[2] = 0xa4; pei_data->ec_present = 0; pei_data->dq_pins_interleaved = 1; pei_data->usb_xhci_on_resume = 1; diff --git a/src/mainboard/google/slippy/romstage.c b/src/mainboard/google/slippy/romstage.c index e571944..47bcb47 100644 --- a/src/mainboard/google/slippy/romstage.c +++ b/src/mainboard/google/slippy/romstage.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <stdint.h> #include <northbridge/intel/haswell/haswell.h> #include <northbridge/intel/haswell/raminit.h> #include <southbridge/intel/lynxpoint/pch.h> @@ -40,10 +41,14 @@ RCBA16(D23IR) = DIR_ROUTE(PIRQH, PIRQH, PIRQH, PIRQH); /* SDIO */ }
+void mb_get_spd_map(uint8_t spd_map[4]) +{ + spd_map[0] = 0xff; + spd_map[2] = 0xff; +} + void mainboard_fill_pei_data(struct pei_data *pei_data) { - pei_data->spd_addresses[0] = 0xff; - pei_data->spd_addresses[2] = 0xff; pei_data->ec_present = 1; pei_data->usb_xhci_on_resume = 1;
diff --git a/src/mainboard/intel/baskingridge/romstage.c b/src/mainboard/intel/baskingridge/romstage.c index 73fc54e..5641775 100644 --- a/src/mainboard/intel/baskingridge/romstage.c +++ b/src/mainboard/intel/baskingridge/romstage.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <stdint.h> #include <northbridge/intel/haswell/haswell.h> #include <northbridge/intel/haswell/raminit.h> #include <southbridge/intel/lynxpoint/pch.h> @@ -40,12 +41,16 @@ RCBA16(D22IR) = DIR_ROUTE(PIRQA, PIRQB, PIRQC, PIRQD); }
+void mb_get_spd_map(uint8_t spd_map[4]) +{ + spd_map[0] = 0xa0; + spd_map[1] = 0xa2; + spd_map[2] = 0xa4; + spd_map[3] = 0xa6; +} + void mainboard_fill_pei_data(struct pei_data *pei_data) { - pei_data->spd_addresses[0] = 0xa0; - pei_data->spd_addresses[1] = 0xa2; - pei_data->spd_addresses[2] = 0xa4; - pei_data->spd_addresses[3] = 0xa6; pei_data->ec_present = 0;
struct usb2_port_setting usb2_ports[MAX_USB2_PORTS] = { diff --git a/src/mainboard/lenovo/t440p/romstage.c b/src/mainboard/lenovo/t440p/romstage.c index 3678760..e83530a 100644 --- a/src/mainboard/lenovo/t440p/romstage.c +++ b/src/mainboard/lenovo/t440p/romstage.c @@ -40,10 +40,14 @@ } }
+void mb_get_spd_map(uint8_t spd_map[4]) +{ + spd_map[0] = 0xa0; + spd_map[2] = 0xa2; +} + void mainboard_fill_pei_data(struct pei_data *pei_data) { - pei_data->spd_addresses[0] = 0xa0; - pei_data->spd_addresses[2] = 0xa2; pei_data->ec_present = 1; pei_data->gbe_enable = 1;
diff --git a/src/mainboard/supermicro/x10slm-f/romstage.c b/src/mainboard/supermicro/x10slm-f/romstage.c index 26f820a..036a89c 100644 --- a/src/mainboard/supermicro/x10slm-f/romstage.c +++ b/src/mainboard/supermicro/x10slm-f/romstage.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */
+#include <stdint.h> #include <northbridge/intel/haswell/haswell.h> #include <northbridge/intel/haswell/raminit.h> #include <southbridge/intel/lynxpoint/pch.h> @@ -16,12 +17,16 @@ RCBA16(D20IR) = DIR_ROUTE(PIRQA, PIRQB, PIRQC, PIRQD); }
+void mb_get_spd_map(uint8_t spd_map[4]) +{ + spd_map[0] = 0xa0; + spd_map[1] = 0xa2; + spd_map[2] = 0xa4; + spd_map[3] = 0xa6; +} + void mainboard_fill_pei_data(struct pei_data *pei_data) { - pei_data->spd_addresses[0] = 0xa0; - pei_data->spd_addresses[1] = 0xa2; - pei_data->spd_addresses[2] = 0xa4; - pei_data->spd_addresses[3] = 0xa6; pei_data->ec_present = 0;
struct usb2_port_setting usb2_ports[MAX_USB2_PORTS] = { diff --git a/src/northbridge/intel/haswell/raminit.h b/src/northbridge/intel/haswell/raminit.h index 140ea20..adf53ea 100644 --- a/src/northbridge/intel/haswell/raminit.h +++ b/src/northbridge/intel/haswell/raminit.h @@ -3,11 +3,15 @@ #ifndef RAMINIT_H #define RAMINIT_H
+#include <stdint.h> #include "pei_data.h"
/* Optional function to copy SPD data for on-board memory */ void copy_spd(struct pei_data *peid);
+/* Mainboard callback to fill in the SPD addresses in MRC format */ +void mb_get_spd_map(uint8_t spd_map[4]); + /* Necessary function to initialize pei_data with mainboard-specific settings */ void mainboard_fill_pei_data(struct pei_data *pei_data);
diff --git a/src/northbridge/intel/haswell/romstage.c b/src/northbridge/intel/haswell/romstage.c index 40b7b87..2961299 100644 --- a/src/northbridge/intel/haswell/romstage.c +++ b/src/northbridge/intel/haswell/romstage.c @@ -87,6 +87,9 @@ /* MRC has hardcoded assumptions of 2 meaning S3 wake. Normalize it here. */ pei_data.boot_mode = wake_from_s3 ? 2 : 0;
+ /* Obtain the SPD addresses from mainboard code */ + mb_get_spd_map(pei_data.spd_addresses); + /* Calculate unimplemented DIMM slots for each channel */ pei_data.dimm_channel0_disabled = make_channel_disabled_mask(&pei_data, 0); pei_data.dimm_channel1_disabled = make_channel_disabled_mask(&pei_data, 1);