Re: [PATCH 2/3] gpio: dwapb: add support for GPIO interrupts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/05/2013 05:55 PM, Alan Tull wrote:
From: Jamie Iles <jamie@xxxxxxxxxxxxx>

The controller supports interrupts on bank A (up to 8 interrupt

s/bank/port/g

sources).  Use the generic IRQ chip to implement interrupt support for
this bank.

Cc: Grant Likely <grant.likely@xxxxxxxxxxxx>
Cc: Linus Walleij <linus.walleij@xxxxxxxxxxxxxx>
Acked-by: Rob Herring <rob.herring@xxxxxxxxxxx>
Signed-off-by: Jamie Iles <jamie@xxxxxxxxxxxxx>
---
  drivers/gpio/Kconfig      |    2 +-
  drivers/gpio/gpio-dwapb.c |  181 +++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 182 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index c5e7868..4cc26ed 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -125,7 +125,7 @@ config GPIO_DWAPB
  	bool "Synopsys DesignWare APB GPIO driver"
  	select GPIO_GENERIC
  	select GENERIC_IRQ_CHIP
-	depends on OF_GPIO
+	depends on OF_GPIO && IRQ_DOMAIN
  	help
  	  Say Y or M here to build support for the Synopsys DesignWare APB
  	  GPIO block.  This requires device tree support.
diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index 3a28697..64c7183 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -10,12 +10,22 @@
  #include <linux/init.h>
  #include <linux/io.h>
  #include <linux/ioport.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
  #include <linux/module.h>
  #include <linux/basic_mmio_gpio.h>
  #include <linux/of.h>
  #include <linux/of_address.h>
+#include <linux/of_irq.h>
  #include <linux/platform_device.h>

+#define INT_EN_REG_OFFS		0x30
+#define INT_MASK_REG_OFFS	0x34
+#define INT_TYPE_REG_OFFS	0x38
+#define INT_POLARITY_REG_OFFS	0x3c
+#define INT_STATUS_REG_OFFS	0x40
+#define EOI_REG_OFFS		0x4c

For sane register names, check [1] pp. 1271. I also suggest
to get rid of _REG_OFFS.

[1] http://www.altera.com/literature/hb/arria-v/hps.pdf

  struct dwapb_gpio;

  struct dwapb_gpio_bank {
@@ -31,6 +41,8 @@ struct dwapb_gpio {
  	void __iomem		*regs;
  	struct dwapb_gpio_bank	*banks;
  	unsigned int		nr_banks;
+	struct irq_chip_generic	*irq_gc;
+	unsigned long		toggle_edge;
  };

  static unsigned int dwapb_gpio_nr_banks(struct device_node *of_node)
@@ -44,6 +56,168 @@ static unsigned int dwapb_gpio_nr_banks(struct device_node *of_node)
  	return nr_banks;
  }

+static int dwapb_gpio_to_irq(struct gpio_chip *gc, unsigned offset)
+{
+	struct bgpio_chip *bgc = to_bgpio_chip(gc);
+	struct dwapb_gpio_bank *bank = container_of(bgc, struct
+						    dwapb_gpio_bank, bgc);
+	struct dwapb_gpio *gpio = bank->gpio;
+
+	return irq_domain_to_irq(&gpio->irq_gc->domain, offset);
+}
+
+static void dwapb_toggle_trigger(struct dwapb_gpio *gpio, unsigned int offs)
+{
+	u32 v = readl(gpio->regs + INT_TYPE_REG_OFFS);

s/readl/readl_relaxed/g
s/writel/writel_relaxed/g ?

+	if (gpio_get_value(gpio->banks[0].bgc.gc.base + offs))
+		v &= ~BIT(offs);
+	else
+		v |= BIT(offs);
+
+	writel(v, gpio->regs + INT_TYPE_REG_OFFS);
+}
+
+static void dwapb_irq_handler(u32 irq, struct irq_desc *desc)
+{
+	struct dwapb_gpio *gpio = irq_get_handler_data(irq);
+	u32 irq_status = readl(gpio->regs + INT_STATUS_REG_OFFS);
+
+	while (irq_status) {
+		int irqoffset = fls(irq_status) - 1;
+		int irq = irq_domain_to_irq(&gpio->irq_gc->domain, irqoffset);
+
+		generic_handle_irq(irq);
+		irq_status &= ~(1 << irqoffset);
+
+		if (gpio->toggle_edge & BIT(irqoffset))
+			dwapb_toggle_trigger(gpio, irqoffset);

if ((irq_get_trigger_type(irq) & IRQ_TYPE_SENSE_MASK) == IRQ_TYPE_EDGE_BOTH)

and get rid of ->toggle_edge cache variable.

+	}
+}
+
+static void dwapb_irq_enable(struct irq_data *d)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct dwapb_gpio *gpio = gc->private;
+
+	u32 val = readl(gpio->regs + INT_EN_REG_OFFS);
+	val |= 1 << d->hwirq;
+	writel(val, gpio->regs + INT_EN_REG_OFFS);
+}
+
+static void dwapb_irq_disable(struct irq_data *d)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct dwapb_gpio *gpio = gc->private;
+
+	u32 val = readl(gpio->regs + INT_EN_REG_OFFS);
+	val &= ~(1 << d->hwirq);
+	writel(val, gpio->regs + INT_EN_REG_OFFS);
+}
+
+static int dwapb_irq_set_type(struct irq_data *d, u32 type)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d);
+	struct dwapb_gpio *gpio = gc->private;
+	int bit = d->hwirq;
+	unsigned long level, polarity;
+
+	if (type & ~(IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING |
+		     IRQ_TYPE_LEVEL_HIGH | IRQ_TYPE_LEVEL_LOW))
+		return -EINVAL;
+
+	level = readl(gpio->regs + INT_TYPE_REG_OFFS);
+	polarity = readl(gpio->regs + INT_POLARITY_REG_OFFS);
+
+	gpio->toggle_edge &= ~BIT(bit);
+	if (type & IRQ_TYPE_EDGE_BOTH) {
+		gpio->toggle_edge |= BIT(bit);
+		level |= (1 << bit);

If you remove ->toggle_edge, read current pin value here and
set next edge accordingly.

+		dwapb_toggle_trigger(gpio, bit);
+	} else if (type & IRQ_TYPE_EDGE_RISING) {
+		level |= (1 << bit);
+		polarity |= (1 << bit);
+	} else if (type & IRQ_TYPE_EDGE_FALLING) {
+		level |= (1 << bit);
+		polarity &= ~(1 << bit);
+	} else if (type & IRQ_TYPE_LEVEL_HIGH) {
+		level &= ~(1 << bit);
+		polarity |= (1 << bit);
+	} else if (type & IRQ_TYPE_LEVEL_LOW) {
+		level &= ~(1 << bit);
+		polarity &= ~(1 << bit);
+	}

