[coreboot-gerrit] New patch to review for coreboot: a2d8270 rockchip/rk3288: Fix operator precedence error in LPDDR init

Patrick Georgi (pgeorgi@google.com) gerrit at coreboot.org
Tue Apr 21 15:19:06 CEST 2015


Patrick Georgi (pgeorgi at google.com) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/9911

-gerrit

commit a2d82700a92a5a6e33ab660a980a9e1b5d1ee419
Author: Julius Werner <jwerner at chromium.org>
Date:   Mon Apr 6 13:51:46 2015 -0700

    rockchip/rk3288: Fix operator precedence error in LPDDR init
    
    Upstream coreboot regularly runs Coverity over the code base. Turns out
    that's a good idea since it's really easy to screw yourself over with a
    missing parenthesis and some unfortunately deceptive line breaking.
    
    This patch fixes a bug in LPDDR3 initialization due to an incorrect
    operator precedence assumption ( ?: does not bind stronger than | ). In
    effect, instead of setting MR11[1:0] to 0b11 or 0b00 based on ODT, we're
    unconditionally setting MR0[1:0] to 0b11. Thankfully, MR0[1:0] seems to
    contain read-only bits so this might have not been a problem when ODT is
    off (which is currently true for all LPDDR boards).
    
    Also adding a redundant LPDDR_OP() around the 0 to make the intent
    clearer and changing 3 and 0 to 0x3 and 0x0 to make it more obvious that
    these are bit masks (right?).
    
    BRANCH=veyron
    BUG=None
    TEST=Running reboot loop on a Minnie, looks good so far...
    
    Change-Id: I06464aaa57e693b1973846a5771162244f7a1c57
    Signed-off-by: Patrick Georgi <pgeorgi at chromium.org>
    Found-by: Coverity Scan
    Original-Commit-Id: 5bd9eba39fb7b0f940fead963bbc1878b031b2cb
    Original-Change-Id: I701ce059472078b5de09a45dd31f54b65a51e641
    Original-Signed-off-by: Julius Werner <jwerner at chromium.org>
    Original-Reviewed-on: https://chromium-review.googlesource.com/264135
    Original-Reviewed-by: David Hendricks <dhendrix at chromium.org>
    Original-Reviewed-by: Jinkun Hong <jinkun.hong at rock-chips.com>
    Original-Tested-by: Jinkun Hong <jinkun.hong at rock-chips.com>
---
 src/soc/rockchip/rk3288/sdram.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/soc/rockchip/rk3288/sdram.c b/src/soc/rockchip/rk3288/sdram.c
index e7d33ea..0fe2638 100644
--- a/src/soc/rockchip/rk3288/sdram.c
+++ b/src/soc/rockchip/rk3288/sdram.c
@@ -1048,8 +1048,8 @@ void sdram_init(const struct rk3288_sdram_params *sdram_params)
 			udelay(10);
 			send_command(ddr_pctl_regs,
 				(sdram_params->ch[channel].rank | 1),
-				MRS_CMD, LPDDR2_MA(11) |
-				sdram_params->odt ? LPDDR2_OP(3) : 0);
+				MRS_CMD, LPDDR2_MA(11) | (sdram_params->odt ?
+				LPDDR2_OP(0x3) : LPDDR2_OP(0x0)));
 			if (channel == 0) {
 				write32(&ddr_pctl_regs->mrrcfg0, 0);
 				send_command(ddr_pctl_regs, 1, MRR_CMD,



More information about the coreboot-gerrit mailing list