diff mbox

[v2,2/9] drivers: irqchip: Add STM32 external interrupts support

Message ID 1459436979-17275-3-git-send-email-mcoquelin.stm32@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maxime Coquelin March 31, 2016, 3:09 p.m. UTC 
The STM32 external interrupt controller consists of edge detectors that
generate interrupts requests or wake-up events.

Each line can be independently configured as interrupt or wake-up source,
and triggers either on rising, fallin or both edges. Each line can also
be masked independently.

Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
---
 drivers/irqchip/Kconfig          |   4 +
 drivers/irqchip/Makefile         |   1 +
 drivers/irqchip/irq-stm32-exti.c | 169 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 174 insertions(+)
 create mode 100644 drivers/irqchip/irq-stm32-exti.c

Comments

Linus Walleij April 8, 2016, 9:38 a.m. UTC | #1 
On Thu, Mar 31, 2016 at 5:09 PM, Maxime Coquelin
<mcoquelin.stm32@gmail.com> wrote:

> +static void stm32_irq_handler(struct irq_desc *desc)
> +{
> +       struct irq_domain *domain = irq_desc_get_handler_data(desc);
> +       struct irq_chip_generic *gc = domain->gc->gc[0];
> +       struct irq_chip *chip = irq_desc_get_chip(desc);
> +       unsigned long pending;
> +       int n;
> +
> +       chained_irq_enter(chip, desc);
> +
> +       pending = irq_reg_readl(gc, EXTI_PR);
> +       for_each_set_bit(n, &pending, BITS_PER_LONG) {
> +               generic_handle_irq(irq_find_mapping(domain, n));
> +       }
> +
> +       chained_irq_exit(chip, desc);
> +}

Is this one of those cases where you should re-read the status register
on every iteration, so as to avoid exiting and immediately re-entering
the irq handler?

C.g irq-vic.c:

static int handle_one_vic(struct vic_device *vic, struct pt_regs *regs)
{
        u32 stat, irq;
        int handled = 0;

        while ((stat = readl_relaxed(vic->base + VIC_IRQ_STATUS))) {
                irq = ffs(stat) - 1;
                handle_domain_irq(vic->domain, irq, regs);
                handled = 1;
        }

        return handled;
}

Yours,
Linus Walleij
Linus Walleij April 8, 2016, 9:47 a.m. UTC | #2 
On Thu, Mar 31, 2016 at 5:09 PM, Maxime Coquelin
<mcoquelin.stm32@gmail.com> wrote:

> +       gc = domain->gc->gc[0];
> +       gc->reg_base                         = base;
> +       gc->chip_types->type               = IRQ_TYPE_EDGE_BOTH;
> +       gc->chip_types->chip.name          = gc->chip_types[0].chip.name;
> +       gc->chip_types->chip.irq_ack       = irq_gc_ack_set_bit;
> +       gc->chip_types->chip.irq_mask      = irq_gc_mask_clr_bit;
> +       gc->chip_types->chip.irq_unmask    = irq_gc_mask_set_bit;
> +       gc->chip_types->chip.irq_set_type  = stm32_irq_set_type;
> +       gc->chip_types->chip.irq_set_wake  = stm32_irq_set_wake;
> +       gc->chip_types->regs.ack           = EXTI_PR;
> +       gc->chip_types->regs.mask          = EXTI_IMR;
> +       gc->chip_types->handler            = handle_edge_irq;

If this is used by a GPIO chip (as happens in another part of
the series), you need to set up the .irq_request_resources()
and .irq_release_resources() to call gpiochip_lock_as_irq()
and gpiochip_unlock_as_irq().

As with the other comment on the GPIO patch, the separation
of concerns between irqchip and gpiochip is a bit artificial
here and breaks down somewhat.

Yours,
Linus Walleij
Maxime Coquelin April 19, 2016, 8 a.m. UTC | #3 
Hi Linus,

Sorry for the late reply, I was off last week.

2016-04-08 11:38 GMT+02:00 Linus Walleij <linus.walleij@linaro.org>:
> On Thu, Mar 31, 2016 at 5:09 PM, Maxime Coquelin
> <mcoquelin.stm32@gmail.com> wrote:
>
>> +static void stm32_irq_handler(struct irq_desc *desc)
>> +{
>> +       struct irq_domain *domain = irq_desc_get_handler_data(desc);
>> +       struct irq_chip_generic *gc = domain->gc->gc[0];
>> +       struct irq_chip *chip = irq_desc_get_chip(desc);
>> +       unsigned long pending;
>> +       int n;
>> +
>> +       chained_irq_enter(chip, desc);
>> +
>> +       pending = irq_reg_readl(gc, EXTI_PR);
>> +       for_each_set_bit(n, &pending, BITS_PER_LONG) {
>> +               generic_handle_irq(irq_find_mapping(domain, n));
>> +       }
>> +
>> +       chained_irq_exit(chip, desc);
>> +}
>
> Is this one of those cases where you should re-read the status register
> on every iteration, so as to avoid exiting and immediately re-entering
> the irq handler?
>
> C.g irq-vic.c:
>
> static int handle_one_vic(struct vic_device *vic, struct pt_regs *regs)
> {
>         u32 stat, irq;
>         int handled = 0;
>
>         while ((stat = readl_relaxed(vic->base + VIC_IRQ_STATUS))) {
>                 irq = ffs(stat) - 1;
>                 handle_domain_irq(vic->domain, irq, regs);
>                 handled = 1;
>         }
>
>         return handled;
> }

