Hi Tim,
On 24/04/2014 10:06, Tim Harvey wrote:
+/* configure mx6 mmdc io registers */ +struct mx6_mmdc_ioregs mmdc_ioregs = {
/* DDR3 */.mmdc_grp_ddr_type = 0x000c0000,/* disable DDR pullups */.mmdc_grp_ddrpke = 0x00000000,/* SDCLK[0:1], CAS, RAS, Reset: Differential input, 40ohm */.mmdc_dram_sdclk_0 = 0x00020030,.mmdc_dram_sdclk_1 = 0x00020030,.mmdc_dram_cas = 0x00020030,.mmdc_dram_ras = 0x00020030,.mmdc_dram_reset = 0x00020030,/* ADDR[00:16], SDBA[0:1]: 40 ohm */.mmdc_grp_addds = 0x00000030,/* SDCKE[0:1]: 100k pull-up */.mmdc_dram_sdcke0 = 0x00003000,.mmdc_dram_sdcke1 = 0x00003000,/* SDBA2: pull-up disabled */.mmdc_dram_sdba2 = 0x00000000,/* SDODT[0:1]: 100k pull-up, 40 ohm */.mmdc_dram_sdodt0 = 0x00003030,.mmdc_dram_sdodt1 = 0x00003030,/* CS0/CS1/SDBA2/CKE0/CKE1/SDWE: 40 ohm */.mmdc_grp_ctlds = 0x00000030,/* SDQS[0:7]: Differential input, 40 ohm */.mmdc_ddrmode_ctl = 0x00020000,.mmdc_dram_sdqs0 = 0x00000030,.mmdc_dram_sdqs1 = 0x00000030,.mmdc_dram_sdqs2 = 0x00000030,.mmdc_dram_sdqs3 = 0x00000030,.mmdc_dram_sdqs4 = 0x00000030,.mmdc_dram_sdqs5 = 0x00000030,.mmdc_dram_sdqs6 = 0x00000030,.mmdc_dram_sdqs7 = 0x00000030,/* DATA[00:63]: Differential input, 40 ohm */.mmdc_grp_ddrmode = 0x00020000,.mmdc_grp_b0ds = 0x00000030,.mmdc_grp_b1ds = 0x00000030,.mmdc_grp_b2ds = 0x00000030,.mmdc_grp_b3ds = 0x00000030,.mmdc_grp_b4ds = 0x00000030,.mmdc_grp_b5ds = 0x00000030,.mmdc_grp_b6ds = 0x00000030,.mmdc_grp_b7ds = 0x00000030,/* DQM[0:7]: Differential input, 40 ohm */.mmdc_dram_dqm0 = 0x00020030,.mmdc_dram_dqm1 = 0x00020030,.mmdc_dram_dqm2 = 0x00020030,.mmdc_dram_dqm3 = 0x00020030,.mmdc_dram_dqm4 = 0x00020030,.mmdc_dram_dqm5 = 0x00020030,.mmdc_dram_dqm6 = 0x00020030,.mmdc_dram_dqm7 = 0x00020030,+};
I will suggest you move these structure in a separate file. It is then easier for a board developer to understand what is very board specific.
hmmm... they are all very board specific but ok.
Well, I am noy saying to move it outside board/gateworks/gw_ventana. My proposal is more as to signalize how to add SPL support for a new board. Anyway, I agree it is a personal taste.
not needed - I used it mainly for debugging and will remove it.
board_init_r(NULL, 0);+}
Mmhhh...apart the access to the eeprom to get the ram size, this function should be common.
maybe, but I think we should wait to see what other boards come up with SPL support to see what actually ends up being common. An SPL that supports SPI or some of the other boot devices may need to do some additional things.
Agree, this makes sense. We can do it later.
yes... considering we currently have 4 baseboards (a fifth on the way), supporting both IMX6DL or IMX6Q on each, and have 2 memory densities (a third on the way) SPL is a must for my sanity to eliminate what would be something like 5*2*3 various build-time configurations.
The dynamic ddr configuration functionality I'm proposing helps out tremendously as well because the DDR3 calibration and testing I've done tells me that each board design (varied layout between SoC and DDR3) needs its own calibration values due to propagation delays and IMX6Q/D vs IMX6DL/SOLO need different calibration values on each board as well. This can all be handled with minimal tables and easily configured at runtime via baseboard detection, cpu detection, and memory density information.
Very nice.
+#define CONFIG_SYS_NAND_U_BOOT_OFFS (14 * 1024 * 1024)
This is ok - it is your decision where to put it.
May I ask why do you need 14 MB at the beginning ? It seems you lose a lot of place. NAND is cheap nowadays, but...
it was perhaps a poor decision I made early on when I was following too much of the reference design without understanding all the details. They used a 16MB area in NAND for the bootstreams so that is what I used as well. I worked through the calculation once and the 16MB wasn't all that crazy considering at the time I was wanting enough room to support a 1MB u-boot. When you add the blocks and redundant blocks for FCB/DBBT (default is to have 2 copies of these), then double the bootloader size (because IMX BOOT ROM supports 2 firmware images for redundancy), and factor in 20% headroom to allow for bad blocks, the 16MB wasn't all that crazy. Now that I'm talking about using 14MB for a <64KB SPL image, its overkill for sure.
I don't want to impose a partition layout change on our existing users that I want to update to the SPL bootloader (with all the improved DDR3 calibration values) so I figure we'll put u-boot.img in the last 2MB of the 16MB partition for the 'bootloader' and leave the first 14MB for SPL to be flashed according to the IMX6 BOOT ROM.
ok, thanks for explanation
+#include "imx6_spl.h" /* common IMX6 SPL configuration */ #include "mx6_common.h" #define CONFIG_MX6 #define CONFIG_DISPLAY_CPUINFO /* display cpu info */ @@ -242,7 +253,7 @@ "mtdparts=nor:512k(uboot),64k(env),2m(kernel),-(rootfs)" #else #define MTDIDS_DEFAULT "nand0=nand" -#define MTDPARTS_DEFAULT "mtdparts=nand:16m(uboot),1m(env),-(rootfs)" +#define MTDPARTS_DEFAULT "mtdparts=nand:14m(spl),2m(uboot),1m(env),-(rootfs)"
and I'll be reverting that last change as well and leaving the original 16M partition for the 'bootloader' meaning the 14MB area flashed by kobs-ng according to the IMX6 BOOT ROM requirements for NAND boot as well as the 2MB area I'm reserving for u-boot.img.
If I were to split the partitions like the above change, it will cause some grief for existing users that are using the 3.0.35 (non device-tree) vendor kernel that has the mtd partitions hard coded in the board support. Instead, I decided to patch the kobs-ng application used to flash the SPL so that it can be passed a max size to use within /dev/mtd0 and users that need to upgrade to the SPL bootloader will need to use that patched kobs-ng to flash the SPL to the first 14MB, then use std mtd utils to flash u-boot.img in the area between 14M and 16M.
Don't forget, at some point soon I hope to add some functionality to u-boot to flash the SPL portion to nand (the way kobs-ng does) so that you don't need to boot to linux and use kobs-ng or use our JTAG tool.
This will be a great improvement ! Thanks for working on this issue.
Best regards, Stefano