Change in coreboot[master]: Treewide: Add '-Wduplicated-branches' warning option

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/39561 )

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
3 files changed, 7 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/1

diff --git a/Makefile.inc b/Makefile.inc
index 43b29c7..f5395ce 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -421,7 +421,7 @@
 CFLAGS_common += -nostdlib -Wall -Wundef -Wstrict-prototypes -Wmissing-prototypes
 CFLAGS_common += -Wwrite-strings -Wredundant-decls -Wno-trigraphs -Wimplicit-fallthrough
 CFLAGS_common += -Wstrict-aliasing -Wshadow -Wdate-time -Wtype-limits -Wvla
-CFLAGS_common += -Wlogical-op -Wduplicated-cond -Wdangling-else
+CFLAGS_common += -Wlogical-op -Wduplicated-cond -Wduplicated-branches -Wdangling-else
 CFLAGS_common += -fno-common -ffreestanding -fno-builtin -fomit-frame-pointer
 CFLAGS_common += -ffunction-sections -fdata-sections -fno-pie
 ifeq ($(CONFIG_COMPILER_GCC),y)
diff --git a/src/device/dram/ddr2.c b/src/device/dram/ddr2.c
index 5319806..9c5cc4f 100644
--- a/src/device/dram/ddr2.c
+++ b/src/device/dram/ddr2.c
@@ -522,10 +522,9 @@
    dimm->ranksize_mb = 128 * reg8;
    /* Module density */
    dimm->size_mb = dimm->ranksize_mb * dimm->ranks;
-	if (dimm->size_mb < 1024)
-		printram("  Capacity           : %u MB\n", dimm->size_mb);
-	else
-		printram("  Capacity           : %u GB\n", dimm->size_mb >> 10);
+	printram("  Capacity           : ", (dimm->size_mb < 1024) ?
+		 "%u MB\n", dimm->size_mb :
+		 "%u GB\n", dimm->size_mb >> 10);

/* SDRAM Maximum Cycle Time (tCKmax) */
    if (spd_decode_bcd_time(&dimm->tCK, spd[43]) != CB_SUCCESS) {
diff --git a/src/device/dram/ddr3.c b/src/device/dram/ddr3.c
index bef3c78..f3615b6 100644
--- a/src/device/dram/ddr3.c
+++ b/src/device/dram/ddr3.c
@@ -162,10 +162,9 @@
    	printram("  Invalid module capacity\n");
    	ret = SPD_STATUS_INVALID_FIELD;
    }
-	if (capacity_shift < 0x02) {
-		printram("  Capacity           : %u Mb\n", 256 << capacity_shift);
-	} else {
-		printram("  Capacity           : %u Gb\n", 1 << (capacity_shift - 2));
+	printram("  Capacity           : ", (capacity_shift < 0x02) ?
+		 "%u Mb\n", 256 << capacity_shift :
+		 "%u Gb\n", 1 << (capacity_shift - 2));
    }

reg8 = spd[5];

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#2).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
3 files changed, 7 insertions(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/2

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#3).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/cpu/x86/smm/smm_module_loader.c
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
M src/northbridge/intel/sandybridge/gma.c
5 files changed, 17 insertions(+), 24 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/3

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#4).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/cpu/x86/smm/smm_module_loader.c
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
M src/northbridge/intel/sandybridge/gma.c
5 files changed, 17 insertions(+), 24 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/4

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39561 )

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39561/4/src/device/dram/ddr3.c 
File src/device/dram/ddr3.c:

https://review.coreboot.org/c/coreboot/+/39561/4/src/device/dram/ddr3.c@165 
PS4, Line 165: 	printram("  Capacity           : ", (capacity_shift < 0x02) ?
Is this required by the '-Wduplicated-branches' check? It might fail only because printram defaults to nothing

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#5).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/5

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39561 )

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/39561/4/src/device/dram/ddr3.c 
File src/device/dram/ddr3.c:

