[U-Boot] [PATCH V3 28/32] mx6q_4x_mt41j128.cfg: add mx6 solo/duallite support

Troy Kisky
2012-10-08T21:08:32Z

On 10/8/2012 11:46 AM, Eric Nelson wrote:
> Hi Troy,
>
> This seems to be the patch where the rubber meets the road in
> much of this series.
>
> On 10/03/2012 06:47 PM, Troy Kisky wrote:
>> Signed-off-by: Troy Kisky<troy.kisky@boundarydevices.com>
>> ---
>> board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg | 120
>> +++++++++++++++++++-------
>> 1 file changed, 87 insertions(+), 33 deletions(-)
>>
>> diff --git a/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg
>> b/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg
>> index 9e20db0..f45f93e 100644
>> --- a/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg
>> +++ b/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg
>> @@ -49,6 +49,15 @@ BOOT_FROM sd
>> * Address absolute address of the register
>> * value value to be stored in the register
>> */
>> +/*
>> + * DDR3 settings
>> + * MX6Q ddr is limited to 1066 Mhz, currently 1056 MHz(528 MHz
>> clock),
>> + * memory bus width: 64 bits, x16/x32/x64
>> + * MX6DL ddr is limited to 800 MHz(400 MHz clock)
>> + * memory bus width: 64 bits, x16/x32/x64
>> + * MX6SOLO ddr is limited to 800 MHz(400 MHz clock)
>> + * memory bus width: 32 bits, x16/x32
>> + */
>
> This comment seems to be critical to understanding some of
> what's below, since now three **different** memory configurations
> are represented in this file.
>
> At a minimum, the file name should be changed to get rid of
> '6q' and '4x' since those don't necessarily apply.

Right, my preference is to put at back in
board/freescale/mx6qsabrelite/imximage.cfg
since MPDGCTRL0/1 DQS GATE registers and MMDC_MPRDDQBYnDL should be very
board specific,
but that may be meet by objections from Fabio.

>
>> WRITE_ENTRY1(IOM_DRAM_SDQS0, 0x00000030)
>> WRITE_ENTRY1(IOM_DRAM_SDQS1, 0x00000030)
>> WRITE_ENTRY1(IOM_DRAM_SDQS2, 0x00000030)
>> @@ -90,6 +99,7 @@ WRITE_ENTRY1(IOM_GRP_B6DS, 0x00000030)
>> WRITE_ENTRY1(IOM_GRP_B7DS, 0x00000030)
>>
>> WRITE_ENTRY1(IOM_GRP_ADDDS, 0x00000030)
>> +
>> /* (differential input) */
>> WRITE_ENTRY1(IOM_DDRMODE_CTL, 0x00020000)
>> /* disable ddr pullups */
>> @@ -119,48 +129,92 @@ WRITE_ENTRY1(MMDC_P0_MDMISC, 0x00081740)
>> * MDSCR, con_req
>> */
>> WRITE_ENTRY1(MMDC_P0_MDSCR, 0x00008000)
>> +
>> /*
>> - * MDCFG0, tRFC=0x56 clocks, tXS=0x5b clocks
>> - * tXP=4 clocks, tXPDLL=13 clocks
>> - * tFAW=24 clocks, cas=8 cycles
>> + * MDCFG0,
>> + * MX6Q:
>> + * tRFC=0x56 clocks, tXS=0x5b clocks, tXP=4 clocks, tXPDLL=13 clocks
>> + * tFAW=24 clocks, cas=8 cycles
>> + * MX6DL/SOLO:
>> + * tRFC=0x6a clocks, tXS=0x6e clocks, tXP=3 clocks, tXPDLL=10 clocks
>> + * tFAW=19 clocks, cas=6 cycles
>> */
>> -WRITE_ENTRY1(MMDC_P0_MDCFG0, 0x555A7975)
>> +WRITE_ENTRY2(MMDC_P0_MDCFG0, 0x555A7975, 0x696D5323)
>> +
>
> Here's where I start to get lost in the macro-fu.
>
> The WRITE_ENTRY1 macros make some sense to me in that they
> always write a single value to an offset whose address
> changes based on the processor type.
>
> In order to understand WRITE_ENTRY2, you really have to
> know that the dual, solo, and sololite share many
> characteristics.

RIght, ENTRY2 is used when there are differences between
mx6q, and mx6dl/solo. In the case above, the value
0x555A7975 is written if a mx6quad, and the value 0x696D5323
is written if a mx6duallite or mx6solo.

