Change in coreboot[master]: nb/intel/sandybridge: Use proper names to refer to training steps
Hello Felix Singer, Nico Huber, Arthur Heymans, Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/48601
to review the following change.
Change subject: nb/intel/sandybridge: Use proper names to refer to training steps ......................................................................
nb/intel/sandybridge: Use proper names to refer to training steps
Now that the purpose of each training algorithm is clear, replace the last instances of the original names in comments and print statements with the current, correct names. Also, print which channel has failed command training, for completeness and consistency with other errors.
Change-Id: I9cc5c4b04499297825ca004c6bd1648a68449d2c Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/sandybridge/raminit_common.c 1 file changed, 11 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/48601/1
diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index 70a856c..9b364ab 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -1231,7 +1231,7 @@ if (!some_works) { /* Guard against roundtrip latency underflow */ if (ctrl->timings[channel][slotrank].roundtrip_latency < 2) { - printk(BIOS_EMERG, "402x discovery failed (1): %d, %d\n", + printk(BIOS_EMERG, "Roundtrip latency underflow: %d, %d\n", channel, slotrank); return MAKE_ERR; } @@ -1250,7 +1250,7 @@
/* Guard against I/O latency overflow */ if (ctrl->timings[channel][slotrank].io_latency >= 0x10) { - printk(BIOS_EMERG, "402x discovery failed (2): %d, %d\n", + printk(BIOS_EMERG, "I/O latency overflow: %d, %d\n", channel, slotrank); return MAKE_ERR; } @@ -1499,7 +1499,7 @@ struct run rn = get_longest_zero_run(stats[lane], ARRAY_SIZE(stats[lane]));
if (rn.all || rn.length < 8) { - printk(BIOS_EMERG, "tx_dq discovery failed: %d, %d, %d\n", + printk(BIOS_EMERG, "tx_dq write leveling failed: %d, %d, %d\n", channel, slotrank, lane); /* * With command training not being done yet, the lane can be erroneous. @@ -1634,7 +1634,7 @@
ctrl->timings[channel][slotrank].lanes[lane].tx_dqs = rn.start; if (rn.all) { - printk(BIOS_EMERG, "tx_dqs discovery failed: %d, %d, %d\n", + printk(BIOS_EMERG, "JEDEC write leveling failed: %d, %d, %d\n", channel, slotrank, lane);
return MAKE_ERR; @@ -2102,7 +2102,7 @@ /* * Dual DIMM per channel: * Issue: - * While c320c discovery seems to succeed raminit will fail in write training. + * While command training seems to succeed, raminit will fail in write training. * * Workaround: * Skip 1T in dual DIMM mode, that's only supported by a few DIMMs. @@ -2124,7 +2124,7 @@ }
if (err) { - printk(BIOS_EMERG, "c320c discovery failed\n"); + printk(BIOS_EMERG, "Command training failed: %d\n", channel); return err; }
@@ -2176,7 +2176,7 @@ edges[lane] = rn.middle;
if (rn.all) { - printk(BIOS_EMERG, "edge discovery failed: %d, %d, %d\n", channel, + printk(BIOS_EMERG, "Read MPR training failed: %d, %d, %d\n", channel, slotrank, lane); return MAKE_ERR; } @@ -2376,7 +2376,7 @@
edges[lane] = (lower[lane] + upper[lane]) / 2; if (rn.all || (lower[lane] > upper[lane])) { - printk(BIOS_EMERG, "edge write discovery failed: " + printk(BIOS_EMERG, "Aggressive read training failed: " "%d, %d, %d\n", channel, slotrank, lane);
return MAKE_ERR; @@ -2482,7 +2482,7 @@ if (enable_iosav_opt) MCHBAR32(MCMNTS_SPARE) = 1;
- printram("discover tx_dq write:\n"); + printram("Aggresive write training:\n");
for (i = 0; i < ARRAY_SIZE(wr_vref_offsets); i++) { FOR_ALL_POPULATED_CHANNELS { @@ -2520,8 +2520,8 @@
rn = get_longest_zero_run(stats, MAX_TX_DQ + 1); if (rn.all) { - printk(BIOS_EMERG, - "tx_dq write discovery failed: " + printk(BIOS_EMERG, "Aggressive " + "write training failed: " "%d, %d, %d\n", channel, slotrank, lane);
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48601 )
Change subject: nb/intel/sandybridge: Use proper names to refer to training steps ......................................................................
Patch Set 1: Code-Review+2
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48601 )
Change subject: nb/intel/sandybridge: Use proper names to refer to training steps ......................................................................
Patch Set 2: Code-Review+2
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48601 )
Change subject: nb/intel/sandybridge: Use proper names to refer to training steps ......................................................................
nb/intel/sandybridge: Use proper names to refer to training steps
Now that the purpose of each training algorithm is clear, replace the last instances of the original names in comments and print statements with the current, correct names. Also, print which channel has failed command training, for completeness and consistency with other errors.
Change-Id: I9cc5c4b04499297825ca004c6bd1648a68449d2c Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/48601 Reviewed-by: Felix Held felix-coreboot@felixheld.de Reviewed-by: Patrick Rudolph siro@das-labor.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/northbridge/intel/sandybridge/raminit_common.c 1 file changed, 11 insertions(+), 11 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved Patrick Rudolph: Looks good to me, approved
diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index 70a856c..9b364ab 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -1231,7 +1231,7 @@ if (!some_works) { /* Guard against roundtrip latency underflow */ if (ctrl->timings[channel][slotrank].roundtrip_latency < 2) { - printk(BIOS_EMERG, "402x discovery failed (1): %d, %d\n", + printk(BIOS_EMERG, "Roundtrip latency underflow: %d, %d\n", channel, slotrank); return MAKE_ERR; } @@ -1250,7 +1250,7 @@
/* Guard against I/O latency overflow */ if (ctrl->timings[channel][slotrank].io_latency >= 0x10) { - printk(BIOS_EMERG, "402x discovery failed (2): %d, %d\n", + printk(BIOS_EMERG, "I/O latency overflow: %d, %d\n", channel, slotrank); return MAKE_ERR; } @@ -1499,7 +1499,7 @@ struct run rn = get_longest_zero_run(stats[lane], ARRAY_SIZE(stats[lane]));
if (rn.all || rn.length < 8) { - printk(BIOS_EMERG, "tx_dq discovery failed: %d, %d, %d\n", + printk(BIOS_EMERG, "tx_dq write leveling failed: %d, %d, %d\n", channel, slotrank, lane); /* * With command training not being done yet, the lane can be erroneous. @@ -1634,7 +1634,7 @@
ctrl->timings[channel][slotrank].lanes[lane].tx_dqs = rn.start; if (rn.all) { - printk(BIOS_EMERG, "tx_dqs discovery failed: %d, %d, %d\n", + printk(BIOS_EMERG, "JEDEC write leveling failed: %d, %d, %d\n", channel, slotrank, lane);
return MAKE_ERR; @@ -2102,7 +2102,7 @@ /* * Dual DIMM per channel: * Issue: - * While c320c discovery seems to succeed raminit will fail in write training. + * While command training seems to succeed, raminit will fail in write training. * * Workaround: * Skip 1T in dual DIMM mode, that's only supported by a few DIMMs. @@ -2124,7 +2124,7 @@ }
if (err) { - printk(BIOS_EMERG, "c320c discovery failed\n"); + printk(BIOS_EMERG, "Command training failed: %d\n", channel); return err; }
@@ -2176,7 +2176,7 @@ edges[lane] = rn.middle;
if (rn.all) { - printk(BIOS_EMERG, "edge discovery failed: %d, %d, %d\n", channel, + printk(BIOS_EMERG, "Read MPR training failed: %d, %d, %d\n", channel, slotrank, lane); return MAKE_ERR; } @@ -2376,7 +2376,7 @@
edges[lane] = (lower[lane] + upper[lane]) / 2; if (rn.all || (lower[lane] > upper[lane])) { - printk(BIOS_EMERG, "edge write discovery failed: " + printk(BIOS_EMERG, "Aggressive read training failed: " "%d, %d, %d\n", channel, slotrank, lane);
return MAKE_ERR; @@ -2482,7 +2482,7 @@ if (enable_iosav_opt) MCHBAR32(MCMNTS_SPARE) = 1;
- printram("discover tx_dq write:\n"); + printram("Aggresive write training:\n");
for (i = 0; i < ARRAY_SIZE(wr_vref_offsets); i++) { FOR_ALL_POPULATED_CHANNELS { @@ -2520,8 +2520,8 @@
rn = get_longest_zero_run(stats, MAX_TX_DQ + 1); if (rn.all) { - printk(BIOS_EMERG, - "tx_dq write discovery failed: " + printk(BIOS_EMERG, "Aggressive " + "write training failed: " "%d, %d, %d\n", channel, slotrank, lane);
-
Angel Pons (Code Review) -
Felix Held (Code Review)
-
Patrick Rudolph (Code Review)