From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756539AbZFGVdd (ORCPT ); Sun, 7 Jun 2009 17:33:33 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753676AbZFGVdZ (ORCPT ); Sun, 7 Jun 2009 17:33:25 -0400 Received: from mail-fx0-f213.google.com ([209.85.220.213]:50294 "EHLO mail-fx0-f213.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752947AbZFGVdY convert rfc822-to-8bit (ORCPT ); Sun, 7 Jun 2009 17:33:24 -0400 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=LmQPAXr4xVbDK8tr8nbjWK9o5EQ/k49beimNhoH07uW0DV7JFC/TZmlNlFYkk5G4n/ UA08uRiMU3QkFdGldJwwLjfzwGMdrqW/+Q7TwqiRZ1zdGpXjVcj5O2RL+913CxebjHzs zTsKVwBHJTWCjV7grnRlvk4+THyNBiRjBf4yw= MIME-Version: 1.0 In-Reply-To: <1244399935-23128-1-git-send-email-baruch@tkos.co.il> References: <1244399935-23128-1-git-send-email-baruch@tkos.co.il> Date: Sun, 7 Jun 2009 23:33:23 +0200 Message-ID: <63386a3d0906071433h516ee358ud2e64a90692fa4f9@mail.gmail.com> Subject: Re: [PATCH v4] gpio: driver for PrimeCell PL061 GPIO controller From: Linus Walleij To: Baruch Siach Cc: linux-kernel@vger.kernel.org, David Brownell , Andrew Morton , linux-arm-kernel@lists.arm.linux.org.uk, Russell King - ARM Linux Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org OK sorry for coming in so late, this is really not so important but anyway: 2009/6/7 Baruch Siach : > This is a driver for the ARM PrimeCell PL061 GPIO AMBA peripheral. The driver > is implemented using the gpiolib framework. > > This driver also includes support for the use of the PL061 as an interrupt > controller (secondary). > > Signed-off-by: Baruch Siach > --- > Changes in v4: >        - Depend on CONFIG_ARM_AMBA >        - Do not request the gpio for IRQ automatically, just warn if it's not >          requested >        - Remove SZ_4K Yes but defined a new one 4*1024 instead, but I'll show, hang on... >        - Iterate through a linked list find the source of interrupt when >          multiple PL061s are connected to the same IRQ trigger >        - Move hardware register defines into the .c file > > Changes in v3: >        - amba driver instead of a platform driver >        - Move include/linux/gpio/pl061.h => include/linux/amba/pl061.h > > Changes in v2: >        - Address Andrew Morton's comments >        - Pass checkpatch.pl >        - Add changelog comment > >  drivers/gpio/Kconfig       |    6 + >  drivers/gpio/Makefile      |    1 + >  drivers/gpio/pl061.c       |  342 ++++++++++++++++++++++++++++++++++++++++++++ >  include/linux/amba/pl061.h |   17 +++ >  4 files changed, 366 insertions(+), 0 deletions(-) >  create mode 100644 drivers/gpio/pl061.c >  create mode 100644 include/linux/amba/pl061.h > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index edb0253..585376f 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -67,6 +67,12 @@ config GPIO_SYSFS > >  comment "Memory mapped GPIO expanders:" > > +config GPIO_PL061 > +       bool "PrimeCell PL061 GPIO support" > +       depends on ARM_AMBA > +       help > +         Say yes here to support the PrimeCell PL061 GPIO device > + >  config GPIO_XILINX >        bool "Xilinx GPIO support" >        depends on PPC_OF > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > index 49ac64e..ef90203 100644 > --- a/drivers/gpio/Makefile > +++ b/drivers/gpio/Makefile > @@ -9,6 +9,7 @@ obj-$(CONFIG_GPIO_MAX732X)      += max732x.o >  obj-$(CONFIG_GPIO_MCP23S08)    += mcp23s08.o >  obj-$(CONFIG_GPIO_PCA953X)     += pca953x.o >  obj-$(CONFIG_GPIO_PCF857X)     += pcf857x.o > +obj-$(CONFIG_GPIO_PL061)       += pl061.o >  obj-$(CONFIG_GPIO_TWL4030)     += twl4030-gpio.o >  obj-$(CONFIG_GPIO_XILINX)      += xilinx_gpio.o >  obj-$(CONFIG_GPIO_BT8XX)       += bt8xxgpio.o > diff --git a/drivers/gpio/pl061.c b/drivers/gpio/pl061.c > new file mode 100644 > index 0000000..c48e82e > --- /dev/null > +++ b/drivers/gpio/pl061.c > @@ -0,0 +1,342 @@ > +/* > + *  linux/drivers/gpio/pl061.c > + * > + *  Copyright (C) 2008, 2009 Provigent Ltd. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * Driver for the ARM PrimeCell(tm) General Purpose Input/Output (PL061) > + * > + * Data sheet: ARM DDI 0190B, September 2000 > + */ > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define GPIODIR 0x400 > +#define GPIOIS  0x404 > +#define GPIOIBE 0x408 > +#define GPIOIEV 0x40C > +#define GPIOIE  0x410 > +#define GPIORIS 0x414 > +#define GPIOMIS 0x418 > +#define GPIOIC  0x41C > + > +#define PL061_GPIO_NR  8 > + > +#define PL061_REG_SIZE (4*1024) Then you could just #define PL061_REG_SIZE SZ_4K But you can remove it, look below: > + > +struct pl061_gpio { > +       struct list_head        list; > + > +       /* Each of the two spinlocks protects a different set of hardware > +        * regiters and data structurs. This decouples the code of the IRQ from > +        * the GPIO code. This also makes the case of a GPIO routine call from > +        * the IRQ code simpler. > +        */ > +       spinlock_t              lock;           /* GPIO registers */ > +       spinlock_t              irq_lock;       /* IRQ registers */ > + > +       void __iomem            *base; > +       struct gpio_chip        gc; > +}; > + > +static int pl061_direction_input(struct gpio_chip *gc, unsigned offset) > +{ > +       struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc); > +       unsigned long flags; > +       unsigned char gpiodir; > + > +       if (offset >= gc->ngpio) > +               return -EINVAL; > + > +       spin_lock_irqsave(&chip->lock, flags); > +       gpiodir = readb(chip->base + GPIODIR); > +       gpiodir &= ~(1 << offset); > +       writeb(gpiodir, chip->base + GPIODIR); > +       spin_unlock_irqrestore(&chip->lock, flags); > + > +       return 0; > +} > + > +static int pl061_direction_output(struct gpio_chip *gc, unsigned offset, > +               int value) > +{ > +       struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc); > +       unsigned long flags; > +       unsigned char gpiodir; > + > +       if (offset >= gc->ngpio) > +               return -EINVAL; > + > +       spin_lock_irqsave(&chip->lock, flags); > +       writeb(!!value << offset, chip->base + (1 << (offset + 2))); > +       gpiodir = readb(chip->base + GPIODIR); > +       gpiodir |= 1 << offset; > +       writeb(gpiodir, chip->base + GPIODIR); > +       spin_unlock_irqrestore(&chip->lock, flags); > + > +       return 0; > +} > + > +static int pl061_get_value(struct gpio_chip *gc, unsigned offset) > +{ > +       struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc); > + > +       return !!readb(chip->base + (1 << (offset + 2))); > +} > + > +static void pl061_set_value(struct gpio_chip *gc, unsigned offset, int value) > +{ > +       struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc); > + > +       writeb(!!value << offset, chip->base + (1 << (offset + 2))); > +} > + > +/* > + * PL061 GPIO IRQ > + */ > +static void pl061_irq_disable(unsigned irq) > +{ > +       struct pl061_gpio *chip = get_irq_chip_data(irq); > +       int offset = irq_to_gpio(irq) - chip->gc.base; > +       unsigned long flags; > +       u8 gpioie; > + > +       spin_lock_irqsave(&chip->irq_lock, flags); > +       gpioie = readb(chip->base + GPIOIE); > +       gpioie &= ~(1 << offset); > +       writeb(gpioie, chip->base + GPIOIE); > +       spin_unlock_irqrestore(&chip->irq_lock, flags); > +} > + > +static void pl061_irq_enable(unsigned irq) > +{ > +       struct pl061_gpio *chip = get_irq_chip_data(irq); > +       int offset = irq_to_gpio(irq) - chip->gc.base; > +       unsigned long flags; > +       u8 gpioie; > + > +       spin_lock_irqsave(&chip->irq_lock, flags); > +       gpioie = readb(chip->base + GPIOIE); > +       gpioie |= 1 << offset; > +       writeb(gpioie, chip->base + GPIOIE); > +       spin_unlock_irqrestore(&chip->irq_lock, flags); > +} > + > +static unsigned int pl061_irq_startup(unsigned irq) > +{ > +       if (gpio_request(irq_to_gpio(irq), "IRQ") == 0) > +               pr_warning("%s: warning: GPIO%d has not been requested\n", > +                               __func__, irq_to_gpio(irq)); > + > +       pl061_irq_enable(irq); > + > +       return 0; > +} > + > +static int pl061_irq_type(unsigned irq, unsigned trigger) > +{ > +       struct pl061_gpio *chip = get_irq_chip_data(irq); > +       int offset = irq_to_gpio(irq) - chip->gc.base; > +       unsigned long flags; > +       u8 gpiois, gpioibe, gpioiev; > + > +       if (irq_to_gpio(irq) < 0) > +               return -EINVAL; > + > +       spin_lock_irqsave(&chip->irq_lock, flags); > + > +       gpioiev = readb(chip->base + GPIOIEV); > + > +       gpiois = readb(chip->base + GPIOIS); > +       if (trigger & (IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW)) { > +               gpiois |= 1 << offset; > +               if (trigger & IRQ_TYPE_LEVEL_HIGH) > +                       gpioiev |= 1 << offset; > +               else > +                       gpioiev &= ~(1 << offset); > +       } else > +               gpiois &= ~(1 << offset); > +       writeb(gpiois, chip->base + GPIOIS); > + > +       gpioibe = readb(chip->base + GPIOIBE); > +       if ((trigger & IRQ_TYPE_EDGE_BOTH) == IRQ_TYPE_EDGE_BOTH) > +               gpioibe |= 1 << offset; > +       else { > +               gpioibe &= ~(1 << offset); > +               if (trigger & IRQ_TYPE_EDGE_RISING) > +                       gpioiev |= 1 << offset; > +               else > +                       gpioiev &= ~(1 << offset); > +       } > +       writeb(gpioibe, chip->base + GPIOIBE); > + > +       writeb(gpioiev, chip->base + GPIOIEV); > + > +       spin_unlock_irqrestore(&chip->irq_lock, flags); > + > +       return 0; > +} > + > +static struct irq_chip pl061_irqchip = { > +       .name           = "GPIO", > +       .startup        = pl061_irq_startup, > +       .enable         = pl061_irq_enable, > +       .disable        = pl061_irq_disable, > +       .set_type       = pl061_irq_type, > +}; > + > +static void pl061_irq_handler(unsigned irq, struct irq_desc *desc) > +{ > +       struct list_head *chip_list = get_irq_chip_data(irq); > +       struct list_head *ptr; > +       struct pl061_gpio *chip; > + > +       desc->chip->ack(irq); > +       list_for_each(ptr, chip_list) { > +               unsigned long pending; > +               int gpio; > + > +               chip = list_entry(ptr, struct pl061_gpio, list); > +               pending = readb(chip->base + GPIOMIS); > +               writeb(pending, chip->base + GPIOIC); > + > +               if (pending == 0) > +                       continue; > + > +               for_each_bit(gpio, &pending, PL061_GPIO_NR) > +                       generic_handle_irq(gpio_to_irq(gpio)); > +       } > +       desc->chip->unmask(irq); > +} > + > +static int __init pl061_probe(struct amba_device *dev, struct amba_id *id) > +{ > +       struct pl061_platform_data *pdata; > +       struct pl061_gpio *chip; > +       struct list_head *chip_list; > +       int ret, irq, i; > + > +       pdata = dev->dev.platform_data; > +       if (pdata == NULL) > +               return -ENODEV; > + > +       chip = kzalloc(sizeof(*chip), GFP_KERNEL); > +       if (chip == NULL) > +               return -ENOMEM; > + > +       if (!request_mem_region(dev->res.start, PL061_REG_SIZE, "pl061")) { > +               ret = -EBUSY; > +               goto free_mem; > +       } > + > +       chip->base = ioremap(dev->res.start, PL061_REG_SIZE); Just do this: chip->base = ioremap(dev->res.start, resource_size(dev->res)); Hm perhaps this should be fixed in a few more primecell drivers, I'll have a check... > +       if (chip->base == NULL) { > +               ret = -ENOMEM; > +               goto release_region; > +       } > + > +       spin_lock_init(&chip->lock); > +       spin_lock_init(&chip->irq_lock); > +       INIT_LIST_HEAD(&chip->list); > + > +       chip->gc.direction_input = pl061_direction_input; > +       chip->gc.direction_output = pl061_direction_output; > +       chip->gc.get = pl061_get_value; > +       chip->gc.set = pl061_set_value; > +       chip->gc.base = pdata->gpio_base; > +       chip->gc.ngpio = PL061_GPIO_NR; > +       chip->gc.label = dev_name(&dev->dev); > +       chip->gc.dev = &dev->dev; > +       chip->gc.owner = THIS_MODULE; > + > +       ret = gpiochip_add(&chip->gc); > +       if (ret) > +               goto iounmap; > + > +       /* > +        * irq_chip support > +        */ > +       writeb(0, chip->base + GPIOIE); /* disable irqs */ > +       irq = dev->irq[0]; > +       if (irq < 0) { > +               ret = -ENODEV; > +               goto iounmap; > +       } > +       set_irq_chained_handler(irq, pl061_irq_handler); > +       if (!pdata->is_irq_initialized || !pdata->is_irq_initialized(irq)) { > +               chip_list = kmalloc(sizeof(*chip_list), GFP_KERNEL); > +               if (chip_list == NULL) { > +                       ret = -ENOMEM; > +                       goto iounmap; > +               } > +               INIT_LIST_HEAD(chip_list); > +               set_irq_chip_data(irq, chip_list); > +       } else > +               chip_list = get_irq_chip_data(irq); > +       list_add(&chip->list, chip_list); > + > +       for (i = 0; i < PL061_GPIO_NR; i++) { > +               if (pdata->directions & (1 << i)) > +                       pl061_direction_output(&chip->gc, i, > +                                       pdata->values & (1 << i)); > +               else > +                       pl061_direction_input(&chip->gc, i); > + > +               set_irq_chip(gpio_to_irq(i+pdata->gpio_base), &pl061_irqchip); > +               set_irq_handler(gpio_to_irq(i+pdata->gpio_base), > +                               handle_simple_irq); > +               set_irq_flags(gpio_to_irq(i+pdata->gpio_base), IRQF_VALID); > +               set_irq_chip_data(gpio_to_irq(i+pdata->gpio_base), chip); > +       } > + > +       return 0; > + > +iounmap: > +       iounmap(chip->base); > +release_region: > +       release_mem_region(dev->res.start, PL061_REG_SIZE); > +free_mem: > +       kfree(chip); > + > +       return ret; > +} > + > +static struct amba_id pl061_ids[] __initdata = { > +       { > +               .id     = 0x00041061, > +               .mask   = 0x000fffff, > +       }, > +       { 0, 0 }, > +}; > + > +static struct amba_driver pl061_gpio_driver = { > +       .drv = { > +               .name   = "pl061_gpio", > +       }, > +       .id_table       = pl061_ids, > +       .probe          = pl061_probe, > +}; > + > +static int __init pl061_gpio_init(void) > +{ > +       return amba_driver_register(&pl061_gpio_driver); > +} > +subsys_initcall(pl061_gpio_init); > + > +MODULE_AUTHOR("Baruch Siach "); > +MODULE_DESCRIPTION("PL061 GPIO driver"); > +MODULE_LICENSE("GPL"); > diff --git a/include/linux/amba/pl061.h b/include/linux/amba/pl061.h > new file mode 100644 > index 0000000..90de39e > --- /dev/null > +++ b/include/linux/amba/pl061.h > @@ -0,0 +1,17 @@ > +/* platform data for the PL061 GPIO driver */ > + > +struct pl061_platform_data { > +       /* number of the first GPIO */ > +       unsigned        gpio_base; > + > +       u8              directions;     /* startup directions, 1: out, 0: in */ > +       u8              values;         /* startup values */ > + > +       /* This callback may be used when you have more than one PL061 > +        * connected to the same IRQ trigger. This callback must return 0 on > +        * the first time it is called for each irq, and 1 on any other call. > +        * In case you are not unfortunate enough to have this setup, this > +        * pointer must be set to NULL. > +        */ > +       int             (*is_irq_initialized)(int irq); > +}; > -- > 1.6.3.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at  http://vger.kernel.org/majordomo-info.html > Please read the FAQ at  http://www.tux.org/lkml/ > Yours, Linus Walleij