>
> It's kinda hard to infer that knowledge from the code
> and I wonder if this structure will hold up under future
> revisions.
>
>> /*
>> - * MDCFG1, tRDC=8, tRP=8, tRC=27,tRAS=20,tRPA=tRP+1,tWR=8
>> - * tMRD=4, tCWL=6
>> + * MDCFG1,
>> + * MX6Q:
>> + * tRDC=8, tRP=8, tRC=27, tRAS=20, tRPA=tRP+1, tWR=8, tMRD=4, tCWL=6
>> + * MX6DL/SOLO:
>> + * tRDC=6, tRP=6, tRC=20, tRAS=15, tRPA=tRP+1, tWR=7, tMRD=4, tCWL=5
>> */
>> -WRITE_ENTRY1(MMDC_P0_MDCFG1, 0xFF538E64)
>> +WRITE_ENTRY2(MMDC_P0_MDCFG1, 0xFF538E64, 0xB66E8C63)
>> +
>> /*
>> * MDCFG2,tDLLK=512,tRTP=4,tWTR=4,tRRD=4
>> */
>> WRITE_ENTRY1(MMDC_P0_MDCFG2, 0x01FF00DB)
>> WRITE_ENTRY1(MMDC_P0_MDRWD, 0x000026D2)
>> -
>> WRITE_ENTRY1(MMDC_P0_MDOR, 0x005B0E21)
>> -WRITE_ENTRY1(MMDC_P0_MDOTC, 0x09444040)
>> -WRITE_ENTRY1(MMDC_P0_MDPDC, 0x00025576)
>>
>> /*
>> - * Mx6Q - 64 bit wide ddr
>> + * MMDC_MDOTC,
>> + * MX6Q:
>> + * tAOFPD=2 cycles, tAONPD=2, tANPD=5, tAXPD=5, tODTLon=5,
>> tODT_idle_off=5
>> + * MX6DL/SOLO:
>> + * tAOFPD=1 cycles, tAONPD=1, tANPD=4, tAXPD=4, tODTLon=4,
>> tODT_idle_off=4
>> + */
>> +WRITE_ENTRY2(MMDC_P0_MDOTC, 0x09444040, 0x00333030)
>> +
>> +/*
>> + * MDPDC - [17:16](2) => CKE pulse width = 3 cycles.
>> + * [15:12](5) => PWDT_1 = 256 cycles
>> + * [11:8](5) =>PWDR_0 = 256 cycles
>> + * MX6Q: [2:0](6) => CKSRE = 6 cycles, [5:3](6) => CKSRX = 6
>> cycles
>> + * MX6DL/SOLO: [2:0](5) => CKSRE = 5 cycles, [5:3](5) => CKSRX = 5
>> cycles
>> + */
>> +WRITE_ENTRY2(MMDC_P0_MDPDC, 0x00025576, 0x0002556D)
>> +
>> +/*
>> + * MX6Q/DL - 64 bit wide ddr
>> * last address is (1<<28 (base) + 1<<30 - 1) / (1<<25) =
>> * 1<<3 + 1<<5 - 1 = 8 + 0x20 -1 = 0x27
>> */
>> +/*
>> + * MX6SOLO - 32 bit wide ddr
>> + * last address is (1<<28 (base) + 1<<29 - 1) / (1<<25) =
>> + * 1<<3 + 1<<4 - 1 = 8 + 0x10 -1 = 0x17
>> + */
>> /* MDASP, CS0_END */
>> -WRITE_ENTRY1(MMDC_P0_MDASP, 0x00000027)
>> +WRITE_ENTRY3(MMDC_P0_MDASP, 0x00000027, 0x00000027, 0x00000017)
>
> Is it unreasonable to think there's a use case for 32-bit wide
> memory on a dual or quad?

Sure, they just would not use this file.

>
> What happens when we populate 256Mx16 DDR chips?
>
> Sabre Auto is already doing this.

Again, definitely a new file would be needed.

