Hi Fabio,

On 01/05/2016 08:07 AM, Fabio Estevam wrote:

Hi,

Currently we have in arch/arm/cpu/armv7/mx6/ddr.c:

   /* MMDC Termination: rtt_nom:2 RZQ/2(120ohm), rtt_nom:1 RZQ/4(60ohm) */
   val = (sysinfo->rtt_nom == 2) ? 0x00011117 : 0x00022227;

,but the mx6 reference manual states

rtt_nom = 1 ---> 120 ohm rtt_nom = 2 ---> 60 ohm

which is the opposite of what the code and comment do.

Also, it is currently not possible to set rtt_nom to any other values other than 1 and 2.

On mx6sl evk, for example, we need rtt_nom = 0, which is not possible to be achieved currently.

Shouldn't we do like this instead?

Yes.

--- a/arch/arm/cpu/armv7/mx6/ddr.c +++ b/arch/arm/cpu/armv7/mx6/ddr.c @@ -834,11 +834,12 @@ void mx6_ddr3_cfg(const struct mx6_ddr_sysinfo *sysinfo, MMDC1(mprddqby3dl, 0x33333333); }

•   /* MMDC Termination: rtt_nom:2 RZQ/2(120ohm), rtt_nom:1 RZQ/4(60ohm) */
  

•   val = (sysinfo->rtt_nom == 2) ? 0x00011117 : 0x00022227;
  

•   mmdc0->mpodtctrl = val;
  

•   /* MMDC Termination */
  

•   val = sysinfo->rtt_nom;
  

•   mmdc0->mpodtctrl = (val << 16) | (val << 12) | (val << 8) | (val << 4) |
  

•                      0x7;
    if (sysinfo->dsize > 1)
  

•           MMDC1(mpodtctrl, val);
  

•           MMDC1(mpodtctrl, mmdc0->mpodtctrl);
  
  

The only thing I would add here is a set of constants to prevent the need to refer to the manual.

The values differ in the i.MX6SDL to allow different meanings for DDR2, so this should suffice:

#define RTT_NOM_Disabled 0 #define RTT_NOM_120_OHM_75_OHM_DDR2 1 #define RTT_NOM_60_OHM_150_OHM_DDR2 2 #define RTT_NOM_40_OHM_50_OHM_DDR2 3 #define RTT_NOM_30_OHM_37.5_OHM_DDR2 4 #define RTT_NOM_24_OHM_30_OHM_DDR2 5 #define RTT_NOM_20_OHM_25_OHM_DDR2 6 #define RTT_NOM_17_OHM_21_OHM_DDR2 7

(or an enum of the same)...

Regards,

Eric