Change in coreboot[master]: nb/intel/sandybridge: Drop none reliable MCHBARx_{AND,OR} macros
HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44506 )
Change subject: nb/intel/sandybridge: Drop none reliable MCHBARx_{AND,OR} macros ......................................................................
nb/intel/sandybridge: Drop none reliable MCHBARx_{AND,OR} macros
Use of MCHBARx_AND_OR gives an error: overflow in conversion from 'int' to 'u16' {aka 'volatile short unsigned int'} changes value from '(int)*(volatile u16 *)((unsigned int)((int)i * 1024) + 4275127072) & -65536 | 26214' to '26214' [-Werror=overflow]
Change-Id: I73f7f22dfa2440b3b44312e614a5d38ae1efcfc8 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/sandybridge/early_init.c M src/northbridge/intel/sandybridge/finalize.c M src/northbridge/intel/sandybridge/northbridge.c M src/northbridge/intel/sandybridge/raminit_common.c M src/northbridge/intel/sandybridge/sandybridge.h 5 files changed, 75 insertions(+), 62 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/44506/1
diff --git a/src/northbridge/intel/sandybridge/early_init.c b/src/northbridge/intel/sandybridge/early_init.c index 2ee273c..af39ffe 100644 --- a/src/northbridge/intel/sandybridge/early_init.c +++ b/src/northbridge/intel/sandybridge/early_init.c @@ -105,18 +105,18 @@ pci_update_config8(PCI_DEV(0, 2, 0), MSAC, ~0x06, 0x02);
/* Erratum workarounds */ - MCHBAR32_OR(SAPMCTL, (1 << 9) | (1 << 10)); + MCHBAR32(SAPMCTL) |= ((1 << 9) | (1 << 10));
/* Enable SA Clock Gating */ - MCHBAR32_OR(SAPMCTL, 1); + MCHBAR32(SAPMCTL) |= 1;
/* GPU RC6 workaround for sighting 366252 */ - MCHBAR32_OR(SSKPD_HI, 1 << 31); + MCHBAR32(SSKPD_HI) |= (1 << 31);
/* VLW (Virtual Legacy Wire?) */ - MCHBAR32_AND(0x6120, ~(1 << 0)); + MCHBAR32(0x6120) &= ~(1 << 0);
- MCHBAR32_OR(INTRDIRCTL, (1 << 4) | (1 << 5)); + MCHBAR32(INTRDIRCTL) |= ((1 << 4) | (1 << 5)); }
static void start_peg_link_training(void) diff --git a/src/northbridge/intel/sandybridge/finalize.c b/src/northbridge/intel/sandybridge/finalize.c index 26c5306..dfa5948 100644 --- a/src/northbridge/intel/sandybridge/finalize.c +++ b/src/northbridge/intel/sandybridge/finalize.c @@ -18,13 +18,13 @@ pci_or_config32(HOST_BRIDGE, TSEGMB, 1 << 0); pci_or_config32(HOST_BRIDGE, TOLUD, 1 << 0);
- MCHBAR32_OR(PAVP_MSG, 1 << 0); /* PAVP */ - MCHBAR32_OR(SAPMCTL, 1 << 31); /* SA PM */ - MCHBAR32_OR(UMAGFXCTL, 1 << 0); /* UMA GFX */ - MCHBAR32_OR(VTDTRKLCK, 1 << 0); /* VTDTRK */ - MCHBAR32_OR(REQLIM, 1 << 31); - MCHBAR32_OR(DMIVCLIM, 1 << 31); - MCHBAR32_OR(CRDTLCK, 1 << 0); + MCHBAR32(PAVP_MSG) |= (1 << 0); /* PAVP */ + MCHBAR32(SAPMCTL) |= (1 << 31); /* SA PM */ + MCHBAR32(UMAGFXCTL) |= (1 << 0); /* UMA GFX */ + MCHBAR32(VTDTRKLCK) |= (1 << 0); /* VTDTRK */ + MCHBAR32(REQLIM) |= (1 << 31); + MCHBAR32(DMIVCLIM) |= (1 << 31); + MCHBAR32(CRDTLCK) |= (1 << 0);
/* Memory Controller Lockdown */ MCHBAR8(MC_LOCK) = 0x8f; diff --git a/src/northbridge/intel/sandybridge/northbridge.c b/src/northbridge/intel/sandybridge/northbridge.c index e670c09..9a345a5 100644 --- a/src/northbridge/intel/sandybridge/northbridge.c +++ b/src/northbridge/intel/sandybridge/northbridge.c @@ -359,10 +359,10 @@ * * FIXME: Never clock gate on Ivy Bridge stepping A0! */ - MCHBAR32_OR(PEGCTL, 1); + MCHBAR32(PEGCTL) |= 1; printk(BIOS_DEBUG, "Disabling PEG IO clock.\n"); } else { - MCHBAR32_AND(PEGCTL, ~1); + MCHBAR32(PEGCTL) &= ~1; } }
diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index 3527c8e..2ff7ac8 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -178,7 +178,7 @@
static void dram_odt_stretch(ramctr_timing *ctrl, int channel) { - u32 addr, stretch; + u32 addr, stretch, reg32;
stretch = ctrl->ref_card_offset[channel]; /* @@ -190,11 +190,17 @@ stretch = 3;
addr = SCHED_SECOND_CBIT_ch(channel); - MCHBAR32_AND_OR(addr, 0xffffc3ff, (stretch << 12) | (stretch << 10)); + reg32 = MCHBAR32(addr); + reg32 &= 0xffffc3ff; + reg32 |= ((stretch << 12) | (stretch << 10)); + MCHBAR32(addr) = reg32; printk(RAM_DEBUG, "OTHP Workaround [%x] = %x\n", addr, MCHBAR32(addr)); } else { addr = TC_OTHP_ch(channel); - MCHBAR32_AND_OR(addr, 0xfff0ffff, (stretch << 16) | (stretch << 18)); + reg32 = MCHBAR32(addr); + reg32 &= 0xfff0ffff; + reg32 |= ((stretch << 16) | (stretch << 18)); + MCHBAR32(addr) = reg32; printk(RAM_DEBUG, "OTHP [%x] = %x\n", addr, MCHBAR32(addr)); } } @@ -254,7 +260,7 @@ MCHBAR32(TC_DTP_ch(channel)) = reg; }
- MCHBAR32_OR(addr, 0x00020000); + MCHBAR32(addr) |= 0x00020000;
dram_odt_stretch(ctrl, channel);
@@ -271,7 +277,7 @@ printram("REFI [%x] = %x\n", TC_RFTP_ch(channel), reg); MCHBAR32(TC_RFTP_ch(channel)) = reg;
- MCHBAR32_OR(TC_RFP_ch(channel), 0xff); + MCHBAR32(TC_RFP_ch(channel)) |= 0xff;
/* Self-refresh timing parameters */ reg = 0; @@ -652,19 +658,19 @@ MCHBAR32(MC_INIT_STATE_G) = reg;
/* Assert DIMM reset signal */ - MCHBAR32_AND(MC_INIT_STATE_G, ~2); + MCHBAR32(MC_INIT_STATE_G) &= ~2;
/* Wait 200us */ udelay(200);
/* Deassert DIMM reset signal */ - MCHBAR32_OR(MC_INIT_STATE_G, 2); + MCHBAR32(MC_INIT_STATE_G) |= 2;
/* Wait 500us */ udelay(500);
/* Enable DCLK */ - MCHBAR32_OR(MC_INIT_STATE_G, 4); + MCHBAR32(MC_INIT_STATE_G) |= 4;
/* XXX Wait 20ns */ udelay(1); @@ -957,10 +963,10 @@ }
/* Refresh enable */ - MCHBAR32_OR(MC_INIT_STATE_G, 8); + MCHBAR32(MC_INIT_STATE_G) |= 8;
FOR_ALL_POPULATED_CHANNELS { - MCHBAR32_AND(SCHED_CBIT_ch(channel), ~0x200000); + MCHBAR32(SCHED_CBIT_ch(channel)) &= ~0x200000;
wait_for_iosav(channel);
@@ -2637,15 +2643,15 @@ int err;
FOR_ALL_POPULATED_CHANNELS - MCHBAR32_OR(TC_RWP_ch(channel), 0x8000000); + MCHBAR32(TC_RWP_ch(channel)) |= 0x8000000;
FOR_ALL_POPULATED_CHANNELS { write_op(ctrl, channel); - MCHBAR32_OR(SCHED_CBIT_ch(channel), 0x200000); + MCHBAR32(SCHED_CBIT_ch(channel)) |= 0x200000; }
/* Refresh disable */ - MCHBAR32_AND(MC_INIT_STATE_G, ~8); + MCHBAR32(MC_INIT_STATE_G) &= ~8; FOR_ALL_POPULATED_CHANNELS { write_op(ctrl, channel); } @@ -2678,10 +2684,10 @@ wait_for_iosav(channel);
/* Refresh enable */ - MCHBAR32_OR(MC_INIT_STATE_G, 8); + MCHBAR32(MC_INIT_STATE_G) |= 8;
FOR_ALL_POPULATED_CHANNELS { - MCHBAR32_AND(SCHED_CBIT_ch(channel), ~0x00200000); + MCHBAR32(SCHED_CBIT_ch(channel)) &= ~0x00200000; MCHBAR32(IOSAV_STATUS_ch(channel)); wait_for_iosav(channel);
@@ -2723,7 +2729,7 @@ printram("CPF\n");
FOR_ALL_CHANNELS FOR_ALL_POPULATED_RANKS FOR_ALL_LANES { - MCHBAR32_AND(IOSAV_By_BW_MASK_ch(channel, lane), 0); + MCHBAR32(IOSAV_By_BW_MASK_ch(channel, lane) &= 0; }
FOR_ALL_POPULATED_CHANNELS { @@ -2747,7 +2753,7 @@ program_timings(ctrl, channel);
FOR_ALL_CHANNELS FOR_ALL_POPULATED_RANKS FOR_ALL_LANES { - MCHBAR32_AND(IOSAV_By_BW_MASK_ch(channel, lane), 0); + MCHBAR32(IOSAV_By_BW_MASK_ch(channel, lane)) &= 0; } return 0; } @@ -2978,11 +2984,11 @@ iosav_run_once(channel);
wait_for_iosav(channel); - MCHBAR32_OR(SCHED_CBIT_ch(channel), 0x200000); + MCHBAR32(SCHED_CBIT_ch(channel)) |= 0x200000; }
/* refresh disable */ - MCHBAR32_AND(MC_INIT_STATE_G, ~8); + MCHBAR32(MC_INIT_STATE_G) &= ~8; FOR_ALL_POPULATED_CHANNELS { wait_for_iosav(channel);
@@ -3964,6 +3970,7 @@ { const u8 rege3c_b24[3] = { 0, 0x0f, 0x2f }; int i, pat; + u32 reg32;
int lower[NUM_CHANNELS][NUM_SLOTRANKS][NUM_LANES]; int upper[NUM_CHANNELS][NUM_SLOTRANKS][NUM_LANES]; @@ -3985,8 +3992,10 @@ FOR_ALL_POPULATED_CHANNELS {
/* FIXME: Setting the Write VREF must only be done on Ivy Bridge */ - MCHBAR32_AND_OR(GDCRCMDDEBUGMUXCFG_Cz_S(channel), - ~0x3f000000, rege3c_b24[i] << 24); + reg32 = MCHBAR32(GDCRCMDDEBUGMUXCFG_Cz_S(channel); + reg32 &= ~0x3f000000; + reg32 |= (rege3c_b24[i] << 24); + MCHBAR32(GDCRCMDDEBUGMUXCFG_Cz_S(channel)) = reg32;
udelay(2);
@@ -4053,7 +4062,7 @@
FOR_ALL_CHANNELS { /* FIXME: Setting the Write VREF must only be done on Ivy Bridge */ - MCHBAR32_AND(GDCRCMDDEBUGMUXCFG_Cz_S(channel), ~0x3f000000); + MCHBAR32(GDCRCMDDEBUGMUXCFG_Cz_S(channel)) &= ~0x3f000000; udelay(2); }
@@ -4423,7 +4432,7 @@
FOR_ALL_POPULATED_CHANNELS { /* Always drive command bus */ - MCHBAR32_OR(TC_RAP_ch(channel), 0x20000000); + MCHBAR32(TC_RAP_ch(channel)) |= 0x20000000; }
udelay(1); @@ -4463,7 +4472,7 @@ int channel; FOR_ALL_POPULATED_CHANNELS { MCHBAR32(MC_INIT_STATE_ch(channel)) = 0x00001000 | ctrl->rankmap[channel]; - MCHBAR32_AND(TC_RAP_ch(channel), ~0x20000000); + MCHBAR32(TC_RAP_ch(channel)) &= ~0x20000000; } }
@@ -4481,13 +4490,17 @@ int channel; int t1_cycles = 0, t1_ns = 0, t2_ns; int t3_ns; - u32 r32; + u32 r32, reg32;
/* FIXME: This register only exists on Ivy Bridge */ MCHBAR32(WMM_READ_CONFIG) = 0x46;
- FOR_ALL_CHANNELS - MCHBAR32_AND_OR(TC_OTHP_ch(channel), 0xffffcfff, 0x1000); + FOR_ALL_CHANNELS { + reg32 = MCHBAR32(TC_OTHP_ch(channel)); + reg32 &= 0xffffcfff; + reg32 |= 0x1000; + MCHBAR32(TC_OTHP_ch(channel)) = reg32; + }
if (is_mobile) /* APD - DLL Off, 64 DCLKs until idle, decision per rank */ @@ -4522,14 +4535,21 @@ }
MCHBAR32(MEM_TRML_ESTIMATION_CONFIG) = 0xca9171e5; - MCHBAR32_AND_OR(MEM_TRML_THRESHOLDS_CONFIG, ~0x00ffffff, 0x00e4d5d0); - MCHBAR32_AND(MEM_TRML_INTERRUPT, ~0x1f); + reg32 = MCHBAR32(MEM_TRML_THRESHOLDS_CONFIG); + reg32 &= ~0x00ffffff; + reg32 |= 0x00e4d5d0; + MCHBAR32(MEM_TRML_THRESHOLDS_CONFIG) = reg32; + MCHBAR32(MEM_TRML_INTERRUPT) &= ~0x1f;
- FOR_ALL_CHANNELS - MCHBAR32_AND_OR(TC_RFP_ch(channel), ~(3 << 16), 1 << 16); + FOR_ALL_CHANNELS { + reg32 = MCHBAR32(TC_RFP_ch(channel)); + reg32 &= ~(3 << 16); + reg32 |= (1 << 16); + MCHBAR32(TC_RFP_ch(channel)) = reg32; + }
- MCHBAR32_OR(MC_INIT_STATE_G, 1); - MCHBAR32_OR(MC_INIT_STATE_G, 0x80); + MCHBAR32(MC_INIT_STATE_G) |= 1; + MCHBAR32(MC_INIT_STATE_G) |= 0x80; MCHBAR32(BANDTIMERS_SNB) = 0xfa;
/* Find a populated channel */ @@ -4555,9 +4575,11 @@
/* The graphics driver will use these watermark values */ printk(BIOS_DEBUG, "t123: %d, %d, %d\n", t1_ns, t2_ns, t3_ns); - MCHBAR32_AND_OR(SSKPD, 0xC0C0C0C0, - ((encode_wm(t1_ns) + encode_wm(t2_ns)) << 16) | (encode_wm(t1_ns) << 8) | - ((encode_wm(t3_ns) + encode_wm(t2_ns) + encode_wm(t1_ns)) << 24) | 0x0c); + reg32 = MCHBAR32(SSKPD); + reg32 &= 0xC0C0C0C0; + reg32 |= (((encode_wm(t1_ns) + encode_wm(t2_ns)) << 16) | (encode_wm(t1_ns) << 8) | + ((encode_wm(t3_ns) + encode_wm(t2_ns) + encode_wm(t1_ns)) << 24) | 0x0c)); + MCHBAR32(SSKPD) = reg32; }
void restore_timings(ramctr_timing *ctrl) @@ -4586,11 +4608,11 @@ }
FOR_ALL_POPULATED_CHANNELS - MCHBAR32_OR(TC_RWP_ch(channel), 0x08000000); + MCHBAR32(TC_RWP_ch(channel)) |= 0x08000000;
FOR_ALL_POPULATED_CHANNELS { udelay(1); - MCHBAR32_OR(SCHED_CBIT_ch(channel), 0x00200000); + MCHBAR32(SCHED_CBIT_ch(channel)) |= 0x00200000; }
printram("CPE\n"); @@ -4646,7 +4668,7 @@ MCHBAR32(GDCRTRAININGMOD_ch(0)) = 0;
FOR_ALL_CHANNELS { - MCHBAR32_AND(GDCRCMDDEBUGMUXCFG_Cz_S(channel), ~0x3f000000); + MCHBAR32(GDCRCMDDEBUGMUXCFG_Cz_S(channel)) &= ~0x3f000000; udelay(2); } } diff --git a/src/northbridge/intel/sandybridge/sandybridge.h b/src/northbridge/intel/sandybridge/sandybridge.h index 72724a3..2fed3a2 100644 --- a/src/northbridge/intel/sandybridge/sandybridge.h +++ b/src/northbridge/intel/sandybridge/sandybridge.h @@ -67,15 +67,6 @@ #define MCHBAR8(x) (*((volatile u8 *)(DEFAULT_MCHBAR + (x)))) #define MCHBAR16(x) (*((volatile u16 *)(DEFAULT_MCHBAR + (x)))) #define MCHBAR32(x) (*((volatile u32 *)(DEFAULT_MCHBAR + (x)))) -#define MCHBAR8_AND(x, and) (MCHBAR8(x) = MCHBAR8(x) & (and)) -#define MCHBAR16_AND(x, and) (MCHBAR16(x) = MCHBAR16(x) & (and)) -#define MCHBAR32_AND(x, and) (MCHBAR32(x) = MCHBAR32(x) & (and)) -#define MCHBAR8_OR(x, or) (MCHBAR8(x) = MCHBAR8(x) | (or)) -#define MCHBAR16_OR(x, or) (MCHBAR16(x) = MCHBAR16(x) | (or)) -#define MCHBAR32_OR(x, or) (MCHBAR32(x) = MCHBAR32(x) | (or)) -#define MCHBAR8_AND_OR(x, and, or) (MCHBAR8(x) = (MCHBAR8(x) & (and)) | (or)) -#define MCHBAR16_AND_OR(x, and, or) (MCHBAR16(x) = (MCHBAR16(x) & (and)) | (or)) -#define MCHBAR32_AND_OR(x, and, or) (MCHBAR32(x) = (MCHBAR32(x) & (and)) | (or))
/* As there are many registers, define them on a separate file */ #include "mchbar_regs.h"
Hello build bot (Jenkins), Angel Pons, Arthur Heymans, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44506
to look at the new patch set (#2).
Change subject: nb/intel/sandybridge: Drop none reliable MCHBARx_{AND,OR} macros ......................................................................
nb/intel/sandybridge: Drop none reliable MCHBARx_{AND,OR} macros
Use of MCHBARx_AND_OR gives an error: overflow in conversion from 'int' to 'u16' {aka 'volatile short unsigned int'} changes value from '(int)*(volatile u16 *)((unsigned int)((int)i * 1024) + 4275127072) & -65536 | 26214' to '26214' [-Werror=overflow]
Change-Id: I73f7f22dfa2440b3b44312e614a5d38ae1efcfc8 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/sandybridge/early_init.c M src/northbridge/intel/sandybridge/finalize.c M src/northbridge/intel/sandybridge/northbridge.c M src/northbridge/intel/sandybridge/raminit_common.c M src/northbridge/intel/sandybridge/sandybridge.h 5 files changed, 75 insertions(+), 62 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/44506/2
Hello build bot (Jenkins), Nico Huber, Angel Pons, Arthur Heymans, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/44506
to look at the new patch set (#5).
Change subject: nb/intel/sandybridge: Drop unreliable MCHBARx_{AND,OR} macros ......................................................................
nb/intel/sandybridge: Drop unreliable MCHBARx_{AND,OR} macros
Use of MCHBARx_AND_OR gives an error: overflow in conversion from 'int' to 'u16' {aka 'volatile short unsigned int'} changes value from '(int)*(volatile u16 *)((unsigned int)((int)i * 1024) + 4275127072) & -65536 | 26214' to '26214' [-Werror=overflow]
Change-Id: I73f7f22dfa2440b3b44312e614a5d38ae1efcfc8 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/northbridge/intel/sandybridge/early_init.c M src/northbridge/intel/sandybridge/finalize.c M src/northbridge/intel/sandybridge/northbridge.c M src/northbridge/intel/sandybridge/raminit_common.c M src/northbridge/intel/sandybridge/sandybridge.h 5 files changed, 75 insertions(+), 62 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/44506/5
HAOUAS Elyes has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/44506 )
Change subject: nb/intel/sandybridge: Drop unreliable MCHBARx_{AND,OR} macros ......................................................................
Abandoned
see 42448
-
HAOUAS Elyes (Code Review)