>
>> /*
>> - * MDCTL, CS0 enable, CS1 disabled, row=14, col=10, burst=8,
>> width=64/32bit
>> - * mx6q : row+col+bank+width=14+10+3+3=30 = 1G
>> + * MDCTL, CS0 enable, CS1 disabled, row=14, col=10, burst=8
>> + * MX6Q/DL: width=64bit row+col+bank+width=14+10+3+3=30 = 1G
>> + * MX6SOLO: width=32bit row+col+bank+width=14+10+3+2=29 = 512M
>> */
>> -WRITE_ENTRY1(MMDC_P0_MDCTL, 0x831A0000)
>> +WRITE_ENTRY3(MMDC_P0_MDCTL, 0x831A0000, 0x831A0000, 0x83190000)
>>
>> -/* MDSCR, con_req, LOAD MR2, CS0, A3,A10 set (CAS Write=6), RZQ/2 */
>> -WRITE_ENTRY1(MMDC_P0_MDSCR, 0x04088032)
>> +/*
>> + * LOAD MR2: MDSCR, con_req, CS0, A10 set - RZQ/2
>> + * MX6Q: A3 set(CAS Write=6)
>> + * MX6DL/SOLO: (CAS Write=5)
>> + */
>> +WRITE_ENTRY2(MMDC_P0_MDSCR, 0x04088032, 0x04008032)
>> /* LOAD MR3, CS0 */
>> WRITE_ENTRY1(MMDC_P0_MDSCR, 0x00008033)
>> -/* LOAD MR1, CS0, A1,A6 set Rtt=RZQ/2, ODI=RZQ/7 */
>> -WRITE_ENTRY1(MMDC_P0_MDSCR, 0x00428031)
>> -/* LOAD MR0, CS0, A6,A8,A11 set CAS=8, WR=8, DLL reset */
>> -WRITE_ENTRY1(MMDC_P0_MDSCR, 0x09408030)
>> +
>> +/*
>> + * LOAD MR1, CS0
>> + * MX6Q: A6 set: Rtt=RZQ/2, A1 set: ODI=RZQ/7
>> + * MX6DL/SOLO: A2 set: Rtt=RZQ/4, ODI=RZQ/6
>> + */
>> +WRITE_ENTRY2(MMDC_P0_MDSCR, 0x00428031, 0x00048031)
>> +
>> +/* LOAD MR0, CS0 A8 set: DLL Reset
>> + * MX6Q: A6 set: CAS=8 A11 set: WR=8
>> + * MX6DL/SOLO: A4 set: CAS=5, A9,A10 set: WR=7
>> + */
>> +WRITE_ENTRY2(MMDC_P0_MDSCR, 0x09408030, 0x07208030)
>>
>> /* ZQ calibrate, CS0 */
>> WRITE_ENTRY1(MMDC_P0_MDSCR, 0x04008040)
>> @@ -173,18 +227,18 @@ WRITE_ENTRY1(MMDC_P0_MPODTCTRL, 0x00022227)
>> WRITE_ENTRY1(MMDC_P1_MPODTCTRL, 0x00022227)
>>
>> /* MPDGCTRL0/1 DQS GATE*/
>> -WRITE_ENTRY1(MMDC_P0_MPDGCTRL0, 0x434B0350)
>> -WRITE_ENTRY1(MMDC_P0_MPDGCTRL1, 0x034C0359)
>> -WRITE_ENTRY1(MMDC_P1_MPDGCTRL0, 0x434B0350)
>> -WRITE_ENTRY1(MMDC_P1_MPDGCTRL1, 0x03650348)
>> -WRITE_ENTRY1(MMDC_P0_MPRDDLCTL, 0x4436383B)
>> -WRITE_ENTRY1(MMDC_P1_MPRDDLCTL, 0x39393341)
>> -WRITE_ENTRY1(MMDC_P0_MPWRDLCTL, 0x35373933)
>> -WRITE_ENTRY1(MMDC_P1_MPWRDLCTL, 0x48254A36)
>> -WRITE_ENTRY1(MMDC_P0_MPWLDECTRL0, 0x001F001F)
>> -WRITE_ENTRY1(MMDC_P0_MPWLDECTRL1, 0x001F001F)
>> -WRITE_ENTRY1(MMDC_P1_MPWLDECTRL0, 0x00440044)
>> -WRITE_ENTRY1(MMDC_P1_MPWLDECTRL1, 0x00440044)
>> +WRITE_ENTRY2(MMDC_P0_MPDGCTRL0, 0x434B0350, 0x42350231)
>> +WRITE_ENTRY2(MMDC_P0_MPDGCTRL1, 0x034C0359, 0x021A0218)
>> +WRITE_ENTRY2(MMDC_P1_MPDGCTRL0, 0x434B0350, 0x42350231)
>> +WRITE_ENTRY2(MMDC_P1_MPDGCTRL1, 0x03650348, 0x021A0218)
>> +WRITE_ENTRY2(MMDC_P0_MPRDDLCTL, 0x4436383B, 0x4B4B4E49)
>> +WRITE_ENTRY2(MMDC_P1_MPRDDLCTL, 0x39393341, 0x4B4B4E49)
>> +WRITE_ENTRY2(MMDC_P0_MPWRDLCTL, 0x35373933, 0x3F3F3035)
>> +WRITE_ENTRY2(MMDC_P1_MPWRDLCTL, 0x48254A36, 0x3F3F3035)
>> +WRITE_ENTRY2(MMDC_P0_MPWLDECTRL0, 0x001F001F, 0x0040003C)
>> +WRITE_ENTRY2(MMDC_P0_MPWLDECTRL1, 0x001F001F, 0x0032003E)
>> +WRITE_ENTRY2(MMDC_P1_MPWLDECTRL0, 0x00440044, 0x0040003C)
>> +WRITE_ENTRY2(MMDC_P1_MPWLDECTRL1, 0x00440044, 0x0032003E)
>>
>> /* MPMUR0 - Complete calibration by forced measurement */
>> WRITE_ENTRY1(MMDC_P0_MPMUR0, 0x00000800)
>
> It appears that only the memory configuration registers use
> WRITE_ENTRY2 or WRITE_ENTRY3 macros, and if so, I'd much
> rather see three separate files expressing the values, i.e.:
>
> mx6q_4x_mt41j128.cfg
> mx6dl_4x_mt41j128.cfg
> mx6s_2x_mt41j128.cfg

