From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?utf-8?Q?Beno=C3=AEt_Th=C3=A9baudeau?= Date: Sat, 18 Aug 2012 21:25:58 +0200 (CEST) Subject: [U-Boot] [PATCH] MX: Set a common gpio.h for all i.MX In-Reply-To: <1345303610-3964-1-git-send-email-sbabic@denx.de> Message-ID: <1281266427.2541001.1345317958784.JavaMail.root@advansee.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Stefano, > Each i.MX has its own gpio.h, defining the same structure. > The internal GPIO controller has the same layout > (at least for the register used by u-boot) and can be shared. Good! > > Signed-off-by: Stefano Babic > CC: Matt Sealey > CC: Marek Vasut > CC: Benoit Thebaudeau > CC: Jason Liu > --- > arch/arm/include/asm/arch-mx25/gpio.h | 12 +-------- > arch/arm/include/asm/arch-mx31/gpio.h | 7 +----- > arch/arm/include/asm/arch-mx35/gpio.h | 12 +-------- > arch/arm/include/asm/arch-mx5/gpio.h | 7 +----- > arch/arm/include/asm/arch-mx6/gpio.h | 7 +----- > arch/arm/include/asm/arch-mx6/imx-regs.h | 2 -- > arch/arm/include/asm/imx-common/gpio.h | 39 > ++++++++++++++++++++++++++++++ > include/configs/mx6qsabrelite.h | 1 + > 8 files changed, 45 insertions(+), 42 deletions(-) > create mode 100644 arch/arm/include/asm/imx-common/gpio.h > > diff --git a/arch/arm/include/asm/arch-mx25/gpio.h > b/arch/arm/include/asm/arch-mx25/gpio.h > index dc6edc7..5ab1f6d 100644 > --- a/arch/arm/include/asm/arch-mx25/gpio.h > +++ b/arch/arm/include/asm/arch-mx25/gpio.h > @@ -30,16 +30,6 @@ > */ > #define MXC_GPIO_PORT_TO_NUM(port, bit) (((port - 1) << 5) + (bit & > 0x1f)) Keeping this is also useless. GPIO_NUMBER() from the new can be used instead everywhere needed. > > -/* GPIO registers */ > -struct gpio_regs { > - u32 gpio_dr; /* data */ > - u32 gpio_dir; /* direction */ > - u32 psr; /* pad satus */ > - u32 icr1; /* interrupt config 1 */ > - u32 icr2; /* interrupt config 2 */ > - u32 imr; /* interrupt mask */ > - u32 isr; /* interrupt status */ > - u32 edge_sel; /* edge select */ > -}; > +#include > > #endif > diff --git a/arch/arm/include/asm/arch-mx31/gpio.h > b/arch/arm/include/asm/arch-mx31/gpio.h > index 95b73bf..55c0afa 100644 > --- a/arch/arm/include/asm/arch-mx31/gpio.h > +++ b/arch/arm/include/asm/arch-mx31/gpio.h > @@ -25,11 +25,6 @@ > #ifndef __ASM_ARCH_MX31_GPIO_H > #define __ASM_ARCH_MX31_GPIO_H > > -/* GPIO Registers */ > -struct gpio_regs { > - u32 gpio_dr; > - u32 gpio_dir; > - u32 gpio_psr; > -}; > +#include > > #endif > diff --git a/arch/arm/include/asm/arch-mx35/gpio.h > b/arch/arm/include/asm/arch-mx35/gpio.h > index 7bcc3e8..1deb292 100644 > --- a/arch/arm/include/asm/arch-mx35/gpio.h > +++ b/arch/arm/include/asm/arch-mx35/gpio.h > @@ -25,16 +25,6 @@ > #ifndef __ASM_ARCH_MX35_GPIO_H > #define __ASM_ARCH_MX35_GPIO_H > > -/* GPIO registers */ > -struct gpio_regs { > - u32 gpio_dr; /* data */ > - u32 gpio_dir; /* direction */ > - u32 psr; /* pad satus */ > - u32 icr1; /* interrupt config 1 */ > - u32 icr2; /* interrupt config 2 */ > - u32 imr; /* interrupt mask */ > - u32 isr; /* interrupt status */ > - u32 edge_sel; /* edge select */ > -}; > +#include > > #endif > diff --git a/arch/arm/include/asm/arch-mx5/gpio.h > b/arch/arm/include/asm/arch-mx5/gpio.h > index 1dc34e9..b1b1218 100644 > --- a/arch/arm/include/asm/arch-mx5/gpio.h > +++ b/arch/arm/include/asm/arch-mx5/gpio.h > @@ -25,11 +25,6 @@ > #ifndef __ASM_ARCH_MX5_GPIO_H > #define __ASM_ARCH_MX5_GPIO_H > > -/* GPIO registers */ > -struct gpio_regs { > - u32 gpio_dr; > - u32 gpio_dir; > - u32 gpio_psr; > -}; > +#include > > #endif > diff --git a/arch/arm/include/asm/arch-mx6/gpio.h > b/arch/arm/include/asm/arch-mx6/gpio.h > index 20c4e57..24c10f8 100644 > --- a/arch/arm/include/asm/arch-mx6/gpio.h > +++ b/arch/arm/include/asm/arch-mx6/gpio.h > @@ -25,11 +25,6 @@ > #ifndef __ASM_ARCH_MX6_GPIO_H > #define __ASM_ARCH_MX6_GPIO_H > > -/* GPIO registers */ > -struct gpio_regs { > - u32 gpio_dr; > - u32 gpio_dir; > - u32 gpio_psr; > -}; > +#include > > #endif /* __ASM_ARCH_MX6_GPIO_H */ Why do you keep all these old ? The new can be included instead everywhere needed. > diff --git a/arch/arm/include/asm/arch-mx6/imx-regs.h > b/arch/arm/include/asm/arch-mx6/imx-regs.h > index 5d77603..f3e58b5 100644 > --- a/arch/arm/include/asm/arch-mx6/imx-regs.h > +++ b/arch/arm/include/asm/arch-mx6/imx-regs.h > @@ -172,8 +172,6 @@ > #define IMX_IIM_BASE OCOTP_BASE_ADDR > #define FEC_QUIRK_ENET_MAC > > -#define GPIO_NUMBER(port, index) ((((port)-1)*32)+((index)&31)) > - > #if !(defined(__KERNEL_STRICT_NAMES) || defined(__ASSEMBLY__)) > #include > > diff --git a/arch/arm/include/asm/imx-common/gpio.h > b/arch/arm/include/asm/imx-common/gpio.h > new file mode 100644 > index 0000000..fcc25e8 > --- /dev/null > +++ b/arch/arm/include/asm/imx-common/gpio.h > @@ -0,0 +1,39 @@ > +/* > + * Copyright (C) 2011 > + * Stefano Babic, DENX Software Engineering, > + * > + * 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_IMX_GPIO_H > +#define __ASM_ARCH_IMX_GPIO_H > + > +#if !(defined(__KERNEL_STRICT_NAMES) || defined(__ASSEMBLY__)) > +/* GPIO registers */ > +struct gpio_regs { > + u32 gpio_dr; /* data */ > + u32 gpio_dir; /* direction */ > + u32 psr; /* pad satus */ Why don't you rename this to gpio_psr to be more consistent? This made me wonder how mxc_gpio.c could build successfully with this naming. It's because gpio_get_value() uses DR instead of PSR. This is an issue. Linux uses PSR, and U-Boot should do so since DR does not always reflect the pin state, while PSR does. This makes a big difference if you want to detect a short circuit on a GPIO pin configured as output, or if you want to read the level of a pin driven by a non-GPIO function. This is another patch to make. > +}; > +#endif > + > +#define GPIO_NUMBER(port, index) ((((port)-1)*32)+((index)&31)) > + > +#endif > diff --git a/include/configs/mx6qsabrelite.h > b/include/configs/mx6qsabrelite.h > index 0d376ba..700268e 100644 > --- a/include/configs/mx6qsabrelite.h > +++ b/include/configs/mx6qsabrelite.h > @@ -31,6 +31,7 @@ > #define CONFIG_MACH_TYPE 3769 > > #include > +#include > > #define CONFIG_CMDLINE_TAG > #define CONFIG_SETUP_MEMORY_TAGS > -- > 1.7.9.5 > > The rest sounds good. This is a bit off topic, but shouldn't the iomux-v3 stuff be moved to a common location for all these i.MXs too? As to the header file, it's already done, but the C file is under arch/arm/cpu/armv7/imx-common/ while it is absolutely not armv7-specific. Unlike Linux's, U-Boot's SoC files are organized per CPU instead of per platform, which makes the organization of such platform common code a bit complicated. This also encourages the duplication of platform code. Perhaps it could be moved to a new arch/arm/cpu/imx-common/ folder (this would require an addition to the main Makefile)? Best regards, Beno?t