[U-Boot] [PATCH v3 7/8] imx6: SPL support for iMX6 SabreSD

Stefano Babic sbabic at denx.de
Tue Nov 11 09:44:24 CET 2014


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 at 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


-- 
=====================================================================
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 at denx.de
=====================================================================


More information about the U-Boot mailing list