That is definitely a valid option. But you lose the common settings
of the iomux registers. Cut-n-pasting the files isn't so bad I guess,
but updates such as
mx6q_4x_mt41j128.cfg: use ddr3 mode for reset

Bits 19-18 of IOMUXC_IOMUXC_SW_PAD_CTL_PAD_DRAM_RESET
should be 3 for DDR3 mode. The current value of 0 is
reserved in TRM.

Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com>

diff --git a/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg
b/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg
index b859e2f..9c622c8 100644
--- a/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg
+++ b/board/freescale/imx/ddr/mx6q_4x_mt41j128.cfg
@@ -72,7 +72,7 @@ WRITE_ENTRY1(IOM_DRAM_RAS, 0x00020030)
WRITE_ENTRY1(IOM_DRAM_SDCLK_0, 0x00020030)
WRITE_ENTRY1(IOM_DRAM_SDCLK_1, 0x00020030)

-WRITE_ENTRY1(IOM_DRAM_RESET, 0x00020030)
+WRITE_ENTRY1(IOM_DRAM_RESET, 0x000e0030)
WRITE_ENTRY1(IOM_DRAM_SDCKE0, 0x00003000)
WRITE_ENTRY1(IOM_DRAM_SDCKE1, 0x00003000)
WRITE_ENTRY1(IOM_DRAM_SDBA2, 0x00000000)

Would then need to be applied to all three files.

>
> I'm not certain the processor part of the name is the
> right designator though. If I understand correctly,
> the differences in values between 6q and 6dl are mostly
> based on the memory speed.
>
> Is that right?

Correct...

>
> It also seems valid that a board would want to use an
> i.mx6quad with a lower memory bus speed.

Especially if there is a layout problem.

>
> If the address differences are taken care of by the
> preprocessor, it seems that a set like this would
> be more appropriate (and processor independent):
>
> mx6_4x_mt41j128_1066.cfg
> mx6_4x_mt41j128_800.cfg
> mx6_2x_mt41j128_800.cfg

Right, this is definitely a valid option. And if different
boards calibrated their DDR to the same settings
(DQS and byte delays) it would make more sense.

But I still see this file as board specific and not so much
memory type specific.

>
> By splitting up the files, we can still check for
> differences between them using 'diff', and it will
> be easier to extend the set.
>
> Note that meaningful diffs will require the use of
> symbolic names rather than varying addresses for
> registers.
>
> It's also conceivable that calibration on a given
> layout will require per-board tweaks, but that's
> not clear at the moment.
>
> You've clearly been through this in more detail than
> I have. Please let me know your thoughts.
>
> Regards,
>
>
> Eric
>