| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CACxGe6uzoJeLst+4T3CNArQ-K1CPD6KQjFHg_komrqNsWHR=xQ@mail.gmail.com>
Date: Wed, 14 Dec 2011 14:39:26 -0700
From: Grant Likely <grant.likely@...retlab.ca>
To: Rob Herring <robherring2@...il.com>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
devicetree-discuss@...ts.ozlabs.org, shawn.guo@...escale.com,
Kukjin Kim <kgene.kim@...sung.com>,
Kevin Hilman <khilman@...com>,
Tony Lindgren <tony@...mide.com>,
Barry Song <baohua.song@....com>,
Linus Walleij <linus.ml.walleij@...il.com>,
Rob Herring <rob.herring@...xeda.com>
Subject: Re: [PATCH 8/9] gpio: pl061: enable interrupts with DT style binding
On Wed, Dec 14, 2011 at 8:28 AM, Rob Herring <robherring2@...il.com> wrote:
> From: Rob Herring <rob.herring@...xeda.com>
>
> Enable DT interrupt binding support for pl061 gpio lines. If the gpio
> node has an interrupt-controller property, then it will be setup to
> handle interrupts on gpio lines.
>
> Signed-off-by: Rob Herring <rob.herring@...xeda.com>
> Cc: Grant Likely <grant.likely@...retlab.ca>
> Cc: Linus Walleij <linus.ml.walleij@...il.com>
> ---
> .../devicetree/bindings/gpio/pl061-gpio.txt | 15 ++++++++++
> drivers/gpio/gpio-pl061.c | 29 +++++++++++--------
> 2 files changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/pl061-gpio.txt b/Documentation/devicetree/bindings/gpio/pl061-gpio.txt
> index a2c416b..9671d4e 100644
> --- a/Documentation/devicetree/bindings/gpio/pl061-gpio.txt
> +++ b/Documentation/devicetree/bindings/gpio/pl061-gpio.txt
> @@ -8,3 +8,18 @@ Required properties:
> - gpio-controller : Marks the device node as a GPIO controller.
> - interrupts : Interrupt mapping for GPIO IRQ.
>
> +Optional properties:
> +- interrupt-controller : Identifies the node as an interrupt controller. Must
> + be present if using gpios lines for interrupts.
> +- #interrupt-cells : Specifies the number of cells needed to encode an
> + interrupt source. The type shall be a <u32> and the value shall be 2.
> +
> + The 1st cell contains the interrupt number 0-7 corresponding to the gpio
> + line.
> +
> + The 2nd cell is the flags, encoding trigger type and level flags.
> + 1 = low-to-high edge triggered
> + 2 = high-to-low edge triggered
> + 4 = active high level-sensitive
> + 8 = active low level-sensitive
> +
> diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
> index 4a1874f..0151798 100644
> --- a/drivers/gpio/gpio-pl061.c
> +++ b/drivers/gpio/gpio-pl061.c
> @@ -16,6 +16,8 @@
> #include <linux/io.h>
> #include <linux/ioport.h>
> #include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/of.h>
> #include <linux/bitops.h>
> #include <linux/workqueue.h>
> #include <linux/gpio.h>
> @@ -52,7 +54,7 @@ struct pl061_gpio {
> spinlock_t lock; /* GPIO registers */
>
> void __iomem *base;
> - int irq_base;
> + struct irq_domain *irq_domain;
> struct gpio_chip gc;
> };
>
> @@ -117,18 +119,16 @@ static void pl061_set_value(struct gpio_chip *gc, unsigned offset, int value)
> static int pl061_to_irq(struct gpio_chip *gc, unsigned offset)
> {
> struct pl061_gpio *chip = container_of(gc, struct pl061_gpio, gc);
> -
> - if (chip->irq_base <= 0)
> - return -EINVAL;
> -
> - return chip->irq_base + offset;
> + if (!chip->irq_domain)
> + return -ENXIO;
> + return irq_domain_to_irq(chip->irq_domain, offset);
> }
>
> static int pl061_irq_type(struct irq_data *d, unsigned trigger)
> {
> struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
> struct pl061_gpio *chip = gc->private;
> - int offset = d->irq - chip->irq_base;
> + int offset = d->hwirq;
> unsigned long flags;
> u8 gpiois, gpioibe, gpioiev;
>
> @@ -212,6 +212,7 @@ static void __init pl061_init_gc(struct pl061_gpio *chip, int irq_base)
>
> irq_setup_generic_chip(gc, IRQ_MSK(PL061_GPIO_NR),
> IRQ_GC_INIT_NESTED_LOCK, IRQ_NOREQUEST, 0);
> + chip->irq_domain = gc->domain;
I would expect the driver to store chip->gc = gc instead. Then the
driver has access to both structures, and if ever needed, the gc could
be freed on driver release.
> }
>
> static int pl061_probe(struct amba_device *dev, const struct amba_id *id)
> @@ -219,7 +220,7 @@ static int pl061_probe(struct amba_device *dev, const struct amba_id *id)
> struct pl061_platform_data *pdata;
> struct pl061_gpio *chip;
> struct list_head *chip_list;
> - int ret, irq, i;
> + int ret, irq, i, irq_base;
> static DECLARE_BITMAP(init_irq, NR_IRQS);
>
> chip = kzalloc(sizeof(*chip), GFP_KERNEL);
> @@ -229,10 +230,13 @@ static int pl061_probe(struct amba_device *dev, const struct amba_id *id)
> pdata = dev->dev.platform_data;
> if (pdata) {
> chip->gc.base = pdata->gpio_base;
> - chip->irq_base = pdata->irq_base;
> + irq_base = pdata->irq_base;
> } else if (dev->dev.of_node) {
> chip->gc.base = -1;
> - chip->irq_base = 0;
> + if (of_get_property(dev->dev.of_node, "interrupt-controller", NULL))
> + irq_base = -1;
> + else
> + irq_base = 0;
> } else {
> ret = -ENODEV;
> goto free_mem;
> @@ -271,10 +275,11 @@ static int pl061_probe(struct amba_device *dev, const struct amba_id *id)
> * irq_chip support
> */
>
> - if (chip->irq_base <= 0)
> + if (!irq_base)
> return 0;
A comment would be useful here as it took a bit for me to work out
what the options are here. What are the possible values of irq_base
at this point, and what do they mean?
Otherwise looks pretty good.
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists