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 theMX29LV400CB 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, =0xD0000000add r3, r3, #0x20000str r4, [r3, #0x80]/* Use R3 as the base for Device Bus registers */add r3, r4, #0x10000/* init MPPs */ldr r6, =EDMINIV2_MPP0_7str r6, [r3, #0x000]ldr r6, =EDMINIV2_MPP8_15str r6, [r3, #0x004]ldr r6, =EDMINIV2_MPP16_23str r6, [r3, #0x050]/* init GPIOs */ldr r6, =EDMINIV2_GPIO_OUT_ENABLEstr r6, [r3, #0x104]/* Use R3 as the base for DRAM registers */add r3, r4, #0x01000/*DDR SDRAM Initialization Control */ldr r6, =0x00000001str r6, [r3, #0x480]/* Use R3 as the base for PCI registers */add r3, r4, #0x31000/* Disable arbiter */ldr r6, =0x00000030str r6, [r3, #0xd00]/* Use R3 as the base for DRAM registers */add r3, r4, #0x01000/* set all dram windows to 0 */mov r6, #0str r6, [r3, #0x504]str r6, [r3, #0x50C]str r6, [r3, #0x514]str r6, [r3, #0x51C]/* 1) Configure SDRAM */ldr r6, =SDRAM_CONFIGstr r6, [r3, #0x400]/* 2) Set SDRAM Control reg */ldr r6, =SDRAM_CONTROLstr r6, [r3, #0x404]/* 3) Write SDRAM address control register */ldr r6, =SDRAM_ADDR_CTRLstr r6, [r3, #0x410]/* 4) Write SDRAM bank 0 size register */ldr r6, =SDRAM_BANK0_SIZEstr r6, [r3, #0x504]/* keep other banks disabled *//* 5) Write SDRAM open pages control register */ldr r6, =SDRAM_OPEN_PAGE_ENstr r6, [r3, #0x414]/* 6) Write SDRAM timing Low register */ldr r6, =SDRAM_TIME_CTRL_LOWstr r6, [r3, #0x408]/* 7) Write SDRAM timing High register */ldr r6, =SDRAM_TIME_CTRL_HIstr r6, [r3, #0x40C]/* 8) Write SDRAM mode register *//* The CPU must not attempt to change the SDRAM Moderegister setting */
/* prior to DRAM controller completion of the DRAMinitialization */
/* sequence. To guarantee this restriction, it isrecommended that */
/* the CPU sets the SDRAM Operation register to NOPcommand, performs */
/* read polling until the register is back in Normaloperation value, */
/* and then sets SDRAM Mode register to its newvalue. */
/* 8.1 write 'nop' to SDRAM operation */ldr r6, =SDRAM_OP_NOPstr r6, [r3, #0x418]/* 8.2 poll SDRAM operation until back in'normal' mode. */
+1:
ldr r6, [r3, #0x418]cmp r6, #0bne 1b/* 8.3 Now its safe to write new value to SDRAM Moderegister */
ldr r6, =SDRAM_MODEstr r6, [r3, #0x41C]/* 8.4 Set new mode */ldr r6, =SDRAM_OP_SETMODEstr r6, [r3, #0x418]/* 8.5 poll SDRAM operation until back in'normal' mode. */
+2:
ldr r6, [r3, #0x418]cmp r6, #0bne 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_ENstr r6, [r3, #0x4C0]bic r6, r6, #SDRAM_PAD_CTRL_WR_ENbic r6, r6, #SDRAM_PAD_CTRL_TUNE_ENbic r6, r6, #SDRAM_PAD_CTRL_DRVN_MASKbic r6, r6, #SDRAM_PAD_CTRL_DRVP_MASK/* Get the final N locked value of driving strength[22:17] */
mov r1, r6mov r1, r1, LSL #9mov 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, r1str 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_ENstr r6, [r3, #0x4C4]bic r6, r6, #SDRAM_PAD_CTRL_WR_ENbic r6, r6, #SDRAM_PAD_CTRL_TUNE_ENbic r6, r6, #SDRAM_PAD_CTRL_DRVN_MASKbic r6, r6, #SDRAM_PAD_CTRL_DRVP_MASK/* Get the final N locked value of driving strength[22:17] */
mov r1, r6mov r1, r1, LSL #9mov r1, r1, LSR #26orr 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, r1str r6, [r3, #0x4C4]/* Implement Guideline (GL# MEM-3) Drive StrengthValue */
/* Relevant for: 88F5181-A1/B0/B1 and 88F5281-A0/B0 */ldr r1, =DDR1_PAD_STRENGTH_DEFAULT/* Enable writes to DDR SDRAM Addr/Ctrl PadsCalibration register */
ldr r6, [r3, #0x4C0]orr r6, r6, #SDRAM_PAD_CTRL_WR_ENstr r6, [r3, #0x4C0]/* Correct strength and disable writes again */bic r6, r6, #SDRAM_PAD_CTRL_WR_ENbic r6, r6, #SDRAM_PAD_CTRL_DRV_STR_MASKorr r6, r6, r1str r6, [r3, #0x4C0]/* Enable writes to DDR SDRAM Data Pads Calibrationregister */
ldr r6, [r3, #0x4C4]orr r6, r6, #SDRAM_PAD_CTRL_WR_ENstr r6, [r3, #0x4C4]/* Correct strength and disable writes again */bic r6, r6, #SDRAM_PAD_CTRL_DRV_STR_MASKbic r6, r6, #SDRAM_PAD_CTRL_WR_ENorr r6, r6, r1str r6, [r3, #0x4C4]/* Implement Guideline (GL# MEM-4) DQS ReferenceDelay Tuning */
/* Relevant for: 88F5181-A1/B0/B1 and 88F5281-A0/B0 *//* Get the "sample on reset" register for the DDRfrequancy */
ldr r3, =0x10000ldr r6, [r3, #0x010]ldr r1, =MSAR_ARMDDRCLCK_MASKand r1, r6, r1ldr r6, =FTDLL_DDR1_166MHZcmp r1, #MSAR_ARMDDRCLCK_333_167beq 3fcmp r1, #MSAR_ARMDDRCLCK_500_167beq 3fcmp r1, #MSAR_ARMDDRCLCK_667_167beq 3fldr r6, =FTDLL_DDR1_200MHZcmp r1, #MSAR_ARMDDRCLCK_400_200_1beq 3fcmp r1, #MSAR_ARMDDRCLCK_400_200beq 3fcmp r1, #MSAR_ARMDDRCLCK_600_200beq 3fcmp r1, #MSAR_ARMDDRCLCK_800_200beq 3fAs 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,