switch (type) for the above if-else-if ?

+	writel(level, gpio->regs + INT_TYPE_REG_OFFS);
+	writel(polarity, gpio->regs + INT_POLARITY_REG_OFFS);
+
+	return 0;
+}
+
+static int dwapb_create_irqchip(struct dwapb_gpio *gpio,
+				struct dwapb_gpio_bank *bank,
+				unsigned int irq_base)
+{
+	struct irq_chip_type *ct;
+
+	gpio->irq_gc = irq_alloc_generic_chip("gpio-dwapb", 1, irq_base,
+					      gpio->regs, handle_level_irq);
+	if (!gpio->irq_gc)
+		return -EIO;
+
+	gpio->irq_gc->domain.of_node = of_node_get(bank->bgc.gc.of_node);
+	gpio->irq_gc->private = gpio;
+	ct = gpio->irq_gc->chip_types;
+	ct->chip.irq_ack = irq_gc_ack_set_bit;
+	ct->chip.irq_mask = irq_gc_mask_set_bit;
+	ct->chip.irq_unmask = irq_gc_mask_clr_bit;
+	ct->chip.irq_set_type = dwapb_irq_set_type;
+	ct->chip.irq_enable = dwapb_irq_enable;
+	ct->chip.irq_disable = dwapb_irq_disable;

I can see no functional difference between enable/mask and always
enable and mask initially. If there are no objections, remove
enable/disable callbacks and mask and enable in _probe().

+	ct->regs.ack = EOI_REG_OFFS;
+	ct->regs.mask = INT_MASK_REG_OFFS;
+	irq_setup_generic_chip(gpio->irq_gc, IRQ_MSK(bank->bgc.gc.ngpio),
+			       IRQ_GC_INIT_NESTED_LOCK, IRQ_NOREQUEST, 0);
+
+	return 0;
+}
+
+static int dwapb_configure_irqs(struct dwapb_gpio *gpio,
+				struct dwapb_gpio_bank *bank)
+{
+	unsigned int m, irq, ngpio = bank->bgc.gc.ngpio;
+	int irq_base;
+
+	for (m = 0; m < ngpio; ++m) {
+		irq = irq_of_parse_and_map(bank->bgc.gc.of_node, m);

Only port A can raise interrupts. Are you sure, that all IP has
one irq per gpio? As you always use the same handler, just map
all irqs passed and install the handler; no need to match against
individual gpios here.

Sebastian

+		if (!irq && m == 0) {
+			dev_warn(gpio->dev, "no irq for bank %s\n",
+				 bank->bgc.gc.of_node->full_name);
+			return -ENXIO;
+		} else if (!irq) {
+			break;
+		}
+
+		irq_set_chained_handler(irq, dwapb_irq_handler);
+		irq_set_handler_data(irq, gpio);
+	}
+	bank->bgc.gc.to_irq = dwapb_gpio_to_irq;
+
+	irq_base = irq_alloc_descs(-1, 0, ngpio, NUMA_NO_NODE);
+	if (irq_base < 0)
+		return irq_base;

I was going to mention irqdomain_linear here, but then I realized
that patch 3/3 reverts most of irq handling here and replaces it
with irqdomain. Alan, I think Jamie is fine with you mangling patches
2 and 3 into one.

+	if (dwapb_create_irqchip(gpio, bank, irq_base))
+		goto out_free_descs;
+
+	return 0;
+
+out_free_descs:
+	irq_free_descs(irq_base, ngpio);
+
+	return -EIO;
+}
+
  static int dwapb_gpio_add_bank(struct dwapb_gpio *gpio,
  			       struct device_node *bank_np,
  			       unsigned int offs)
@@ -81,6 +255,13 @@ static int dwapb_gpio_add_bank(struct dwapb_gpio *gpio,
  	bank->bgc.gc.ngpio = ngpio;
  	bank->bgc.gc.of_node = bank_np;

+	/*
+	 * Only bank A can provide interrupts in all configurations of the IP.
+	 */
+	if (bank_idx == 0 &&
+	    of_get_property(bank_np, "interrupt-controller", NULL))
+		dwapb_configure_irqs(gpio, bank);
+
  	err = gpiochip_add(&bank->bgc.gc);
  	if (err)
  		dev_err(gpio->dev, "failed to register gpiochip for %s\n",


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux