[U-Boot] [PATCH 16/31] iMX28: Add GPIO control

[Stefano Babic <sbabic@denx.de>]

On 09/08/2011 10:42 PM, Marek Vasut wrote:
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Detlev Zundel <dzu@denx.de>
> ---
>  arch/arm/cpu/arm926ejs/mx28/Makefile  |    2 +-
>  arch/arm/cpu/arm926ejs/mx28/gpio.c    |  108 +++++++++++++++++++++++++++++++++
>  arch/arm/cpu/arm926ejs/mx28/mx28.c    |   10 +++
>  arch/arm/include/asm/arch-mx28/gpio.h |   32 ++++++++++
>  4 files changed, 151 insertions(+), 1 deletions(-)
>  create mode 100644 arch/arm/cpu/arm926ejs/mx28/gpio.c
>  create mode 100644 arch/arm/include/asm/arch-mx28/gpio.h
> 
> diff --git a/arch/arm/cpu/arm926ejs/mx28/Makefile b/arch/arm/cpu/arm926ejs/mx28/Makefile
> index 7845310..e3f0a12 100644
> --- a/arch/arm/cpu/arm926ejs/mx28/Makefile
> +++ b/arch/arm/cpu/arm926ejs/mx28/Makefile
> @@ -25,7 +25,7 @@ include $(TOPDIR)/config.mk
>  
>  LIB	= $(obj)lib$(SOC).o
>  
> -COBJS	= clock.o mx28.o iomux.o timer.o
> +COBJS	= clock.o mx28.o gpio.o iomux.o timer.o
>  
>  SRCS	:= $(START:.o=.S) $(COBJS:.o=.c)
>  OBJS	:= $(addprefix $(obj),$(COBJS))
> diff --git a/arch/arm/cpu/arm926ejs/mx28/gpio.c b/arch/arm/cpu/arm926ejs/mx28/gpio.c

No. All drivers go into the drivers directory (we try to follow the
Linux's rule..).

See drivers/gpio/.

As name, I suggest you steal the name from kernel (mxs_gpio.c seems to
me appropriate).


As remark: in the kernel, the structure for this SOC are identified by
the prefix mxs. This is valid for both MX28 and MX23. I want only to
point out how it is done in the kernel, I know we have not (yet) support
for MX23 in u-boot. Any chance to uniform names with the  kernel ?

> new file mode 100644
> index 0000000..00d2cca
> --- /dev/null
> +++ b/arch/arm/cpu/arm926ejs/mx28/gpio.c
> @@ -0,0 +1,108 @@
> +/*
> + * Freescale i.MX28 GPIO control code
> + *
> + * Copyright (C) 2011 Marek Vasut <marek.vasut@gmail.com>
> + * on behalf of DENX Software Engineering GmbH
> + *
> + * 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
> + */
> +
> +#include <common.h>
> +#include <netdev.h>
> +#include <asm/errno.h>
> +#include <asm/io.h>
> +#include <asm/arch/clock.h>
> +#include <asm/arch/regs-common.h>
> +#include <asm/arch/regs-base.h>
> +#include <asm/arch/iomux.h>
> +
> +#if	defined(CONFIG_MX23)

Ok. So you *want* to add support for MX23, too. Thanks for doing that !

Then probably the name "mxs" is not very far to be used....

> +#define	PINCTRL_BANKS		3
> +#define	PINCTRL_DOUT(n)		(0x0500 + ((n) * 0x10))
> +#define	PINCTRL_DIN(n)		(0x0600 + ((n) * 0x10))
> +#define	PINCTRL_DOE(n)		(0x0700 + ((n) * 0x10))
> +#define	PINCTRL_PIN2IRQ(n)	(0x0800 + ((n) * 0x10))
> +#define	PINCTRL_IRQEN(n)	(0x0900 + ((n) * 0x10))
> +#define	PINCTRL_IRQSTAT(n)	(0x0c00 + ((n) * 0x10))

No structures for that ?

> +void mxs_gpio_direction(iomux_cfg_t pad, int output)
> +{

This is not correct. Blackfin addes as first a common way to use gpio,
and we try to uniform the other SOC to the same rule. We should have not
special SOC_something_gpio_function.

The interface is in asm/asm/gpio.h. Each SOC must implement (you can
check on other i.MX SOCs) only those functions:

int gpio_request(int gp, const char *label);
void gpio_free(int gp);
void gpio_toggle_value(int gp);
int gpio_direction_input(int gp);
int gpio_direction_output(int gp, int value);
int gpio_get_value(int gp);
void gpio_set_value(int gp, int value);

This is the common interface - special definition for each SOC can be
put into asm/arch/gpio.h


> +void mxs_gpio_init(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < PINCTRL_BANKS; i++) {
> +		writel(0, MXS_PINCTRL_BASE + PINCTRL_PIN2IRQ(i));
> +		writel(0, MXS_PINCTRL_BASE + PINCTRL_IRQEN(i));
> +		/* Use SCT address here to clear the IRQSTAT bits */
> +		writel(0xffffffff, MXS_PINCTRL_BASE + PINCTRL_IRQSTAT(i) + 8);
> +	}

I am not sure this stuff should be done here. It should be
responsibility of the board maintainer to execute it - not only, to
decide how to set up the pinmux at all.

> diff --git a/arch/arm/cpu/arm926ejs/mx28/mx28.c b/arch/arm/cpu/arm926ejs/mx28/mx28.c
> index dca54d2..68f0da3 100644
> --- a/arch/arm/cpu/arm926ejs/mx28/mx28.c
> +++ b/arch/arm/cpu/arm926ejs/mx28/mx28.c
> @@ -30,6 +30,8 @@
>  #include <asm/errno.h>
>  #include <asm/io.h>
>  #include <asm/arch/clock.h>
> +#include <asm/arch/iomux.h>
> +#include <asm/arch/gpio.h>
>  #include <asm/arch/regs-common.h>
>  #include <asm/arch/regs-base.h>
>  #include <asm/arch/regs-clkctrl.h>
> @@ -118,6 +120,14 @@ int mx28_reset_block(struct mx28_register *reg)
>  	return 0;
>  }
>  
> +#ifdef	CONFIG_ARCH_CPU_INIT
> +int arch_cpu_init(void)
> +{
> +	mxs_gpio_init();
> +	return 0;
> +}

Please squash your patches - I am already confused after a few patches.

> +#endif
> +
>  #if defined(CONFIG_DISPLAY_CPUINFO)
>  int print_cpuinfo(void)
>  {
> diff --git a/arch/arm/include/asm/arch-mx28/gpio.h b/arch/arm/include/asm/arch-mx28/gpio.h
> new file mode 100644
> index 0000000..9435538
> --- /dev/null
> +++ b/arch/arm/include/asm/arch-mx28/gpio.h
> @@ -0,0 +1,32 @@
> +/*
> + * Freescale i.MX28 GPIO
> + *
> + * Copyright (C) 2011 Marek Vasut <marek.vasut@gmail.com>
> + * on behalf of DENX Software Engineering GmbH
> + *
> + * 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 __GPIO_H__
> +#define __GPIO_H__
> +
> +void mxs_gpio_set(iomux_cfg_t pad, int value);
> +int mxs_gpio_get(iomux_cfg_t pad);
> +void mxs_gpio_direction(iomux_cfg_t pad, int output);

No. This file should be dropped, it is not required.

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-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================