[U-Boot,1/2] gpio: add gpio api support to mx27 (v3)

There is a little change on gpio_get_value because
on mx27 the register to read is ssr and not dr.

Signed-off-by: Philippe Reynes <tremyfr@yahoo.fr>
---
 arch/arm/cpu/arm926ejs/mx27/generic.c     |   11 +++---
 arch/arm/include/asm/arch-mx27/gpio.h     |   55 +++++++++++++++++++++++++++++
 arch/arm/include/asm/arch-mx27/imx-regs.h |   30 ++++------------
 drivers/gpio/mxc_gpio.c                   |   12 +++++--
 4 files changed, 77 insertions(+), 31 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-mx27/gpio.h

On 24/08/2012 22:36, Philippe Reynes wrote:
> There is a little change on gpio_get_value because
> on mx27 the register to read is ssr and not dr.
> 
> Signed-off-by: Philippe Reynes <tremyfr@yahoo.fr>
> ---

Hi Philippe,

>  arch/arm/cpu/arm926ejs/mx27/generic.c     |   11 +++---
>  arch/arm/include/asm/arch-mx27/gpio.h     |   55 +++++++++++++++++++++++++++++
>  arch/arm/include/asm/arch-mx27/imx-regs.h |   30 ++++------------
>  drivers/gpio/mxc_gpio.c                   |   12 +++++--
>  4 files changed, 77 insertions(+), 31 deletions(-)
>  create mode 100644 arch/arm/include/asm/arch-mx27/gpio.h
> 
> diff --git a/arch/arm/cpu/arm926ejs/mx27/generic.c b/arch/arm/cpu/arm926ejs/mx27/generic.c
> index 65c4813..41bb84b 100644
> --- a/arch/arm/cpu/arm926ejs/mx27/generic.c
> +++ b/arch/arm/cpu/arm926ejs/mx27/generic.c
> @@ -24,6 +24,7 @@
>  #include <asm/io.h>
>  #include <asm/arch/imx-regs.h>
>  #include <asm/arch/clock.h>
> +#include <asm/arch/gpio.h>
>  #ifdef CONFIG_MXC_MMC
>  #include <asm/arch/mxcmmc.h>
>  #endif
> @@ -209,7 +210,7 @@ int cpu_mmc_init(bd_t *bis)
>  
>  void imx_gpio_mode(int gpio_mode)
>  {
> -	struct gpio_regs *regs = (struct gpio_regs *)IMX_GPIO_BASE;
> +	struct gpio_port_regs *regs = (struct gpio_port_regs *)IMX_GPIO_BASE;
>  	unsigned int pin = gpio_mode & GPIO_PIN_MASK;
>  	unsigned int port = (gpio_mode & GPIO_PORT_MASK) >> GPIO_PORT_SHIFT;
>  	unsigned int ocr = (gpio_mode & GPIO_OCR_MASK) >> GPIO_OCR_SHIFT;
> @@ -228,11 +229,11 @@ void imx_gpio_mode(int gpio_mode)
>  
>  	/* Data direction */
>  	if (gpio_mode & GPIO_OUT) {
> -		writel(readl(&regs->port[port].ddir) | 1 << pin,
> -				&regs->port[port].ddir);
> +		writel(readl(&regs->port[port].gpio_dir) | 1 << pin,
> +				&regs->port[port].gpio_dir);
>  	} else {
> -		writel(readl(&regs->port[port].ddir) & ~(1 << pin),
> -				&regs->port[port].ddir);
> +		writel(readl(&regs->port[port].gpio_dir) & ~(1 << pin),
> +				&regs->port[port].gpio_dir);
>  	}
>  
>  	/* Primary / alternate function */
> diff --git a/arch/arm/include/asm/arch-mx27/gpio.h b/arch/arm/include/asm/arch-mx27/gpio.h
> new file mode 100644
> index 0000000..965b584
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-mx27/gpio.h
> @@ -0,0 +1,55 @@
> +/*
> + * Copyright (C) 2012
> + * Philippe Reynes <tremyfr@yahoo.fr>
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +
> +#ifndef __ASM_ARCH_MX27_GPIO_H
> +#define __ASM_ARCH_MX27_GPIO_H
> +
> +/* GPIO registers */
> +struct gpio_regs {
> +	u32 gpio_dir; /* DDIR */
> +	u32 ocr1;
> +	u32 ocr2;
> +	u32 iconfa1;
> +	u32 iconfa2;
> +	u32 iconfb1;
> +	u32 iconfb2;
> +	u32 gpio_dr; /* DR */
> +	u32 gius;
> +	u32 ssr;

What about my proposal to change this into gpio_psr ? I mean, if
Freescale calls some times the register "Sample Status Register" and on
other SOCs "Pad Status Register", but anyway the register does the same,
it is not a good reason to follow it.

> +	u32 icr1;
> +	u32 icr2;
> +	u32 imr;
> +	u32 isr;
> +	u32 gpr;
> +	u32 swr;
> +	u32 puen;
> +	u32 res[0x2f];
> +};
> +
> +/* This structure is used by the function imx_gpio_mode */
> +struct gpio_port_regs {
> +	struct gpio_regs port[6];
> +};
> +
> +#endif
> diff --git a/arch/arm/include/asm/arch-mx27/imx-regs.h b/arch/arm/include/asm/arch-mx27/imx-regs.h
> index f7cf85b..f78d5f2 100644
> --- a/arch/arm/include/asm/arch-mx27/imx-regs.h
> +++ b/arch/arm/include/asm/arch-mx27/imx-regs.h
> @@ -164,29 +164,6 @@ struct gpt_regs {
>  #define PORTE 4
>  #define PORTF 5
>  
> -struct gpio_regs {
> -	struct {
> -		u32 ddir;
> -		u32 ocr1;
> -		u32 ocr2;
> -		u32 iconfa1;
> -		u32 iconfa2;
> -		u32 iconfb1;
> -		u32 iconfb2;
> -		u32 dr;
> -		u32 gius;
> -		u32 ssr;
> -		u32 icr1;
> -		u32 icr2;
> -		u32 imr;
> -		u32 isr;
> -		u32 gpr;
> -		u32 swr;
> -		u32 puen;
> -		u32 res[0x2f];
> -	} port[6];
> -};
> -
>  /* IIM Control Registers */
>  struct iim_regs {
>  	u32 iim_stat;
> @@ -474,6 +451,13 @@ struct fuse_bank0_regs {
>  #define TSTAT_CAPT	(1 << 1)	/* Capture event */
>  #define TSTAT_COMP	1		/* Compare event */
>  
> +#define GPIO1_BASE_ADDR 0x10015000
> +#define GPIO2_BASE_ADDR 0x10015100
> +#define GPIO3_BASE_ADDR 0x10015200
> +#define GPIO4_BASE_ADDR 0x10015300
> +#define GPIO5_BASE_ADDR 0x10015400
> +#define GPIO6_BASE_ADDR 0x10015500
> +
>  #define GPIO_PIN_MASK	0x1f
>  
>  #define GPIO_PORT_SHIFT	5
> diff --git a/drivers/gpio/mxc_gpio.c b/drivers/gpio/mxc_gpio.c
> index e9bf278..2c2c4bb 100644
> --- a/drivers/gpio/mxc_gpio.c
> +++ b/drivers/gpio/mxc_gpio.c
> @@ -41,13 +41,15 @@ static unsigned long gpio_ports[] = {
>  	[0] = GPIO1_BASE_ADDR,
>  	[1] = GPIO2_BASE_ADDR,
>  	[2] = GPIO3_BASE_ADDR,
> -#if defined(CONFIG_MX25) || defined(CONFIG_MX51) || defined(CONFIG_MX53) || \
> -		defined(CONFIG_MX6Q)
> +#if defined(CONFIG_MX25) || defined(CONFIG_MX27) || defined(CONFIG_MX51) || \
> +		defined(CONFIG_MX53) || defined(CONFIG_MX6Q)
>  	[3] = GPIO4_BASE_ADDR,
>  #endif
> -#if defined(CONFIG_MX53) || defined(CONFIG_MX6Q)
> +#if defined(CONFIG_MX27) || defined(CONFIG_MX53) || defined(CONFIG_MX6Q)
>  	[4] = GPIO5_BASE_ADDR,
>  	[5] = GPIO6_BASE_ADDR,
> +#endif
> +#if defined(CONFIG_MX53) || defined(CONFIG_MX6Q)
>  	[6] = GPIO7_BASE_ADDR,
>  #endif
>  };
> @@ -116,7 +118,11 @@ int gpio_get_value(unsigned gpio)
>  
>  	regs = (struct gpio_regs *)gpio_ports[port];
>  
> +#ifndef CONFIG_MX27
>  	val = (readl(&regs->gpio_psr) >> gpio) & 0x01;
> +#else
> +	val = (readl(&regs->ssr) >> gpio) & 0x01;
> +#endif

Not required if ssr becomes gpio_psr

Best regards,
Stefano Babic

Hi Stefano,


> +    u32 gpio_dr; /* DR */
> +    u32 gius;
> +    u32 ssr;

What about my proposal to change this into gpio_psr ? I mean, if
Freescale calls some times the register "Sample Status Register" and on
other SOCs "Pad Status Register", but anyway the register does the same,
it is not a good reason to follow it.


I think you're right, sorry, I read your feedback too fast and missed this one.
I update my patch and send a v4.



> +#ifndef CONFIG_MX27
>      val = (readl(&regs->gpio_psr) >> gpio) & 0x01;
> +#else
> +    val = (readl(&regs->ssr) >> gpio) & 0x01;
> +#endif

Not required if ssr becomes gpio_psr

I agree too.


Regards,
Philippe