7 comments:
File src/northbridge/intel/sandybridge/raminit_common.h:
Patch Set #7, Line 60: val_320c
this has a known name
Patch Set #7, Line 112: reg_5064b0
this has also a known name
File src/northbridge/intel/sandybridge/sandybridge.h:
#define CxLy(r, x, y) ((r) + ((x) * 0x400) + ((y) * 4))
#define LyCx(r, x, y) ((r) + ((y) * 4) + ((x) * 0x400))
these two are basically the same, but with different order of the parameters; please drop the second one and use the first one everywhere
#define GDCRTRAININGRESULT1_Cz_B0(z) Gz(0x0004, z) /* Results according to PI settings */
#define GDCRTRAININGRESULT2_Cz_B0(z) Gz(0x0008, z)
#define GDCRRXRANK0_Cz_B0(z) Gz(0x0010, z) /* Time setting for Rx data byte */
#define GDCRRXRANK1_Cz_B0(z) Gz(0x0014, z)
#define GDCRRXRANK2_Cz_B0(z) Gz(0x0018, z)
#define GDCRRXRANK3_Cz_B0(z) Gz(0x001c, z)
#define GDCRTXRANK0_Cz_B0(z) Gz(0x0020, z) /* Time setting for Tx data byte */
#define GDCRTXRANK1_Cz_B0(z) Gz(0x0024, z)
#define GDCRTXRANK2_Cz_B0(z) Gz(0x0028, z)
#define GDCRTXRANK3_Cz_B0(z) Gz(0x002c, z)
these don't seem to be registers, but register offsets. they also seem unused
Patch Set #7, Line 184: MC Channel 0
this isn't just for channel 0, but also channel 1
Patch Set #7, Line 185: #define TC_DBP_Cx(x) Cx(0x4000, x) /* Timing of DDR - bin parameters */
i'd strongly recommend using channel instead of x here; that would make it clearer what happens here. also for the following defines
/* Sub-sequence special command address */
#define IOSAV_y_SP_CMD_ADDR_Cx(x, y) LyCx(0x4200, x, y)
/* Address update after command execution */
#define IOSAV_y_ADDR_UPD_Cx(x, y) LyCx(0x4210, x, y)
/* Command signals in sub-sequence command */
#define IOSAV_y_SP_CMD_CTL_Cx(x, y) LyCx(0x4220, x, y)
/* Sub-sequence command parameter control */
#define IOSAV_y_SUBSEQ_CTL_Cx(x, y) LyCx(0x4230, x, y)
/* 23-bit LFSR value of the sequence */
#define IOSAV_y_ADDRESS_LFSR_Cx(x, y) LyCx(0x4240, x, y)
please use the CxLy macro here.
Maybe also swap the x and y parameters in the IOSAV_* macros; it would make more sense to me to have the sequencer slot number (0..3) as first parameter and the memory channel as second. this is just my opinion though ;)
To view, visit change 38036. To unsubscribe, or for help writing mail filters, visit settings.