Hi Cyril, On Fri, Mar 26, 2010 at 03:13:57, Chemparathy, Cyril wrote: > Support for tnetv107x gpio controller. Note that this controller does not use > the gpio related definitions from davinci_soc_info.
... because this code is used only on one chip right now? How about when more devices of the same family are added? > > Further, this patch modifies davinci's inlined gpio functions to range check > against davinci_soc_info contents instead of DAVINCI_N_GPIO. This way, > tnetv107x systems can define davinci_soc_info.gpio_num to 0 and bypass the > inline routines. Why? Tnetv107x would benefit from inline GPIO functions as well. So, why not define separate inline functions for that chip? > > Signed-off-by: Cyril Chemparathy <[email protected]> > --- > arch/arm/mach-davinci/Makefile | 1 + > arch/arm/mach-davinci/gpio-tnetv107x.c | 155 > ++++++++++++++++++++++++ > arch/arm/mach-davinci/include/mach/gpio.h | 6 +- > arch/arm/mach-davinci/include/mach/tnetv107x.h | 4 + > 4 files changed, 163 insertions(+), 3 deletions(-) > create mode 100644 arch/arm/mach-davinci/gpio-tnetv107x.c > > diff --git a/arch/arm/mach-davinci/Makefile b/arch/arm/mach-davinci/Makefile > index 6aac880..0284a4c 100644 > --- a/arch/arm/mach-davinci/Makefile > +++ b/arch/arm/mach-davinci/Makefile > @@ -16,6 +16,7 @@ obj-$(CONFIG_ARCH_DAVINCI_DM646x) += dm646x.o > devices.o > obj-$(CONFIG_ARCH_DAVINCI_DM365) += dm365.o devices.o > obj-$(CONFIG_ARCH_DAVINCI_DA830) += da830.o devices-da8xx.o > obj-$(CONFIG_ARCH_DAVINCI_DA850) += da850.o devices-da8xx.o > +obj-$(CONFIG_ARCH_DAVINCI_TNETV107X) += gpio-tnetv107x.o Err, you are building in support for both DaVinci and tnetv107x gpios. I thought the DaVinci GPIO code is not applicable for this chip. > > obj-$(CONFIG_AINTC) += irq.o > obj-$(CONFIG_CP_INTC) += cp_intc.o > diff --git a/arch/arm/mach-davinci/gpio-tnetv107x.c > b/arch/arm/mach-davinci/gpio-tnetv107x.c > new file mode 100644 > index 0000000..d868b46 > --- /dev/null > +++ b/arch/arm/mach-davinci/gpio-tnetv107x.c > @@ -0,0 +1,155 @@ > +/* > + * TI TNETV107X GPIO Support > + * > + * Author: Cyril Chemparathy <[email protected]> > + * > + * 2009 (c) Texas Instruments, Inc. This file is licensed under > + * the terms of the GNU General Public License version 2. This program > + * is licensed "as is" without any warranty of any kind, whether express > + * or implied. > + */ > +#include <linux/kernel.h> > +#include <linux/init.h> > +#include <linux/gpio.h> > + > +#include <mach/common.h> > +#include <mach/tnetv107x.h> > + > +struct tnetv107x_gpio_regs { > + u32 idver; > + u32 data_in[3]; > + u32 data_out[3]; > + u32 direction[3]; > + u32 enable[3]; > +}; > + > +struct tnetv107x_gpio_controller { > + struct tnetv107x_gpio_regs __iomem *regs; > + struct gpio_chip chip; > +}; > + > +static struct tnetv107x_gpio_controller tnetv107x_gpio_controller; > + > +#define gpio_reg_index(gpio) ((gpio) >> 5) > +#define gpio_reg_bit(gpio) BIT((gpio) & 0x1f) > + > +#define gpio_reg_rmw(reg, mask, val) \ > + __raw_writel((__raw_readl(reg) & ~(mask)) | (val), (reg)) > + > +#define gpio_reg_set_bit(reg, gpio) \ > + gpio_reg_rmw((reg) + gpio_reg_index(gpio), 0, gpio_reg_bit(gpio)) > + > +#define gpio_reg_clear_bit(reg, gpio) \ > + gpio_reg_rmw((reg) + gpio_reg_index(gpio), gpio_reg_bit(gpio), 0) > + > +#define gpio_reg_get_bit(reg, gpio) \ > + (__raw_readl((reg) + gpio_reg_index(gpio)) & gpio_reg_bit(gpio)) > + > +#define chip_to_gpio(chip) \ > + container_of(chip, struct tnetv107x_gpio_controller, chip) > + > +static int tnetv107x_gpio_request(struct gpio_chip *chip, unsigned gpio) > +{ > + struct tnetv107x_gpio_controller *ctlr = chip_to_gpio(chip); > + unsigned long flags; > + > + local_irq_save(flags); Should use spin_lock() for protection. > + > + gpio_reg_set_bit(&ctlr->regs->enable, gpio); > + > + local_irq_restore(flags); > + > + return 0; > +} > + [...] > +void __init tnetv107x_gpio_init(void) > +{ > + struct tnetv107x_gpio_controller *ctlr = &tnetv107x_gpio_controller; > + > + ctlr->regs = ioremap(TNETV107X_GPIO_BASE, PAGE_SIZE); > + > + ctlr->chip.label = "TNETV107X"; > + ctlr->chip.request = tnetv107x_gpio_request; > + ctlr->chip.free = tnetv107x_gpio_free; > + ctlr->chip.direction_input = tnetv107x_gpio_dir_in; > + ctlr->chip.get = tnetv107x_gpio_get; > + ctlr->chip.direction_output = tnetv107x_gpio_dir_out; > + ctlr->chip.set = tnetv107x_gpio_set; > + ctlr->chip.base = 0; > + ctlr->chip.ngpio = TNETV107X_N_GPIOS; > + ctlr->chip.can_sleep = 0; > + > + gpiochip_add(&ctlr->chip); > +} Why make it a global? Why not an initcall like the regular DaVinci gpio driver does? Thanks, Sekhar _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
