HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32938
Change subject: nb/intel/sandybridge: Remove variable set but not used ......................................................................
nb/intel/sandybridge: Remove variable set but not used
Change-Id: I75f5d821e018932d3f10d84b7ebed362777fb17d Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/sandybridge/raminit_common.c 1 file changed, 6 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/32938/1
diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index 5347c5c..900633a 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -1452,9 +1452,8 @@ int lane;
FOR_ALL_LANES { - volatile u32 tmp; MCHBAR32(0x4340 + 0x400 * channel + 4 * lane) = 0; - tmp = MCHBAR32(0x4140 + 0x400 * channel + 4 * lane); + MCHBAR32(0x4140 + 0x400 * channel + 4 * lane); }
wait_428c(channel); @@ -1997,9 +1996,8 @@ MCHBAR32_OR(0x5030, 8);
FOR_ALL_POPULATED_CHANNELS { - volatile u32 tmp; MCHBAR32_AND(0x4020 + 0x400 * channel, ~0x00200000); - tmp = MCHBAR32(0x428c + 0x400 * channel); + MCHBAR32(0x428c + 0x400 * channel); wait_428c(channel);
/* DRAM command ZQCS */ @@ -2344,9 +2342,8 @@ program_timings(ctrl, channel);
FOR_ALL_LANES { - volatile u32 tmp; MCHBAR32(0x4340 + 0x400 * channel + 4 * lane) = 0; - tmp = MCHBAR32(0x400 * channel + 4 * lane + 0x4140); + MCHBAR32(0x400 * channel + 4 * lane + 0x4140); }
wait_428c(channel); @@ -2425,8 +2422,7 @@ fill_pattern0(ctrl, channel, 0, 0); MCHBAR32(0x4288 + (channel << 10)) = 0; FOR_ALL_LANES { - volatile u32 tmp; - tmp = MCHBAR32(0x400 * channel + lane * 4 + 0x4140); + MCHBAR32(0x400 * channel + lane * 4 + 0x4140); }
FOR_ALL_POPULATED_RANKS FOR_ALL_LANES { @@ -2626,10 +2622,9 @@ program_timings(ctrl, channel);
FOR_ALL_LANES { - volatile u32 tmp; MCHBAR32(0x4340 + 0x400 * channel + 4 * lane) = 0; - tmp = MCHBAR32(0x400 * channel + + MCHBAR32(0x400 * channel + 4 * lane + 0x4140); } wait_428c(channel); @@ -2674,8 +2669,7 @@
wait_428c(channel); FOR_ALL_LANES { - volatile u32 tmp; - tmp = MCHBAR32(0x4340 + + MCHBAR32(0x4340 + 0x400 * channel + lane * 4); }
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32938 )
Change subject: nb/intel/sandybridge: Remove variable set but not used ......................................................................
Patch Set 1: Code-Review-1
(1 comment)
https://review.coreboot.org/#/c/32938/1/src/northbridge/intel/sandybridge/ra... File src/northbridge/intel/sandybridge/raminit_common.c:
https://review.coreboot.org/#/c/32938/1/src/northbridge/intel/sandybridge/ra... PS1, Line 1455: Those are all MMIO reads, where it may be important to push the result of the read to the stack. The use of a volatile variable seems quite intentional here.
HAOUAS Elyes has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/32938 )
Change subject: nb/intel/sandybridge: Remove variable set but not used ......................................................................
Abandoned
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32938 )
Change subject: nb/intel/sandybridge: Remove variable set but not used ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32938/1/src/northbridge/intel/sandybridge/ra... File src/northbridge/intel/sandybridge/raminit_common.c:
https://review.coreboot.org/#/c/32938/1/src/northbridge/intel/sandybridge/ra... PS1, Line 1455:
Those are all MMIO reads, where it may be important to push the result of the read to the stack. […]
See CB:32948 commentary.
It's certainly important for the MMIO read to happen, but I don't see how storing the value on the stack (cachelines) could have any importance.
HAOUAS Elyes has restored this change. ( https://review.coreboot.org/c/coreboot/+/32938 )
Change subject: nb/intel/sandybridge: Remove variable set but not used ......................................................................
Restored
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32938 )
Change subject: nb/intel/sandybridge: Remove variable set but not used ......................................................................
Patch Set 1: -Code-Review
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32938 )
Change subject: nb/intel/sandybridge: Remove variable set but not used ......................................................................
Patch Set 1: Code-Review+1
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32938 )
Change subject: nb/intel/sandybridge: Remove variable set but not used ......................................................................
Patch Set 3: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32938 )
Change subject: nb/intel/sandybridge: Remove variable set but not used ......................................................................
nb/intel/sandybridge: Remove variable set but not used
Change-Id: I75f5d821e018932d3f10d84b7ebed362777fb17d Signed-off-by: Elyes HAOUAS ehaouas@noos.fr Reviewed-on: https://review.coreboot.org/c/coreboot/+/32938 Reviewed-by: Patrick Georgi pgeorgi@google.com Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/northbridge/intel/sandybridge/raminit_common.c 1 file changed, 6 insertions(+), 12 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved Kyösti Mälkki: Looks good to me, but someone else must approve
diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index 53f28c6..4974173 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -1452,9 +1452,8 @@ int lane;
FOR_ALL_LANES { - volatile u32 tmp; MCHBAR32(0x4340 + 0x400 * channel + 4 * lane) = 0; - tmp = MCHBAR32(0x4140 + 0x400 * channel + 4 * lane); + MCHBAR32(0x4140 + 0x400 * channel + 4 * lane); }
wait_428c(channel); @@ -2026,9 +2025,8 @@ MCHBAR32_OR(0x5030, 8);
FOR_ALL_POPULATED_CHANNELS { - volatile u32 tmp; MCHBAR32_AND(0x4020 + 0x400 * channel, ~0x00200000); - tmp = MCHBAR32(0x428c + 0x400 * channel); + MCHBAR32(0x428c + 0x400 * channel); wait_428c(channel);
/* DRAM command ZQCS */ @@ -2373,9 +2371,8 @@ program_timings(ctrl, channel);
FOR_ALL_LANES { - volatile u32 tmp; MCHBAR32(0x4340 + 0x400 * channel + 4 * lane) = 0; - tmp = MCHBAR32(0x400 * channel + 4 * lane + 0x4140); + MCHBAR32(0x400 * channel + 4 * lane + 0x4140); }
wait_428c(channel); @@ -2454,8 +2451,7 @@ fill_pattern0(ctrl, channel, 0, 0); MCHBAR32(0x4288 + (channel << 10)) = 0; FOR_ALL_LANES { - volatile u32 tmp; - tmp = MCHBAR32(0x400 * channel + lane * 4 + 0x4140); + MCHBAR32(0x400 * channel + lane * 4 + 0x4140); }
FOR_ALL_POPULATED_RANKS FOR_ALL_LANES { @@ -2655,10 +2651,9 @@ program_timings(ctrl, channel);
FOR_ALL_LANES { - volatile u32 tmp; MCHBAR32(0x4340 + 0x400 * channel + 4 * lane) = 0; - tmp = MCHBAR32(0x400 * channel + + MCHBAR32(0x400 * channel + 4 * lane + 0x4140); } wait_428c(channel); @@ -2703,8 +2698,7 @@
wait_428c(channel); FOR_ALL_LANES { - volatile u32 tmp; - tmp = MCHBAR32(0x4340 + + MCHBAR32(0x4340 + 0x400 * channel + lane * 4); }
-
Arthur Heymans (Code Review) -
HAOUAS Elyes (Code Review)
-
Kyösti Mälkki (Code Review)
-
Patrick Georgi (Code Review)