[U-Boot] [PATCH V6] ARM: mx6: Add support for Kosagi Novena

[Nikolay Dimitrov <picmaster@mail.bg>]

Hi Marek,

The usual stuff follows :D.


On 10/10/2014 02:22 AM, Marek Vasut wrote:
> +#define NOVENA_BUTTON_GPIO	IMX_GPIO_NR(4, 14)
> +#define NOVENA_SD_WP		IMX_GPIO_NR(1, 2)
> +#define NOVENA_SD_CD		IMX_GPIO_NR(1, 4)

NOVENA_BUTTON_GPIO is initialized as input in this source file, but the 
NOVENA_SD_WP and NOVENA_SD_CD are not. They're initialized only in 
novena_spl.c, and if you change the SPL configuration, they won't be 
explicitly initialized.

While by pure luck the iomuxc reset values are those of the GPIO pins, I 
would vote for explicit initialization of WP & CD.

And btw, it's cool that you added the WP pin, I totally missed this out 
last time I was staring at the schematic.


> +/*
> + * USB
> + */
> +#ifdef CONFIG_USB_EHCI_MX6
> +static iomux_v3_cfg_t usb_pads[] = {
> +	MX6_PAD_GPIO_17__GPIO7_IO12 | MUX_PAD_CTRL(NO_PAD_CTRL),
> +};
> +
> +static void novena_spl_setup_iomux_usb(void)
> +{
> +	imx_iomux_v3_setup_multiple_pads(usb_pads, ARRAY_SIZE(usb_pads));
> +}
> +#else
> +static void novena_spl_setup_iomux_usb(void) {}
> +#endif

The patch comments are saying this is gone :).


> +/*
> + * 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)
> +{
> +	/*
> +	 * Zero out global data:
> +	 *  - this should be done by crt0.S
> +	 *  - failure to zero it will cause i2c_setup to fail
> +	 */
> +	memset((void *)gd, 0, sizeof(struct global_data));

Same here.


> +/*
> + * Environment is on MMC, starting at offset 64KiB from start of the card.
> + * Please place first partition at offset 1MiB from the start of the card
> + * as recommended by GNU/fdisk. See below for details:
> + * http://homepage.ntlworld.com./jonathan.deboynepollard/FGA/disc-partition-alignment.html
> + */
> +#ifdef CONFIG_CMD_MMC
> +#define CONFIG_ENV_IS_IN_MMC
> +#define CONFIG_SYS_MMC_ENV_DEV		0
> +#define CONFIG_ENV_OFFSET		(512 * 1024)

Please fix either the comment saying offset is 64KiB from start, or fix 
the CONFIG_ENV_OFFSET.


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


Marek, would you agree that after finishing this patch which enables the 
initial Novena support, to handle all other fixes by separate 
independent patches, which are easier to review, resubmit, test, etc?

Kind regards,
Nikolay