[coreboot-gerrit] Change in coreboot[master]: nb/intel/sandybridge: Clarify command timing calculation

[Angel Pons (Code Review)]

Attention is currently required from: Nico Huber.
Hello Nico Huber,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/49320

to review the following change.

Change subject: nb/intel/sandybridge: Clarify command timing calculation
......................................................................

nb/intel/sandybridge: Clarify command timing calculation

Command timing is the absolute value of the most negative `pi_coding`
value across all ranks, or zero if there are no negative values. Use the
MAX() macro to ease proving that `cmd_delay` can never be negative, and
then drop the always-false underflow check.

The variable type for `cmd_delay` still needs to be signed because of
the comparisons with `pi_coding`, which is a signed value. Using an
unsigned type would result in undefined and also undesired behavior.

Change-Id: I714d3cf57d0f62376a1107af63bcd761f952bc3a
Signed-off-by: Angel Pons 
---
M src/northbridge/intel/sandybridge/raminit_common.c
1 file changed, 3 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/49320/1

diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c
index f766867..cef0131 100644
--- a/src/northbridge/intel/sandybridge/raminit_common.c
+++ b/src/northbridge/intel/sandybridge/raminit_common.c
@@ -938,16 +938,11 @@
    u32 clk_logic_dly = 0;

/*
-	 * Apply command delay if desired setting is negative. Find the
-	 * most negative value: 'cmd_delay' will be the absolute value.
+	 * Compute command timing as abs() of the most negative PI code
+	 * across all ranks. Use zero if none of the values is negative.
     */
    FOR_ALL_POPULATED_RANKS {
-		if (cmd_delay < -ctrl->timings[channel][slotrank].pi_coding)
-			cmd_delay = -ctrl->timings[channel][slotrank].pi_coding;
-	}
-	if (cmd_delay < 0) {
-		printk(BIOS_ERR, "C%d command delay underflow: %d\n", channel, cmd_delay);
-		cmd_delay = 0;
+		cmd_delay = MAX(cmd_delay, -ctrl->timings[channel][slotrank].pi_coding);
    }
    if (cmd_delay > CCC_MAX_PI) {
    	printk(BIOS_ERR, "C%d command delay overflow: %d\n", channel, cmd_delay);