Indeed, it would be better doing it like this.
Do you think I could even do this with two nested loops to reduce the
number of reg accesses?
It would look like this (just compiled, not tested):

static void stm32_irq_handler(struct irq_desc *desc)
{
    struct irq_domain *domain = irq_desc_get_handler_data(desc);
    struct irq_chip_generic *gc = domain->gc->gc[0];
    struct irq_chip *chip = irq_desc_get_chip(desc);
    unsigned long pending;
    int n;

    chained_irq_enter(chip, desc);

    while ((pending = irq_reg_readl(gc, EXTI_PR))) {
        for_each_set_bit(n, &pending, BITS_PER_LONG) {
            generic_handle_irq(irq_find_mapping(domain, n));
        }
    }

    chained_irq_exit(chip, desc);
}

Thanks,
Maxime
Linus Walleij April 29, 2016, 8:45 a.m. UTC | #4 
On Tue, Apr 19, 2016 at 10:00 AM, Maxime Coquelin
<mcoquelin.stm32@gmail.com> wrote:
> 2016-04-08 11:38 GMT+02:00 Linus Walleij <linus.walleij@linaro.org>:

>>         while ((stat = readl_relaxed(vic->base + VIC_IRQ_STATUS))) {
>>                 irq = ffs(stat) - 1;
>>                 handle_domain_irq(vic->domain, irq, regs);
>>                 handled = 1;
>>         }
>>
>>         return handled;
>> }
>
> Indeed, it would be better doing it like this.
> Do you think I could even do this with two nested loops to reduce the
> number of reg accesses?
> It would look like this (just compiled, not tested):
>
> static void stm32_irq_handler(struct irq_desc *desc)
> {
>     struct irq_domain *domain = irq_desc_get_handler_data(desc);
>     struct irq_chip_generic *gc = domain->gc->gc[0];
>     struct irq_chip *chip = irq_desc_get_chip(desc);
>     unsigned long pending;
>     int n;
>
>     chained_irq_enter(chip, desc);
>
>     while ((pending = irq_reg_readl(gc, EXTI_PR))) {
>         for_each_set_bit(n, &pending, BITS_PER_LONG) {
>             generic_handle_irq(irq_find_mapping(domain, n));
>         }
>     }

Looks clever! :)

If it also works, I'm in for it.

Yours,
Linus Walleij
diff mbox

Patch

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 3e124793e224..149a30e0eec0 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -244,3 +244,7 @@  config IRQ_MXS
 config MVEBU_ODMI
 	bool
 	select GENERIC_MSI_IRQ_DOMAIN
+
+config STM32_EXTI
+	bool
+	select IRQ_DOMAIN
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index b03cfcbbac6b..2caeae000307 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -65,3 +65,4 @@  obj-$(CONFIG_INGENIC_IRQ)		+= irq-ingenic.o
 obj-$(CONFIG_IMX_GPCV2)			+= irq-imx-gpcv2.o
 obj-$(CONFIG_PIC32_EVIC)		+= irq-pic32-evic.o
 obj-$(CONFIG_MVEBU_ODMI)		+= irq-mvebu-odmi.o
