Attention is currently required from: Nico Huber, Arthur Heymans, Patrick Rudolph. Hello Nico Huber, Arthur Heymans, Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/55043
to review the following change.
Change subject: nb/intel/sandybridge: Wrap DMI recipe with macros ......................................................................
nb/intel/sandybridge: Wrap DMI recipe with macros
To transform the odd-looking dummy-read-then-write sequences into proper read-modify-write sequences (which is what they are supposed to be), add a temporary `DMIBAR32_UPDATE` macro and use it to wrap all the instances of the pattern to be changed. With this method, non-reproducible changes do not change any lines with magic numbers, and should thus be easier to review.
Tested with BUILD_TIMELESS=1, Asus P8Z77-V LX2 remains identical.
Change-Id: Ia608c8204e3c4753c0753963b939fd0fba5b7b0a Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/sandybridge/early_dmi.c 1 file changed, 36 insertions(+), 60 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/55043/1
diff --git a/src/northbridge/intel/sandybridge/early_dmi.c b/src/northbridge/intel/sandybridge/early_dmi.c index 122dea9..cfd0d0b 100644 --- a/src/northbridge/intel/sandybridge/early_dmi.c +++ b/src/northbridge/intel/sandybridge/early_dmi.c @@ -6,6 +6,12 @@ #include <northbridge/intel/sandybridge/sandybridge.h> #include <southbridge/intel/bd82x6x/pch.h>
+#define DMIBAR32_UPDATE(x, val, clear, set) \ + do { \ + dmibar_read32(x); \ + dmibar_write32(x, ((val) & ~(clear)) | (set)); \ + } while (0) + static void dmi_recipe(void) { const u32 cpuid = cpu_get_cpuid(); @@ -36,136 +42,106 @@ }
for (i = 0; i < 2; i++) { - dmibar_read32(0x0904 + (i << 5)); // !!! = 0x7a1842ec - dmibar_write32(0x0904 + (i << 5), (0x7a1842ec & ~(0x3f << 16)) | 0x18 << 16); - dmibar_read32(0x090c + (i << 5)); // !!! = 0x00000208 - dmibar_write32(0x090c + (i << 5), (0x00000208 & ~(0x3f << 5)) | 0x09 << 5); + DMIBAR32_UPDATE(0x0904 + (i << 5), 0x7a1842ec, 0x3f << 16, 0x18 << 16); + DMIBAR32_UPDATE(0x090c + (i << 5), 0x00000208, 0x3f << 5, 0x09 << 5); }
for (i = 0; i < 2; i++) { - dmibar_read32(0x0700 + (i << 5)); // !!! = 0x46139008 - dmibar_write32(0x0700 + (i << 5), (0x46139008 & ~0) | 1 << 15); + DMIBAR32_UPDATE(0x0700 + (i << 5), 0x46139008, 0, 1 << 15); }
- dmibar_read32(0x0c04); // !!! = 0x2e680008 - dmibar_write32(0x0c04, (0x2e680008 & ~(0xff << 21)) | 0x73 << 21); + DMIBAR32_UPDATE(0x0c04, 0x2e680008, 0xff << 21, 0x73 << 21);
for (i = 0; i < 2; i++) { - dmibar_read32(0x0904 + (i << 5)); // !!! = 0x7a1842ec - dmibar_write32(0x0904 + (i << 5), (0x7a1842ec & ~(3 << 29)) | 1 << 29); + DMIBAR32_UPDATE(0x0904 + (i << 5), 0x7a1842ec, 3 << 29, 1 << 29); }
for (i = 0; i < 2; i++) { - dmibar_read32(0x0910 + (i << 5)); // !!! = 0x00006300 - dmibar_write32(0x0910 + (i << 5), (0x00006300 & ~(0xf << 10)) | 0 << 10); + DMIBAR32_UPDATE(0x0910 + (i << 5), 0x00006300, 0xf << 10, 0 << 10); }
for (i = 0; i < 4; i++) { - dmibar_read32(0x0a00 + (i << 4)); // !!! = 0x03042010 - dmibar_write32(0x0a00 + (i << 4), (0x03042010 & ~(0x1f << 1)) | 12 << 1); + DMIBAR32_UPDATE(0x0a00 + (i << 4), 0x03042010, 0x1f << 1, 12 << 1); }
- dmibar_read32(0x0c00); // !!! = 0x29700c08 - dmibar_write32(0x0c00, (0x29700c08 & ~(1 << 4)) | 0 << 4); + DMIBAR32_UPDATE(0x0c00, 0x29700c08, 1 << 4, 0 << 4);
for (i = 0; i < 4; i++) { - dmibar_read32(0x0a04 + (i << 4)); // !!! = 0x0c0708f0 - dmibar_write32(0x0a04 + (i << 4), (0x0c0708f0 & ~(1 << 12)) | 1 << 12); + DMIBAR32_UPDATE(0x0a04 + (i << 4), 0x0c0708f0, 1 << 12, 1 << 12); }
for (i = 0; i < 2; i++) { - dmibar_read32(0x0900 + (i << 5)); // !!! = 0x50000000 - dmibar_write32(0x0900 + (i << 5), (0x50000000 & ~(3 << 26)) | 0 << 26); + DMIBAR32_UPDATE(0x0900 + (i << 5), 0x50000000, 3 << 26, 0 << 26); }
for (i = 0; i < 2; i++) { - dmibar_read32(0x0908 + (i << 5)); // !!! = 0x51ffffff - dmibar_write32(0x0908 + (i << 5), (0x51ffffff & ~(3 << 27)) | 2 << 27); + DMIBAR32_UPDATE(0x0908 + (i << 5), 0x51ffffff, 3 << 27, 2 << 27); }
for (i = 0; i < 4; i++) { - dmibar_read32(0x0a00 + (i << 4)); // !!! = 0x03042018 - dmibar_write32(0x0a00 + (i << 4), (0x03042018 & ~(3 << 17)) | 2 << 17); + DMIBAR32_UPDATE(0x0a00 + (i << 4), 0x03042018, 3 << 17, 2 << 17); }
for (i = 0; i < 2; i++) { - dmibar_read32(0x0700 + (i << 5)); // !!! = 0x46139008 - dmibar_write32(0x0700 + (i << 5), (0x46139008 & ~(1 << 3)) | 1 << 3); + DMIBAR32_UPDATE(0x0700 + (i << 5), 0x46139008, 1 << 3, 1 << 3); }
for (i = 0; i < 2; i++) { - dmibar_read32(0x0904 + (i << 5)); // !!! = 0x3a1842ec - dmibar_write32(0x0904 + (i << 5), (0x3a1842ec & ~(3 << 10)) | 1 << 10); + DMIBAR32_UPDATE(0x0904 + (i << 5), 0x3a1842ec, 3 << 10, 1 << 10); }
for (i = 0; i < 4; i++) { - dmibar_read32(0x0a00 + (i << 4)); // !!! = 0x03042018 - dmibar_write32(0x0a00 + (i << 4), (0x03042018 & ~(3 << 24)) | 3 << 24); + DMIBAR32_UPDATE(0x0a00 + (i << 4), 0x03042018, 3 << 24, 3 << 24); }
for (i = 0; i < 2; i++) { - dmibar_read32(0x0908 + (i << 5)); // !!! = 0x51ffffff - dmibar_write32(0x0908 + (i << 5), (0x51ffffff & ~(3 << 30)) | 1 << 30); + DMIBAR32_UPDATE(0x0908 + (i << 5), 0x51ffffff, 3 << 30, 1 << 30); }
- dmibar_read32(0x0c00); // !!! = 0x29700c08 - dmibar_write32(0x0c00, (0x29700c08 & ~(0x1f << 25)) | 0x14 << 25); + DMIBAR32_UPDATE(0x0c00, 0x29700c08, 0x1f << 25, 0x14 << 25);
- dmibar_read32(0x0c0c); // !!! = 0x16063400 - dmibar_write32(0x0c0c, (0x16063400 & ~(0x3f << 23)) | 0x00 << 23); + DMIBAR32_UPDATE(0x0c0c, 0x16063400, 0x3f << 23, 0x00 << 23);
for (i = 0; i < 2; i++) { - dmibar_read32(0x0700 + (i << 5)); // !!! = 0x46139008 - dmibar_write32(0x0700 + (i << 5), (0x46139008 & ~(1 << 21)) | 1 << 21); + DMIBAR32_UPDATE(0x0700 + (i << 5), 0x46139008, 1 << 21, 1 << 21); }
for (i = 0; i < 2; i++) { - dmibar_read32(0x0700 + (i << 5)); // !!! = 0x46339008 - dmibar_write32(0x0700 + (i << 5), (0x46339008 & ~(3 << 24)) | 1 << 24); + DMIBAR32_UPDATE(0x0700 + (i << 5), 0x46339008, 3 << 24, 1 << 24); }
for (i = 0; i < 2; i++) { - dmibar_read32(0x0700 + (i << 5)); // !!! = 0x45339008 - dmibar_write32(0x0700 + (i << 5), (0x45339008 & ~(1 << 19)) | 1 << 19); + DMIBAR32_UPDATE(0x0700 + (i << 5), 0x45339008, 1 << 19, 1 << 19); }
for (i = 0; i < 2; i++) { - dmibar_read32(0x0700 + (i << 5)); // !!! = 0x453b9008 - dmibar_write32(0x0700 + (i << 5), (0x453b9008 & ~(1 << 23)) | 1 << 23); + DMIBAR32_UPDATE(0x0700 + (i << 5), 0x453b9008, 1 << 23, 1 << 23); }
for (i = 0; i < 2; i++) { - dmibar_read32(0x0700 + (i << 5)); // !!! = 0x45bb9008 - dmibar_write32(0x0700 + (i << 5), (0x45bb9008 & ~(1 << 22)) | 1 << 22); + DMIBAR32_UPDATE(0x0700 + (i << 5), 0x45bb9008, 1 << 22, 1 << 22); }
for (i = 0; i < 2; i++) { - dmibar_read32(0x0914 + (i << 5)); // !!! = 0x9021a080 - dmibar_write32(0x0914 + (i << 5), (0x9021a080 & ~(1 << 9)) | 1 << 9); + DMIBAR32_UPDATE(0x0914 + (i << 5), 0x9021a080, 1 << 9, 1 << 9); }
for (i = 0; i < 2; i++) { - dmibar_read32(0x0914 + (i << 5)); // !!! = 0x9021a280 - dmibar_write32(0x0914 + (i << 5), (0x9021a280 & ~(1 << 27)) | 1 << 27); + DMIBAR32_UPDATE(0x0914 + (i << 5), 0x9021a280, 1 << 27, 1 << 27); }
for (i = 0; i < 4; i++) { - dmibar_read32(0x0a00 + (i << 4)); // !!! = 0x03042018 - dmibar_write32(0x0a00 + (i << 4), (0x03042018 & ~(1 << 21)) | 1 << 21); + DMIBAR32_UPDATE(0x0a00 + (i << 4), 0x03042018, 1 << 21, 1 << 21); }
- dmibar_read32(0x0258); // !!! = 0x40000600 - dmibar_write32(0x0258, (0x40000600 & ~(1 << 29)) | 1 << 29); + DMIBAR32_UPDATE(0x0258, 0x40000600, 1 << 29, 1 << 29);
for (i = 0; i < 2; i++) { - dmibar_read32(0x0904 + (i << 5)); // !!! = 0x3a1846ec - dmibar_write32(0x0904 + (i << 5), (0x3a1846ec & ~(0xf << 25)) | 0x5 << 25); - dmibar_read32(0x0914 + (i << 5)); // !!! = 0x9821a280 - dmibar_write32(0x0914 + (i << 5), (0x9821a280 & ~(0xf << 13)) | 0x0 << 13); + DMIBAR32_UPDATE(0x0904 + (i << 5), 0x3a1846ec, 0xf << 25, 0x5 << 25); + DMIBAR32_UPDATE(0x0914 + (i << 5), 0x9821a280, 0xf << 13, 0x0 << 13); }
- dmibar_read32(DMIL0SLAT); // !!! = 0x00c26460 - dmibar_write32(DMIL0SLAT, (0x00c26460 & ~(0xffff << 0)) | 0x403c << 0); + DMIBAR32_UPDATE(DMIL0SLAT, 0x00c26460, 0xffff << 0, 0x403c << 0); }
void early_init_dmi(void)
-
Angel Pons (Code Review)