Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39622 )
Change subject: nb/intel/sandybridge: Deduplicate report_memory_config ......................................................................
nb/intel/sandybridge: Deduplicate report_memory_config
Use the version from native raminit, as it takes the reference clock into account.
Change-Id: I00e979bec236167d22561e3eb44b30b4a34ad663 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/sandybridge/Makefile.inc M src/northbridge/intel/sandybridge/raminit.c M src/northbridge/intel/sandybridge/raminit_mrc.c A src/northbridge/intel/sandybridge/raminit_shared.c M src/northbridge/intel/sandybridge/sandybridge.h 5 files changed, 70 insertions(+), 97 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/39622/1
diff --git a/src/northbridge/intel/sandybridge/Makefile.inc b/src/northbridge/intel/sandybridge/Makefile.inc index 5fb9fdb..7718bf9 100644 --- a/src/northbridge/intel/sandybridge/Makefile.inc +++ b/src/northbridge/intel/sandybridge/Makefile.inc @@ -29,6 +29,7 @@ romstage-y += common.c smm-y += common.c
+romstage-y += raminit_shared.c ifeq ($(CONFIG_USE_NATIVE_RAMINIT),y) romstage-y += early_dmi.c romstage-y += raminit.c diff --git a/src/northbridge/intel/sandybridge/raminit.c b/src/northbridge/intel/sandybridge/raminit.c index ca78eb3..34fb499 100644 --- a/src/northbridge/intel/sandybridge/raminit.c +++ b/src/northbridge/intel/sandybridge/raminit.c @@ -38,13 +38,6 @@ /* FIXME: no ECC support */ /* FIXME: no support for 3-channel chipsets */
-static const char *ecc_decoder[] = { - "inactive", - "active on IO", - "disabled on IO", - "active", -}; - static void wait_txt_clear(void) { struct cpuid_result cp = cpuid_ext(1, 0); @@ -90,49 +83,6 @@ } }
-#define ON_OFF(val) (((val) & 1) ? "on" : "off") - -/* Print the memory controller configuration as read from the memory controller registers. */ -static void report_memory_config(void) -{ - u32 addr_decoder_common, addr_decode_ch[NUM_CHANNELS]; - int i; - - addr_decoder_common = MCHBAR32(MAD_CHNL); - addr_decode_ch[0] = MCHBAR32(MAD_DIMM_CH0); - addr_decode_ch[1] = MCHBAR32(MAD_DIMM_CH1); - - const int refclk = MCHBAR32(MC_BIOS_REQ) & 0x100 ? 100 : 133; - - printk(BIOS_DEBUG, "memcfg DDR3 ref clock %d MHz\n", refclk); - printk(BIOS_DEBUG, "memcfg DDR3 clock %d MHz\n", - (MCHBAR32(MC_BIOS_DATA) * refclk * 100 * 2 + 50) / 100); - - printk(BIOS_DEBUG, "memcfg channel assignment: A: %d, B % d, C % d\n", - (addr_decoder_common >> 0) & 3, - (addr_decoder_common >> 2) & 3, - (addr_decoder_common >> 4) & 3); - - for (i = 0; i < ARRAY_SIZE(addr_decode_ch); i++) { - u32 ch_conf = addr_decode_ch[i]; - printk(BIOS_DEBUG, "memcfg channel[%d] config (%8.8x):\n", i, ch_conf); - printk(BIOS_DEBUG, " ECC %s\n", ecc_decoder[(ch_conf >> 24) & 3]); - printk(BIOS_DEBUG, " enhanced interleave mode %s\n", ON_OFF(ch_conf >> 22)); - printk(BIOS_DEBUG, " rank interleave %s\n", ON_OFF(ch_conf >> 21)); - printk(BIOS_DEBUG, " DIMMA %d MB width x%d %s rank%s\n", - ((ch_conf >> 0) & 0xff) * 256, - ((ch_conf >> 19) & 1) ? 16 : 8, - ((ch_conf >> 17) & 1) ? "dual" : "single", - ((ch_conf >> 16) & 1) ? "" : ", selected"); - printk(BIOS_DEBUG, " DIMMB %d MB width x%d %s rank%s\n", - ((ch_conf >> 8) & 0xff) * 256, - ((ch_conf >> 20) & 1) ? 16 : 8, - ((ch_conf >> 18) & 1) ? "dual" : "single", - ((ch_conf >> 16) & 1) ? ", selected" : ""); - } -} -#undef ON_OFF - /* Return CRC16 match for all SPDs */ static int verify_crc16_spds_ddr3(spd_raw_data *spd, ramctr_timing *ctrl) { diff --git a/src/northbridge/intel/sandybridge/raminit_mrc.c b/src/northbridge/intel/sandybridge/raminit_mrc.c index 5b4b46c..6d00afa 100644 --- a/src/northbridge/intel/sandybridge/raminit_mrc.c +++ b/src/northbridge/intel/sandybridge/raminit_mrc.c @@ -126,53 +126,6 @@ pei_data->mrc_input_len); }
-static const char *ecc_decoder[] = { - "inactive", - "active on IO", - "disabled on IO", - "active", -}; - -#define ON_OFF(val) (((val) & 1) ? "on" : "off") - -/* Print the memory controller configuration as read from the memory controller registers. */ -static void report_memory_config(void) -{ - u32 addr_decoder_common, addr_decode_ch[2]; - int i; - - addr_decoder_common = MCHBAR32(MAD_CHNL); - addr_decode_ch[0] = MCHBAR32(MAD_DIMM_CH0); - addr_decode_ch[1] = MCHBAR32(MAD_DIMM_CH1); - - printk(BIOS_DEBUG, "memcfg DDR3 clock %d MHz\n", - (MCHBAR32(MC_BIOS_DATA) * 13333 * 2 + 50) / 100); - - printk(BIOS_DEBUG, "memcfg channel assignment: A: %d, B % d, C % d\n", - (addr_decoder_common >> 0) & 3, - (addr_decoder_common >> 2) & 3, - (addr_decoder_common >> 4) & 3); - - for (i = 0; i < ARRAY_SIZE(addr_decode_ch); i++) { - u32 ch_conf = addr_decode_ch[i]; - printk(BIOS_DEBUG, "memcfg channel[%d] config (%8.8x):\n", i, ch_conf); - printk(BIOS_DEBUG, " ECC %s\n", ecc_decoder[(ch_conf >> 24) & 3]); - printk(BIOS_DEBUG, " enhanced interleave mode %s\n", ON_OFF(ch_conf >> 22)); - printk(BIOS_DEBUG, " rank interleave %s\n", ON_OFF(ch_conf >> 21)); - printk(BIOS_DEBUG, " DIMMA %d MB width x%d %s rank%s\n", - ((ch_conf >> 0) & 0xff) * 256, - ((ch_conf >> 19) & 1) ? 16 : 8, - ((ch_conf >> 17) & 1) ? "dual" : "single", - ((ch_conf >> 16) & 1) ? "" : ", selected"); - printk(BIOS_DEBUG, " DIMMB %d MB width x%d %s rank%s\n", - ((ch_conf >> 8) & 0xff) * 256, - ((ch_conf >> 20) & 1) ? 16 : 8, - ((ch_conf >> 18) & 1) ? "dual" : "single", - ((ch_conf >> 16) & 1) ? ", selected" : ""); - } -} -#undef ON_OFF - /** * Find PEI executable in coreboot filesystem and execute it. * diff --git a/src/northbridge/intel/sandybridge/raminit_shared.c b/src/northbridge/intel/sandybridge/raminit_shared.c new file mode 100644 index 0000000..6162544 --- /dev/null +++ b/src/northbridge/intel/sandybridge/raminit_shared.c @@ -0,0 +1,68 @@ +/* + * 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. + */ + +#include <console/console.h> +#include <device/mmio.h> +#include <types.h> + +#include "sandybridge.h" + +static const char *const ecc_decoder[] = { + "inactive", + "active on IO", + "disabled on IO", + "active", +}; + +#define ON_OFF(val) (((val) & 1) ? "on" : "off") + +/* Print the memory controller configuration as read from the memory controller registers. */ +void report_memory_config(void) +{ + u32 addr_decoder_common, addr_decode_ch[2]; + int i; + + addr_decoder_common = MCHBAR32(MAD_CHNL); + addr_decode_ch[0] = MCHBAR32(MAD_DIMM_CH0); + addr_decode_ch[1] = MCHBAR32(MAD_DIMM_CH1); + + const int refclk = MCHBAR32(MC_BIOS_REQ) & 0x100 ? 100 : 133; + + printk(BIOS_DEBUG, "memcfg DDR3 ref clock %d MHz\n", refclk); + printk(BIOS_DEBUG, "memcfg DDR3 clock %d MHz\n", + (MCHBAR32(MC_BIOS_DATA) * refclk * 100 * 2 + 50) / 100); + + printk(BIOS_DEBUG, "memcfg channel assignment: A: %d, B % d, C % d\n", + (addr_decoder_common >> 0) & 3, + (addr_decoder_common >> 2) & 3, + (addr_decoder_common >> 4) & 3); + + for (i = 0; i < ARRAY_SIZE(addr_decode_ch); i++) { + u32 ch_conf = addr_decode_ch[i]; + printk(BIOS_DEBUG, "memcfg channel[%d] config (%8.8x):\n", i, ch_conf); + printk(BIOS_DEBUG, " ECC %s\n", ecc_decoder[(ch_conf >> 24) & 3]); + printk(BIOS_DEBUG, " enhanced interleave mode %s\n", ON_OFF(ch_conf >> 22)); + printk(BIOS_DEBUG, " rank interleave %s\n", ON_OFF(ch_conf >> 21)); + printk(BIOS_DEBUG, " DIMMA %d MB width x%d %s rank%s\n", + ((ch_conf >> 0) & 0xff) * 256, + ((ch_conf >> 19) & 1) ? 16 : 8, + ((ch_conf >> 17) & 1) ? "dual" : "single", + ((ch_conf >> 16) & 1) ? "" : ", selected"); + printk(BIOS_DEBUG, " DIMMB %d MB width x%d %s rank%s\n", + ((ch_conf >> 8) & 0xff) * 256, + ((ch_conf >> 20) & 1) ? 16 : 8, + ((ch_conf >> 18) & 1) ? "dual" : "single", + ((ch_conf >> 16) & 1) ? ", selected" : ""); + } +} +#undef ON_OFF diff --git a/src/northbridge/intel/sandybridge/sandybridge.h b/src/northbridge/intel/sandybridge/sandybridge.h index 07d7904..8fb72cc 100644 --- a/src/northbridge/intel/sandybridge/sandybridge.h +++ b/src/northbridge/intel/sandybridge/sandybridge.h @@ -236,6 +236,7 @@ void mainboard_early_init(int s3resume); int mainboard_should_reset_usb(int s3resume); void perform_raminit(int s3resume); +void report_memory_config(void); enum platform_type get_platform_type(void);
#include <device/device.h>
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39622 )
Change subject: nb/intel/sandybridge: Deduplicate report_memory_config ......................................................................
Patch Set 2: Code-Review+2
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/39622 )
Change subject: nb/intel/sandybridge: Deduplicate report_memory_config ......................................................................
nb/intel/sandybridge: Deduplicate report_memory_config
Use the version from native raminit, as it takes the reference clock into account.
Change-Id: I00e979bec236167d22561e3eb44b30b4a34ad663 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/39622 Reviewed-by: Arthur Heymans arthur@aheymans.xyz Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/northbridge/intel/sandybridge/Makefile.inc M src/northbridge/intel/sandybridge/raminit.c M src/northbridge/intel/sandybridge/raminit_mrc.c A src/northbridge/intel/sandybridge/raminit_shared.c M src/northbridge/intel/sandybridge/sandybridge.h 5 files changed, 70 insertions(+), 97 deletions(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved
diff --git a/src/northbridge/intel/sandybridge/Makefile.inc b/src/northbridge/intel/sandybridge/Makefile.inc index 5fb9fdb..7718bf9 100644 --- a/src/northbridge/intel/sandybridge/Makefile.inc +++ b/src/northbridge/intel/sandybridge/Makefile.inc @@ -29,6 +29,7 @@ romstage-y += common.c smm-y += common.c
+romstage-y += raminit_shared.c ifeq ($(CONFIG_USE_NATIVE_RAMINIT),y) romstage-y += early_dmi.c romstage-y += raminit.c diff --git a/src/northbridge/intel/sandybridge/raminit.c b/src/northbridge/intel/sandybridge/raminit.c index ca78eb3..34fb499 100644 --- a/src/northbridge/intel/sandybridge/raminit.c +++ b/src/northbridge/intel/sandybridge/raminit.c @@ -38,13 +38,6 @@ /* FIXME: no ECC support */ /* FIXME: no support for 3-channel chipsets */
-static const char *ecc_decoder[] = { - "inactive", - "active on IO", - "disabled on IO", - "active", -}; - static void wait_txt_clear(void) { struct cpuid_result cp = cpuid_ext(1, 0); @@ -90,49 +83,6 @@ } }
-#define ON_OFF(val) (((val) & 1) ? "on" : "off") - -/* Print the memory controller configuration as read from the memory controller registers. */ -static void report_memory_config(void) -{ - u32 addr_decoder_common, addr_decode_ch[NUM_CHANNELS]; - int i; - - addr_decoder_common = MCHBAR32(MAD_CHNL); - addr_decode_ch[0] = MCHBAR32(MAD_DIMM_CH0); - addr_decode_ch[1] = MCHBAR32(MAD_DIMM_CH1); - - const int refclk = MCHBAR32(MC_BIOS_REQ) & 0x100 ? 100 : 133; - - printk(BIOS_DEBUG, "memcfg DDR3 ref clock %d MHz\n", refclk); - printk(BIOS_DEBUG, "memcfg DDR3 clock %d MHz\n", - (MCHBAR32(MC_BIOS_DATA) * refclk * 100 * 2 + 50) / 100); - - printk(BIOS_DEBUG, "memcfg channel assignment: A: %d, B % d, C % d\n", - (addr_decoder_common >> 0) & 3, - (addr_decoder_common >> 2) & 3, - (addr_decoder_common >> 4) & 3); - - for (i = 0; i < ARRAY_SIZE(addr_decode_ch); i++) { - u32 ch_conf = addr_decode_ch[i]; - printk(BIOS_DEBUG, "memcfg channel[%d] config (%8.8x):\n", i, ch_conf); - printk(BIOS_DEBUG, " ECC %s\n", ecc_decoder[(ch_conf >> 24) & 3]); - printk(BIOS_DEBUG, " enhanced interleave mode %s\n", ON_OFF(ch_conf >> 22)); - printk(BIOS_DEBUG, " rank interleave %s\n", ON_OFF(ch_conf >> 21)); - printk(BIOS_DEBUG, " DIMMA %d MB width x%d %s rank%s\n", - ((ch_conf >> 0) & 0xff) * 256, - ((ch_conf >> 19) & 1) ? 16 : 8, - ((ch_conf >> 17) & 1) ? "dual" : "single", - ((ch_conf >> 16) & 1) ? "" : ", selected"); - printk(BIOS_DEBUG, " DIMMB %d MB width x%d %s rank%s\n", - ((ch_conf >> 8) & 0xff) * 256, - ((ch_conf >> 20) & 1) ? 16 : 8, - ((ch_conf >> 18) & 1) ? "dual" : "single", - ((ch_conf >> 16) & 1) ? ", selected" : ""); - } -} -#undef ON_OFF - /* Return CRC16 match for all SPDs */ static int verify_crc16_spds_ddr3(spd_raw_data *spd, ramctr_timing *ctrl) { diff --git a/src/northbridge/intel/sandybridge/raminit_mrc.c b/src/northbridge/intel/sandybridge/raminit_mrc.c index 5b4b46c..6d00afa 100644 --- a/src/northbridge/intel/sandybridge/raminit_mrc.c +++ b/src/northbridge/intel/sandybridge/raminit_mrc.c @@ -126,53 +126,6 @@ pei_data->mrc_input_len); }
-static const char *ecc_decoder[] = { - "inactive", - "active on IO", - "disabled on IO", - "active", -}; - -#define ON_OFF(val) (((val) & 1) ? "on" : "off") - -/* Print the memory controller configuration as read from the memory controller registers. */ -static void report_memory_config(void) -{ - u32 addr_decoder_common, addr_decode_ch[2]; - int i; - - addr_decoder_common = MCHBAR32(MAD_CHNL); - addr_decode_ch[0] = MCHBAR32(MAD_DIMM_CH0); - addr_decode_ch[1] = MCHBAR32(MAD_DIMM_CH1); - - printk(BIOS_DEBUG, "memcfg DDR3 clock %d MHz\n", - (MCHBAR32(MC_BIOS_DATA) * 13333 * 2 + 50) / 100); - - printk(BIOS_DEBUG, "memcfg channel assignment: A: %d, B % d, C % d\n", - (addr_decoder_common >> 0) & 3, - (addr_decoder_common >> 2) & 3, - (addr_decoder_common >> 4) & 3); - - for (i = 0; i < ARRAY_SIZE(addr_decode_ch); i++) { - u32 ch_conf = addr_decode_ch[i]; - printk(BIOS_DEBUG, "memcfg channel[%d] config (%8.8x):\n", i, ch_conf); - printk(BIOS_DEBUG, " ECC %s\n", ecc_decoder[(ch_conf >> 24) & 3]); - printk(BIOS_DEBUG, " enhanced interleave mode %s\n", ON_OFF(ch_conf >> 22)); - printk(BIOS_DEBUG, " rank interleave %s\n", ON_OFF(ch_conf >> 21)); - printk(BIOS_DEBUG, " DIMMA %d MB width x%d %s rank%s\n", - ((ch_conf >> 0) & 0xff) * 256, - ((ch_conf >> 19) & 1) ? 16 : 8, - ((ch_conf >> 17) & 1) ? "dual" : "single", - ((ch_conf >> 16) & 1) ? "" : ", selected"); - printk(BIOS_DEBUG, " DIMMB %d MB width x%d %s rank%s\n", - ((ch_conf >> 8) & 0xff) * 256, - ((ch_conf >> 20) & 1) ? 16 : 8, - ((ch_conf >> 18) & 1) ? "dual" : "single", - ((ch_conf >> 16) & 1) ? ", selected" : ""); - } -} -#undef ON_OFF - /** * Find PEI executable in coreboot filesystem and execute it. * diff --git a/src/northbridge/intel/sandybridge/raminit_shared.c b/src/northbridge/intel/sandybridge/raminit_shared.c new file mode 100644 index 0000000..6162544 --- /dev/null +++ b/src/northbridge/intel/sandybridge/raminit_shared.c @@ -0,0 +1,68 @@ +/* + * 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. + */ + +#include <console/console.h> +#include <device/mmio.h> +#include <types.h> + +#include "sandybridge.h" + +static const char *const ecc_decoder[] = { + "inactive", + "active on IO", + "disabled on IO", + "active", +}; + +#define ON_OFF(val) (((val) & 1) ? "on" : "off") + +/* Print the memory controller configuration as read from the memory controller registers. */ +void report_memory_config(void) +{ + u32 addr_decoder_common, addr_decode_ch[2]; + int i; + + addr_decoder_common = MCHBAR32(MAD_CHNL); + addr_decode_ch[0] = MCHBAR32(MAD_DIMM_CH0); + addr_decode_ch[1] = MCHBAR32(MAD_DIMM_CH1); + + const int refclk = MCHBAR32(MC_BIOS_REQ) & 0x100 ? 100 : 133; + + printk(BIOS_DEBUG, "memcfg DDR3 ref clock %d MHz\n", refclk); + printk(BIOS_DEBUG, "memcfg DDR3 clock %d MHz\n", + (MCHBAR32(MC_BIOS_DATA) * refclk * 100 * 2 + 50) / 100); + + printk(BIOS_DEBUG, "memcfg channel assignment: A: %d, B % d, C % d\n", + (addr_decoder_common >> 0) & 3, + (addr_decoder_common >> 2) & 3, + (addr_decoder_common >> 4) & 3); + + for (i = 0; i < ARRAY_SIZE(addr_decode_ch); i++) { + u32 ch_conf = addr_decode_ch[i]; + printk(BIOS_DEBUG, "memcfg channel[%d] config (%8.8x):\n", i, ch_conf); + printk(BIOS_DEBUG, " ECC %s\n", ecc_decoder[(ch_conf >> 24) & 3]); + printk(BIOS_DEBUG, " enhanced interleave mode %s\n", ON_OFF(ch_conf >> 22)); + printk(BIOS_DEBUG, " rank interleave %s\n", ON_OFF(ch_conf >> 21)); + printk(BIOS_DEBUG, " DIMMA %d MB width x%d %s rank%s\n", + ((ch_conf >> 0) & 0xff) * 256, + ((ch_conf >> 19) & 1) ? 16 : 8, + ((ch_conf >> 17) & 1) ? "dual" : "single", + ((ch_conf >> 16) & 1) ? "" : ", selected"); + printk(BIOS_DEBUG, " DIMMB %d MB width x%d %s rank%s\n", + ((ch_conf >> 8) & 0xff) * 256, + ((ch_conf >> 20) & 1) ? 16 : 8, + ((ch_conf >> 18) & 1) ? "dual" : "single", + ((ch_conf >> 16) & 1) ? ", selected" : ""); + } +} +#undef ON_OFF diff --git a/src/northbridge/intel/sandybridge/sandybridge.h b/src/northbridge/intel/sandybridge/sandybridge.h index 07d7904..8fb72cc 100644 --- a/src/northbridge/intel/sandybridge/sandybridge.h +++ b/src/northbridge/intel/sandybridge/sandybridge.h @@ -236,6 +236,7 @@ void mainboard_early_init(int s3resume); int mainboard_should_reset_usb(int s3resume); void perform_raminit(int s3resume); +void report_memory_config(void); enum platform_type get_platform_type(void);
#include <device/device.h>
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Felix Held (Code Review)