Re: [PATCH v4 2/5] irqchip: add virtual demultiplexer implementation
On Thu, Jan 29, 2015 at 11:33:37AM +0100, Boris Brezillon wrote: +#ifdef CONFIG_VIRT_IRQ_DEMUX_CHIP +/** + * struct irq_chip_virt_demux - Dumb demultiplexer irq chip data structure s/Dumb/Virtual/ ? + * @domain: irq domain pointer + * @available: Bitfield of valid irqs + * @unmasked:Bitfield containing irqs status + * @flags: irq_virt_demux_flags flags + * @src_irq: irq feeding the virt demux chip + * + * Note, that irq_chip_generic can have multiple irq_chip_type + * implementations which can be associated to a particular irq line of + * an irq_chip_generic instance. That allows to share and protect + * state in an irq_chip_generic instance when we need to implement + * different flow mechanisms (level/edge) for it. This seems like a copy/paste from struct irq_chip_generic; it seems not relevant, seeing how irq_chip_virt_demux does not even have an irq_chip_type pointer list. Also, with a demuxer like this, we're bound to whatever flow type its host is, no? +# Dumb interrupt demuxer chip implementation s/Dumb/Virtual/ ? +#ifdef CONFIG_VIRT_IRQ_DEMUX_CHIP +/** + * handle_virt_demux_irq - Dumb demuxer irq handle function. idem + * @irq: the interrupt number + * @desc: the interrupt description structure for this irq + * + * Dumb demux interrupts are sent from a demultiplexing interrupt handler idem + * which is not able to decide which child interrupt handler should be + * called. + * + * Note: The caller is expected to handle the ack, clear, mask and + * unmask issues if necessary. + */ If we're calling multiple handlers, how can they all do this and not collide? Over all it looks good -- in as far as my (admittedly stale IRQ braincells) go. I'll go queue up these bits, if you could send me a delta patch addressing these 'minor' comment issues? -- To unsubscribe from this list: send the line unsubscribe devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/5] irqchip: add virtual demultiplexer implementation
Hello Peter, On Tue, 10 Feb 2015 16:00:13 +0100 Peter Zijlstra pet...@infradead.org wrote: On Thu, Jan 29, 2015 at 11:33:37AM +0100, Boris Brezillon wrote: +#ifdef CONFIG_VIRT_IRQ_DEMUX_CHIP +/** + * struct irq_chip_virt_demux - Dumb demultiplexer irq chip data structure s/Dumb/Virtual/ ? + * @domain:irq domain pointer + * @available: Bitfield of valid irqs + * @unmasked: Bitfield containing irqs status + * @flags: irq_virt_demux_flags flags + * @src_irq: irq feeding the virt demux chip + * + * Note, that irq_chip_generic can have multiple irq_chip_type + * implementations which can be associated to a particular irq line of + * an irq_chip_generic instance. That allows to share and protect + * state in an irq_chip_generic instance when we need to implement + * different flow mechanisms (level/edge) for it. This seems like a copy/paste from struct irq_chip_generic; it seems not relevant, seeing how irq_chip_virt_demux does not even have an irq_chip_type pointer list. Also, with a demuxer like this, we're bound to whatever flow type its host is, no? Absolutely, I'll fix the comment by removing those lines. +# Dumb interrupt demuxer chip implementation s/Dumb/Virtual/ ? Yep, I'll fix that one too. +#ifdef CONFIG_VIRT_IRQ_DEMUX_CHIP +/** + * handle_virt_demux_irq - Dumb demuxer irq handle function. idem + * @irq: the interrupt number + * @desc: the interrupt description structure for this irq + * + * Dumb demux interrupts are sent from a demultiplexing interrupt handler idem + * which is not able to decide which child interrupt handler should be + * called. + * + * Note: The caller is expected to handle the ack, clear, mask and + * unmask issues if necessary. + */ If we're calling multiple handlers, how can they all do this and not collide? It's the same problem as you noted above: a copy/paste that should have been reworded. I'll remove those lines too. Over all it looks good -- in as far as my (admittedly stale IRQ braincells) go. I'll go queue up these bits, if you could send me a delta patch addressing these 'minor' comment issues? Thanks, I'll send you a patch addressing your comments (it would be great if you could squash it with this patch). Regards, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- To unsubscribe from this list: send the line unsubscribe devicetree in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/5] irqchip: add virtual demultiplexer implementation
[...]
+static int __init virt_irq_demux_of_init(struct device_node *node,
+struct device_node *parent)
+{
+ struct irq_chip_virt_demux *demux;
+ unsigned int irq;
+ u32 valid_irqs;
+ int ret;
+
+ irq = irq_of_parse_and_map(node, 0);
+ if (!irq) {
+ pr_err(Failed to retrieve virt irq demuxer source\n);
+ return -EINVAL;
+ }
+
+ ret = of_property_read_u32(node, irqs, valid_irqs);
+ if (ret) {
+ pr_err(Invalid of missing 'irqs' property\n);
+ return ret;
+ }
+
+ demux = irq_alloc_virt_demux_chip(irq, valid_irqs,
+ IRQ_NOREQUEST | IRQ_NOPROBE |
+ IRQ_NOAUTOEN, 0);
+ if (!demux) {
+ pr_err(Failed to allocate virt irq demuxer struct\n);
+ return -ENOMEM;
+ }
+
+ demux-domain = irq_domain_add_linear(node, BITS_PER_LONG,
+ irq_virt_demux_domain_ops,
+ demux);
+ if (!demux-domain) {
+ ret = -ENOMEM;
+ goto err_free_demux;
+ }
+
+ ret = irq_set_handler_data(irq, demux);
+ if (ret) {
+ pr_err(Failed to assign handler data\n);
+ goto err_free_domain;
+ }
+
+ irq_set_chained_handler_nostartup(irq, irq_virt_demux_handler);
+
+ return 0;
+
+err_free_domain:
+ irq_domain_remove(demux-domain);
+
+err_free_demux:
+ kfree(demux);
+
+ return ret;
+}
+IRQCHIP_DECLARE(virt_irq_demux, virtual,irq-demux, virt_irq_demux_of_init);
As mentioned on the DT binding patch, I really don't think this should
be in the DT. It corresponds only to Linux internal details, not a piece
of hardware. If we need this internally, I don't see why it can't be
instanciated as required.
If we _must_ have this in the DT, I have concerns with the binding
w.r.t. the irqs property being a 32-bit bitmask (as opposed to say
being something like num-irqs which could be far larger, and is
easuier to read).
Thanks,
Mark.
--
To unsubscribe from this list: send the line unsubscribe devicetree in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/5] irqchip: add virtual demultiplexer implementation
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 boris.brezil...@free-electrons.com
Acked-by: Nicolas Ferre nicolas.fe...@atmel.com
---
drivers/irqchip/Kconfig | 4 ++
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-virt-demux.c | 72
include/linux/irq.h | 38 +++
include/linux/irqdomain.h| 1 +
kernel/irq/Kconfig | 5 ++
kernel/irq/Makefile | 1 +
kernel/irq/chip.c| 41
kernel/irq/handle.c | 31 -
kernel/irq/internals.h | 3 +
kernel/irq/virt-demux-chip.c | 140 +++
11 files changed, 335 insertions(+), 2 deletions(-)
create mode 100644 drivers/irqchip/irq-virt-demux.c
create mode 100644 kernel/irq/virt-demux-chip.c
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index cc79d2a..268f01f 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 VIRT_DEMUX_IRQ
+ bool
+ select VIRT_IRQ_DEMUX_CHIP
+
config DW_APB_ICTL
bool
select GENERIC_IRQ_CHIP
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 9516a32..ee81262 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_VIRT_DEMUX_IRQ) += irq-virt-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-virt-demux.c b/drivers/irqchip/irq-virt-demux.c
new file mode 100644
index 000..501978e
--- /dev/null
+++ b/drivers/irqchip/irq-virt-demux.c
@@ -0,0 +1,72 @@
+/*
+ * Virtual irq demux chip driver
+ *
+ * Copyright (C) 2015 Atmel
+ *
+ * Author: Boris Brezillon boris.brezil...@free-electrons.com
+ *
+ * This file is licensed under GPLv2.
+ */
+#include linux/interrupt.h
+#include linux/irq.h
+#include linux/irqdomain.h
+#include linux/of_irq.h
+#include linux/slab.h
+
+#include irqchip.h
+
+static int __init virt_irq_demux_of_init(struct device_node *node,
+struct device_node *parent)
+{
+ struct irq_chip_virt_demux *demux;
+ unsigned int irq;
+ u32 valid_irqs;
+ int ret;
+
+ irq = irq_of_parse_and_map(node, 0);
+ if (!irq) {
+ pr_err(Failed to retrieve virt irq demuxer source\n);
+ return -EINVAL;
+ }
+
+ ret = of_property_read_u32(node, irqs, valid_irqs);
+ if (ret) {
+ pr_err(Invalid of missing 'irqs' property\n);
+ return ret;
+ }
+
+ demux = irq_alloc_virt_demux_chip(irq, valid_irqs,
+ IRQ_NOREQUEST | IRQ_NOPROBE |
+ IRQ_NOAUTOEN, 0);
+ if (!demux) {
+ pr_err(Failed to allocate virt irq demuxer struct\n);
+ return -ENOMEM;
+ }
+
+ demux-domain = irq_domain_add_linear(node, BITS_PER_LONG,
+ irq_virt_demux_domain_ops,
+ demux);
+ if (!demux-domain) {
+ ret = -ENOMEM;
+ goto err_free_demux;
+ }
+
+ ret = irq_set_handler_data(irq, demux);
+ if (ret) {
+ pr_err(Failed to assign handler data\n);
+ goto err_free_domain;
+ }
+
+ irq_set_chained_handler_nostartup(irq, irq_virt_demux_handler);
+
+ return 0;
+
+err_free_domain:
+ irq_domain_remove(demux-domain);
+
+err_free_demux:
+ kfree(demux);
+
+ return ret;
+}
+IRQCHIP_DECLARE(virt_irq_demux, virtual,irq-demux, virt_irq_demux_of_init);
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 247b2d1..b703093 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -445,6 +445,10 @@ extern void handle_fasteoi_irq(unsigned int irq, struct
irq_desc *desc);
extern void handle_edge_irq(unsigned int irq, struct