https://review.coreboot.org/c/coreboot/+/39561/4/src/device/dram/ddr3.c@165 
PS4, Line 165: 	printram("  Capacity           : ", (capacity_shift < 0x02) ?
> Is this required by the '-Wduplicated-branches' check? It might fail only because printram defaults  […]

yes

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#6).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
3 files changed, 8 insertions(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/6

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#7).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
3 files changed, 8 insertions(+), 10 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/7

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#8).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/cpu/x86/smm/smm_module_loader.c
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
4 files changed, 10 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/8

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#9).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/cpu/x86/smm/smm_module_loader.c
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
M src/soc/mediatek/mt8173/dramc_pi_calibration_api.c
5 files changed, 12 insertions(+), 23 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/9

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Julius Werner, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#10).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/cpu/x86/smm/smm_module_loader.c
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
M src/northbridge/intel/sandybridge/gma.c
M src/soc/mediatek/mt8173/dramc_pi_calibration_api.c
6 files changed, 16 insertions(+), 27 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/10

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Julius Werner, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#11).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/cpu/x86/smm/smm_module_loader.c
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
M src/northbridge/intel/sandybridge/gma.c
M src/soc/mediatek/mt8173/dramc_pi_calibration_api.c
M src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbInitTN/GfxIntegratedInfoTableTN.c
M src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c
8 files changed, 19 insertions(+), 30 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/11

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Julius Werner, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#12).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/cpu/x86/smm/smm_module_loader.c
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
M src/northbridge/intel/pineview/raminit.c
M src/northbridge/intel/sandybridge/gma.c
M src/soc/mediatek/mt8173/dramc_pi_calibration_api.c
M src/soc/nvidia/tegra124/sor.c
M src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbInitTN/GfxIntegratedInfoTableTN.c
M src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c
10 files changed, 49 insertions(+), 59 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/12

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Damien Zammit, Patrick Georgi, Martin Roth, Julius Werner, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#13).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/cpu/x86/smm/smm_module_loader.c
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
M src/northbridge/intel/pineview/raminit.c
M src/northbridge/intel/sandybridge/gma.c
M src/soc/mediatek/mt8173/dramc_pi_calibration_api.c
M src/soc/nvidia/tegra124/sor.c
M src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbInitTN/GfxIntegratedInfoTableTN.c
M src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c
M src/vendorcode/cavium/bdk/libdram/dram-spd.c
11 files changed, 56 insertions(+), 61 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/13

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Damien Zammit, Patrick Georgi, Martin Roth, Julius Werner, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#14).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/cpu/x86/smm/smm_module_loader.c
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
M src/northbridge/intel/pineview/raminit.c
M src/northbridge/intel/sandybridge/gma.c
M src/soc/mediatek/mt8173/dramc_pi_calibration_api.c
M src/soc/nvidia/tegra124/sor.c
M src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbInitTN/GfxIntegratedInfoTableTN.c
M src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c
M src/vendorcode/cavium/bdk/libdram/dram-spd.c
11 files changed, 56 insertions(+), 61 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/14

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Damien Zammit, Patrick Georgi, Martin Roth, Julius Werner, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#15).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/cpu/x86/smm/smm_module_loader.c
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
M src/northbridge/intel/pineview/raminit.c
M src/northbridge/intel/sandybridge/gma.c
M src/soc/mediatek/mt8173/dramc_pi_calibration_api.c
M src/soc/nvidia/tegra124/sor.c
M src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbInitTN/GfxIntegratedInfoTableTN.c
M src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c
M src/vendorcode/cavium/bdk/libdram/dram-spd.c
11 files changed, 54 insertions(+), 61 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/15

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Damien Zammit, Patrick Georgi, Martin Roth, Julius Werner, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#16).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/cpu/x86/smm/smm_module_loader.c
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
M src/northbridge/intel/pineview/raminit.c
M src/northbridge/intel/sandybridge/gma.c
M src/soc/mediatek/mt8173/dramc_pi_calibration_api.c
M src/soc/nvidia/tegra124/sor.c
M src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbInitTN/GfxIntegratedInfoTableTN.c
M src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c
M src/vendorcode/cavium/bdk/libdram/dram-spd.c
11 files changed, 57 insertions(+), 64 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/16

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Damien Zammit, Patrick Georgi, Martin Roth, Julius Werner, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#17).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/cpu/x86/smm/smm_module_loader.c
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
M src/northbridge/intel/pineview/raminit.c
M src/northbridge/intel/sandybridge/gma.c
M src/soc/mediatek/mt8173/dramc_pi_calibration_api.c
M src/soc/nvidia/tegra124/sor.c
M src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbInitTN/GfxIntegratedInfoTableTN.c
M src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c
M src/vendorcode/amd/agesa/f16kb/Proc/GNB/Modules/GnbGfxConfig/GfxConfigLib.c
M src/vendorcode/cavium/bdk/libdram/dram-spd.c
12 files changed, 58 insertions(+), 65 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/17

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Damien Zammit, Patrick Georgi, Martin Roth, Julius Werner, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#18).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/cpu/x86/smm/smm_module_loader.c
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
M src/northbridge/intel/pineview/raminit.c
M src/northbridge/intel/sandybridge/gma.c
M src/soc/mediatek/mt8173/dramc_pi_calibration_api.c
M src/soc/nvidia/tegra124/sor.c
M src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbInitTN/GfxIntegratedInfoTableTN.c
M src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c
M src/vendorcode/amd/agesa/f16kb/Proc/GNB/Modules/GnbGfxConfig/GfxConfigLib.c
M src/vendorcode/cavium/bdk/libdram/dram-spd.c
12 files changed, 59 insertions(+), 66 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/18

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Damien Zammit, Patrick Georgi, Martin Roth, Julius Werner, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#19).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/cpu/x86/smm/smm_module_loader.c
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
M src/northbridge/intel/pineview/raminit.c
M src/northbridge/intel/sandybridge/gma.c
M src/soc/mediatek/mt8173/dramc_pi_calibration_api.c
M src/soc/nvidia/tegra124/sor.c
M src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbInitTN/GfxIntegratedInfoTableTN.c
M src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c
M src/vendorcode/amd/agesa/f16kb/Proc/GNB/Modules/GnbGfxConfig/GfxConfigLib.c
M src/vendorcode/cavium/bdk/libdram/dram-spd.c
12 files changed, 63 insertions(+), 70 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/19

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Damien Zammit, Patrick Georgi, Martin Roth, Julius Werner, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#20).

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Treewide: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
M src/cpu/x86/smm/smm_module_loader.c
M src/device/dram/ddr2.c
M src/device/dram/ddr3.c
M src/northbridge/intel/pineview/raminit.c
M src/northbridge/intel/sandybridge/gma.c
M src/soc/mediatek/mt8173/dramc_pi_calibration_api.c
M src/soc/nvidia/tegra124/sor.c
M src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbInitTN/GfxIntegratedInfoTableTN.c
M src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c
M src/vendorcode/amd/agesa/f16kb/Proc/GNB/Modules/GnbGfxConfig/GfxConfigLib.c
M src/vendorcode/cavium/bdk/libdram/dram-spd.c
12 files changed, 63 insertions(+), 70 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/20

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39561 )

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Patch Set 20: Code-Review-1

(2 comments)

I feel that this is a bad idea...

https://review.coreboot.org/c/coreboot/+/39561/20/src/vendorcode/amd/agesa/f... 
File src/vendorcode/amd/agesa/f16kb/Proc/Fch/Sata/Family/Yangtze/YangtzeSataService.c:

https://review.coreboot.org/c/coreboot/+/39561/20/src/vendorcode/amd/agesa/f... 
PS20, Line 371:       AndMaskDword |= BIT10;
So now we have duplicated code, does GCC warn on that?

https://review.coreboot.org/c/coreboot/+/39561/20/src/vendorcode/amd/agesa/f... 
PS20, Line 435:       RwMem ((Bar5 + 0x0F8), AccessWidth32, 0x00FFFFFF, 0x00);
Same here

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39561 )

