[U-Boot] Allowing rtt_nom to be configurable to all possible values

[U-Boot] Allowing rtt_nom to be configurable to all possible values

[Fabio Estevam]

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?

--- 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,

Fabio Estevam

[U-Boot] Allowing rtt_nom to be configurable to all possible values

[Eric Nelson]

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

[U-Boot] Allowing rtt_nom to be configurable to all possible values

[Fabio Estevam]

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.