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

[Tim Harvey]

On Tue, May 13, 2014 at 10:03 PM, Tim Harvey <tharvey at gateworks.com> wrote:
>
> On Tue, May 13, 2014 at 9:58 PM, Tim Harvey <tharvey at gateworks.com> wrote:
> > On Wed, May 7, 2014 at 2:29 AM, Stefano Babic <sbabic at denx.de> wrote:
> >> Hi Tim,
> >>
> >> On 06/05/2014 20:18, Tim Harvey wrote:
> >>>
> >>> 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
> >>
> >> I think your analyses is correct.
> >>
> >>>
> >>> 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);
> >>> }
> >>
> >> It seems reasonable, go on this way.
> >>
> >> Regards,
> >> Stefano
> >>
> >
> > Stefano,
> >
> > I've just found that one of my boards fails with the above re-org.
> > Strangely a board which has the same mem layout, mem width/size, CPU
> > and nand does not fail.
> >
> > If I make the following change:
>
> (sorry, accidentally fat fingered and sent instead of pasting).  I'll continue:
>
> --- a/board/gateworks/gw_ventana/gw_ventana_spl.c
> +++ b/board/gateworks/gw_ventana/gw_ventana_spl.c
> @@ -380,9 +380,6 @@ void board_init_f(ulong dummy)
>          */
>         memset((void *)gd, 0, sizeof(struct global_data));
>
> -       /* setup AIPS and disable watchdog */
> -       arch_cpu_init();
> -
>         /* iomux and setup of i2c */
>         board_early_init_f();
>         i2c_setup_iomux();
> @@ -407,6 +404,9 @@ void board_init_f(ulong dummy)
>                       16 << ventana_info.sdram_size,
>                       board_model);
>
> +       /* setup AIPS and disable watchdog */
> +       arch_cpu_init();
> +
>         /* Clear the BSS. */
>         memset(__bss_start, 0, __bss_end - __bss_start);
>
> Things start to work properly. I took a look at arch_cpu_init() and
> the call that really needs to be moved (or just duplicated here) is
> mxs_dma_init() to start APBH DMA. The failure mode of the one board is
> that apparently it hangs while loading u-boot.img from NAND (which of
> course uses mxs_nand and thus APBH DMA).
>
> Anyone know what's going on here and why mxs_dma_init() needs to be
> called after MMDC setup, and before clearing the BSS? I don't like
> having magic placement of functions without understanding why.
>
> Thanks,
>
> Tim

I figured this one out - it has nothing to do with the order of
calling arch_cpu_init() its that the MMDC isn't always 'ready' by the
time the BSS is cleared and thus in my failure case the BSS isn't
getting entirely cleared which causes the spl_image global var to not
be cleared as expected and triggers an invalid codepath. I will update
the mmdc config patch when I find the right solution.

Sorry for the noise.

Tim