From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wang Huan-B18965 <B18965@freescale.com>
Date: Wed, 22 May 2013 05:22:09 +0000
Subject: [U-Boot] [PATCH v3 2/6] arm: mvf600: Add IOMUX support for
 Vybrid MVF600
Message-ID: <81BA6E5E0BC2344391CABCEE22D1B6D8336C9E@039-SN1MPN1-003.039d.mgd.msft.net>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de


Hi, Benoit, 

On Tuesday, May 21, 2013 11:02:57 AM, Alison Wang wrote:
> This patch adds the IOMUX support for Vybrid MVF600 platform.
>
> There is a little difference for IOMUXC module between MVF600 and i.MX
> platform, the muxmode and pad configuration share one 32bit register on
> MVF600, but they are two independent registers on I.MX platform. A
> CONFIG_IOMUX_SHARE_CONFIG_REG was introduced to fit this difference.
>
> Signed-off-by: Alison Wang <b18965@freescale.com>
> ---
> Changes in v3:
> - Define PAD_CTL_PUE with PKE enabled
>
> Changes in v2:
> - Use common iomux-v3 code
>
>  arch/arm/imx-common/Makefile               |  2 +-
>  arch/arm/imx-common/iomux-v3.c             |  6 ++++++
>  arch/arm/include/asm/imx-common/iomux-v3.h | 18 ++++++++++++++++++
>  3 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/imx-common/Makefile b/arch/arm/imx-common/Makefile
> index 8bba8a5..3378931 100644
> --- a/arch/arm/imx-common/Makefile
> +++ b/arch/arm/imx-common/Makefile
> @@ -27,7 +27,7 @@ include $(TOPDIR)/config.mk
>
>  LIB     = $(obj)libimx-common.o
>
> -ifeq ($(SOC),$(filter $(SOC),mx25 mx35 mx5 mx6))
> +ifeq ($(SOC),$(filter $(SOC),mx25 mx35 mx5 mx6 mvf600))
>  COBJS-y      = iomux-v3.o
>  endif
>  ifeq ($(SOC),$(filter $(SOC),mx5 mx6))
> diff --git a/arch/arm/imx-common/iomux-v3.c b/arch/arm/imx-common/iomux-v3.c
> index 7fe5ce7..35880c7 100644
> --- a/arch/arm/imx-common/iomux-v3.c
> +++ b/arch/arm/imx-common/iomux-v3.c
> @@ -48,8 +48,14 @@ void imx_iomux_v3_setup_pad(iomux_v3_cfg_t pad)
>       if (sel_input_ofs)
>               __raw_writel(sel_input, base + sel_input_ofs);
>
> +#ifdef CONFIG_IOMUX_SHARE_CONF_REG
> +     if (!(pad_ctrl & NO_PAD_CTRL))
> +             __raw_writel((mux_mode << PAD_MUX_MODE_SHIFT) | pad_ctrl,
> +                     base + pad_ctrl_ofs);
> +#else
>       if (!(pad_ctrl & NO_PAD_CTRL) && pad_ctrl_ofs)
>               __raw_writel(pad_ctrl, base + pad_ctrl_ofs);
> +#endif
>  }
>
>  void imx_iomux_v3_setup_multiple_pads(iomux_v3_cfg_t const *pad_list,
> diff --git a/arch/arm/include/asm/imx-common/iomux-v3.h
> b/arch/arm/include/asm/imx-common/iomux-v3.h
> index 0b4e763..012d66a 100644
> --- a/arch/arm/include/asm/imx-common/iomux-v3.h
> +++ b/arch/arm/include/asm/imx-common/iomux-v3.h
> @@ -121,6 +121,24 @@ typedef u64 iomux_v3_cfg_t;
>  #define PAD_CTL_DSE_40ohm    (6 << 3)
>  #define PAD_CTL_DSE_34ohm    (7 << 3)
>
> +#elif defined(CONFIG_MVF600)
> +
> +#define PAD_MUX_MODE_SHIFT   20
> +
> +#define      PAD_CTL_PUS_47K_UP      (1 << 4)
> +#define      PAD_CTL_PUS_100K_UP     (2 << 4)

PAD_CTL_PUE should be bitwise-OR-ed to the two values above to make it easy to
use, just like you added PAD_CTL_PKE to PAD_CTL_PUE below.
[Alison Wang] Agree.

> +#define PAD_CTL_PUE          (1 << 2 | PAD_CTL_PKE)
> +#define      PAD_CTL_PKE             (1 << 3)
> +
> +#define PAD_CTL_SPEED_HIGH   (3 << 12)
> +#define PAD_CTL_SPEED_MED    (1 << 12)
> +
> +#define PAD_CTL_DSE_20ohm    (7 << 6)
> +#define PAD_CTL_DSE_25ohm    (6 << 6)
> +#define PAD_CTL_DSE_50ohm    (3 << 6)
> +
> +#define PAD_CTL_OBE_IBE_ENABLE       (3 << 0)

Please organize those definitions by decreasing offset value and increasing
value in order to follow the reference manual order for easier review:
PAD_MUX_MODE_SHIFT, then PAD_CTL_SPEED_MED, PAD_CTL_SPEED_HIGH,
PAD_CTL_DSE_50ohm, PAD_CTL_DSE_25ohm, PAD_CTL_DSE_20ohm, PAD_CTL_PUS_47K_UP,
PAD_CTL_PUS_100K_UP, PAD_CTL_PKE, PAD_CTL_PUE, and PAD_CTL_OBE_IBE_ENABLE.
[Alison Wang] Agree.

Also, the spaces/tabs are still not correct in those definitions. They should
be:
#define<1 space>NAME<1 or more tabs to align column>value

> +
>  #else
>
>  #define PAD_CTL_DVS          (1 << 13)
[Alison Wang] Yes. I will change. Thanks.

Best regards,
Alison Wang