[PATCH 14/15] gpio: locomo: implement per-pin irq handling

Subsystems: gpio subsystem, the rest

STALE4339d

3 messages, 3 authors, 2014-10-31 · open the first message on its own page

[PATCH 14/15] gpio: locomo: implement per-pin irq handling

From: Dmitry Eremin-Solenikov <hidden>
Date: 2014-10-28 00:02:07

LoCoMo has a possibility to generate per-GPIO edge irqs. Support for
that was there in old locomo driver, got 'cleaned up' during old driver
IRQ cascading cleanup and is now reimplemented. It is expected that
SL-5500 (collie) will use locomo gpio irqs for mmc detection irq.

Signed-off-by: Dmitry Eremin-Solenikov <redacted>
---
 drivers/gpio/gpio-locomo.c | 158 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 158 insertions(+)
diff --git a/drivers/gpio/gpio-locomo.c b/drivers/gpio/gpio-locomo.c
index 3b54b07..6ef0fc4 100644
--- a/drivers/gpio/gpio-locomo.c
+++ b/drivers/gpio/gpio-locomo.c
@@ -18,13 +18,18 @@
 #include <linux/gpio.h>
 #include <linux/io.h>
 #include <linux/spinlock.h>
+#include <linux/irq.h>
 #include <linux/mfd/locomo.h>
 
+#define LOCOMO_GPIO_NR_IRQS 16
+
 struct locomo_gpio {
 	void __iomem *regs;
+	int irq;
 
 	spinlock_t lock;
 	struct gpio_chip gpio;
+	int irq_base;
 
 	u16 rising_edge;
 	u16 falling_edge;
@@ -114,6 +119,148 @@ static int locomo_gpio_direction_output(struct gpio_chip *chip,
 	return 0;
 }
 
+static int locomo_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+	struct locomo_gpio *lg = container_of(chip, struct locomo_gpio, gpio);
+
+	return lg->irq_base + offset;
+}
+
+static void
+locomo_gpio_handler(unsigned int irq, struct irq_desc *desc)
+{
+	u16 req;
+	struct locomo_gpio *lg = irq_get_handler_data(irq);
+	int i = lg->irq_base;
+
+	req = readw(lg->regs + LOCOMO_GIR) &
+	      readw(lg->regs + LOCOMO_GPD);
+
+	while (req) {
+		if (req & 1)
+			generic_handle_irq(i);
+		req >>= 1;
+		i++;
+	}
+}
+
+static void locomo_gpio_ack_irq(struct irq_data *d)
+{
+	struct locomo_gpio *lg = irq_data_get_irq_chip_data(d);
+	unsigned long flags;
+	u16 r;
+
+	spin_lock_irqsave(&lg->lock, flags);
+
+	r = readw(lg->regs + LOCOMO_GWE);
+	r |= (0x0001 << (d->irq - lg->irq_base));
+	writew(r, lg->regs + LOCOMO_GWE);
+
+	r = readw(lg->regs + LOCOMO_GIS);
+	r &= ~(0x0001 << (d->irq - lg->irq_base));
+	writew(r, lg->regs + LOCOMO_GIS);
+
+	r = readw(lg->regs + LOCOMO_GWE);
+	r &= ~(0x0001 << (d->irq - lg->irq_base));
+	writew(r, lg->regs + LOCOMO_GWE);
+
+	spin_unlock_irqrestore(&lg->lock, flags);
+}
+
+static void locomo_gpio_mask_irq(struct irq_data *d)
+{
+	struct locomo_gpio *lg = irq_data_get_irq_chip_data(d);
+	unsigned long flags;
+	u16 r;
+
+	spin_lock_irqsave(&lg->lock, flags);
+
+	r = readw(lg->regs + LOCOMO_GIE);
+	r &= ~(0x0001 << (d->irq - lg->irq_base));
+	writew(r, lg->regs + LOCOMO_GIE);
+
+	spin_unlock_irqrestore(&lg->lock, flags);
+}
+
+static void locomo_gpio_unmask_irq(struct irq_data *d)
+{
+	struct locomo_gpio *lg = irq_data_get_irq_chip_data(d);
+	unsigned long flags;
+	u16 r;
+
+	spin_lock_irqsave(&lg->lock, flags);
+
+	r = readw(lg->regs + LOCOMO_GIE);
+	r |= (0x0001 << (d->irq - lg->irq_base));
+	writew(r, lg->regs + LOCOMO_GIE);
+
+	spin_unlock_irqrestore(&lg->lock, flags);
+}
+
+static int locomo_gpio_type(struct irq_data *d, unsigned int type)
+{
+	unsigned int mask;
+	struct locomo_gpio *lg = irq_data_get_irq_chip_data(d);
+	unsigned long flags;
+
+	mask = 1 << (d->irq - lg->irq_base);
+
+	if (type = IRQ_TYPE_PROBE) {
+		if ((lg->rising_edge | lg->falling_edge) & mask)
+			return 0;
+		type = IRQ_TYPE_EDGE_RISING | IRQ_TYPE_EDGE_FALLING;
+	}
+
+	if (type & IRQ_TYPE_EDGE_RISING)
+		lg->rising_edge |= mask;
+	else
+		lg->rising_edge &= ~mask;
+	if (type & IRQ_TYPE_EDGE_FALLING)
+		lg->falling_edge |= mask;
+	else
+		lg->falling_edge &= ~mask;
+
+	spin_lock_irqsave(&lg->lock, flags);
+
+	writew(lg->rising_edge, lg->regs + LOCOMO_GRIE);
+	writew(lg->falling_edge, lg->regs + LOCOMO_GFIE);
+
+	spin_unlock_irqrestore(&lg->lock, flags);
+
+	return 0;
+}
+
+static struct irq_chip locomo_gpio_chip = {
+	.name		= "LOCOMO-gpio",
+	.irq_ack	= locomo_gpio_ack_irq,
+	.irq_mask	= locomo_gpio_mask_irq,
+	.irq_unmask	= locomo_gpio_unmask_irq,
+	.irq_set_type	= locomo_gpio_type,
+};
+
+static void locomo_gpio_setup_irq(struct locomo_gpio *lg)
+{
+	int irq;
+
+	lg->irq_base = irq_alloc_descs(-1, 0, LOCOMO_GPIO_NR_IRQS, -1);
+
+	/* Install handlers for IRQ_LOCOMO_* */
+	for (irq = lg->irq_base;
+			irq < lg->irq_base + LOCOMO_GPIO_NR_IRQS;
+			irq++) {
+		irq_set_chip_and_handler(irq, &locomo_gpio_chip,
+				handle_edge_irq);
+		irq_set_chip_data(irq, lg);
+		set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
+	}
+
+	/*
+	 * Install handler for IRQ_LOCOMO_HW.
+	 */
+	irq_set_handler_data(lg->irq, lg);
+	irq_set_chained_handler(lg->irq, locomo_gpio_handler);
+}
+
 #ifdef CONFIG_PM_SLEEP
 static int locomo_gpio_suspend(struct device *dev)
 {
@@ -169,6 +316,10 @@ static int locomo_gpio_probe(struct platform_device *pdev)
 	if (!lg)
 		return -ENOMEM;
 
+	lg->irq = platform_get_irq(pdev, 0);
+	if (lg->irq < 0)
+		return -ENXIO;
+
 	lg->regs = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(lg->regs))
 		return PTR_ERR(lg->regs);
