On 09/08/2011 10:42 PM, Marek Vasut wrote:
> Signed-off-by: Marek Vasut <marek.va...@gmail.com>
> Cc: Stefano Babic <sba...@denx.de>
> Cc: Wolfgang Denk <w...@denx.de>
> Cc: Detlev Zundel <d...@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.va...@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.va...@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: off...@denx.de
=====================================================================
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
