Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48613 )
Change subject: nb/intel/sandybridge: Refactor ODT stretch table code ......................................................................
nb/intel/sandybridge: Refactor ODT stretch table code
Leverage existing `ch_dimms` value and use constants for brevity.
Change-Id: I4e08166c8e9fbd15ff1dcd266abb0689e4b159f7 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/sandybridge/raminit.c 1 file changed, 5 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/48613/1
diff --git a/src/northbridge/intel/sandybridge/raminit.c b/src/northbridge/intel/sandybridge/raminit.c index 17772c6..3077c0a 100644 --- a/src/northbridge/intel/sandybridge/raminit.c +++ b/src/northbridge/intel/sandybridge/raminit.c @@ -233,10 +233,11 @@ printk(BIOS_DEBUG, "channel[%d] rankmap = 0x%x\n", channel, ctrl->rankmap[channel]); } - if ((ctrl->rankmap[channel] & 0x03) && (ctrl->rankmap[channel] & 0x0c) - && ctrl->info.dimm[channel][0].reference_card <= 5 - && ctrl->info.dimm[channel][1].reference_card <= 5) {
+ const u8 rc_0 = ctrl->info.dimm[channel][0].reference_card; + const u8 rc_1 = ctrl->info.dimm[channel][1].reference_card; + + if (ch_dimms == NUM_SLOTS && rc_0 < 6 && rc_1 < 6) { const int ref_card_offset_table[6][6] = { { 0, 0, 0, 0, 2, 2 }, { 0, 0, 0, 0, 2, 2 }, @@ -245,9 +246,7 @@ { 2, 2, 2, 1, 0, 0 }, { 2, 2, 2, 1, 0, 0 }, }; - ctrl->ref_card_offset[channel] = ref_card_offset_table - [ctrl->info.dimm[channel][0].reference_card] - [ctrl->info.dimm[channel][1].reference_card]; + ctrl->ref_card_offset[channel] = ref_card_offset_table[rc_0][rc_1]; } else { ctrl->ref_card_offset[channel] = 0; }
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48613 )
Change subject: nb/intel/sandybridge: Refactor ODT stretch table code ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/48613/2/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/raminit.c:
https://review.coreboot.org/c/coreboot/+/48613/2/src/northbridge/intel/sandy... PS2, Line 240: NUM_SLOTS out of scope for this patch, but the NUM_SLOTS define in the header file could really use a comment that that is per channel and not per socket/system; found that a bit confusing and had to look that up
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48613 )
Change subject: nb/intel/sandybridge: Refactor ODT stretch table code ......................................................................
nb/intel/sandybridge: Refactor ODT stretch table code
Leverage existing `ch_dimms` value and use constants for brevity.
Change-Id: I4e08166c8e9fbd15ff1dcd266abb0689e4b159f7 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/48613 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Felix Held felix-coreboot@felixheld.de --- M src/northbridge/intel/sandybridge/raminit.c 1 file changed, 5 insertions(+), 6 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved
diff --git a/src/northbridge/intel/sandybridge/raminit.c b/src/northbridge/intel/sandybridge/raminit.c index 17772c6..3077c0a 100644 --- a/src/northbridge/intel/sandybridge/raminit.c +++ b/src/northbridge/intel/sandybridge/raminit.c @@ -233,10 +233,11 @@ printk(BIOS_DEBUG, "channel[%d] rankmap = 0x%x\n", channel, ctrl->rankmap[channel]); } - if ((ctrl->rankmap[channel] & 0x03) && (ctrl->rankmap[channel] & 0x0c) - && ctrl->info.dimm[channel][0].reference_card <= 5 - && ctrl->info.dimm[channel][1].reference_card <= 5) {
+ const u8 rc_0 = ctrl->info.dimm[channel][0].reference_card; + const u8 rc_1 = ctrl->info.dimm[channel][1].reference_card; + + if (ch_dimms == NUM_SLOTS && rc_0 < 6 && rc_1 < 6) { const int ref_card_offset_table[6][6] = { { 0, 0, 0, 0, 2, 2 }, { 0, 0, 0, 0, 2, 2 }, @@ -245,9 +246,7 @@ { 2, 2, 2, 1, 0, 0 }, { 2, 2, 2, 1, 0, 0 }, }; - ctrl->ref_card_offset[channel] = ref_card_offset_table - [ctrl->info.dimm[channel][0].reference_card] - [ctrl->info.dimm[channel][1].reference_card]; + ctrl->ref_card_offset[channel] = ref_card_offset_table[rc_0][rc_1]; } else { ctrl->ref_card_offset[channel] = 0; }
-
Angel Pons (Code Review) -
Felix Held (Code Review)