@@ -189,11 +340,14 @@ static int locomo_gpio_probe(struct platform_device *pdev)
 	lg->gpio.get = locomo_gpio_get;
 	lg->gpio.direction_input = locomo_gpio_direction_input;
 	lg->gpio.direction_output = locomo_gpio_direction_output;
+	lg->gpio.to_irq = locomo_gpio_to_irq;
 
 	ret = gpiochip_add(&lg->gpio);
 	if (ret)
 		return ret;
 
+	locomo_gpio_setup_irq(lg);
+
 	return 0;
 }
 
@@ -208,6 +362,10 @@ static int locomo_gpio_remove(struct platform_device *pdev)
 		return ret;
 	}
 
+	irq_set_chained_handler(lg->irq, NULL);
+	irq_set_handler_data(lg->irq, NULL);
+	irq_free_descs(lg->irq_base, LOCOMO_GPIO_NR_IRQS);
+
 	return 0;
 }
 
-- 
2.1.1

Re: [PATCH 14/15] gpio: locomo: implement per-pin irq handling

From: Linus Walleij <hidden>
Date: 2014-10-31 08:00:49

On Tue, Oct 28, 2014 at 1:02 AM, Dmitry Eremin-Solenikov
[off-list ref] wrote:
LoCoMo has a possibility to generate per-GPIO edge irqs. Support for
that was there in old locomo driver, got 'cleaned up' during old driver
IRQ cascading cleanup and is now reimplemented. It is expected that
SL-5500 (collie) will use locomo gpio irqs for mmc detection irq.

Signed-off-by: Dmitry Eremin-Solenikov <redacted>
Please don't use open-coded IRQ handling like this, we are moving
away from that.

In Kconfig,

select GPIOLIB_IRQCHIP

and look at the other drivers selecting this for inspiration. There
is even some documentation in Documentation/gpio/driver.txt