Change subject: Treewide: Add '-Wduplicated-branches' warning option
......................................................................

Patch Set 20:

(2 comments)

Please, put the fixes in separate commits, as some of your fixes introduced errors.

Angel, the warning found some valid errors.

```
    CC         ramstage/northbridge/intel/sandybridge/gma.o
src/northbridge/intel/sandybridge/gma.c: In function 'gma_pm_init_pre_vbios':
src/northbridge/intel/sandybridge/gma.c:473:5: error: this condition has identical branches [-Werror=duplicated-branches]
  if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_IVB) {
     ^
```

```
        /* 11a: Enable Render Standby (RC6) */
        if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_IVB) {
                /*
                 * IvyBridge should also support DeepRenderStandby.
                 *
                 * Unfortunately it does not work reliably on all SKUs so
                 * disable it here and it can be enabled by the kernel.
                 */
                gtt_write(0xa090, 0x88040000); /* HW RC Control */
        } else {
                gtt_write(0xa090, 0x88040000); /* HW RC Control */
        }
```

Commit da83a5f1 (Fixes to enable RC6 on IvyBridge) [1].

[1]: https://review.coreboot.org/1268
     I69d005ba56be8c7684a4ea1133a1d761f7c07acc

https://review.coreboot.org/c/coreboot/+/39561/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/39561/1//COMMIT_MSG@7 
PS1, Line 7: Treewide
src

https://review.coreboot.org/c/coreboot/+/39561/20/src/vendorcode/amd/agesa/f... 
File src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbInitTN/GfxIntegratedInfoTableTN.c:

https://review.coreboot.org/c/coreboot/+/39561/20/src/vendorcode/amd/agesa/f... 
PS20, Line 188:     ((Channel == 0) ? D18F2x94_dct0_ADDRESS : D18F2x94_dct1_ADDRESS),
What is the warning here? The code looks correct?

