[U-Boot] [PATCH] mx6: Add initial SPL support for HummingBoard-i2eX

[Nikolay Dimitrov]

Hi Fabio,

On 04/15/2015 11:57 PM, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam at freescale.com>
>
> Add the initial SPL support for HummingBoard-i2eX, which is based on a
> MX6 Dual.
>
> For more information about HummingBoard, please check:
> http://www.solid-run.com/products/hummingboard/
>
> Based on the work from Jon Nettleton and Rabeeh Khoury.
>
> Signed-off-by: Fabio Estevam <fabio.estevam at freescale.com>
> ---
> Currently only HummingBoard-i2eX is supported.
>
> After this initial patch is accepted, then we can add the other Hummingboard
> and Cubox-i variants as well.

The patch looks nice and clean.

> +static struct mx6_ddr3_cfg mem_ddr = {
> +	.mem_speed = 1600,
> +	.density = 4,
> +	.width = 64,
> +	.banks = 8,
> +	.rowaddr = 14,
> +	.coladdr = 10,
> +	.pagesz = 2,
> +	.trcd = 1375,
> +	.trcmin = 4875,
> +	.trasmin = 3500,
> +};
> +

This mem_speed (1600) means that the MMDC imx6 code will calculate
timings for 1600 MT/s, but in reality the memory clock will be 528 or
532 MHz. It would be better to override the speed to 1066 to make
sure imx6 ddr code calculates proper timings.

To be honest, I don't understand why the code (arch/arm/cpu/armv7
/mx6/ddr.c) supports 1333/1600 MT/s when it's obvious that it's outside
chip specs.

> +static void ccgr_init(void)
> +{
> +	struct mxc_ccm_reg *ccm = (struct mxc_ccm_reg *)CCM_BASE_ADDR;
> +
> +	writel(0x00C03F3F, &ccm->CCGR0);
> +	writel(0x0030FC03, &ccm->CCGR1);
> +	writel(0x0FFFC000, &ccm->CCGR2);
> +	writel(0x3FF00000, &ccm->CCGR3);
> +	writel(0x00FFF300, &ccm->CCGR4);
> +	writel(0x0F0000C3, &ccm->CCGR5);
> +	writel(0x000003FF, &ccm->CCGR6);
> +}

This is nice - clocks are optimal, to the dual-core imx6 won't overheat
if left for long time in U-Boot shell.

And a couple of more notes:
- I see that you don't use the extlinux boot mechanism (from distro
defaults). Do you see any advantages of not using it?
- Would be nice to have HDMI support :)
- git patch gives a warning about one empty line

Reviewed-by: Nikolay Dimitrov <picmaster at mail.bg>

Thanks for the hard work. Regards,
Nikolay