[U-Boot] [PATCH 1/2] mx5: Add default pin initializers

[Stefano Babic <sbabic@denx.de>]

On 14/08/2012 17:46, Beno?t Th?baudeau wrote:
> Create default pin initialization functions for the default iomux function
> assignments of the main peripherals.
> 
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau@advansee.com>
> Cc: Stefano Babic <sbabic@denx.de>
> ---

Hi Beno?t,

>  .../arch/arm/cpu/armv7/mx5/soc.c                   |  139 ++++++++++++++++++++
>  .../arch/arm/include/asm/arch-mx5/sys_proto.h      |    5 +
>  2 files changed, 144 insertions(+)
> 
> diff --git u-boot-4d3c95f.orig/arch/arm/cpu/armv7/mx5/soc.c u-boot-4d3c95f/arch/arm/cpu/armv7/mx5/soc.c
> index 3f5a4f7..ee19b54 100644
> --- u-boot-4d3c95f.orig/arch/arm/cpu/armv7/mx5/soc.c
> +++ u-boot-4d3c95f/arch/arm/cpu/armv7/mx5/soc.c
> @@ -25,6 +25,8 @@
>  
>  #include <common.h>
>  #include <asm/arch/imx-regs.h>
> +#include <asm/arch/mx5x_pins.h>
> +#include <asm/arch/iomux.h>
>  #include <asm/arch/clock.h>
>  #include <asm/arch/sys_proto.h>
>  
> @@ -71,6 +73,143 @@ u32 get_cpu_rev(void)
>  	return system_rev;
>  }
>  
> +#ifdef CONFIG_MXC_UART
> +#if CONFIG_MXC_UART_BASE == UART1_BASE
> +#ifdef CONFIG_MX51
> +void mx51_uart1_init_pins(void)
> +{
> +	int in_pad, out_pad;
> +
> +	/* Set up pins for UART1. */
> +	mxc_request_iomux(MX51_PIN_UART1_RXD, IOMUX_CONFIG_ALT0);
> +	mxc_request_iomux(MX51_PIN_UART1_TXD, IOMUX_CONFIG_ALT0);
> +	mxc_request_iomux(MX51_PIN_UART1_RTS, IOMUX_CONFIG_ALT0);
> +	mxc_request_iomux(MX51_PIN_UART1_CTS, IOMUX_CONFIG_ALT0);
> +
> +	mxc_iomux_set_input(MX51_UART1_IPP_UART_RXD_MUX_SELECT_INPUT,
> +			    INPUT_CTL_PATH0);
> +	mxc_iomux_set_input(MX51_UART1_IPP_UART_RTS_B_SELECT_INPUT,
> +			    INPUT_CTL_PATH0);
> +
> +	in_pad = PAD_CTL_HYS_ENABLE | PAD_CTL_PKE_ENABLE | PAD_CTL_PUE_PULL |
> +		PAD_CTL_100K_PU | PAD_CTL_ODE_OPENDRAIN_NONE |
> +		PAD_CTL_DRV_HIGH | PAD_CTL_SRE_SLOW;
> +	out_pad = PAD_CTL_HYS_NONE | PAD_CTL_PKE_NONE |
> +		PAD_CTL_ODE_OPENDRAIN_NONE | PAD_CTL_DRV_HIGH |
> +		PAD_CTL_SRE_SLOW;
> +
> +	mxc_iomux_set_pad(MX51_PIN_UART1_RXD, in_pad);
> +	mxc_iomux_set_pad(MX51_PIN_UART1_TXD, out_pad);
> +	mxc_iomux_set_pad(MX51_PIN_UART1_RTS, in_pad);
> +	mxc_iomux_set_pad(MX51_PIN_UART1_CTS, out_pad);
> +}

Yes, the intention is surely to rationalize code and to avoid
duplication. But it requires to add such kind of function for any
instance of a peripheral (mx51_uart2_init_pins, for example) and cannot
worked on all boards. For example, a board will not use RTS and CTS, and
these pins are GPIOs (very common example). Or it can use other pins for
RX and TXD: the SOC allows this, and maybe someone want to use it.

The other thing I see (not in this patch, but checking the current
implementation in mainline) is that we have different implementation for
MX51, MX53 / MX6. This is bad.

I think that setting each pin with mxc_request_iomux() is not a great
idea. Maybe another solution would be to provide a table with the pinmux
for the whole SOC and a funtion to set all of them, as
imx_iomux_v3_setup_multiple_pads. This is also similar to the current
implementation in u-boot for other SOCs, for example TI.

IMHO to improve the current situation in i.MX pinmux code we should try
to use the same code, or at least, the same API for the board
maintainer, and putting the pinmux configuration in a table. This is
also what the legacy kernel does, putting in the table only the pins
required by u-boot itself (current kernel sets itself the pinmux, via
device tree or initializing the board when dts is not available).

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================