On Wed, 14 Jan 2015 14:36:42 +0100
Nicolas Ferre [off-list ref] wrote:

Le 13/01/2015 19:46, Boris Brezillon a ?crit :

quoted

Some interrupt controllers are multiplexing several peripheral IRQs on
a single interrupt line.
While this is not a problem for most IRQs (as long as all peripherals
request the interrupt with IRQF_SHARED flag set), multiplexing timers and
other type of peripherals will generate a WARNING (mixing IRQF_NO_SUSPEND
and !IRQF_NO_SUSPEND is prohibited).

Create a dumb irq demultiplexer which simply forwards interrupts to all
peripherals (exactly what's happening with IRQ_SHARED) but keep a unique
irq number for each peripheral, thus preventing the IRQF_NO_SUSPEND
and !IRQF_NO_SUSPEND mix on a given interrupt.

Signed-off-by: Boris Brezillon <redacted>
---
 drivers/irqchip/Kconfig          |   4 ++
 drivers/irqchip/Makefile         |   1 +
 drivers/irqchip/irq-dumb-demux.c |  70 ++++++++++++++++++++
 include/linux/irq.h              |  49 ++++++++++++++
 include/linux/irqdomain.h        |   1 +
 kernel/irq/Kconfig               |   5 ++
 kernel/irq/Makefile              |   1 +
 kernel/irq/chip.c                |  41 ++++++++++++
 kernel/irq/dumb-demux-chip.c     | 140 +++++++++++++++++++++++++++++++++++++++
 kernel/irq/handle.c              |  31 ++++++++-
 kernel/irq/internals.h           |   3 +
 11 files changed, 344 insertions(+), 2 deletions(-)
 create mode 100644 drivers/irqchip/irq-dumb-demux.c
 create mode 100644 kernel/irq/dumb-demux-chip.c

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index cc79d2a..8a9df88 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig

@@ -70,6 +70,10 @@ config BRCMSTB_L2_IRQ
 	select GENERIC_IRQ_CHIP
 	select IRQ_DOMAIN
 
+config DUMB_DEMUX_IRQ
+	bool
+	select DUMB_IRQ_DEMUX_CHIP
+
 config DW_APB_ICTL
 	bool
 	select GENERIC_IRQ_CHIP

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 9516a32..77f3c51 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile

@@ -8,6 +8,7 @@ obj-$(CONFIG_ARCH_MVEBU)		+= irq-armada-370-xp.o
 obj-$(CONFIG_ARCH_MXS)			+= irq-mxs.o
 obj-$(CONFIG_ARCH_S3C24XX)		+= irq-s3c24xx.o
 obj-$(CONFIG_DW_APB_ICTL)		+= irq-dw-apb-ictl.o
+obj-$(CONFIG_DUMB_DEMUX_IRQ)		+= irq-dumb-demux.o
 obj-$(CONFIG_METAG)			+= irq-metag-ext.o
 obj-$(CONFIG_METAG_PERFCOUNTER_IRQS)	+= irq-metag.o
 obj-$(CONFIG_ARCH_MOXART)		+= irq-moxart.o

diff --git a/drivers/irqchip/irq-dumb-demux.c b/drivers/irqchip/irq-dumb-demux.c
new file mode 100644
index 0000000..dfa05ce
--- /dev/null
+++ b/drivers/irqchip/irq-dumb-demux.c

@@ -0,0 +1,70 @@

Maybe add a little file header here. It's always better.

Sure, I just forgot it.

quoted

+#ifdef CONFIG_DUMB_IRQ_DEMUX_CHIP
+/**
+ *	handle_dumb_demux_irq - Dumb demuxer irq handle function.
+ *	@irq:	the interrupt number
+ *	@desc:	the interrupt description structure for this irq
+ *
+ *	Dumb demux interrupts are sent from a demultiplexing interrupt handler
+ *	which is not able to decide which child interrupt interrupt handler

typo: "interrupt interrupt"

I'll fix that.

quoted

+ *	should be called.
+ *
+ *	Note: The caller is expected to handle the ack, clear, mask and
+ *	unmask issues if necessary.
+ */

[...]

quoted

+
 /*
  * Called unconditionally from handle_level_irq() and only for oneshot
  * interrupts from handle_fasteoi_irq()

diff --git a/kernel/irq/dumb-demux-chip.c b/kernel/irq/dumb-demux-chip.c
new file mode 100644
index 0000000..8e2de1d
--- /dev/null
+++ b/kernel/irq/dumb-demux-chip.c

@@ -0,0 +1,140 @@
+/*
+ * Library implementing common dumb irq demux chip functions
+ *
+ * Copyright (C) 2015, Boris Brezillon

License here, please.

Yep, I'll add it.

quoted

+ */
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/slab.h>
+#include <linux/export.h>
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/interrupt.h>
+#include <linux/kernel_stat.h>
+#include <linux/syscore_ops.h>
+
+#include "internals.h"
+
+static void irq_dumb_demux_mask(struct irq_data *d)
+{
+	struct irq_chip_dumb_demux *demux = irq_data_get_irq_chip_data(d);
+
+	clear_bit(d->hwirq, &demux->unmasked);
+
+	if (!demux->unmasked)
+		disable_irq_nosync(demux->src_irq);
+}
+
+static void irq_dumb_demux_unmask(struct irq_data *d)
+{
+	struct irq_chip_dumb_demux *demux = irq_data_get_irq_chip_data(d);
+	bool enable_src_irq = !demux->unmasked;

Why this additional "bool" unlike the other function above?

Because set_bit will modify the unmasked status and we must check if it
is equal to 0 (in other terms, all irqs are masked) before modifying it
in order to know whether we should enable the src irq or not.

quoted

+
+	set_bit(d->hwirq, &demux->unmasked);
+
+	if (enable_src_irq)
+		enable_irq(demux->src_irq);
+}
+

[...]

quoted

+
+/**
+ * irq_alloc_dumb_demux_chip - Allocate a dumb demux chip
+ * @src_irq:		irq feeding the dumb demux chip
+ * @dd_flags:		irq_dumb_demux_flags flags
+ * @valid_irqs:		Bitmask representing valid irqs
+ * @clr_flags:		irq_flags to clear when mapping an interrupt
+ * @set_flags:		irq_flags to set when mapping an interrupt

Nit. not same order as the function parameters...

I'll fix that.

Thanks,

Boris
-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com