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. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