You will find that it cuts down a lot of overhead from your driver
and does everything in the right way in a central place.
 struct locomo_gpio {
        void __iomem *regs;
+       int irq;

        spinlock_t lock;
        struct gpio_chip gpio;
+       int irq_base;
gpiolib irqchip helpers uses irqdomain to do all this debasing
and rebasing for you. Go with that.
+static int locomo_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+       struct locomo_gpio *lg = container_of(chip, struct locomo_gpio, gpio);
+
+       return lg->irq_base + offset;
+}
And it implements .to_irq() in the gpiolib core.
+static void
+locomo_gpio_handler(unsigned int irq, struct irq_desc *desc)
It's locomo_gpio_irq_handler() right?
+{
+       u16 req;
+       struct locomo_gpio *lg = irq_get_handler_data(irq);
+       int i = lg->irq_base;
+
+       req = readw(lg->regs + LOCOMO_GIR) &
+             readw(lg->regs + LOCOMO_GPD);
+
+       while (req) {
+               if (req & 1)
+                       generic_handle_irq(i);
+               req >>= 1;
+               i++;
+       }
Same thing as the MFD device, look closer at how you construct
the IRQ handling loop, so the register gets re-read each iteration.
+static void locomo_gpio_ack_irq(struct irq_data *d)
+{
+       struct locomo_gpio *lg = irq_data_get_irq_chip_data(d);
+       unsigned long flags;
+       u16 r;
+
+       spin_lock_irqsave(&lg->lock, flags);
+
+       r = readw(lg->regs + LOCOMO_GWE);
+       r |= (0x0001 << (d->irq - lg->irq_base));
+       writew(r, lg->regs + LOCOMO_GWE);
+
+       r = readw(lg->regs + LOCOMO_GIS);
+       r &= ~(0x0001 << (d->irq - lg->irq_base));
+       writew(r, lg->regs + LOCOMO_GIS);
+
+       r = readw(lg->regs + LOCOMO_GWE);
+       r &= ~(0x0001 << (d->irq - lg->irq_base));
+       writew(r, lg->regs + LOCOMO_GWE);
+
+       spin_unlock_irqrestore(&lg->lock, flags);
+}
I really wonder if this locking is needed around these
regioster accesses. It seems more like a habit than
like something that is actually needed. Think it over.

*irqsave* versions of spinlocks are definately wrong
in the irqchip callbacks, if you give it a minute I think
you quickly realize why.
+static int locomo_gpio_type(struct irq_data *d, unsigned int type)
+{
+       unsigned int mask;
+       struct locomo_gpio *lg = irq_data_get_irq_chip_data(d);
+       unsigned long flags;
+
+       mask = 1 << (d->irq - lg->irq_base);
This should just use d->hwirq with irqdomain implemented
correctly.

(...)
+static void locomo_gpio_setup_irq(struct locomo_gpio *lg)
+{
+       int irq;
+
+       lg->irq_base = irq_alloc_descs(-1, 0, LOCOMO_GPIO_NR_IRQS, -1);
+
+       /* Install handlers for IRQ_LOCOMO_* */
+       for (irq = lg->irq_base;
+                       irq < lg->irq_base + LOCOMO_GPIO_NR_IRQS;
+                       irq++) {
+               irq_set_chip_and_handler(irq, &locomo_gpio_chip,
+                               handle_edge_irq);
+               irq_set_chip_data(irq, lg);
+               set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
+       }
+
+       /*
+        * Install handler for IRQ_LOCOMO_HW.
+        */
+       irq_set_handler_data(lg->irq, lg);
+       irq_set_chained_handler(lg->irq, locomo_gpio_handler);
+}
All this gets redundant with gpiochip_irqchip_add()
and gpiochip_set_chained_irqchip().

Yours,
Linus Walleij

Re: [PATCH 14/15] gpio: locomo: implement per-pin irq handling

From: Dmitry Eremin-Solenikov <hidden>
Date: 2014-10-31 09:35:18

2014-10-31 11:00 GMT+03:00 Linus Walleij [off-list ref]:
On Tue, Oct 28, 2014 at 1:02 AM, Dmitry Eremin-Solenikov
[off-list ref] wrote:
quoted
LoCoMo has a possibility to generate per-GPIO edge irqs. Support for
that was there in old locomo driver, got 'cleaned up' during old driver
IRQ cascading cleanup and is now reimplemented. It is expected that
SL-5500 (collie) will use locomo gpio irqs for mmc detection irq.

Signed-off-by: Dmitry Eremin-Solenikov <redacted>
Please don't use open-coded IRQ handling like this, we are moving
away from that.

In Kconfig,

select GPIOLIB_IRQCHIP

and look at the other drivers selecting this for inspiration. There
is even some documentation in Documentation/gpio/driver.txt

You will find that it cuts down a lot of overhead from your driver
and does everything in the right way in a central place.
Thanks, I will take a look.
[Skipped the rest of the comments mostly dedicated to gpiolib_irqchip].


-- 
With best wishes
Dmitry
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help