[coreboot-gerrit] Change in coreboot[master]: rk3399: sdram: Remove obs_err variable to avoid uninitialized use

[Julius Werner (Code Review)]

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/51079 )

Change subject: rk3399: sdram: Remove obs_err variable to avoid uninitialized use
......................................................................

rk3399: sdram: Remove obs_err variable to avoid uninitialized use

CB:50863 refactored the data_training() function to split out read gate
training into a separate function, but in the course of this forgot to
correctly initialize the local obs_err varible in the new function to 0.
This means that it will be used uninitialized, and when it happens to be
non-zero it makes the training process fail. Due to the convoluted
control flow in the function, it seems that GCC's static analyzer
couldn't pick up on this uninitialized use.

The whole variable is unnecessary anyway, all it's used for is to force
the function to return two lines below without doing anything with
side-effects in between. This patch removes the variable and simplifies
the code in all three training functions to avoid this uninitialized use
issue and make everything a bit more readable. (Also restore the
original pre-clang-format continuation line intendations for more
readability.)

Signed-off-by: Julius Werner 
Change-Id: Ia475d64c06f2ec1bf9295742d173ce66717b821c
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51079
Reviewed-by: Moritz Fischer 
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
M src/soc/rockchip/rk3399/sdram.c
1 file changed, 29 insertions(+), 21 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved
  Moritz Fischer: Looks good to me, but someone else must approve

diff --git a/src/soc/rockchip/rk3399/sdram.c b/src/soc/rockchip/rk3399/sdram.c
index 7a9f0ba..0fdef71 100644
--- a/src/soc/rockchip/rk3399/sdram.c
+++ b/src/soc/rockchip/rk3399/sdram.c
@@ -639,7 +639,7 @@
 {
    u32 *denali_pi = rk3399_ddr_pi[channel]->denali_pi;
    u32 *denali_phy = rk3399_ddr_publ[channel]->denali_phy;
-	u32 obs_0, obs_1, obs_2, obs_err = 0;
+	u32 obs_0, obs_1, obs_2;
    const u32 rank_mask = get_rank_mask(channel, params);
    u32 i, tmp;

@@ -668,13 +668,15 @@
    		obs_0 = read32(&denali_phy[532]);
    		obs_1 = read32(&denali_phy[660]);
    		obs_2 = read32(&denali_phy[788]);
-			if (((obs_0 >> 30) & 0x3) || ((obs_1 >> 30) & 0x3)
-			    || ((obs_2 >> 30) & 0x3))
-				obs_err = 1;
-			if ((((tmp >> 11) & 0x1) == 0x1) && (((tmp >> 13) & 0x1) == 0x1)
-			    && (((tmp >> 5) & 0x1) == 0x0) && (obs_err == 0))
+			if (((obs_0 >> 30) & 0x3) ||
+			    ((obs_1 >> 30) & 0x3) ||
+			    ((obs_2 >> 30) & 0x3))
+				return -1;
+			if ((((tmp >> 11) & 0x1) == 0x1) &&
+			    (((tmp >> 13) & 0x1) == 0x1) &&
+			    (((tmp >> 5) & 0x1) == 0x0))
    			break;
-			else if ((((tmp >> 5) & 0x1) == 0x1) || (obs_err == 1))
+			else if (((tmp >> 5) & 0x1) == 0x1)
    			return -1;
    	}
    	/* clear interrupt,PI_175 PI_INT_ACK:WR:0:17 */
@@ -689,7 +691,7 @@
 {
    u32 *denali_pi = rk3399_ddr_pi[channel]->denali_pi;
    u32 *denali_phy = rk3399_ddr_publ[channel]->denali_phy;
-	u32 obs_0, obs_1, obs_2, obs_3, obs_err = 0;
+	u32 obs_0, obs_1, obs_2, obs_3;
    u32 rank = params->ch[channel].rank;
    u32 i, tmp;

@@ -716,13 +718,16 @@
    		obs_1 = read32(&denali_phy[168]);
    		obs_2 = read32(&denali_phy[296]);
    		obs_3 = read32(&denali_phy[424]);
-			if (((obs_0 >> 12) & 0x1) || ((obs_1 >> 12) & 0x1)
-			    || ((obs_2 >> 12) & 0x1) || ((obs_3 >> 12) & 0x1))
-				obs_err = 1;
-			if ((((tmp >> 10) & 0x1) == 0x1) && (((tmp >> 13) & 0x1) == 0x1)
-			    && (((tmp >> 4) & 0x1) == 0x0) && (obs_err == 0))
+			if (((obs_0 >> 12) & 0x1) ||
+			    ((obs_1 >> 12) & 0x1) ||
+			    ((obs_2 >> 12) & 0x1) ||
+			    ((obs_3 >> 12) & 0x1))
+				return -1;
+			if ((((tmp >> 10) & 0x1) == 0x1) &&
+			    (((tmp >> 13) & 0x1) == 0x1) &&
+			    (((tmp >> 4) & 0x1) == 0x0))
    			break;
-			else if ((((tmp >> 4) & 0x1) == 0x1) || (obs_err == 1))
+			else if (((tmp >> 4) & 0x1) == 0x1)
    			return -1;
    	}
    	/* clear interrupt,PI_175 PI_INT_ACK:WR:0:17 */
@@ -740,7 +745,7 @@
    u32 *denali_pi = rk3399_ddr_pi[channel]->denali_pi;
    u32 *denali_phy = rk3399_ddr_publ[channel]->denali_phy;
    u32 rank = params->ch[channel].rank;
-	u32 obs_0, obs_1, obs_2, obs_3, obs_err;
+	u32 obs_0, obs_1, obs_2, obs_3;
    u32 reg_value = 0;
    u32 i, tmp;

@@ -793,13 +798,16 @@
    		obs_1 = read32(&denali_phy[171]);
    		obs_2 = read32(&denali_phy[299]);
    		obs_3 = read32(&denali_phy[427]);
-			if (((obs_0 >> (16 + 6)) & 0x3) || ((obs_1 >> (16 + 6)) & 0x3)
-			    || ((obs_2 >> (16 + 6)) & 0x3) || ((obs_3 >> (16 + 6)) & 0x3))
-				obs_err = 1;
-			if ((((tmp >> 9) & 0x1) == 0x1) && (((tmp >> 13) & 0x1) == 0x1)
-			    && (((tmp >> 3) & 0x1) == 0x0) && (obs_err == 0))
+			if (((obs_0 >> (16 + 6)) & 0x3) ||
+			    ((obs_1 >> (16 + 6)) & 0x3) ||
+			    ((obs_2 >> (16 + 6)) & 0x3) ||
+			    ((obs_3 >> (16 + 6)) & 0x3))
+				return -1;
+			if ((((tmp >> 9) & 0x1) == 0x1) &&
+			    (((tmp >> 13) & 0x1) == 0x1) &&
+			    (((tmp >> 3) & 0x1) == 0x0))
    			break;
-			else if ((((tmp >> 3) & 0x1) == 0x1) || (obs_err == 1))
+			else if (((tmp >> 3) & 0x1) == 0x1)
    			return -1;
    	}
    	/* clear interrupt,PI_175 PI_INT_ACK:WR:0:17 */