HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31144
Change subject: nb/intel/i945: Correct mask for CxDRT1 ......................................................................
nb/intel/i945: Correct mask for CxDRT1
[27:24] and [31:30] are reserved, so we shouldn't change them. Tested on i945G-M4 board.
Change-Id: I0cac9160c756405838706edb0461575241faae35 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/i945/raminit.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/31144/1
diff --git a/src/northbridge/intel/i945/raminit.c b/src/northbridge/intel/i945/raminit.c index f3c3df6..2a62ca3 100644 --- a/src/northbridge/intel/i945/raminit.c +++ b/src/northbridge/intel/i945/raminit.c @@ -1475,7 +1475,7 @@
/* Calculate DRT1 */
- temp_drt = MCHBAR32(C0DRT1) & 0x00020088; + temp_drt = MCHBAR32(C0DRT1) & 0xcf020088;
/* DRAM RASB Precharge */ temp_drt |= (sysinfo->trp - 2) << 0;
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31144 )
Change subject: nb/intel/i945: Correct mask for CxDRT1 ......................................................................
Patch Set 1:
vendor did the same on macbook21.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31144 )
Change subject: nb/intel/i945: Correct mask for CxDRT1 ......................................................................
Patch Set 2: Code-Review-1
(2 comments)
https://review.coreboot.org/#/c/31144/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31144/2//COMMIT_MSG@9 PS2, Line 9: [27:24] and [31:30] are reserved The code explicitly sets bits [31:30]. The one who wrote this code had better documentation than the publicly available one.
https://review.coreboot.org/#/c/31144/2//COMMIT_MSG@10 PS2, Line 10: Tested on i945G-M4 board. What does it do better with this change?
HAOUAS Elyes has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/31144 )
Change subject: nb/intel/i945: Correct mask for CxDRT1 ......................................................................
Abandoned
HAOUAS Elyes has restored this change. ( https://review.coreboot.org/c/coreboot/+/31144 )
Change subject: nb/intel/i945: Correct mask for CxDRT1 ......................................................................
Restored
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31144 )
Change subject: nb/intel/i945: Correct mask for CxDRT1 ......................................................................
Patch Set 6: Code-Review-1
"Reserved" in Intel's datasheets means "doesn't concern you" (which is wrong anyway) but not that the hardware/firmware doesn't know the bits.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31144 )
Change subject: nb/intel/i945: Correct mask for CxDRT1 ......................................................................
Patch Set 6:
Patch Set 6: Code-Review-1
"Reserved" in Intel's datasheets means "doesn't concern you" (which is wrong anyway) but not that the hardware/firmware doesn't know the bits.
indeed, as they are Reserved, we don't have to mask with 0s . I mean Reserved & 1 = Reserved and Reserved & 0 = 0 (I agree, it isn't a good dialectic, but vendor uses 0xcf020088)
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31144 )
Change subject: nb/intel/i945: Correct mask for CxDRT1 ......................................................................
Patch Set 6: Code-Review-1
Patch Set 6:
Patch Set 6: Code-Review-1
"Reserved" in Intel's datasheets means "doesn't concern you" (which is wrong anyway) but not that the hardware/firmware doesn't know the bits.
indeed, as they are Reserved, we don't have to mask with 0s . I mean Reserved & 1 = Reserved and Reserved & 0 = 0 (I agree, it isn't a good dialectic, but vendor uses 0xcf020088)
What does vendor do with that value? Is it what gets written to the CxDRT1 registers?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31144 )
Change subject: nb/intel/i945: Correct mask for CxDRT1 ......................................................................
Patch Set 6: -Code-Review
Patch Set 6: Code-Review-1
Patch Set 6:
Patch Set 6: Code-Review-1
"Reserved" in Intel's datasheets means "doesn't concern you" (which is wrong anyway) but not that the hardware/firmware doesn't know the bits.
indeed, as they are Reserved, we don't have to mask with 0s . I mean Reserved & 1 = Reserved and Reserved & 0 = 0 (I agree, it isn't a good dialectic, but vendor uses 0xcf020088)
What does vendor do with that value? Is it what gets written to the CxDRT1 registers?
i915 uses 0xcf080488, so I guess these registers are truly unused.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31144 )
Change subject: nb/intel/i945: Correct mask for CxDRT1 ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/31144/6//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/31144/6//COMMIT_MSG@9 PS6, Line 9: [27:24] On i915, this is RCVEN_COARSE. It is written to as part of the receive enable sequence, so it is not really reserved.
HAOUAS Elyes has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/31144 )
Change subject: nb/intel/i945: Correct mask for CxDRT1 ......................................................................
Abandoned
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
HAOUAS Elyes (Code Review)
-
Nico Huber (Code Review)