+obj-$(CONFIG_STM32_EXTI) 		+= irq-stm32-exti.o
diff --git a/drivers/irqchip/irq-stm32-exti.c b/drivers/irqchip/irq-stm32-exti.c
new file mode 100644
index 000000000000..02bfa807db6c
--- /dev/null
+++ b/drivers/irqchip/irq-stm32-exti.c
@@ -0,0 +1,169 @@ 
+/*
+ * Copyright (C) Maxime Coquelin 2015
+ * Author:  Maxime Coquelin <mcoquelin.stm32@gmail.com>
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#include <linux/bitops.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+#define EXTI_IMR	0x0
+#define EXTI_EMR	0x4
+#define EXTI_RTSR	0x8
+#define EXTI_FTSR	0xc
+#define EXTI_SWIER	0x10
+#define EXTI_PR		0x14
+
+static void stm32_irq_handler(struct irq_desc *desc)
+{
+	struct irq_domain *domain = irq_desc_get_handler_data(desc);
+	struct irq_chip_generic *gc = domain->gc->gc[0];
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	unsigned long pending;
+	int n;
+
+	chained_irq_enter(chip, desc);
+
+	pending = irq_reg_readl(gc, EXTI_PR);
+	for_each_set_bit(n, &pending, BITS_PER_LONG) {
+		generic_handle_irq(irq_find_mapping(domain, n));
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static int stm32_irq_set_type(struct irq_data *data, unsigned int type)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
+	u32 rtsr, ftsr;
+	int pin = data->hwirq;
+
+	irq_gc_lock(gc);
+
+	rtsr = irq_reg_readl(gc, EXTI_RTSR);
+	ftsr = irq_reg_readl(gc, EXTI_FTSR);
+
+	switch (type) {
+	case IRQ_TYPE_EDGE_RISING:
+		rtsr |= BIT(pin);
+		ftsr &= ~BIT(pin);
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		rtsr &= ~BIT(pin);
+		ftsr |= BIT(pin);
+		break;
+	case IRQ_TYPE_EDGE_BOTH:
+		rtsr |= BIT(pin);
+		ftsr |= BIT(pin);
+		break;
+	default:
+		irq_gc_unlock(gc);
+		return -EINVAL;
+	}
+
+	irq_reg_writel(gc, rtsr, EXTI_RTSR);
+	irq_reg_writel(gc, ftsr, EXTI_FTSR);
+
+	irq_gc_unlock(gc);
+
+	return 0;
+}
+
+static int stm32_irq_set_wake(struct irq_data *data, unsigned int on)
+{
+	struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
+	int pin = data->hwirq;
+	u32 emr;
+
+	irq_gc_lock(gc);
+
+	emr = irq_reg_readl(gc, EXTI_EMR);
+	if (on)
+		emr |= BIT(pin);
+	else
+		emr &= ~BIT(pin);
+	irq_reg_writel(gc, emr, EXTI_EMR);
+
+	irq_gc_unlock(gc);
+
+	return 0;
+}
+
+static int __init stm32_exti_init(struct device_node *node,
+				  struct device_node *parent)
+{
+	int nr_irqs, nr_exti, ret, i;
+	unsigned int clr = IRQ_NOREQUEST | IRQ_NOPROBE | IRQ_NOAUTOEN;
+	struct irq_domain *domain;
+	struct irq_chip_generic *gc;
+	void *base;
+
+	base = of_iomap(node, 0);
+	if (!base) {
+		pr_err("%s: Unable to map registers\n", node->full_name);
+		return -ENOMEM;
+	}
+
+	/* Determine number of irqs supported */
+	writel_relaxed(~0UL, base + EXTI_RTSR);
+	nr_exti = fls(readl_relaxed(base + EXTI_RTSR));
+	writel_relaxed(0, base + EXTI_RTSR);
+
+	pr_info("%s: %d External IRQs detected\n", node->full_name, nr_exti);
+
+	domain = irq_domain_add_linear(node, nr_exti,
+				       &irq_generic_chip_ops, NULL);
+	if (!domain) {
+		pr_err("%s: Could not register interrupt domain.\n",
+				node->name);
+		ret = -ENOMEM;
+		goto out_unmap;
+	}
+
+	ret = irq_alloc_domain_generic_chips(domain, nr_exti, 1, "exti",
+					     handle_edge_irq, clr, 0, 0);
+	if (ret) {
+		pr_err("%s: Could not allocate generic interrupt chip.\n",
+			node->full_name);
+		goto out_free_domain;
+	}
+
+	gc = domain->gc->gc[0];
+	gc->reg_base                         = base;
+	gc->chip_types->type               = IRQ_TYPE_EDGE_BOTH;
+	gc->chip_types->chip.name          = gc->chip_types[0].chip.name;
+	gc->chip_types->chip.irq_ack       = irq_gc_ack_set_bit;
+	gc->chip_types->chip.irq_mask      = irq_gc_mask_clr_bit;
+	gc->chip_types->chip.irq_unmask    = irq_gc_mask_set_bit;
+	gc->chip_types->chip.irq_set_type  = stm32_irq_set_type;
+	gc->chip_types->chip.irq_set_wake  = stm32_irq_set_wake;
+	gc->chip_types->regs.ack           = EXTI_PR;
+	gc->chip_types->regs.mask          = EXTI_IMR;
+	gc->chip_types->handler            = handle_edge_irq;
+
+	nr_irqs = of_irq_count(node);
+	for (i = 0; i < nr_irqs; i++) {
+		unsigned int irq = irq_of_parse_and_map(node, i);
+
+		irq_set_handler_data(irq, domain);
+		irq_set_chained_handler(irq, stm32_irq_handler);
+	}
+
+	return 0;
+
+out_free_domain:
+	irq_domain_remove(domain);
+out_unmap:
+	iounmap(base);
+	return ret;
+}
+
+IRQCHIP_DECLARE(stm32_exti, "st,stm32-exti", stm32_exti_init);