Re: [PATCH] ARM: add minimal support for the Freescale Quad UDOO Board

[Lucas Stach <l.stach@pengutronix.de>]

Hi Raphaël,

Am Freitag, den 14.02.2014, 09:50 +0100 schrieb Raphaël Poggi:
> From: Raphael Poggi <poggi.raph@gmail.com>
> 
> Adding minimal support for the UDOO board.
> 
> For more information about the board: http://www.udoo.org/
> 
> Signed-off-by: Raphael Poggi <poggi.raph@gmail.com>
> ---
>  arch/arm/boards/Makefile                          |    1 +
>  arch/arm/boards/udoo/Makefile                     |    3 +
>  arch/arm/boards/udoo/board.c                      |  214 +++++++++++++++++++++
>  arch/arm/boards/udoo/env/config-board             |    6 +
>  arch/arm/boards/udoo/flash-header-mx6-udoo.imxcfg |  104 ++++++++++
>  arch/arm/boards/udoo/lowlevel.c                   |   17 ++
>  arch/arm/configs/udoo_quad_defconfig              |   86 +++++++++
>  arch/arm/dts/Makefile                             |    4 +-
>  arch/arm/dts/imx6q-udoo.dts                       |   23 +++
>  arch/arm/dts/imx6qdl-udoo.dtsi                    |  108 +++++++++++
>  arch/arm/mach-imx/Kconfig                         |    5 +
>  arch/arm/tools/mach-types                         |    1 +
>  images/Makefile.imx                               |    5 +
>  13 files changed, 576 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/boards/udoo/Makefile
>  create mode 100644 arch/arm/boards/udoo/board.c
>  create mode 100644 arch/arm/boards/udoo/env/config-board
>  create mode 100644 arch/arm/boards/udoo/flash-header-mx6-udoo.imxcfg
>  create mode 100644 arch/arm/boards/udoo/lowlevel.c
>  create mode 100644 arch/arm/configs/udoo_quad_defconfig
>  create mode 100644 arch/arm/dts/imx6q-udoo.dts
>  create mode 100644 arch/arm/dts/imx6qdl-udoo.dtsi
> 
[...]
> +
> +static int udoo_mem_init(void)
> +{
> +	arm_add_mem_device("ram0", 0x10000000, SZ_1G);
> +
> +	return 0;
> +}
> +mem_initcall(udoo_mem_init);
> +
Do you need this? Barebox should be able to figure this out from the
device tree.

> +static int ksz9021rn_phy_fixup(struct phy_device *dev)
> +{
> +
> +	phy_write( dev , 0x09 , 0x1c00 );
> +	phy_write( dev , 0x4 , 0x0000 );
> +	phy_write( dev , 0x5 , 0x0000 );
> +	phy_write( dev , 0x6 , 0x0000 );
> +	phy_write( dev , 0x8 , 0x03ff );
> +
Whitespace is looking weird here.

> +
> +	/* do same as linux kernel */
> +	/* min rx data delay */
> +	phy_write(dev, 0x0b, 0x8105);
> +	phy_write(dev, 0x0c, 0x0000);
> +
> +	/* max rx/tx clock delay, min rx/tx control delay */
> +	phy_write(dev, 0x0b, 0x8104);
> +	phy_write(dev, 0x0c, 0xf0f0);
> +	phy_write(dev, 0x0b, 0x104);
> +
> +	return 0;
> +}
> +
> +static int udoo_ksz9021rn_setup(void)
> +{
> +	mxc_iomux_v3_setup_multiple_pads(udoo_enet_gpio_pads_1,
> +			ARRAY_SIZE(udoo_enet_gpio_pads_1));
> +
> +	gpio_direction_output( IMX_GPIO_NR( 2 , 31 ) , 1 ); /* Power on enet */
> +
> +	/* MODE strap-in pins: advertise all capabilities */
> +	gpio_direction_output(185, 1); /* GPIO 6-25 */
> +	gpio_direction_output(187, 1); /* GPIO 6-27 */
> +	gpio_direction_output(188, 1); /* GPIO 6-28*/
> +	gpio_direction_output(189, 1); /* GPIO 6-29 */
> +
> +	/* Enable 125 MHz clock output */
> +    gpio_direction_output(184, 1); /* GPIO 6-24 */
> +
> +	mdelay(10);
> +
> +	mdelay( 100 );
> +
Why the double delay? Is it needed? If yes, please coalesce into one
statement.

> +	gpio_free(IMX_GPIO_NR(6, 24));
> +	gpio_free(IMX_GPIO_NR(6, 25));
> +	gpio_free(IMX_GPIO_NR(6, 27));
> +	gpio_free(IMX_GPIO_NR(6, 28));
> +	gpio_free(IMX_GPIO_NR(6, 29));
> +
> +	mxc_iomux_v3_setup_multiple_pads(udoo_enet_gpio_pads_2, ARRAY_SIZE(udoo_enet_gpio_pads_2));
> +
> +	return 0;
> +}
[...]
> +
> +static void udoo_epit_init( void )
> +{
> +
> +	writel( 0x0000000 , 0x20D0000 );
> +	writel( 0x142000F , 0x20D0000 );
> +	writel( 0x10000 , 0x20D0008 );
> +	writel( 0x0 , 0x20D000C );
> +}
Whitespace.
> +
> +static int udoo_devices_init(void)
> +{
> +	if (!of_machine_is_compatible("fsl,imx6q-udoo"))
> +		return 0;
> +
> +	setup_iomux_wdog();
> +	udoo_ehci_init();
> +	udoo_epit_init();
> +
> +	armlinux_set_bootparams((void *)0x10000100);
> +	armlinux_set_architecture(4772);
> +
This machine ID is registered as "i.mx6 Clous". I don't think this is
equal to the board you are adding here.
Do you even need the machine ID, or is your board DT enabled in the
linux kernel. If so, please remove this code.

[...]
> diff --git a/arch/arm/configs/udoo_quad_defconfig b/arch/arm/configs/udoo_quad_defconfig
> new file mode 100644
> index 0000000..880318f
> --- /dev/null
> +++ b/arch/arm/configs/udoo_quad_defconfig
> @@ -0,0 +1,86 @@

You are adding a multi-image board, please don't add a separate
defconfig, but instead add the board and maybe required options to the
imx_v7_defconfig.

Regards,
Lucas
-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox