| Message ID | 1415482079-9531-8-git-send-email-john.tobias.ph@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
Hi John, On 08/11/2014 22:27, John Tobias wrote: > This patch will enable the support for SPL on iMX6 SabreSD. > It tested on SD2 and SD3 mmc port. > --- > board/freescale/mx6sabresd/mx6sabresd.c | 211 +++++++++++++++++++++++++++++++- > 1 file changed, 209 insertions(+), 2 deletions(-) > > diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c > index 3d81fff..f443337 100644 > --- a/board/freescale/mx6sabresd/mx6sabresd.c > +++ b/board/freescale/mx6sabresd/mx6sabresd.c > @@ -55,8 +55,7 @@ DECLARE_GLOBAL_DATA_PTR; > > int dram_init(void) > { > - gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE); > - > + gd->ram_size = imx_ddr_size(); > return 0; > } > > @@ -607,3 +606,211 @@ int checkboard(void) > puts("Board: MX6-SabreSD\n"); > return 0; > } > + > +#ifdef CONFIG_SPL_BUILD > +#include <spl.h> > +#include <libfdt.h> > + > +#define BOOT_CFG 0x20D8004 > +#define __REG(x) (*((volatile u32 *)(x))) > + > +struct fsl_esdhc_cfg spl_usdhc_cfg; > +/* > + * Got it from mx6q_4x_mt41j128.cfg file > + */ > +void set_mt41j128_ddr(void) > +{ > + __REG(0x020e05a8) = 0x00000028; > + __REG(0x020e05b0) = 0x00000028; > + __REG(0x020e0524) = 0x00000028; > + __REG(0x020e051c) = 0x00000028; > + > + __REG(0x020e0518) = 0x00000028; > + __REG(0x020e050c) = 0x00000028; > + __REG(0x020e05b8) = 0x00000028; > + __REG(0x020e05c0) = 0x00000028; > + > + __REG(0x020e05ac) = 0x00000028; > + __REG(0x020e05b4) = 0x00000028; > + __REG(0x020e0528) = 0x00000028; > + __REG(0x020e0520) = 0x00000028; > + > + __REG(0x020e0514) = 0x00000028; > + __REG(0x020e0510) = 0x00000028; > + __REG(0x020e05bc) = 0x00000028; > + __REG(0x020e05c4) = 0x00000028; > + > + __REG(0x020e056c) = 0x00000030; > + __REG(0x020e0578) = 0x00000030; > + __REG(0x020e0588) = 0x00000030; > + __REG(0x020e0594) = 0x00000030; > + > + __REG(0x020e057c) = 0x00000030; > + __REG(0x020e0590) = 0x00000030; > + __REG(0x020e0598) = 0x00000030; > + __REG(0x020e058c) = 0x00000000; > + > + __REG(0x020e059c) = 0x00003030; > + __REG(0x020e05a0) = 0x00003030; > + __REG(0x020e0784) = 0x00000028; > + __REG(0x020e0788) = 0x00000028; > + > + __REG(0x020e0794) = 0x00000028; > + __REG(0x020e079c) = 0x00000028; > + __REG(0x020e07a0) = 0x00000028; > + __REG(0x020e07a4) = 0x00000028; > + > + __REG(0x020e07a8) = 0x00000028; > + __REG(0x020e0748) = 0x00000028; > + __REG(0x020e074c) = 0x00000030; > + __REG(0x020e0750) = 0x00020000; > + > + __REG(0x020e0758) = 0x00000000; > + __REG(0x020e0774) = 0x00020000; > + __REG(0x020e078c) = 0x00000030; > + __REG(0x020e0798) = 0x000C0000; > + > + __REG(0x021b081c) = 0x33333333; > + __REG(0x021b0820) = 0x33333333; > + __REG(0x021b0824) = 0x33333333; > + __REG(0x021b0828) = 0x33333333; > + > + __REG(0x021b481c) = 0x33333333; > + __REG(0x021b4820) = 0x33333333; > + __REG(0x021b4824) = 0x33333333; > + __REG(0x021b4828) = 0x33333333; > + > + __REG(0x021b0018) = 0x00001740; > + > + __REG(0x021b001c) = 0x00008000; > + __REG(0x021b000c) = 0x8A8F7975; > + __REG(0x021b0010) = 0xFF538E64; > + __REG(0x021b0014) = 0x01FF00DB; > + __REG(0x021b002c) = 0x000026D2; > + > + __REG(0x021b0030) = 0x008F0E21; > + __REG(0x021b0008) = 0x09444040; > + __REG(0x021b0004) = 0x00020036; > + __REG(0x021b0040) = 0x00000047; > + __REG(0x021b0000) = 0x841A0000; > + > + __REG(0x021b001c) = 0x04088032; > + __REG(0x021b001c) = 0x00008033; > + __REG(0x021b001c) = 0x00428031; > + __REG(0x021b001c) = 0x09408030; > + > + __REG(0x021b001c) = 0x04008040; > + __REG(0x021b0800) = 0xA1380003; > + __REG(0x021b0020) = 0x00005800; > + __REG(0x021b0818) = 0x00000007; > + __REG(0x021b4818) = 0x00000007; > + > + /* Calibration values based on ARD and 528MHz */ > + __REG(0x021b083c) = 0x434B0358; > + __REG(0x021b0840) = 0x033D033C; > + __REG(0x021b483c) = 0x03520362; > + __REG(0x021b4840) = 0x03480318; > + __REG(0x021b0848) = 0x41383A3C; > + __REG(0x021b4848) = 0x3F3C374A; > + __REG(0x021b0850) = 0x42434444; > + __REG(0x021b4850) = 0x4932473A; > + > + __REG(0x021b080c) = 0x001F001F; > + __REG(0x021b0810) = 0x001F001F; > + > + __REG(0x021b480c) = 0x001F001F; > + __REG(0x021b4810) = 0x001F001F; > + > + __REG(0x021b08b8) = 0x00000800; > + __REG(0x021b48b8) = 0x00000800; > + > + __REG(0x021b0404) = 0x00011006; > + __REG(0x021b0004) = 0x00025576; > + > + __REG(0x021b001c) = 0x00000000; > + > + __REG(0x020c4068) = 0x00C03F3F; > + __REG(0x020c406c) = 0x0030FC00; > + __REG(0x020c4070) = 0x0FFFC000; > + __REG(0x020c4074) = 0x3FF00000; > + __REG(0x020c4078) = 0x00FFF300; > + __REG(0x020c407c) = 0x0F0000C3; > + __REG(0x020c4080) = 0x000003FF; > +} > + This cannot be accepted. Firstly, you cannot use fixed offset. The code you propose here is very difficult to maintain. Then, please take a look at the i.MX6 boards that are already supporting SPL (ventana and novena). There is a logical separation between iomux, calibration, and so on. You have to provide tables, and you should then call the common functions: mx6dq_dram_iocfg(); mx6_dram_cfg(); Again, please take a look at the boards I mentioned. > +/* > + * This section require the differentiation > + * between iMX6 Sabre Families. > + * But for now, it will configure only for > + * SabreSD. > + */ > +static void spl_dram_init(void) > +{ > + set_mt41j128_ddr(); > +} > + > +int spl_board_mmc_init(bd_t *bis) > +{ > + unsigned reg = readl(BOOT_CFG); > + /* > + * Upon reading BOOT_CFG register the following map is done: > + * (U-boot device node) (Physical Port) > + * 0x840 SD2 > + * 0x1040 SD3 > + * 0x1840 eMMC > + */ You add a new entry point here (spl_board_mmc_init), but why cannot you do this in the functions already supplied by SPL ? Why not in board_init_f ? > + switch (reg) { > + case 0x840: > + imx_iomux_v3_setup_multiple_pads( > + usdhc2_pads, ARRAY_SIZE(usdhc2_pads)); > + spl_usdhc_cfg.esdhc_base = USDHC2_BASE_ADDR; > + spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC2_CLK); > + gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk; > + break; > + case 0x1040: > + imx_iomux_v3_setup_multiple_pads( > + usdhc3_pads, ARRAY_SIZE(usdhc3_pads)); > + spl_usdhc_cfg.esdhc_base = USDHC3_BASE_ADDR; > + spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK); > + gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk; > + break; > + case 0x1840: > + imx_iomux_v3_setup_multiple_pads( > + usdhc4_pads, ARRAY_SIZE(usdhc4_pads)); > + spl_usdhc_cfg.esdhc_base = USDHC4_BASE_ADDR; > + spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC4_CLK); > + gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk; > + break; > + } > + > + return fsl_esdhc_initialize(bis, &spl_usdhc_cfg); > +} > + > +void board_init_f(ulong dummy) > +{ > + /* setup AIPS and disable watchdog */ > + arch_cpu_init(); > + > + /* iomux and setup of i2c */ > + board_early_init_f(); ?? A "early" function is generally called by common code at a different point. It makes no sense to call it inside board_init_f() > + > + /* setup GP timer */ > + timer_init(); > + Then I am expecting iomux setuup--- > + /* UART clocks enabled and gd valid - init serial console */ > + preloader_console_init(); > + > + /* DDR initialization */ > + spl_dram_init(); and here calling the already supplied functions. > + > + /* Clear the BSS. */ > + memset(__bss_start, 0, __bss_end - __bss_start); > + > + /* load/boot image from boot device */ > + board_init_r(NULL, 0); > +} > + > +void reset_cpu(ulong addr) > +{ > +} > +#endif > Best regards, Stefano Babic
Hi Stefano, On Sun, Nov 9, 2014 at 1:16 PM, Stefano Babic <sbabic@denx.de> wrote: > Hi John, > > On 08/11/2014 22:27, John Tobias wrote: >> This patch will enable the support for SPL on iMX6 SabreSD. >> It tested on SD2 and SD3 mmc port. >> --- >> board/freescale/mx6sabresd/mx6sabresd.c | 211 +++++++++++++++++++++++++++++++- >> 1 file changed, 209 insertions(+), 2 deletions(-) >> >> diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c >> index 3d81fff..f443337 100644 >> --- a/board/freescale/mx6sabresd/mx6sabresd.c >> +++ b/board/freescale/mx6sabresd/mx6sabresd.c >> @@ -55,8 +55,7 @@ DECLARE_GLOBAL_DATA_PTR; >> >> int dram_init(void) >> { >> - gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE); >> - >> + gd->ram_size = imx_ddr_size(); >> return 0; >> } >> >> @@ -607,3 +606,211 @@ int checkboard(void) >> puts("Board: MX6-SabreSD\n"); >> return 0; >> } >> + >> +#ifdef CONFIG_SPL_BUILD >> +#include <spl.h> >> +#include <libfdt.h> >> + >> +#define BOOT_CFG 0x20D8004 >> +#define __REG(x) (*((volatile u32 *)(x))) >> + >> +struct fsl_esdhc_cfg spl_usdhc_cfg; >> +/* >> + * Got it from mx6q_4x_mt41j128.cfg file >> + */ >> +void set_mt41j128_ddr(void) >> +{ >> + __REG(0x020e05a8) = 0x00000028; >> + __REG(0x020e05b0) = 0x00000028; >> + __REG(0x020e0524) = 0x00000028; >> + __REG(0x020e051c) = 0x00000028; >> + >> + __REG(0x020e0518) = 0x00000028; >> + __REG(0x020e050c) = 0x00000028; >> + __REG(0x020e05b8) = 0x00000028; >> + __REG(0x020e05c0) = 0x00000028; >> + >> + __REG(0x020e05ac) = 0x00000028; >> + __REG(0x020e05b4) = 0x00000028; >> + __REG(0x020e0528) = 0x00000028; >> + __REG(0x020e0520) = 0x00000028; >> + >> + __REG(0x020e0514) = 0x00000028; >> + __REG(0x020e0510) = 0x00000028; >> + __REG(0x020e05bc) = 0x00000028; >> + __REG(0x020e05c4) = 0x00000028; >> + >> + __REG(0x020e056c) = 0x00000030; >> + __REG(0x020e0578) = 0x00000030; >> + __REG(0x020e0588) = 0x00000030; >> + __REG(0x020e0594) = 0x00000030; >> + >> + __REG(0x020e057c) = 0x00000030; >> + __REG(0x020e0590) = 0x00000030; >> + __REG(0x020e0598) = 0x00000030; >> + __REG(0x020e058c) = 0x00000000; >> + >> + __REG(0x020e059c) = 0x00003030; >> + __REG(0x020e05a0) = 0x00003030; >> + __REG(0x020e0784) = 0x00000028; >> + __REG(0x020e0788) = 0x00000028; >> + >> + __REG(0x020e0794) = 0x00000028; >> + __REG(0x020e079c) = 0x00000028; >> + __REG(0x020e07a0) = 0x00000028; >> + __REG(0x020e07a4) = 0x00000028; >> + >> + __REG(0x020e07a8) = 0x00000028; >> + __REG(0x020e0748) = 0x00000028; >> + __REG(0x020e074c) = 0x00000030; >> + __REG(0x020e0750) = 0x00020000; >> + >> + __REG(0x020e0758) = 0x00000000; >> + __REG(0x020e0774) = 0x00020000; >> + __REG(0x020e078c) = 0x00000030; >> + __REG(0x020e0798) = 0x000C0000; >> + >> + __REG(0x021b081c) = 0x33333333; >> + __REG(0x021b0820) = 0x33333333; >> + __REG(0x021b0824) = 0x33333333; >> + __REG(0x021b0828) = 0x33333333; >> + >> + __REG(0x021b481c) = 0x33333333; >> + __REG(0x021b4820) = 0x33333333; >> + __REG(0x021b4824) = 0x33333333; >> + __REG(0x021b4828) = 0x33333333; >> + >> + __REG(0x021b0018) = 0x00001740; >> + >> + __REG(0x021b001c) = 0x00008000; >> + __REG(0x021b000c) = 0x8A8F7975; >> + __REG(0x021b0010) = 0xFF538E64; >> + __REG(0x021b0014) = 0x01FF00DB; >> + __REG(0x021b002c) = 0x000026D2; >> + >> + __REG(0x021b0030) = 0x008F0E21; >> + __REG(0x021b0008) = 0x09444040; >> + __REG(0x021b0004) = 0x00020036; >> + __REG(0x021b0040) = 0x00000047; >> + __REG(0x021b0000) = 0x841A0000; >> + >> + __REG(0x021b001c) = 0x04088032; >> + __REG(0x021b001c) = 0x00008033; >> + __REG(0x021b001c) = 0x00428031; >> + __REG(0x021b001c) = 0x09408030; >> + >> + __REG(0x021b001c) = 0x04008040; >> + __REG(0x021b0800) = 0xA1380003; >> + __REG(0x021b0020) = 0x00005800; >> + __REG(0x021b0818) = 0x00000007; >> + __REG(0x021b4818) = 0x00000007; >> + >> + /* Calibration values based on ARD and 528MHz */ >> + __REG(0x021b083c) = 0x434B0358; >> + __REG(0x021b0840) = 0x033D033C; >> + __REG(0x021b483c) = 0x03520362; >> + __REG(0x021b4840) = 0x03480318; >> + __REG(0x021b0848) = 0x41383A3C; >> + __REG(0x021b4848) = 0x3F3C374A; >> + __REG(0x021b0850) = 0x42434444; >> + __REG(0x021b4850) = 0x4932473A; >> + >> + __REG(0x021b080c) = 0x001F001F; >> + __REG(0x021b0810) = 0x001F001F; >> + >> + __REG(0x021b480c) = 0x001F001F; >> + __REG(0x021b4810) = 0x001F001F; >> + >> + __REG(0x021b08b8) = 0x00000800; >> + __REG(0x021b48b8) = 0x00000800; >> + >> + __REG(0x021b0404) = 0x00011006; >> + __REG(0x021b0004) = 0x00025576; >> + >> + __REG(0x021b001c) = 0x00000000; >> + >> + __REG(0x020c4068) = 0x00C03F3F; >> + __REG(0x020c406c) = 0x0030FC00; >> + __REG(0x020c4070) = 0x0FFFC000; >> + __REG(0x020c4074) = 0x3FF00000; >> + __REG(0x020c4078) = 0x00FFF300; >> + __REG(0x020c407c) = 0x0F0000C3; >> + __REG(0x020c4080) = 0x000003FF; >> +} >> + > > This cannot be accepted. Firstly, you cannot use fixed offset. The code > you propose here is very difficult to maintain. > > Then, please take a look at the i.MX6 boards that are already supporting > SPL (ventana and novena). There is a logical separation between iomux, > calibration, and so on. You have to provide tables, and you should then > call the common functions: > > mx6dq_dram_iocfg(); > mx6_dram_cfg(); The reason why I did not use the two functions you've mentioned is that the contents of mx6q_4x_mt41j128.cfg are very difficult to convert according to the data structures being required by the two functions above. It's barely plain text and it requires a lot of time to look for the register address name it the datasheet. > > Again, please take a look at the boards I mentioned. > >> +/* >> + * This section require the differentiation >> + * between iMX6 Sabre Families. >> + * But for now, it will configure only for >> + * SabreSD. >> + */ >> +static void spl_dram_init(void) >> +{ >> + set_mt41j128_ddr(); >> +} >> + >> +int spl_board_mmc_init(bd_t *bis) >> +{ >> + unsigned reg = readl(BOOT_CFG); >> + /* >> + * Upon reading BOOT_CFG register the following map is done: >> + * (U-boot device node) (Physical Port) >> + * 0x840 SD2 >> + * 0x1040 SD3 >> + * 0x1840 eMMC >> + */ > > You add a new entry point here (spl_board_mmc_init), but why cannot you > do this in the functions already supplied by SPL ? Why not in board_init_f ? When the spl_mmc_load_image function being called, it call mmc_initialize. By default, the mmc_initialize call board_mmc_init. By looking the said function, it initialize all mmc ports. While, in spl_board_mmc_init, only initialize the current mmc port. > >> + switch (reg) { >> + case 0x840: >> + imx_iomux_v3_setup_multiple_pads( >> + usdhc2_pads, ARRAY_SIZE(usdhc2_pads)); >> + spl_usdhc_cfg.esdhc_base = USDHC2_BASE_ADDR; >> + spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC2_CLK); >> + gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk; >> + break; >> + case 0x1040: >> + imx_iomux_v3_setup_multiple_pads( >> + usdhc3_pads, ARRAY_SIZE(usdhc3_pads)); >> + spl_usdhc_cfg.esdhc_base = USDHC3_BASE_ADDR; >> + spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK); >> + gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk; >> + break; >> + case 0x1840: >> + imx_iomux_v3_setup_multiple_pads( >> + usdhc4_pads, ARRAY_SIZE(usdhc4_pads)); >> + spl_usdhc_cfg.esdhc_base = USDHC4_BASE_ADDR; >> + spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC4_CLK); >> + gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk; >> + break; >> + } >> + >> + return fsl_esdhc_initialize(bis, &spl_usdhc_cfg); >> +} >> + >> +void board_init_f(ulong dummy) >> +{ >> + /* setup AIPS and disable watchdog */ >> + arch_cpu_init(); >> + >> + /* iomux and setup of i2c */ >> + board_early_init_f(); > > ?? > > A "early" function is generally called by common code at a different > point. It makes no sense to call it inside board_init_f() > >> + >> + /* setup GP timer */ >> + timer_init(); >> + > > Then I am expecting iomux setuup--- The iomux setup being called in board_early_init_f... > >> + /* UART clocks enabled and gd valid - init serial console */ >> + preloader_console_init(); >> + >> + /* DDR initialization */ >> + spl_dram_init(); > > and here calling the already supplied functions. > >> + >> + /* Clear the BSS. */ >> + memset(__bss_start, 0, __bss_end - __bss_start); >> + >> + /* load/boot image from boot device */ >> + board_init_r(NULL, 0); >> +} >> + >> +void reset_cpu(ulong addr) >> +{ >> +} >> +#endif >> > > Best regards, > Stefano Babic > > -- > ===================================================================== > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de > ===================================================================== Regards, john
Hi John, On 10/11/2014 01:23, John Tobias wrote: > Hi Stefano, > > > On Sun, Nov 9, 2014 at 1:16 PM, Stefano Babic <sbabic@denx.de> wrote: >> Hi John, >> >> On 08/11/2014 22:27, John Tobias wrote: >>> This patch will enable the support for SPL on iMX6 SabreSD. >>> It tested on SD2 and SD3 mmc port. >>> --- >>> board/freescale/mx6sabresd/mx6sabresd.c | 211 +++++++++++++++++++++++++++++++- >>> 1 file changed, 209 insertions(+), 2 deletions(-) >>> >>> diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c >>> index 3d81fff..f443337 100644 >>> --- a/board/freescale/mx6sabresd/mx6sabresd.c >>> +++ b/board/freescale/mx6sabresd/mx6sabresd.c >>> @@ -55,8 +55,7 @@ DECLARE_GLOBAL_DATA_PTR; >>> >>> int dram_init(void) >>> { >>> - gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE); >>> - >>> + gd->ram_size = imx_ddr_size(); >>> return 0; >>> } >>> >>> @@ -607,3 +606,211 @@ int checkboard(void) >>> puts("Board: MX6-SabreSD\n"); >>> return 0; >>> } >>> + >>> +#ifdef CONFIG_SPL_BUILD >>> +#include <spl.h> >>> +#include <libfdt.h> >>> + >>> +#define BOOT_CFG 0x20D8004 >>> +#define __REG(x) (*((volatile u32 *)(x))) >>> + >>> +struct fsl_esdhc_cfg spl_usdhc_cfg; >>> +/* >>> + * Got it from mx6q_4x_mt41j128.cfg file >>> + */ >>> +void set_mt41j128_ddr(void) >>> +{ >>> + __REG(0x020e05a8) = 0x00000028; >>> + __REG(0x020e05b0) = 0x00000028; >>> + __REG(0x020e0524) = 0x00000028; >>> + __REG(0x020e051c) = 0x00000028; >>> + >>> + __REG(0x020e0518) = 0x00000028; >>> + __REG(0x020e050c) = 0x00000028; >>> + __REG(0x020e05b8) = 0x00000028; >>> + __REG(0x020e05c0) = 0x00000028; >>> + >>> + __REG(0x020e05ac) = 0x00000028; >>> + __REG(0x020e05b4) = 0x00000028; >>> + __REG(0x020e0528) = 0x00000028; >>> + __REG(0x020e0520) = 0x00000028; >>> + >>> + __REG(0x020e0514) = 0x00000028; >>> + __REG(0x020e0510) = 0x00000028; >>> + __REG(0x020e05bc) = 0x00000028; >>> + __REG(0x020e05c4) = 0x00000028; >>> + >>> + __REG(0x020e056c) = 0x00000030; >>> + __REG(0x020e0578) = 0x00000030; >>> + __REG(0x020e0588) = 0x00000030; >>> + __REG(0x020e0594) = 0x00000030; >>> + >>> + __REG(0x020e057c) = 0x00000030; >>> + __REG(0x020e0590) = 0x00000030; >>> + __REG(0x020e0598) = 0x00000030; >>> + __REG(0x020e058c) = 0x00000000; >>> + >>> + __REG(0x020e059c) = 0x00003030; >>> + __REG(0x020e05a0) = 0x00003030; >>> + __REG(0x020e0784) = 0x00000028; >>> + __REG(0x020e0788) = 0x00000028; >>> + >>> + __REG(0x020e0794) = 0x00000028; >>> + __REG(0x020e079c) = 0x00000028; >>> + __REG(0x020e07a0) = 0x00000028; >>> + __REG(0x020e07a4) = 0x00000028; >>> + >>> + __REG(0x020e07a8) = 0x00000028; >>> + __REG(0x020e0748) = 0x00000028; >>> + __REG(0x020e074c) = 0x00000030; >>> + __REG(0x020e0750) = 0x00020000; >>> + >>> + __REG(0x020e0758) = 0x00000000; >>> + __REG(0x020e0774) = 0x00020000; >>> + __REG(0x020e078c) = 0x00000030; >>> + __REG(0x020e0798) = 0x000C0000; >>> + >>> + __REG(0x021b081c) = 0x33333333; >>> + __REG(0x021b0820) = 0x33333333; >>> + __REG(0x021b0824) = 0x33333333; >>> + __REG(0x021b0828) = 0x33333333; >>> + >>> + __REG(0x021b481c) = 0x33333333; >>> + __REG(0x021b4820) = 0x33333333; >>> + __REG(0x021b4824) = 0x33333333; >>> + __REG(0x021b4828) = 0x33333333; >>> + >>> + __REG(0x021b0018) = 0x00001740; >>> + >>> + __REG(0x021b001c) = 0x00008000; >>> + __REG(0x021b000c) = 0x8A8F7975; >>> + __REG(0x021b0010) = 0xFF538E64; >>> + __REG(0x021b0014) = 0x01FF00DB; >>> + __REG(0x021b002c) = 0x000026D2; >>> + >>> + __REG(0x021b0030) = 0x008F0E21; >>> + __REG(0x021b0008) = 0x09444040; >>> + __REG(0x021b0004) = 0x00020036; >>> + __REG(0x021b0040) = 0x00000047; >>> + __REG(0x021b0000) = 0x841A0000; >>> + >>> + __REG(0x021b001c) = 0x04088032; >>> + __REG(0x021b001c) = 0x00008033; >>> + __REG(0x021b001c) = 0x00428031; >>> + __REG(0x021b001c) = 0x09408030; >>> + >>> + __REG(0x021b001c) = 0x04008040; >>> + __REG(0x021b0800) = 0xA1380003; >>> + __REG(0x021b0020) = 0x00005800; >>> + __REG(0x021b0818) = 0x00000007; >>> + __REG(0x021b4818) = 0x00000007; >>> + >>> + /* Calibration values based on ARD and 528MHz */ >>> + __REG(0x021b083c) = 0x434B0358; >>> + __REG(0x021b0840) = 0x033D033C; >>> + __REG(0x021b483c) = 0x03520362; >>> + __REG(0x021b4840) = 0x03480318; >>> + __REG(0x021b0848) = 0x41383A3C; >>> + __REG(0x021b4848) = 0x3F3C374A; >>> + __REG(0x021b0850) = 0x42434444; >>> + __REG(0x021b4850) = 0x4932473A; >>> + >>> + __REG(0x021b080c) = 0x001F001F; >>> + __REG(0x021b0810) = 0x001F001F; >>> + >>> + __REG(0x021b480c) = 0x001F001F; >>> + __REG(0x021b4810) = 0x001F001F; >>> + >>> + __REG(0x021b08b8) = 0x00000800; >>> + __REG(0x021b48b8) = 0x00000800; >>> + >>> + __REG(0x021b0404) = 0x00011006; >>> + __REG(0x021b0004) = 0x00025576; >>> + >>> + __REG(0x021b001c) = 0x00000000; >>> + >>> + __REG(0x020c4068) = 0x00C03F3F; >>> + __REG(0x020c406c) = 0x0030FC00; >>> + __REG(0x020c4070) = 0x0FFFC000; >>> + __REG(0x020c4074) = 0x3FF00000; >>> + __REG(0x020c4078) = 0x00FFF300; >>> + __REG(0x020c407c) = 0x0F0000C3; >>> + __REG(0x020c4080) = 0x000003FF; >>> +} >>> + >> >> This cannot be accepted. Firstly, you cannot use fixed offset. The code >> you propose here is very difficult to maintain. >> >> Then, please take a look at the i.MX6 boards that are already supporting >> SPL (ventana and novena). There is a logical separation between iomux, >> calibration, and so on. You have to provide tables, and you should then >> call the common functions: >> >> mx6dq_dram_iocfg(); >> mx6_dram_cfg(); > > The reason why I did not use the two functions you've mentioned is that the > contents of mx6q_4x_mt41j128.cfg are very difficult to convert according to the > data structures being required by the two functions above. Yes, but these functions are the result of a very long discussion on the ML about how it is possible to support several flavours of the SOC (MX6Q, MX6DL), having one single binary for all of them. > > It's barely plain text and it requires a lot of time to look for the > register address > name it the datasheet. This is also a reason to get rid of them. DCD tables are more difficult to maintain. However, the .cfg file can be parsed by the compiler. The sabresd was added to mainline before the patch for gcc was added. See other i.MX6 boards, for example nitrogen6x. > >> >> Again, please take a look at the boards I mentioned. >> >>> +/* >>> + * This section require the differentiation >>> + * between iMX6 Sabre Families. >>> + * But for now, it will configure only for >>> + * SabreSD. >>> + */ >>> +static void spl_dram_init(void) >>> +{ >>> + set_mt41j128_ddr(); >>> +} >>> + >>> +int spl_board_mmc_init(bd_t *bis) >>> +{ >>> + unsigned reg = readl(BOOT_CFG); >>> + /* >>> + * Upon reading BOOT_CFG register the following map is done: >>> + * (U-boot device node) (Physical Port) >>> + * 0x840 SD2 >>> + * 0x1040 SD3 >>> + * 0x1840 eMMC >>> + */ >> >> You add a new entry point here (spl_board_mmc_init), but why cannot you >> do this in the functions already supplied by SPL ? Why not in board_init_f ? > > When the spl_mmc_load_image function being called, it call > mmc_initialize. By default, the mmc_initialize > call board_mmc_init. By looking the said function, it initialize all mmc ports. > > While, in spl_board_mmc_init, only initialize the current mmc port. ok - but which is the issue by initializing all ports ? I mean, if board_mmc_init initialize all ports including what you need in SPL, which is the reason to exclude the other ones ? > >> >>> + switch (reg) { >>> + case 0x840: >>> + imx_iomux_v3_setup_multiple_pads( >>> + usdhc2_pads, ARRAY_SIZE(usdhc2_pads)); >>> + spl_usdhc_cfg.esdhc_base = USDHC2_BASE_ADDR; >>> + spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC2_CLK); >>> + gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk; >>> + break; >>> + case 0x1040: >>> + imx_iomux_v3_setup_multiple_pads( >>> + usdhc3_pads, ARRAY_SIZE(usdhc3_pads)); >>> + spl_usdhc_cfg.esdhc_base = USDHC3_BASE_ADDR; >>> + spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK); >>> + gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk; >>> + break; >>> + case 0x1840: >>> + imx_iomux_v3_setup_multiple_pads( >>> + usdhc4_pads, ARRAY_SIZE(usdhc4_pads)); >>> + spl_usdhc_cfg.esdhc_base = USDHC4_BASE_ADDR; >>> + spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC4_CLK); >>> + gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk; >>> + break; >>> + } >>> + >>> + return fsl_esdhc_initialize(bis, &spl_usdhc_cfg); >>> +} >>> + >>> +void board_init_f(ulong dummy) >>> +{ >>> + /* setup AIPS and disable watchdog */ >>> + arch_cpu_init(); >>> + >>> + /* iomux and setup of i2c */ >>> + board_early_init_f(); >> >> ?? >> >> A "early" function is generally called by common code at a different >> point. It makes no sense to call it inside board_init_f() >> >>> + >>> + /* setup GP timer */ >>> + timer_init(); >>> + >> >> Then I am expecting iomux setuup--- > > The iomux setup being called in board_early_init_f... > >> >>> + /* UART clocks enabled and gd valid - init serial console */ >>> + preloader_console_init(); >>> + >>> + /* DDR initialization */ >>> + spl_dram_init(); >> >> and here calling the already supplied functions. >> >>> + >>> + /* Clear the BSS. */ >>> + memset(__bss_start, 0, __bss_end - __bss_start); >>> + >>> + /* load/boot image from boot device */ >>> + board_init_r(NULL, 0); >>> +} >>> + >>> +void reset_cpu(ulong addr) >>> +{ >>> +} >>> +#endif >>> >> Best regards, Stefano Babic
On Tue, Nov 11, 2014 at 12:44 AM, Stefano Babic <sbabic@denx.de> wrote: > Hi John, > > On 10/11/2014 01:23, John Tobias wrote: >> Hi Stefano, >> >> >> On Sun, Nov 9, 2014 at 1:16 PM, Stefano Babic <sbabic@denx.de> wrote: >>> Hi John, >>> >>> On 08/11/2014 22:27, John Tobias wrote: >>>> This patch will enable the support for SPL on iMX6 SabreSD. >>>> It tested on SD2 and SD3 mmc port. >>>> --- >>>> board/freescale/mx6sabresd/mx6sabresd.c | 211 +++++++++++++++++++++++++++++++- >>>> 1 file changed, 209 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c >>>> index 3d81fff..f443337 100644 >>>> --- a/board/freescale/mx6sabresd/mx6sabresd.c >>>> +++ b/board/freescale/mx6sabresd/mx6sabresd.c >>>> @@ -55,8 +55,7 @@ DECLARE_GLOBAL_DATA_PTR; >>>> >>>> int dram_init(void) >>>> { >>>> - gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE); >>>> - >>>> + gd->ram_size = imx_ddr_size(); >>>> return 0; >>>> } >>>> >>>> @@ -607,3 +606,211 @@ int checkboard(void) >>>> puts("Board: MX6-SabreSD\n"); >>>> return 0; >>>> } >>>> + >>>> +#ifdef CONFIG_SPL_BUILD >>>> +#include <spl.h> >>>> +#include <libfdt.h> >>>> + >>>> +#define BOOT_CFG 0x20D8004 >>>> +#define __REG(x) (*((volatile u32 *)(x))) >>>> + >>>> +struct fsl_esdhc_cfg spl_usdhc_cfg; >>>> +/* >>>> + * Got it from mx6q_4x_mt41j128.cfg file >>>> + */ >>>> +void set_mt41j128_ddr(void) >>>> +{ >>>> + __REG(0x020e05a8) = 0x00000028; >>>> + __REG(0x020e05b0) = 0x00000028; >>>> + __REG(0x020e0524) = 0x00000028; >>>> + __REG(0x020e051c) = 0x00000028; >>>> + >>>> + __REG(0x020e0518) = 0x00000028; >>>> + __REG(0x020e050c) = 0x00000028; >>>> + __REG(0x020e05b8) = 0x00000028; >>>> + __REG(0x020e05c0) = 0x00000028; >>>> + >>>> + __REG(0x020e05ac) = 0x00000028; >>>> + __REG(0x020e05b4) = 0x00000028; >>>> + __REG(0x020e0528) = 0x00000028; >>>> + __REG(0x020e0520) = 0x00000028; >>>> + >>>> + __REG(0x020e0514) = 0x00000028; >>>> + __REG(0x020e0510) = 0x00000028; >>>> + __REG(0x020e05bc) = 0x00000028; >>>> + __REG(0x020e05c4) = 0x00000028; >>>> + >>>> + __REG(0x020e056c) = 0x00000030; >>>> + __REG(0x020e0578) = 0x00000030; >>>> + __REG(0x020e0588) = 0x00000030; >>>> + __REG(0x020e0594) = 0x00000030; >>>> + >>>> + __REG(0x020e057c) = 0x00000030; >>>> + __REG(0x020e0590) = 0x00000030; >>>> + __REG(0x020e0598) = 0x00000030; >>>> + __REG(0x020e058c) = 0x00000000; >>>> + >>>> + __REG(0x020e059c) = 0x00003030; >>>> + __REG(0x020e05a0) = 0x00003030; >>>> + __REG(0x020e0784) = 0x00000028; >>>> + __REG(0x020e0788) = 0x00000028; >>>> + >>>> + __REG(0x020e0794) = 0x00000028; >>>> + __REG(0x020e079c) = 0x00000028; >>>> + __REG(0x020e07a0) = 0x00000028; >>>> + __REG(0x020e07a4) = 0x00000028; >>>> + >>>> + __REG(0x020e07a8) = 0x00000028; >>>> + __REG(0x020e0748) = 0x00000028; >>>> + __REG(0x020e074c) = 0x00000030; >>>> + __REG(0x020e0750) = 0x00020000; >>>> + >>>> + __REG(0x020e0758) = 0x00000000; >>>> + __REG(0x020e0774) = 0x00020000; >>>> + __REG(0x020e078c) = 0x00000030; >>>> + __REG(0x020e0798) = 0x000C0000; >>>> + >>>> + __REG(0x021b081c) = 0x33333333; >>>> + __REG(0x021b0820) = 0x33333333; >>>> + __REG(0x021b0824) = 0x33333333; >>>> + __REG(0x021b0828) = 0x33333333; >>>> + >>>> + __REG(0x021b481c) = 0x33333333; >>>> + __REG(0x021b4820) = 0x33333333; >>>> + __REG(0x021b4824) = 0x33333333; >>>> + __REG(0x021b4828) = 0x33333333; >>>> + >>>> + __REG(0x021b0018) = 0x00001740; >>>> + >>>> + __REG(0x021b001c) = 0x00008000; >>>> + __REG(0x021b000c) = 0x8A8F7975; >>>> + __REG(0x021b0010) = 0xFF538E64; >>>> + __REG(0x021b0014) = 0x01FF00DB; >>>> + __REG(0x021b002c) = 0x000026D2; >>>> + >>>> + __REG(0x021b0030) = 0x008F0E21; >>>> + __REG(0x021b0008) = 0x09444040; >>>> + __REG(0x021b0004) = 0x00020036; >>>> + __REG(0x021b0040) = 0x00000047; >>>> + __REG(0x021b0000) = 0x841A0000; >>>> + >>>> + __REG(0x021b001c) = 0x04088032; >>>> + __REG(0x021b001c) = 0x00008033; >>>> + __REG(0x021b001c) = 0x00428031; >>>> + __REG(0x021b001c) = 0x09408030; >>>> + >>>> + __REG(0x021b001c) = 0x04008040; >>>> + __REG(0x021b0800) = 0xA1380003; >>>> + __REG(0x021b0020) = 0x00005800; >>>> + __REG(0x021b0818) = 0x00000007; >>>> + __REG(0x021b4818) = 0x00000007; >>>> + >>>> + /* Calibration values based on ARD and 528MHz */ >>>> + __REG(0x021b083c) = 0x434B0358; >>>> + __REG(0x021b0840) = 0x033D033C; >>>> + __REG(0x021b483c) = 0x03520362; >>>> + __REG(0x021b4840) = 0x03480318; >>>> + __REG(0x021b0848) = 0x41383A3C; >>>> + __REG(0x021b4848) = 0x3F3C374A; >>>> + __REG(0x021b0850) = 0x42434444; >>>> + __REG(0x021b4850) = 0x4932473A; >>>> + >>>> + __REG(0x021b080c) = 0x001F001F; >>>> + __REG(0x021b0810) = 0x001F001F; >>>> + >>>> + __REG(0x021b480c) = 0x001F001F; >>>> + __REG(0x021b4810) = 0x001F001F; >>>> + >>>> + __REG(0x021b08b8) = 0x00000800; >>>> + __REG(0x021b48b8) = 0x00000800; >>>> + >>>> + __REG(0x021b0404) = 0x00011006; >>>> + __REG(0x021b0004) = 0x00025576; >>>> + >>>> + __REG(0x021b001c) = 0x00000000; >>>> + >>>> + __REG(0x020c4068) = 0x00C03F3F; >>>> + __REG(0x020c406c) = 0x0030FC00; >>>> + __REG(0x020c4070) = 0x0FFFC000; >>>> + __REG(0x020c4074) = 0x3FF00000; >>>> + __REG(0x020c4078) = 0x00FFF300; >>>> + __REG(0x020c407c) = 0x0F0000C3; >>>> + __REG(0x020c4080) = 0x000003FF; >>>> +} >>>> + >>> >>> This cannot be accepted. Firstly, you cannot use fixed offset. The code >>> you propose here is very difficult to maintain. >>> >>> Then, please take a look at the i.MX6 boards that are already supporting >>> SPL (ventana and novena). There is a logical separation between iomux, >>> calibration, and so on. You have to provide tables, and you should then >>> call the common functions: >>> >>> mx6dq_dram_iocfg(); >>> mx6_dram_cfg(); >> >> The reason why I did not use the two functions you've mentioned is that the >> contents of mx6q_4x_mt41j128.cfg are very difficult to convert according to the >> data structures being required by the two functions above. > > Yes, but these functions are the result of a very long discussion on the > ML about how it is possible to support several flavours of the SOC > (MX6Q, MX6DL), having one single binary for all of them. > >> >> It's barely plain text and it requires a lot of time to look for the >> register address >> name it the datasheet. > > This is also a reason to get rid of them. DCD tables are more difficult > to maintain. However, the .cfg file can be parsed by the compiler. The > sabresd was added to mainline before the patch for gcc was added. See > other i.MX6 boards, for example nitrogen6x. >> >>> >>> Again, please take a look at the boards I mentioned. >>> >>>> +/* >>>> + * This section require the differentiation >>>> + * between iMX6 Sabre Families. >>>> + * But for now, it will configure only for >>>> + * SabreSD. >>>> + */ >>>> +static void spl_dram_init(void) >>>> +{ >>>> + set_mt41j128_ddr(); >>>> +} >>>> + >>>> +int spl_board_mmc_init(bd_t *bis) >>>> +{ >>>> + unsigned reg = readl(BOOT_CFG); >>>> + /* >>>> + * Upon reading BOOT_CFG register the following map is done: >>>> + * (U-boot device node) (Physical Port) >>>> + * 0x840 SD2 >>>> + * 0x1040 SD3 >>>> + * 0x1840 eMMC >>>> + */ >>> >>> You add a new entry point here (spl_board_mmc_init), but why cannot you >>> do this in the functions already supplied by SPL ? Why not in board_init_f ? >> >> When the spl_mmc_load_image function being called, it call >> mmc_initialize. By default, the mmc_initialize >> call board_mmc_init. By looking the said function, it initialize all mmc ports. >> >> While, in spl_board_mmc_init, only initialize the current mmc port. > > ok - but which is the issue by initializing all ports ? I mean, if > board_mmc_init initialize all ports including what you need in SPL, > which is the reason to exclude the other ones ? > When board_mmc_init initialize all ports, we can issue a command at uboot console to switch to different port. e.g: mmc dev 0 (e.g. SD4) mmc dev 1 (e.g. SD2) mmc dev 2 (e.g. SD3) We can also re-map which is index 0, 1 and 2 by re-arranging the contents of struct fsl_esdhc_cfg usdhc_cfg[3] = { {USDHC4_BASE_ADDR}, {USDHC2_BASE_ADDR}, {USDHC3_BASE_ADDR}, }; Assuming the bootstrap is configured to port 3 (USDHC3). The SPL image will call spl_mmc_load_image to load the uboot image. In order to do that, it will call the find_mmc_device(0), based on example above the index 0 is port 4, index 1 is port 2 and index 2 is port 3. The find_mmc_device will return with errors because it couldn't initialize the said port. So by default, the user should switch back the bootstrap to port 4 in order to load the uboot correctly. Then, if the user really want to use the different ports, they have to change the arrangement of the contents of usdhc_cfg and compile the u-boot again. In spl_board_mmc_init function, it will read the bootstrap and configure it. The settings are passed to fsl_esdhc_initialize which is to be stored in index 0. So, find_mmc_device(0) could get a correct information. Then, switching to different bootstrap doesn't require to re-compile the image. >> >>> >>>> + switch (reg) { >>>> + case 0x840: >>>> + imx_iomux_v3_setup_multiple_pads( >>>> + usdhc2_pads, ARRAY_SIZE(usdhc2_pads)); >>>> + spl_usdhc_cfg.esdhc_base = USDHC2_BASE_ADDR; >>>> + spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC2_CLK); >>>> + gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk; >>>> + break; >>>> + case 0x1040: >>>> + imx_iomux_v3_setup_multiple_pads( >>>> + usdhc3_pads, ARRAY_SIZE(usdhc3_pads)); >>>> + spl_usdhc_cfg.esdhc_base = USDHC3_BASE_ADDR; >>>> + spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK); >>>> + gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk; >>>> + break; >>>> + case 0x1840: >>>> + imx_iomux_v3_setup_multiple_pads( >>>> + usdhc4_pads, ARRAY_SIZE(usdhc4_pads)); >>>> + spl_usdhc_cfg.esdhc_base = USDHC4_BASE_ADDR; >>>> + spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC4_CLK); >>>> + gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk; >>>> + break; >>>> + } >>>> + >>>> + return fsl_esdhc_initialize(bis, &spl_usdhc_cfg); >>>> +} >>>> + >>>> +void board_init_f(ulong dummy) >>>> +{ >>>> + /* setup AIPS and disable watchdog */ >>>> + arch_cpu_init(); >>>> + >>>> + /* iomux and setup of i2c */ >>>> + board_early_init_f(); >>> >>> ?? >>> >>> A "early" function is generally called by common code at a different >>> point. It makes no sense to call it inside board_init_f() >>> >>>> + >>>> + /* setup GP timer */ >>>> + timer_init(); >>>> + >>> >>> Then I am expecting iomux setuup--- >> >> The iomux setup being called in board_early_init_f... >> >>> >>>> + /* UART clocks enabled and gd valid - init serial console */ >>>> + preloader_console_init(); >>>> + >>>> + /* DDR initialization */ >>>> + spl_dram_init(); >>> >>> and here calling the already supplied functions. >>> >>>> + >>>> + /* Clear the BSS. */ >>>> + memset(__bss_start, 0, __bss_end - __bss_start); >>>> + >>>> + /* load/boot image from boot device */ >>>> + board_init_r(NULL, 0); >>>> +} >>>> + >>>> +void reset_cpu(ulong addr) >>>> +{ >>>> +} >>>> +#endif >>>> >>> > > Best regards, > Stefano Babic > > > -- > ===================================================================== > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de > =====================================================================
Hi John, On 11/11/2014 18:15, John Tobias wrote: >>>> You add a new entry point here (spl_board_mmc_init), but why cannot you >>>> do this in the functions already supplied by SPL ? Why not in board_init_f ? >>> >>> When the spl_mmc_load_image function being called, it call >>> mmc_initialize. By default, the mmc_initialize >>> call board_mmc_init. By looking the said function, it initialize all mmc ports. >>> >>> While, in spl_board_mmc_init, only initialize the current mmc port. >> >> ok - but which is the issue by initializing all ports ? I mean, if >> board_mmc_init initialize all ports including what you need in SPL, >> which is the reason to exclude the other ones ? >> > > When board_mmc_init initialize all ports, we can issue a command > at uboot console to switch to different port. > e.g: > > mmc dev 0 (e.g. SD4) > mmc dev 1 (e.g. SD2) > mmc dev 2 (e.g. SD3) > > We can also re-map which is index 0, 1 and 2 by re-arranging the contents of > > struct fsl_esdhc_cfg usdhc_cfg[3] = { > {USDHC4_BASE_ADDR}, > {USDHC2_BASE_ADDR}, > {USDHC3_BASE_ADDR}, > }; > > Assuming the bootstrap is configured to port 3 (USDHC3). The SPL image will > call spl_mmc_load_image to load the uboot image. In order to do that, > it will call > the find_mmc_device(0), based on example above the index 0 is port 4, index 1 > is port 2 and index 2 is port 3. The find_mmc_device will return with errors > because it couldn't initialize the said port. > > So by default, the user should switch back the bootstrap to port 4 in > order to load the > uboot correctly. Then, if the user really want to use the different > ports, they have > to change the arrangement of the contents of usdhc_cfg and compile the > u-boot again. > > In spl_board_mmc_init function, it will read the bootstrap and configure it. The > settings are passed to fsl_esdhc_initialize which is to be stored in > index 0. So, > find_mmc_device(0) could get a correct information. > > Then, switching to different bootstrap doesn't require to re-compile the image. This is not enough for adding a new exported weak function, that reamins undocumented, and does not avoid using #ifdef, only moved into drivers/mmc/mmc.c. We have other cases where the behavior is different between SPL and U-Boot, and this is handled in board file using the same #ifdef CONFIG_SPL_BUILD. The question is: why do you need to change the API (as I said, it remains undocumented) instead of doing : int board_mmc_init(bd_t *bis) { #ifdef CONFIG_SPL_BUILD <initialization single port based on bootstrap> #else <instantiate all ports as usual> #endif Best regards, Stefano Babic
Hi Stefano, That's fine.. I'll fix it. Regards, john On Wed, Nov 12, 2014 at 2:13 AM, Stefano Babic <sbabic@denx.de> wrote: > Hi John, > > On 11/11/2014 18:15, John Tobias wrote: > >>>>> You add a new entry point here (spl_board_mmc_init), but why cannot you >>>>> do this in the functions already supplied by SPL ? Why not in board_init_f ? >>>> >>>> When the spl_mmc_load_image function being called, it call >>>> mmc_initialize. By default, the mmc_initialize >>>> call board_mmc_init. By looking the said function, it initialize all mmc ports. >>>> >>>> While, in spl_board_mmc_init, only initialize the current mmc port. >>> >>> ok - but which is the issue by initializing all ports ? I mean, if >>> board_mmc_init initialize all ports including what you need in SPL, >>> which is the reason to exclude the other ones ? >>> >> >> When board_mmc_init initialize all ports, we can issue a command >> at uboot console to switch to different port. >> e.g: >> >> mmc dev 0 (e.g. SD4) >> mmc dev 1 (e.g. SD2) >> mmc dev 2 (e.g. SD3) >> >> We can also re-map which is index 0, 1 and 2 by re-arranging the contents of >> >> struct fsl_esdhc_cfg usdhc_cfg[3] = { >> {USDHC4_BASE_ADDR}, >> {USDHC2_BASE_ADDR}, >> {USDHC3_BASE_ADDR}, >> }; >> >> Assuming the bootstrap is configured to port 3 (USDHC3). The SPL image will >> call spl_mmc_load_image to load the uboot image. In order to do that, >> it will call >> the find_mmc_device(0), based on example above the index 0 is port 4, index 1 >> is port 2 and index 2 is port 3. The find_mmc_device will return with errors >> because it couldn't initialize the said port. >> >> So by default, the user should switch back the bootstrap to port 4 in >> order to load the >> uboot correctly. Then, if the user really want to use the different >> ports, they have >> to change the arrangement of the contents of usdhc_cfg and compile the >> u-boot again. >> >> In spl_board_mmc_init function, it will read the bootstrap and configure it. The >> settings are passed to fsl_esdhc_initialize which is to be stored in >> index 0. So, >> find_mmc_device(0) could get a correct information. >> >> Then, switching to different bootstrap doesn't require to re-compile the image. > > This is not enough for adding a new exported weak function, that reamins > undocumented, and does not avoid using #ifdef, only moved into > drivers/mmc/mmc.c. > > We have other cases where the behavior is different between SPL and > U-Boot, and this is handled in board file using the same #ifdef > CONFIG_SPL_BUILD. > > The question is: why do you need to change the API (as I said, it > remains undocumented) instead of doing : > > int board_mmc_init(bd_t *bis) { > > #ifdef CONFIG_SPL_BUILD > <initialization single port based on bootstrap> > #else > <instantiate all ports as usual> > #endif > > Best regards, > Stefano Babic > > > -- > ===================================================================== > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de > =====================================================================
diff --git a/board/freescale/mx6sabresd/mx6sabresd.c b/board/freescale/mx6sabresd/mx6sabresd.c index 3d81fff..f443337 100644 --- a/board/freescale/mx6sabresd/mx6sabresd.c +++ b/board/freescale/mx6sabresd/mx6sabresd.c @@ -55,8 +55,7 @@ DECLARE_GLOBAL_DATA_PTR; int dram_init(void) { - gd->ram_size = get_ram_size((void *)PHYS_SDRAM, PHYS_SDRAM_SIZE); - + gd->ram_size = imx_ddr_size(); return 0; } @@ -607,3 +606,211 @@ int checkboard(void) puts("Board: MX6-SabreSD\n"); return 0; } + +#ifdef CONFIG_SPL_BUILD +#include <spl.h> +#include <libfdt.h> + +#define BOOT_CFG 0x20D8004 +#define __REG(x) (*((volatile u32 *)(x))) + +struct fsl_esdhc_cfg spl_usdhc_cfg; +/* + * Got it from mx6q_4x_mt41j128.cfg file + */ +void set_mt41j128_ddr(void) +{ + __REG(0x020e05a8) = 0x00000028; + __REG(0x020e05b0) = 0x00000028; + __REG(0x020e0524) = 0x00000028; + __REG(0x020e051c) = 0x00000028; + + __REG(0x020e0518) = 0x00000028; + __REG(0x020e050c) = 0x00000028; + __REG(0x020e05b8) = 0x00000028; + __REG(0x020e05c0) = 0x00000028; + + __REG(0x020e05ac) = 0x00000028; + __REG(0x020e05b4) = 0x00000028; + __REG(0x020e0528) = 0x00000028; + __REG(0x020e0520) = 0x00000028; + + __REG(0x020e0514) = 0x00000028; + __REG(0x020e0510) = 0x00000028; + __REG(0x020e05bc) = 0x00000028; + __REG(0x020e05c4) = 0x00000028; + + __REG(0x020e056c) = 0x00000030; + __REG(0x020e0578) = 0x00000030; + __REG(0x020e0588) = 0x00000030; + __REG(0x020e0594) = 0x00000030; + + __REG(0x020e057c) = 0x00000030; + __REG(0x020e0590) = 0x00000030; + __REG(0x020e0598) = 0x00000030; + __REG(0x020e058c) = 0x00000000; + + __REG(0x020e059c) = 0x00003030; + __REG(0x020e05a0) = 0x00003030; + __REG(0x020e0784) = 0x00000028; + __REG(0x020e0788) = 0x00000028; + + __REG(0x020e0794) = 0x00000028; + __REG(0x020e079c) = 0x00000028; + __REG(0x020e07a0) = 0x00000028; + __REG(0x020e07a4) = 0x00000028; + + __REG(0x020e07a8) = 0x00000028; + __REG(0x020e0748) = 0x00000028; + __REG(0x020e074c) = 0x00000030; + __REG(0x020e0750) = 0x00020000; + + __REG(0x020e0758) = 0x00000000; + __REG(0x020e0774) = 0x00020000; + __REG(0x020e078c) = 0x00000030; + __REG(0x020e0798) = 0x000C0000; + + __REG(0x021b081c) = 0x33333333; + __REG(0x021b0820) = 0x33333333; + __REG(0x021b0824) = 0x33333333; + __REG(0x021b0828) = 0x33333333; + + __REG(0x021b481c) = 0x33333333; + __REG(0x021b4820) = 0x33333333; + __REG(0x021b4824) = 0x33333333; + __REG(0x021b4828) = 0x33333333; + + __REG(0x021b0018) = 0x00001740; + + __REG(0x021b001c) = 0x00008000; + __REG(0x021b000c) = 0x8A8F7975; + __REG(0x021b0010) = 0xFF538E64; + __REG(0x021b0014) = 0x01FF00DB; + __REG(0x021b002c) = 0x000026D2; + + __REG(0x021b0030) = 0x008F0E21; + __REG(0x021b0008) = 0x09444040; + __REG(0x021b0004) = 0x00020036; + __REG(0x021b0040) = 0x00000047; + __REG(0x021b0000) = 0x841A0000; + + __REG(0x021b001c) = 0x04088032; + __REG(0x021b001c) = 0x00008033; + __REG(0x021b001c) = 0x00428031; + __REG(0x021b001c) = 0x09408030; + + __REG(0x021b001c) = 0x04008040; + __REG(0x021b0800) = 0xA1380003; + __REG(0x021b0020) = 0x00005800; + __REG(0x021b0818) = 0x00000007; + __REG(0x021b4818) = 0x00000007; + + /* Calibration values based on ARD and 528MHz */ + __REG(0x021b083c) = 0x434B0358; + __REG(0x021b0840) = 0x033D033C; + __REG(0x021b483c) = 0x03520362; + __REG(0x021b4840) = 0x03480318; + __REG(0x021b0848) = 0x41383A3C; + __REG(0x021b4848) = 0x3F3C374A; + __REG(0x021b0850) = 0x42434444; + __REG(0x021b4850) = 0x4932473A; + + __REG(0x021b080c) = 0x001F001F; + __REG(0x021b0810) = 0x001F001F; + + __REG(0x021b480c) = 0x001F001F; + __REG(0x021b4810) = 0x001F001F; + + __REG(0x021b08b8) = 0x00000800; + __REG(0x021b48b8) = 0x00000800; + + __REG(0x021b0404) = 0x00011006; + __REG(0x021b0004) = 0x00025576; + + __REG(0x021b001c) = 0x00000000; + + __REG(0x020c4068) = 0x00C03F3F; + __REG(0x020c406c) = 0x0030FC00; + __REG(0x020c4070) = 0x0FFFC000; + __REG(0x020c4074) = 0x3FF00000; + __REG(0x020c4078) = 0x00FFF300; + __REG(0x020c407c) = 0x0F0000C3; + __REG(0x020c4080) = 0x000003FF; +} + +/* + * This section require the differentiation + * between iMX6 Sabre Families. + * But for now, it will configure only for + * SabreSD. + */ +static void spl_dram_init(void) +{ + set_mt41j128_ddr(); +} + +int spl_board_mmc_init(bd_t *bis) +{ + unsigned reg = readl(BOOT_CFG); + /* + * Upon reading BOOT_CFG register the following map is done: + * (U-boot device node) (Physical Port) + * 0x840 SD2 + * 0x1040 SD3 + * 0x1840 eMMC + */ + switch (reg) { + case 0x840: + imx_iomux_v3_setup_multiple_pads( + usdhc2_pads, ARRAY_SIZE(usdhc2_pads)); + spl_usdhc_cfg.esdhc_base = USDHC2_BASE_ADDR; + spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC2_CLK); + gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk; + break; + case 0x1040: + imx_iomux_v3_setup_multiple_pads( + usdhc3_pads, ARRAY_SIZE(usdhc3_pads)); + spl_usdhc_cfg.esdhc_base = USDHC3_BASE_ADDR; + spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC3_CLK); + gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk; + break; + case 0x1840: + imx_iomux_v3_setup_multiple_pads( + usdhc4_pads, ARRAY_SIZE(usdhc4_pads)); + spl_usdhc_cfg.esdhc_base = USDHC4_BASE_ADDR; + spl_usdhc_cfg.sdhc_clk = mxc_get_clock(MXC_ESDHC4_CLK); + gd->arch.sdhc_clk = spl_usdhc_cfg.sdhc_clk; + break; + } + + return fsl_esdhc_initialize(bis, &spl_usdhc_cfg); +} + +void board_init_f(ulong dummy) +{ + /* setup AIPS and disable watchdog */ + arch_cpu_init(); + + /* iomux and setup of i2c */ + board_early_init_f(); + + /* setup GP timer */ + timer_init(); + + /* UART clocks enabled and gd valid - init serial console */ + preloader_console_init(); + + /* DDR initialization */ + spl_dram_init(); + + /* Clear the BSS. */ + memset(__bss_start, 0, __bss_end - __bss_start); + + /* load/boot image from boot device */ + board_init_r(NULL, 0); +} + +void reset_cpu(ulong addr) +{ +} +#endif