Message ID | CAOMZO5DrW=HRedSQhOP5CxZk48MriHJJwJoSrYys_sOZCSNa+Q@mail.gmail.com |
---|---|
State | Changes Requested |
Delegated to: | Stefano Babic |
Headers | show |
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
Hi Eric, On Tue, Jan 5, 2016 at 3:54 PM, Eric Nelson <eric@nelint.com> wrote: > 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)... Thanks for your feedback. Will submit such patch after 2016.01 is out.
--- 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); Regards,