From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefano Babic <sbabic@denx.de>
Date: Fri, 11 Jul 2014 13:10:13 +0200
Subject: [U-Boot] [PATCH v1 1/4] arm,
	imx6: add mxc_iomux_set_gpr_register function
In-Reply-To: <1401272177-16107-2-git-send-email-hs@denx.de>
References: <1401272177-16107-1-git-send-email-hs@denx.de>
	<1401272177-16107-2-git-send-email-hs@denx.de>
Message-ID: <53BFC615.6000102@denx.de>
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 Heiko,

On 28/05/2014 12:16, Heiko Schocher wrote:
> add mxc_iomux_set_gpr_register fucntion
> 
> Signed-off-by: Heiko Schocher <hs@denx.de>
> Cc: Stefano Babic <sbabic@denx.de>
> ---
>  arch/arm/imx-common/iomux-v3.c             | 15 +++++++++++++++
>  arch/arm/include/asm/imx-common/iomux-v3.h |  2 ++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/arch/arm/imx-common/iomux-v3.c b/arch/arm/imx-common/iomux-v3.c
> index 6e46ea8..8739b4c 100644
> --- a/arch/arm/imx-common/iomux-v3.c
> +++ b/arch/arm/imx-common/iomux-v3.c
> @@ -63,3 +63,18 @@ void imx_iomux_v3_setup_multiple_pads(iomux_v3_cfg_t const *pad_list,
>  	for (i = 0; i < count; i++)
>  		imx_iomux_v3_setup_pad(*p++);
>  }
> +
> +void mxc_iomux_set_gpr_register(int group, int start_bit, int num_bits,
> +				int value)
> +{
> +	int i = 0;
> +	u32 reg;
> +	reg = __raw_readl(base + group * 4);
> +	while (num_bits) {
> +		reg &= ~(1<<(start_bit + i));
> +		i++;
> +		num_bits--;
> +	}
> +	reg |= (value << start_bit);
> +	__raw_writel(reg, base + group * 4);
> +}

It is not clear to me why we need to add this or better, I want to avoid
having twice quite the same code doing the same ;-).

We have already a field for gpr:
struct iomuxc {
        u32 gpr[14];
        u32 omux[5];
        /* mux and pad registers */
};

+	reg = __raw_readl(base + group * 4);

this could not work when Freescale introduce a new variant with a slight
different layout. Thanks to Fabio, we know this is already reality:

	http://patchwork.ozlabs.org/patch/368336/

Your code *does* not work with the solox.

Generally, we access register and we use setbits_ and clrsetbits_ to
modify single bits. What is the advantage here ?

> diff --git a/arch/arm/include/asm/imx-common/iomux-v3.h b/arch/arm/include/asm/imx-common/iomux-v3.h
> index cca920b..4c83617 100644
> --- a/arch/arm/include/asm/imx-common/iomux-v3.h
> +++ b/arch/arm/include/asm/imx-common/iomux-v3.h
> @@ -174,5 +174,7 @@ typedef u64 iomux_v3_cfg_t;
>  void imx_iomux_v3_setup_pad(iomux_v3_cfg_t pad);
>  void imx_iomux_v3_setup_multiple_pads(iomux_v3_cfg_t const *pad_list,
>  				     unsigned count);
> +void mxc_iomux_set_gpr_register(int group, int start_bit, int num_bits,
> +				int value);
>  
>  #endif	/* __MACH_IOMUX_V3_H__*/
> 

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
=====================================================================