Hello Felix Singer, Nico Huber, Arthur Heymans, Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/48414
to review the following change.
Change subject: nb/intel/sandybridge: Introduce `dmi_update_bundles` function ......................................................................
nb/intel/sandybridge: Introduce `dmi_update_bundles` function
This eliminates redundancy in the DMI recipe function.
Change-Id: I025ccc79ac323d4bb6c0e1552019ec7fbb847d4b Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/sandybridge/early_dmi.c 1 file changed, 36 insertions(+), 56 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/48414/1
diff --git a/src/northbridge/intel/sandybridge/early_dmi.c b/src/northbridge/intel/sandybridge/early_dmi.c index 5a0f098..307f2a3 100644 --- a/src/northbridge/intel/sandybridge/early_dmi.c +++ b/src/northbridge/intel/sandybridge/early_dmi.c @@ -14,6 +14,12 @@ DMIBAR32(offset) = reg32; }
+static void dmi_update_bundles(const u32 offset, const u32 mask, const u32 val, const u8 shift) +{ + for (int i = 0; i < 2; i++) + dmi_update_field(offset + (i << 5), mask, val, shift); +} + static void set_load_bus(const int lane, u32 load_sel, u32 load_data, const u8 stepping) { u32 lbcvalue = stepping == IVB_STEP_A0 ? 0x30000000 : 0x70000000; @@ -37,10 +43,8 @@ return;
/* E1 PWRFSM fix */ - if (stepping == IVB_STEP_E1) { - for (i = 0; i < 2; i++) - dmi_update_field(0x0914 + (i << 5), 1, 1, 31); - } + if (stepping == IVB_STEP_E1) + dmi_update_bundles(0x0914, 1, 1, 31);
/* UneqMM steps */ for (i = 0; i < 4; i++) { @@ -52,10 +56,8 @@ dmi_update_field(0x0c30, 0xf, 4, 28);
/* Disable the 2nd order loop */ - for (i = 0; i < 2; i++) { - dmi_update_field(0x0904 + (i << 5), 7, 0, 22); - dmi_update_field(0x090c + (i << 5), 7, 0, 17); - } + dmi_update_bundles(0x0904, 7, 0, 22); + dmi_update_bundles(0x090c, 7, 0, 17);
if (IS_IVY_CPU(cpuid)) { if (stepping <= IVB_STEP_B0) @@ -68,8 +70,7 @@ value = 0; }
- for (i = 0; i < 2; i++) - dmi_update_field(0x090c + (i << 5), 0xf, value, 21); + dmi_update_bundles(0x090c, 0xf, value, 21);
if (stepping <= IVB_STEP_B0) {
@@ -84,27 +85,21 @@ }
/* setcdrpg (PGACQ and PGTRK) */ - for (i = 0; i < 2; i++) { - dmi_update_field(0x0904 + (i << 5), 0x3f, 0x18, 16); - dmi_update_field(0x090c + (i << 5), 0x3f, 0x09, 5); - } + dmi_update_bundles(0x0904, 0x3f, 0x18, 16); + dmi_update_bundles(0x090c, 0x3f, 0x09, 5);
/* setl0sfixes */ - if (stepping >= IVB_STEP_C0) { - for (i = 0; i < 2; i++) - dmi_update_field(0x0700 + (i << 5), 1, 1, 15); - } + if (stepping >= IVB_STEP_C0) + dmi_update_bundles(0x0700, 1, 1, 15);
/* setrxctlectl */ dmi_update_field(0x0c04, 0xff, 0x73, 21);
/* setdfegainacq */ - for (i = 0; i < 2; i++) - dmi_update_field(0x0904 + (i << 5), 3, 1, 29); + dmi_update_bundles(0x0904, 3, 1, 29);
/* setg2rxctlepeak */ - for (i = 0; i < 2; i++) - dmi_update_field(0x0910 + (i << 5), 0xf, 0, 10); + dmi_update_bundles(0x0910, 0xf, 0, 10);
/* setVrefAdap */ for (i = 0; i < 4; i++) @@ -121,24 +116,20 @@
if (stepping >= IVB_STEP_C0) { /* setdfelsbsel */ - for (i = 0; i < 2; i++) - dmi_update_field(0x0900 + (i << 5), 3, 0, 26); + dmi_update_bundles(0x0900, 3, 0, 26);
/* setagcgainacq */ - for (i = 0; i < 2; i++) - dmi_update_field(0x0908 + (i << 5), 3, 2, 27); + dmi_update_bundles(0x0908, 3, 2, 27);
/* setagcacqlen */ for (i = 0; i < 4; i++) dmi_update_field(0x0a00 + (i << 4), 3, 2, 17);
/* setsampleratspeedcal */ - for (i = 0; i < 2; i++) - dmi_update_field(0x0700 + (i << 5), 1, 1, 3); + dmi_update_bundles(0x0700, 1, 1, 3);
/* setoffcorgain */ - for (i = 0; i < 2; i++) - dmi_update_field(0x0904 + (i << 5), 3, 1, 10); + dmi_update_bundles(0x0904, 3, 1, 10);
/* setdfesumctl */ for (i = 0; i < 4; i++) @@ -146,8 +137,7 @@ }
/* setrxvcmdssel */ - for (i = 0; i < 2; i++) - dmi_update_field(0x0908 + (i << 5), 3, 1, 30); + dmi_update_bundles(0x0908, 3, 1, 30);
/* Swing Control: Reduced */ if (stepping >= IVB_STEP_C0) { @@ -160,40 +150,32 @@
if (stepping >= IVB_STEP_K0) { /* setK0ctocfix */ - for (i = 0; i < 2; i++) - dmi_update_field(0x0700 + (i << 5), 1, 1, 21); + dmi_update_bundles(0x0700, 1, 1, 21);
/* setK0l0sfreezefix */ - for (i = 0; i < 2; i++) - dmi_update_field(0x0700 + (i << 5), 3, 1, 24); + dmi_update_bundles(0x0700, 3, 1, 24);
/* setK0samplerfix */ - for (i = 0; i < 2; i++) - dmi_update_field(0x0700 + (i << 5), 1, 1, 19); + dmi_update_bundles(0x0700, 1, 1, 19);
/* setK0resetalignfix */ - for (i = 0; i < 2; i++) - dmi_update_field(0x0700 + (i << 5), 1, 1, 23); + dmi_update_bundles(0x0700, 1, 1, 23);
/* setK0cdcprotfix */ - for (i = 0; i < 2; i++) - dmi_update_field(0x0700 + (i << 5), 1, 1, 22); + dmi_update_bundles(0x0700, 1, 1, 22); }
/* setctleocgen2 */ - if (stepping >= IVB_STEP_D0) { - for (i = 0; i < 2; i++) - dmi_update_field(0x0914 + (i << 5), 1, 1, 9); - } + if (stepping >= IVB_STEP_D0) + dmi_update_bundles(0x0914, 1, 1, 9);
/* * ECO(3621419): Aggresive Rx L0s freeze resulting in Rx * errors since AGC is in open loop for long time (L0s Exit) */ - if (stepping >= IVB_STEP_E0) { - for (i = 0; i < 2; i++) - dmi_update_field(0x0914 + (i << 5), 1, 1, 27); - } + if (stepping >= IVB_STEP_E0) + dmi_update_bundles(0x0914, 1, 1, 27); +
/* setD0disableL1exitfix */ if (stepping == IVB_STEP_D0) @@ -212,13 +194,11 @@ dmi_update_field(0x0258, 1, 1, 29);
if (stepping >= IVB_STEP_D0) { - for (i = 0; i < 2; i++) { - /* setD0dfeidacpdgen1 */ - dmi_update_field(0x0904 + (i << 5), 0xf, 5, 25); + /* setD0dfeidacpdgen1 */ + dmi_update_bundles(0x0904, 0xf, 5, 25);
- /* setD0dfeidacpdgen2 */ - dmi_update_field(0x0914 + (i << 5), 0xf, 0, 13); - } + /* setD0dfeidacpdgen2 */ + dmi_update_bundles(0x0914, 0xf, 0, 13); }
/* Update the N_FTS values */
Hello Felix Singer, build bot (Jenkins), Nico Huber, Arthur Heymans, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48414
to look at the new patch set (#3).
Change subject: nb/intel/sandybridge: Introduce `dmi_update_bundles` function ......................................................................
nb/intel/sandybridge: Introduce `dmi_update_bundles` function
This eliminates redundancy in the DMI recipe function. The order in which these registers are programmed does not seem to matter, as long as all registers end up with the correct values and are consistent.
Tested on Asus P8Z77-V LX2, still boots with Ivy Bridge E1 CPU.
Change-Id: I025ccc79ac323d4bb6c0e1552019ec7fbb847d4b Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/sandybridge/early_dmi.c 1 file changed, 36 insertions(+), 56 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/48414/3
Angel Pons has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/48414 )
Change subject: nb/intel/sandybridge: Introduce `dmi_update_bundles` function ......................................................................
Abandoned
Unreviewable