Change in coreboot[master]: src: Add '-Wduplicated-branches' warning option

[HAOUAS Elyes (Code Review)]

Hello build bot (Jenkins), Damien Zammit, Patrick Georgi, Martin Roth, Angel Pons, Julius Werner, Patrick Rudolph,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#21).

Change subject: src: Add '-Wduplicated-branches' warning option
......................................................................

src: Add '-Wduplicated-branches' warning option

Change-Id: Ie433f32a423ce32ea2bd5ae09a1718341cd2b672
Signed-off-by: Elyes HAOUAS 
---
M Makefile.inc
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/39561/21

Change in coreboot[master]: src: Add '-Wduplicated-branches' warning option

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39561 )

Change subject: src: Add '-Wduplicated-branches' warning option
......................................................................

Patch Set 21:

vendorcode is a mess!

Is there a way to use '-Wduplicated-branches' warning option on coreboot tree but not on vendorcode?

Thank you

Change in coreboot[master]: src: Add '-Wduplicated-branches' warning option

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39561 )

Change subject: src: Add '-Wduplicated-branches' warning option
......................................................................

Patch Set 22:

(1 comment)

Such warning options can produce a lot of noise, e.g. false
positives. Unless there was a serious bug in the past that
would have been discovered by this, I'd prefer _not_ to use
this option.

https://review.coreboot.org/c/coreboot/+/39561/20/src/vendorcode/amd/agesa/f... 
File src/vendorcode/amd/agesa/f15tn/Proc/GNB/Modules/GnbInitTN/GfxIntegratedInfoTableTN.c:

https://review.coreboot.org/c/coreboot/+/39561/20/src/vendorcode/amd/agesa/f... 
PS20, Line 188:     ((Channel == 0) ? D18F2x94_dct0_ADDRESS : D18F2x94_dct1_ADDRESS),
> What is the warning here? The code looks correct?

Maybe both types are just the same?

Change in coreboot[master]: src: Add '-Wduplicated-branches' warning option

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/39561 )

Change subject: src: Add '-Wduplicated-branches' warning option
......................................................................

Abandoned

Change in coreboot[master]: src: Add '-Wduplicated-branches' warning option

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39561 )

Change subject: src: Add '-Wduplicated-branches' warning option
......................................................................

Patch Set 23:
> Patch Set 22:
>
> (1 comment)
>
> Such warning options can produce a lot of noise, e.g. false
> positives. Unless there was a serious bug in the past that
> would have been discovered by this, I'd prefer _not_ to use
> this option.

The nvidia/tegra124 code (https://review.coreboot.org/c/coreboot/+/40430) looks like a real bug.

If the non-vendorcode code would build with that option, I believe it’s a good thing, as developers can often reorganize the code (helping other developers to understand it better).

Change in coreboot[master]: src: Add '-Wduplicated-branches' warning option

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39561 )

Change subject: src: Add '-Wduplicated-branches' warning option
......................................................................

Patch Set 23:
> Patch Set 23:
> > Patch Set 22:
> >
> > (1 comment)
> >
> > Such warning options can produce a lot of noise, e.g. false
> > positives. Unless there was a serious bug in the past that
> > would have been discovered by this, I'd prefer _not_ to use
> > this option.
>
> The nvidia/tegra124 code (https://review.coreboot.org/c/coreboot/+/40430) looks like a real bug.
>
> If the non-vendorcode code would build with that option, I believe it’s a good thing, as developers can often reorganize the code (helping other developers to understand it better).

This warning was added and not enabled because it results in too many false positives. And since we use -Werror, a false positive means a build failure. Refer to: https://patchwork.ozlabs.org/project/gcc/patch/20161019110707.GJ2576@redhat....

I don't want this option, as it can and will result in stuff like this: https://github.com/fmtlib/fmt/commit/bef89db6e7d10499a91affca7a933f264c7956d...

Change in coreboot[master]: src: Add '-Wduplicated-branches' warning option

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39561 )

Change subject: src: Add '-Wduplicated-branches' warning option
......................................................................

Patch Set 23:
> > Such warning options can produce a lot of noise, e.g. false
> > positives. Unless there was a serious bug in the past that
> > would have been discovered by this, I'd prefer _not_ to use
> > this option.
>
> The nvidia/tegra124 code (https://review.coreboot.org/c/coreboot/+/40430) looks like a real bug.

Thanks, that's a real bug indeed. I wonder if there is no
simpler warning option for no-op statements (the branches
are the same in this case because both do nothing).
> If the non-vendorcode code would build with that option, I believe it’s a good thing, as developers can often reorganize the code (helping other developers to understand it better).

The problem is to get it to build without making it harder to read.