Carl-Daniel Hailfinger wrote:
On 22.07.2008 01:02, Marc Jones wrote:
Add AMD Fam10 B3 default settings to match AMD example code.
Includes setting for most resent errata.
Signed-off-by: Marc Jones <[EMAIL PROTECTED]>
{ 3, 0x44, AMD_DR_ALL, AMD_PTYPE_ALL,
- 0x0A100044, 0x0A300044 }, /* [27] NB MCA to CPU0 Enable,
- [25] DisPciCfgCpuErrRsp,
- [21] SyncOnErr=0,
+ 0x4A30005C, 0x4A30005C }, /* [30] SyncOnDramAdrParErrEn=1,
+ [27] NbMcaToMstCpuEn=1,
+ [25] DisPciCfgCpuErrRsp=1,
+ [21] SyncOnAnyErrEn=1,
[20] SyncOnWDTEn=1,
- [6] CpuErrDis,
+ [6] CpuErrDis=1,
+ [4] SyncPktPropDis=1,
+ [3] SyncPktGenDis=1,
[2] SyncOnUcEccEn=1 */
This is not a complaint about the patch, but I'd like to understand why
the value pair had internal differences before (0x0A100044, 0x0A300044)
and is identical now. The comments didn't make the old situation so
obvious, however the new situation conforms nciely with the comments.
The second value is a mask, so it was clearing SyncOnAnyErrEn and not
setting it. I think it was intentional at the time.
/* Extended NB MCA Config Register */
{ 3, 0x180, AMD_DR_ALL, AMD_PTYPE_ALL,
- 0x00700022, 0x00700022 }, /* [5] = DisPciCfgCpuMstAbtRsp
- [22:20] = SyncFloodOn_Err = 7,
- [1] = SyncFloodOnUsPwDataErr = 1 */
+ 0x007003E2, 0x007003E2 }, /* [22:20] = SyncFloodOn_Err = 7,
+ [9] SyncOnUncNbAryEn = 1 ,
+ [8] SyncOnProtEn = 1,
+ [7] SyncFloodOnTgtAbtErr] = 1,
+ [6] SyncFloodOnDatErr] = 1,
Both lines above have a superfluous closing square bracket.
fixed.
+ [5] DisPciCfgCpuMstAbtRsp=1,
+ [1] SyncFloodOnUsPwDataErr = 1 */
The style of the datasheet-referencing comments is not entirely
consistent: Sometimes it's "a=1", sometimes "a = 1". I don't care about
this, but Uwe may.
fixed.
/* L3 Control Register */
{ 3, 0x1B8, AMD_DR_ALL, AMD_PTYPE_ALL,
Index: coreboot-v2/src/cpu/amd/model_10xxx/init_cpus.c
===================================================================
--- coreboot-v2.orig/src/cpu/amd/model_10xxx/init_cpus.c 2008-07-21
15:39:58.000000000 -0600
+++ coreboot-v2/src/cpu/amd/model_10xxx/init_cpus.c 2008-07-21
16:45:47.000000000 -0600
@@ -669,6 +669,35 @@
}
+void AMD_SetupPSIVID_d (u32 platform_type, u8 node)
+{
+ u32 dword;
+ int i;
+ msr_t msr;
+
+ if (platform_type & (AMD_PTYPE_MOB | AMD_PTYPE_DSK)) {
+
+ /* The following code sets the PSIVID to the highest supported P state
+ * assuming that the VID for the highest power state is below
+ * the VDD voltage regulator threshold. (This also assumes that there
+ * is a Pstate higher than P0)
+ */
+
+ for( i = 4; i >= 0; i--) {
+ msr = rdmsr(0xC0010064 + i);
Having a #define for 0xC0010064 would be nice.
used #define PS_REG_BASE
+ /* Pstate valid? */
+ if ((msr.hi >> 31) & 1) {
I guess this is personal preference, but I'd like to suggest an alternative:
#define MSR_FOO_PSTATE_VALID (1 << 31)
if (msr.hi & MSR_FOO_PSTATE_VALID) {
There was a define PS_EN_MASK
for(i = 0; i < sizeof(fam10_pci_default)/sizeof(fam10_pci_default[0]);
i++) {
I admit this was not part of your current patch, but please use the
ARRAY_SIZE() macro for such calculations.
This is done in a few places and could use a separate patch to clean
them all up.
if ((fam10_pci_default[i].revision & revision) &&
(fam10_pci_default[i].platform & platform)) {
With the comments above answered or addressed (I trust your judgement):
Acked-by: Carl-Daniel Hailfinger <[EMAIL PROTECTED]>
r 3435
thanks
--
Marc Jones
Senior Firmware Engineer
(970) 226-9684 Office
mailto:[EMAIL PROTECTED]
http://www.amd.com/embeddedprocessors
--
coreboot mailing list
[email protected]
http://www.coreboot.org/mailman/listinfo/coreboot
