[U-Boot] [PATCH V4 3/3] Add support for the LaCie ED Mini V2 board
Albert ARIBAUD
albert.aribaud at free.fr
Wed Jan 13 18:17:30 CET 2010
Prafulla Wadaskar a écrit :
>> I can add some comments, although here most of the comments
>> would simply
>> paraphrase the code one way or the other, e.g.
>>
>>>> + info->device_id = 0xBA; /* device ID of the
>> MX29LV400CB is 0xBA */
>
> This level documentation is good rather than nothing :-)
Ok, but I prefer to summarize it above the assignments, e.g. 'commands
and unlock addresses are AMD-compliant for an 8-bit mode, 8-bit bus
device' is enough to let the reader understand the assignments to
portwidth, chipwidth, vendor, cmd_reset, interface, addr_unlock1 and
addr_unlock2; I'll make sure all fields are covered.
> ...snip...
>>>> +
>>>> +lowlevel_init:
>>>> +
>>>> + /* Use 'r4 as the base for internal register accesses */
>>>> + ldr r4, =EDMINIV2_INTERNAL_BASE
>>>> +
>>>> + /* move internal registers from the default 0xD0000000
>>>> + * to their intended location of 0xf1000000 */
>>>> + ldr r3, =0xD0000000
>>>> + add r3, r3, #0x20000
>>>> + str r4, [r3, #0x80]
>>>> +
>>>> + /* Use R3 as the base for Device Bus registers */
>>>> + add r3, r4, #0x10000
>>>> +
>>>> + /* init MPPs */
>>>> + ldr r6, =EDMINIV2_MPP0_7
>>>> + str r6, [r3, #0x000]
>>>> + ldr r6, =EDMINIV2_MPP8_15
>>>> + str r6, [r3, #0x004]
>>>> + ldr r6, =EDMINIV2_MPP16_23
>>>> + str r6, [r3, #0x050]
>>>> +
>>>> + /* init GPIOs */
>>>> + ldr r6, =EDMINIV2_GPIO_OUT_ENABLE
>>>> + str r6, [r3, #0x104]
>>>> +
>>>> + /* Use R3 as the base for DRAM registers */
>>>> + add r3, r4, #0x01000
>>>> +
>>>> + /*DDR SDRAM Initialization Control */
>>>> + ldr r6, =0x00000001
>>>> + str r6, [r3, #0x480]
>>>> +
>>>> + /* Use R3 as the base for PCI registers */
>>>> + add r3, r4, #0x31000
>>>> +
>>>> + /* Disable arbiter */
>>>> + ldr r6, =0x00000030
>>>> + str r6, [r3, #0xd00]
>>>> +
>>>> + /* Use R3 as the base for DRAM registers */
>>>> + add r3, r4, #0x01000
>>>> +
>>>> + /* set all dram windows to 0 */
>>>> + mov r6, #0
>>>> + str r6, [r3, #0x504]
>>>> + str r6, [r3, #0x50C]
>>>> + str r6, [r3, #0x514]
>>>> + str r6, [r3, #0x51C]
>>>> +
>>>> + /* 1) Configure SDRAM */
>>>> + ldr r6, =SDRAM_CONFIG
>>>> + str r6, [r3, #0x400]
>>>> +
>>>> + /* 2) Set SDRAM Control reg */
>>>> + ldr r6, =SDRAM_CONTROL
>>>> + str r6, [r3, #0x404]
>>>> +
>>>> + /* 3) Write SDRAM address control register */
>>>> + ldr r6, =SDRAM_ADDR_CTRL
>>>> + str r6, [r3, #0x410]
>>>> +
>>>> + /* 4) Write SDRAM bank 0 size register */
>>>> + ldr r6, =SDRAM_BANK0_SIZE
>>>> + str r6, [r3, #0x504]
>>>> + /* keep other banks disabled */
>>>> +
>>>> + /* 5) Write SDRAM open pages control register */
>>>> + ldr r6, =SDRAM_OPEN_PAGE_EN
>>>> + str r6, [r3, #0x414]
>>>> +
>>>> + /* 6) Write SDRAM timing Low register */
>>>> + ldr r6, =SDRAM_TIME_CTRL_LOW
>>>> + str r6, [r3, #0x408]
>>>> +
>>>> + /* 7) Write SDRAM timing High register */
>>>> + ldr r6, =SDRAM_TIME_CTRL_HI
>>>> + str r6, [r3, #0x40C]
>>>> +
>>>> + /* 8) Write SDRAM mode register */
>>>> + /* The CPU must not attempt to change the SDRAM Mode
>>>> register setting */
>>>> + /* prior to DRAM controller completion of the DRAM
>>>> initialization */
>>>> + /* sequence. To guarantee this restriction, it is
>>>> recommended that */
>>>> + /* the CPU sets the SDRAM Operation register to NOP
>>>> command, performs */
>>>> + /* read polling until the register is back in Normal
>>>> operation value, */
>>>> + /* and then sets SDRAM Mode register to its new
>>>> value. */
>>>> +
>>>> + /* 8.1 write 'nop' to SDRAM operation */
>>>> + ldr r6, =SDRAM_OP_NOP
>>>> + str r6, [r3, #0x418]
>>>> +
>>>> + /* 8.2 poll SDRAM operation until back in
>> 'normal' mode. */
>>>> +1:
>>>> + ldr r6, [r3, #0x418]
>>>> + cmp r6, #0
>>>> + bne 1b
>>>> +
>>>> + /* 8.3 Now its safe to write new value to SDRAM Mode
>>>> register */
>>>> + ldr r6, =SDRAM_MODE
>>>> + str r6, [r3, #0x41C]
>>>> +
>>>> + /* 8.4 Set new mode */
>>>> + ldr r6, =SDRAM_OP_SETMODE
>>>> + str r6, [r3, #0x418]
>>>> +
>>>> + /* 8.5 poll SDRAM operation until back in
>> 'normal' mode. */
>>>> +2:
>>>> + ldr r6, [r3, #0x418]
>>>> + cmp r6, #0
>>>> + bne 2b
>>>> +
>>>> + /* DDR SDRAM Address/Control Pads Calibration */
>>>> + ldr r6, [r3, #0x4C0]
>>>> +
>>>> + /* Set Bit [31] to make the register writable
>>>> */
>>>> + orr r6, r6, #SDRAM_PAD_CTRL_WR_EN
>>>> + str r6, [r3, #0x4C0]
>>>> +
>>>> + bic r6, r6, #SDRAM_PAD_CTRL_WR_EN
>>>> + bic r6, r6, #SDRAM_PAD_CTRL_TUNE_EN
>>>> + bic r6, r6, #SDRAM_PAD_CTRL_DRVN_MASK
>>>> + bic r6, r6, #SDRAM_PAD_CTRL_DRVP_MASK
>>>> +
>>>> + /* Get the final N locked value of driving strength
>>>> [22:17] */
>>>> + mov r1, r6
>>>> + mov r1, r1, LSL #9
>>>> + mov r1, r1, LSR #26 /* r1[5:0]<DrvN> =
>>>> r3[22:17]<LockN> */
>>>> + orr r1, r1, r1, LSL #6 /* r1[11:6]<DrvP> =
>>>> r1[5:0]<DrvN> */
>>>> +
>>>> + /* Write to both <DrvN> bits [5:0] and <DrvP> bits
>>>> [11:6] */
>>>> + orr r6, r6, r1
>>>> + str r6, [r3, #0x4C0]
>>>> +
>>>> + /* DDR SDRAM Data Pads Calibration
>>>> */
>>>> + ldr r6, [r3, #0x4C4]
>>>> +
>>>> + /* Set Bit [31] to make the register writable
>>>> */
>>>> + orr r6, r6, #SDRAM_PAD_CTRL_WR_EN
>>>> + str r6, [r3, #0x4C4]
>>>> +
>>>> + bic r6, r6, #SDRAM_PAD_CTRL_WR_EN
>>>> + bic r6, r6, #SDRAM_PAD_CTRL_TUNE_EN
>>>> + bic r6, r6, #SDRAM_PAD_CTRL_DRVN_MASK
>>>> + bic r6, r6, #SDRAM_PAD_CTRL_DRVP_MASK
>>>> +
>>>> + /* Get the final N locked value of driving strength
>>>> [22:17] */
>>>> + mov r1, r6
>>>> + mov r1, r1, LSL #9
>>>> + mov r1, r1, LSR #26
>>>> + orr r1, r1, r1, LSL #6 /* r1[5:0] = r3[22:17]<LockN> */
>>>> +
>>>> + /* Write to both <DrvN> bits [5:0] and <DrvP> bits
>>>> [11:6] */
>>>> + orr r6, r6, r1
>>>> +
>>>> + str r6, [r3, #0x4C4]
>>>> +
>>>> + /* Implement Guideline (GL# MEM-3) Drive Strength
>>>> Value */
>>>> + /* Relevant for: 88F5181-A1/B0/B1 and 88F5281-A0/B0
>>>> */
>>>> +
>>>> + ldr r1, =DDR1_PAD_STRENGTH_DEFAULT
>>>> +
>>>> + /* Enable writes to DDR SDRAM Addr/Ctrl Pads
>>>> Calibration register */
>>>> + ldr r6, [r3, #0x4C0]
>>>> + orr r6, r6, #SDRAM_PAD_CTRL_WR_EN
>>>> + str r6, [r3, #0x4C0]
>>>> +
>>>> + /* Correct strength and disable writes again */
>>>> + bic r6, r6, #SDRAM_PAD_CTRL_WR_EN
>>>> + bic r6, r6, #SDRAM_PAD_CTRL_DRV_STR_MASK
>>>> + orr r6, r6, r1
>>>> + str r6, [r3, #0x4C0]
>>>> +
>>>> + /* Enable writes to DDR SDRAM Data Pads Calibration
>> register */
>>>> + ldr r6, [r3, #0x4C4]
>>>> + orr r6, r6, #SDRAM_PAD_CTRL_WR_EN
>>>> + str r6, [r3, #0x4C4]
>>>> +
>>>> + /* Correct strength and disable writes again */
>>>> + bic r6, r6, #SDRAM_PAD_CTRL_DRV_STR_MASK
>>>> + bic r6, r6, #SDRAM_PAD_CTRL_WR_EN
>>>> + orr r6, r6, r1
>>>> + str r6, [r3, #0x4C4]
>>>> +
>>>> + /* Implement Guideline (GL# MEM-4) DQS Reference
>>>> Delay Tuning */
>>>> + /* Relevant for: 88F5181-A1/B0/B1 and 88F5281-A0/B0
>>>> */
>>>> +
>>>> + /* Get the "sample on reset" register for the DDR
>>>> frequancy */
>>>> + ldr r3, =0x10000
>>>> + ldr r6, [r3, #0x010]
>>>> + ldr r1, =MSAR_ARMDDRCLCK_MASK
>>>> + and r1, r6, r1
>>>> +
>>>> + ldr r6, =FTDLL_DDR1_166MHZ
>>>> + cmp r1, #MSAR_ARMDDRCLCK_333_167
>>>> + beq 3f
>>>> + cmp r1, #MSAR_ARMDDRCLCK_500_167
>>>> + beq 3f
>>>> + cmp r1, #MSAR_ARMDDRCLCK_667_167
>>>> + beq 3f
>>>> +
>>>> + ldr r6, =FTDLL_DDR1_200MHZ
>>>> + cmp r1, #MSAR_ARMDDRCLCK_400_200_1
>>>> + beq 3f
>>>> + cmp r1, #MSAR_ARMDDRCLCK_400_200
>>>> + beq 3f
>>>> + cmp r1, #MSAR_ARMDDRCLCK_600_200
>>>> + beq 3f
>>>> + cmp r1, #MSAR_ARMDDRCLCK_800_200
>>>> + beq 3f
>>>> +
>>> As reported earlier comment for v3,
>>> this should only have simple DRAM initialization, which is
>> only dependency to copy and start binary.
>>
>> Hmm... Those are fixes to allow/ensure DDRAM access, so I'd
>> say this is
>> a dependency to copy and start the binary.
>
> The code here looks very long
> It's purpose is to initialize certain CPU registers.
> For specific board there is no need of any conditional initializations.
>
> Similar to board/Marvell/Sheevaplug/kwbimage.cfg, can you abstract a data for CPU registers and values to be initialized through some data structures and a small function to read and copy them to respective registers?
>
> That will give better readability and easy updates for future users.
It's not only setting registers, there are some loops, so several
register+value tables would be needed, but yes, I can give it a shot.
> lets keep least ASM code.
Ok.
> We cannot avoid lowlevel_init otherwise I could have preferred to omit it.
I would have too, but there's no choice there.
>>> Common to SoC stuff to be moved to cpu.c/h
>> This I can agree upon, however I don't see much that is common across
>> SoCs here. Does that warrant creating a function at SoC level
>> which will
>> do practically nothing?
>
> For ex. Mpp_init, GPIO_init and other init can go in cpu.c,
> You can declare respective init macros in edminv2.h and function calls in edminv2.c
> Thus those will be re-usable for other orion5X boards.
>
> Also you can do basic CPU registers initialization as suggested above and
> Further tuning like reading "sample on reset" and updating some specific registers can be pushed in cpu.c
> Because this will be a standard need to all board using this SoC
>
> What do you think?
I'll give a shot at moving as much code from lowlevel_init as I can into
cpu.c with constants in edminiv2.h.
> Regards.
> Prafulla . .
Amicalement,
--
Albert.
More information about the U-Boot
mailing list