[U-Boot] [PATCH 12/12] imx: ventana: switch to SPL

[York Sun]

On 05/06/2014 11:18 AM, Tim Harvey wrote:
> On Mon, Apr 28, 2014 at 1:17 PM, Tim Harvey <tharvey at gateworks.com> wrote:
>>
>> Switch to an SPL image. The SPL for Ventana does the following:
>>  - setup i2c and read the factory programmed EEPROM to obtain DRAM config
>>    and model for board-specific calibration data
>>  - configure DRAM per CPU/size/layout/devices/calibration
>>  - load u-boot.img from NAND or MTD depending on boot device  and jump to it
>>
>> This allows for a single SPL+u-boot.img to replace the previous multiple board
>> configurations.
>>
> <snip>
>> +
>> +static void i2c_setup_iomux(void)
>> +{
>> +       if (is_cpu_type(MXC_CPU_MX6Q))
>> +               setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, &mx6q_i2c_pad_info0);
>> +       else
>> +               setup_i2c(0, CONFIG_SYS_I2C_SPEED, 0x7f, &mx6dl_i2c_pad_info0);
>> +}
>> +
> <snip>
>> +
>> +/*
>> + * called from C runtime startup code (arch/arm/lib/crt0.S:_main)
>> + * - we have a stack and a place to store GD, both in SRAM
>> + * - no variable global data is available
>> + */
>> +void board_init_f(ulong dummy)
>> +{
>> +       struct ventana_board_info ventana_info;
>> +       int board_model;
>> +
>> +       /* iomux and setup of i2c */
>> +       i2c_setup_iomux();
>> +       timer_init();
>> +       board_model = read_eeprom(I2C_GSC, &ventana_info);
>> +
>> +       /* provide some some default: 32bit 128MB */
>> +       if (GW_UNKNOWN == board_model) {
>> +               ventana_info.sdram_width = 2;
>> +               ventana_info.sdram_size = 3;
>> +       }
>> +       spl_dram_init(8 << ventana_info.sdram_width,
>> +                     16 << ventana_info.sdram_size,
>> +                     board_model);
>> +
>> +       arch_cpu_init();
>> +
>> +       /* Clear the BSS. */
>> +       memset(__bss_start, 0, __bss_end - __bss_start);
>> +
>> +       /* Set global data pointer. */
>> +       gd = &gdata;
>> +
>> +       board_early_init_f();
>> +
>> +       preloader_console_init();
>> +
>> +       board_init_r(NULL, 0);
>> +}
> 
> Stefano / York,
> 
> While preparing for a v3 patch series of my IMX6 SPL bootloader, I
> find that commit dec1861be90c948ea9fb771927d3d26a994d2e20 [1] breaks
> the above code because gd is now needed within setup_i2c.
> 
> I've always been a bit fuzzy on the order of the above calls so I dug
> through the code and I think I understand things better. Please
> correct any wrong assumptions I'm making below:
>  - assignment to gd should 'always' be first (before anything needs
> it, so why not do it first)
>  - arch_cpu_init() should go next as this sets up very low level
> CPU/SoC resources (in this case AIPS config and watchdog disable)
>  - board_early_init_f() should be next as that sets up any board-specific iomux
>  - any additional iomux necessary for SPL should go next (I take care
> of i2c iomux and setup here)
>  - timer_init() next as you need a timer for UART and mxc i2c (for
> delays and busy checks)
>  - preloader_console_init() next as we are now able to send something
> over the UART (this gives me early debug for sdram config now too!)
>  - sdram setup goes next
>  - after sdram is setup, the bss can be cleared
>  - board_init_r - pass over to generic SPL code which will load/call
> an image based on boot device
> 
> So, if the above is correct, I should rework the above function as follows:
> 
> void board_init_f(ulong dummy)
> {
>         struct ventana_board_info ventana_info;
>         int board_model;
> 
>         /* Set global data pointer. */
>         gd = &gdata;
> 
>         /* setup AIPS and disable watchdog */
>         arch_cpu_init();
> 
>         /* iomux and setup of i2c */
>         board_early_init_f();
>         i2c_setup_iomux();
> 
>         /* setup GP timer */
>         timer_init();
> 
>         /* UART clocks enabled and gd valid - init serial console */
>         preloader_console_init();
> 
>         /* read/validate EEPROM info to determine board model and SDRAM cfg */
>         board_model = read_eeprom(I2C_GSC, &ventana_info);
> 
>         /* provide some some default: 32bit 128MB */
>         if (GW_UNKNOWN == board_model) {
>                 ventana_info.sdram_width = 2;
>                 ventana_info.sdram_size = 3;
>         }
> 
>         /* configure MMDC for SDRAM width/size and per-model calibration */
>         spl_dram_init(8 << ventana_info.sdram_width,
>                       16 << ventana_info.sdram_size,
>                       board_model);
> 
>         /* Clear the BSS. */
>         memset(__bss_start, 0, __bss_end - __bss_start);
> 
>         /* load/boot image from boot device */
>         board_init_r(NULL, 0);
> }
> 
> Does this make sense?
>

Tim,

I agree gd should be set first if the memory is available. My previous change to
this i2c driver was to make sure it works without DRAM. Since it uses a data
structure, it has no other place to go.

York