Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42697 )
Change subject: nb/intel/ironlake: Drop copy-pasted and dead code ......................................................................
nb/intel/ironlake: Drop copy-pasted and dead code
This function was copy-pasted, comments included, from Sandy Bridge. However, it is only called with 0x0044 as the northbridge's PCI ID. Therefore, `bridge_silicon_revision() & BASE_REV_MASK` will always evaluate to 0x40, which never equals `BASE_REV_SNB`, that is, 0x00. As the condition is always false, treat this code as dead and drop it.
Following a similar reasoning, all direct comparisons against SNB steppings will always be true, because `bridge_silicon_revision()` returns at least 0x40 which is always larger than either `SNB_STEP_D0` or `SNB_STEP_D1`. So, drop all but the code path that is actually used.
Change-Id: I5219a6af3df98ed77c9c4abfb9a63c2ebf8171bb Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/ironlake/northbridge.c 1 file changed, 3 insertions(+), 29 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/42697/1
diff --git a/src/northbridge/intel/ironlake/northbridge.c b/src/northbridge/intel/ironlake/northbridge.c index 267baf7..6b631b3 100644 --- a/src/northbridge/intel/ironlake/northbridge.c +++ b/src/northbridge/intel/ironlake/northbridge.c @@ -155,39 +155,13 @@ DMIBAR32(0x1c4) = 0xffffffff; DMIBAR32(0x1d0) = 0xffffffff;
- /* Steps prior to DMI ASPM */ - if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_SNB) { - reg32 = DMIBAR32(0x250); - reg32 &= ~((1 << 22) | (1 << 20)); - reg32 |= (1 << 21); - DMIBAR32(0x250) = reg32; - } - reg32 = DMIBAR32(0x238); reg32 |= (1 << 29); DMIBAR32(0x238) = reg32;
- if (bridge_silicon_revision() >= SNB_STEP_D0) { - reg32 = DMIBAR32(0x1f8); - reg32 |= (1 << 16); - DMIBAR32(0x1f8) = reg32; - } else if (bridge_silicon_revision() >= SNB_STEP_D1) { - reg32 = DMIBAR32(0x1f8); - reg32 &= ~(1 << 26); - reg32 |= (1 << 16); - DMIBAR32(0x1f8) = reg32; - - reg32 = DMIBAR32(0x1fc); - reg32 |= (1 << 12) | (1 << 23); - DMIBAR32(0x1fc) = reg32; - } - - /* Enable ASPM on SNB link, should happen before PCH link */ - if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_SNB) { - reg32 = DMIBAR32(0xd04); - reg32 |= (1 << 4); - DMIBAR32(0xd04) = reg32; - } + reg32 = DMIBAR32(0x1f8); + reg32 |= (1 << 16); + DMIBAR32(0x1f8) = reg32;
reg32 = DMIBAR32(0x88); reg32 |= (1 << 1) | (1 << 0);
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42697 )
Change subject: nb/intel/ironlake: Drop copy-pasted and dead code ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42697 )
Change subject: nb/intel/ironlake: Drop copy-pasted and dead code ......................................................................
nb/intel/ironlake: Drop copy-pasted and dead code
This function was copy-pasted, comments included, from Sandy Bridge. However, it is only called with 0x0044 as the northbridge's PCI ID. Therefore, `bridge_silicon_revision() & BASE_REV_MASK` will always evaluate to 0x40, which never equals `BASE_REV_SNB`, that is, 0x00. As the condition is always false, treat this code as dead and drop it.
Following a similar reasoning, all direct comparisons against SNB steppings will always be true, because `bridge_silicon_revision()` returns at least 0x40 which is always larger than either `SNB_STEP_D0` or `SNB_STEP_D1`. So, drop all but the code path that is actually used.
Change-Id: I5219a6af3df98ed77c9c4abfb9a63c2ebf8171bb Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/42697 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Georgi pgeorgi@google.com --- M src/northbridge/intel/ironlake/northbridge.c 1 file changed, 3 insertions(+), 29 deletions(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved
diff --git a/src/northbridge/intel/ironlake/northbridge.c b/src/northbridge/intel/ironlake/northbridge.c index f53b03e..415c142 100644 --- a/src/northbridge/intel/ironlake/northbridge.c +++ b/src/northbridge/intel/ironlake/northbridge.c @@ -161,39 +161,13 @@ DMIBAR32(0x1c4) = 0xffffffff; DMIBAR32(0x1d0) = 0xffffffff;
- /* Steps prior to DMI ASPM */ - if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_SNB) { - reg32 = DMIBAR32(0x250); - reg32 &= ~((1 << 22) | (1 << 20)); - reg32 |= (1 << 21); - DMIBAR32(0x250) = reg32; - } - reg32 = DMIBAR32(0x238); reg32 |= (1 << 29); DMIBAR32(0x238) = reg32;
- if (bridge_silicon_revision() >= SNB_STEP_D0) { - reg32 = DMIBAR32(0x1f8); - reg32 |= (1 << 16); - DMIBAR32(0x1f8) = reg32; - } else if (bridge_silicon_revision() >= SNB_STEP_D1) { - reg32 = DMIBAR32(0x1f8); - reg32 &= ~(1 << 26); - reg32 |= (1 << 16); - DMIBAR32(0x1f8) = reg32; - - reg32 = DMIBAR32(0x1fc); - reg32 |= (1 << 12) | (1 << 23); - DMIBAR32(0x1fc) = reg32; - } - - /* Enable ASPM on SNB link, should happen before PCH link */ - if ((bridge_silicon_revision() & BASE_REV_MASK) == BASE_REV_SNB) { - reg32 = DMIBAR32(0xd04); - reg32 |= (1 << 4); - DMIBAR32(0xd04) = reg32; - } + reg32 = DMIBAR32(0x1f8); + reg32 |= (1 << 16); + DMIBAR32(0x1f8) = reg32;
reg32 = DMIBAR32(0x88); reg32 |= (1 << 1) | (1 << 0);
-
Angel Pons (Code Review) -
Patrick Georgi (Code Review)