2014-11-25 08:04:29

by Yingjoe Chen

[permalink] [raw]
Subject: [PATCH v8 0/4] ARM: mediatek: Add support for interrupt polarity

This series is 8th version of interrupt polarity support for MediaTek SoCs.

I rebased previous version to latest tip/irq/irqdomain, and fix issues
raised by Marc & Mark. The interrupt & irq free work fine on mt8135 board.

Simplified block diagram for interrupt on my system:

+-------+ +-------+
---| SYSIRQ|------|ARM GIC|
---| |------| |
---| |------| |
---| |------| |
---| |------| |
+-------+ +-------+

In device tree, interrupt-parent for other devices is sysirq, child of gic.
This describe HW better and allow device to specify polarity as it is sent
by the device.

When using hierarchy irq domain, gic will use irq_domain_add_linear to
create irqdomain and all interrupt numbers must come from device tree. My
/proc/interrupts looks like this now:

# cat /proc/interrupts
CPU0
16: 149578 MT_SYSIRQ 113 mtk_timer
20: 1082 MT_SYSIRQ 54 serial

Changes in v8:
- Rebased to new tip/irq/irqdomain
- Update mediatek,sysirq.txt bindinds to document interrupt-cells format
and interrupt-parents
- Change interrupt-cells format check in mtk_sysirq.
- Minor coding style fix

Changes in v7:
- Discussed in [1]
- Rebased to tip/irq/irqdomain
- In mtk_sysirq_domain_alloc, use copy of irq_data to workaound of_node
check in gic_irq_domain_xlate.

Changes in v6:
- Discussed in [2]
- Rebased to tip/irq/irqdomain

Changes in v5:
- Discussed in [3]
- Fix bug on mt6589 reported by Matthias
- Fix bug for irq_find_mapping in irq_create_of_mapping
- Merge Marc's change to proper handle non-DT case in gic_init_bases

Changes in v4:
- Discussed in [4]
- Remove arm,hierarchy-irq-domain. When GIC is probed by DT, it will
support hierarchy irqdomain.

Changes in v3:
- Discussed in [5]
- First implementation using hierarchy irqdomain

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/304119.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/302221.html
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/298161.html
[4] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/296911.html
[5] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/293766.html

Yingjoe Chen (4):
irqchip: gic: Support hierarchy irq domain.
ARM: mediatek: Add sysirq interrupt polarity support
ARM: mediatek: Add sysirq in mt6589/mt8135/mt8127 dtsi
dt-bindings: add bindings for mediatek sysirq

.../bindings/arm/mediatek/mediatek,sysirq.txt | 28 ++++
arch/arm/boot/dts/mt6589.dtsi | 14 +-
arch/arm/boot/dts/mt8127.dtsi | 14 +-
arch/arm/boot/dts/mt8135.dtsi | 14 +-
drivers/irqchip/Kconfig | 1 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-gic.c | 77 +++++++---
drivers/irqchip/irq-mtk-sysirq.c | 163 +++++++++++++++++++++
8 files changed, 282 insertions(+), 30 deletions(-)
create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,sysirq.txt
create mode 100644 drivers/irqchip/irq-mtk-sysirq.c


2014-11-25 08:04:45

by Yingjoe Chen

[permalink] [raw]
Subject: [PATCH v8 1/4] irqchip: gic: Support hierarchy irq domain.

Add support to use gic as a parent for stacked irq domain.

Signed-off-by: Yingjoe Chen <[email protected]>
---
drivers/irqchip/Kconfig | 1 +
drivers/irqchip/irq-gic.c | 77 ++++++++++++++++++++++++++++++++---------------
2 files changed, 54 insertions(+), 24 deletions(-)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index b21f12f..7f34138 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -5,6 +5,7 @@ config IRQCHIP
config ARM_GIC
bool
select IRQ_DOMAIN
+ select IRQ_DOMAIN_HIERARCHY
select MULTI_IRQ_HANDLER

config GIC_NON_BANKED
diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 38493ff..ab6069b 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -788,17 +788,16 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
{
if (hw < 32) {
irq_set_percpu_devid(irq);
- irq_set_chip_and_handler(irq, &gic_chip,
- handle_percpu_devid_irq);
+ irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
+ handle_percpu_devid_irq, NULL, NULL);
set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
} else {
- irq_set_chip_and_handler(irq, &gic_chip,
- handle_fasteoi_irq);
+ irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
+ handle_fasteoi_irq, NULL, NULL);
set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);

gic_routable_irq_domain_ops->map(d, irq, hw);
}
- irq_set_chip_data(irq, d->host_data);
return 0;
}

@@ -858,6 +857,31 @@ static struct notifier_block gic_cpu_notifier = {
};
#endif

+static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
+ unsigned int nr_irqs, void *arg)
+{
+ int i, ret;
+ irq_hw_number_t hwirq;
+ unsigned int type = IRQ_TYPE_NONE;
+ struct of_phandle_args *irq_data = arg;
+
+ ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args,
+ irq_data->args_count, &hwirq, &type);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < nr_irqs; i++)
+ gic_irq_domain_map(domain, virq + i, hwirq + i);
+
+ return 0;
+}
+
+static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = {
+ .xlate = gic_irq_domain_xlate,
+ .alloc = gic_irq_domain_alloc,
+ .free = irq_domain_free_irqs_top,
+};
+
static const struct irq_domain_ops gic_irq_domain_ops = {
.map = gic_irq_domain_map,
.unmap = gic_irq_domain_unmap,
@@ -948,18 +972,6 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
gic_cpu_map[i] = 0xff;

/*
- * For primary GICs, skip over SGIs.
- * For secondary GICs, skip over PPIs, too.
- */
- if (gic_nr == 0 && (irq_start & 31) > 0) {
- hwirq_base = 16;
- if (irq_start != -1)
- irq_start = (irq_start & ~31) + 16;
- } else {
- hwirq_base = 32;
- }
-
- /*
* Find out how many interrupts are supported.
* The GIC only supports up to 1020 interrupt sources.
*/
@@ -969,10 +981,31 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
gic_irqs = 1020;
gic->gic_irqs = gic_irqs;

- gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
+ if (node) { /* DT case */
+ const struct irq_domain_ops *ops = &gic_irq_domain_hierarchy_ops;
+
+ if (!of_property_read_u32(node, "arm,routable-irqs",
+ &nr_routable_irqs)) {
+ ops = &gic_irq_domain_ops;
+ gic_irqs = nr_routable_irqs;
+ }
+
+ gic->domain = irq_domain_add_linear(node, gic_irqs, ops, gic);
+ } else { /* Non-DT case */
+ /*
+ * For primary GICs, skip over SGIs.
+ * For secondary GICs, skip over PPIs, too.
+ */
+ if (gic_nr == 0 && (irq_start & 31) > 0) {
+ hwirq_base = 16;
+ if (irq_start != -1)
+ irq_start = (irq_start & ~31) + 16;
+ } else {
+ hwirq_base = 32;
+ }
+
+ gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */

- if (of_property_read_u32(node, "arm,routable-irqs",
- &nr_routable_irqs)) {
irq_base = irq_alloc_descs(irq_start, 16, gic_irqs,
numa_node_id());
if (IS_ERR_VALUE(irq_base)) {
@@ -983,10 +1016,6 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,

gic->domain = irq_domain_add_legacy(node, gic_irqs, irq_base,
hwirq_base, &gic_irq_domain_ops, gic);
- } else {
- gic->domain = irq_domain_add_linear(node, nr_routable_irqs,
- &gic_irq_domain_ops,
- gic);
}

if (WARN_ON(!gic->domain))
--
1.8.1.1.dirty

2014-11-25 08:04:55

by Yingjoe Chen

[permalink] [raw]
Subject: [PATCH v8 2/4] ARM: mediatek: Add sysirq interrupt polarity support

Mediatek SoCs have interrupt polarity support in sysirq which
allows to invert polarity for given interrupt. Add this support
using hierarchy irq domain.

Signed-off-by: Yingjoe Chen <[email protected]>
---
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-mtk-sysirq.c | 163 +++++++++++++++++++++++++++++++++++++++
2 files changed, 164 insertions(+)
create mode 100644 drivers/irqchip/irq-mtk-sysirq.c

diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 173bb5f..4e0f254 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -38,3 +38,4 @@ obj-$(CONFIG_IRQ_CROSSBAR) += irq-crossbar.o
obj-$(CONFIG_BRCMSTB_L2_IRQ) += irq-brcmstb-l2.o \
irq-bcm7120-l2.o
obj-$(CONFIG_KEYSTONE_IRQ) += irq-keystone.o
+obj-$(CONFIG_ARCH_MEDIATEK) += irq-mtk-sysirq.o
diff --git a/drivers/irqchip/irq-mtk-sysirq.c b/drivers/irqchip/irq-mtk-sysirq.c
new file mode 100644
index 0000000..7e342df
--- /dev/null
+++ b/drivers/irqchip/irq-mtk-sysirq.c
@@ -0,0 +1,163 @@
+/*
+ * Copyright (c) 2014 MediaTek Inc.
+ * Author: Joe.C <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/of_address.h>
+#include <linux/io.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#include "irqchip.h"
+
+#define MT6577_SYS_INTPOL_NUM (224)
+
+struct mtk_sysirq_chip_data {
+ spinlock_t lock;
+ void __iomem *intpol_base;
+};
+
+static int mtk_sysirq_set_type(struct irq_data *data, unsigned int type)
+{
+ irq_hw_number_t hwirq = data->hwirq;
+ struct mtk_sysirq_chip_data *chip_data = data->chip_data;
+ u32 offset, reg_index, value;
+ unsigned long flags;
+ int ret;
+
+ offset = hwirq & 0x1f;
+ reg_index = hwirq >> 5;
+
+ spin_lock_irqsave(&chip_data->lock, flags);
+ value = readl_relaxed(chip_data->intpol_base + reg_index * 4);
+ if (type == IRQ_TYPE_LEVEL_LOW || type == IRQ_TYPE_EDGE_FALLING) {
+ if (type == IRQ_TYPE_LEVEL_LOW)
+ type = IRQ_TYPE_LEVEL_HIGH;
+ else
+ type = IRQ_TYPE_EDGE_RISING;
+ value |= (1 << offset);
+ } else {
+ value &= ~(1 << offset);
+ }
+ writel(value, chip_data->intpol_base + reg_index * 4);
+
+ data = data->parent_data;
+ ret = data->chip->irq_set_type(data, type);
+ spin_unlock_irqrestore(&chip_data->lock, flags);
+ return ret;
+}
+
+static struct irq_chip mtk_sysirq_chip = {
+ .name = "MT_SYSIRQ",
+ .irq_mask = irq_chip_mask_parent,
+ .irq_unmask = irq_chip_unmask_parent,
+ .irq_eoi = irq_chip_eoi_parent,
+ .irq_set_type = mtk_sysirq_set_type,
+ .irq_retrigger = irq_chip_retrigger_hierarchy,
+ .irq_set_affinity = irq_chip_set_affinity_parent,
+};
+
+static int mtk_sysirq_domain_xlate(struct irq_domain *d,
+ struct device_node *controller,
+ const u32 *intspec, unsigned int intsize,
+ unsigned long *out_hwirq,
+ unsigned int *out_type)
+{
+ if (intsize != 3)
+ return -EINVAL;
+
+ /* sysirq doesn't support PPI */
+ if (intspec[0])
+ return -EINVAL;
+
+ *out_hwirq = intspec[1];
+ *out_type = intspec[2] & IRQ_TYPE_SENSE_MASK;
+ return 0;
+}
+
+static int mtk_sysirq_domain_alloc(struct irq_domain *domain, unsigned int virq,
+ unsigned int nr_irqs, void *arg)
+{
+ int i;
+ irq_hw_number_t hwirq;
+ struct of_phandle_args *irq_data = arg;
+ struct of_phandle_args gic_data = *irq_data;
+
+ if (irq_data->args_count != 3)
+ return -EINVAL;
+
+ /* sysirq doesn't support PPI */
+ if (irq_data->args[0])
+ return -EINVAL;
+
+ hwirq = irq_data->args[1];
+ for (i = 0; i < nr_irqs; i++)
+ irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+ &mtk_sysirq_chip,
+ domain->host_data);
+
+ gic_data.np = domain->parent->of_node;
+ return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &gic_data);
+}
+
+static struct irq_domain_ops sysirq_domain_ops = {
+ .xlate = mtk_sysirq_domain_xlate,
+ .alloc = mtk_sysirq_domain_alloc,
+ .free = irq_domain_free_irqs_common,
+};
+
+static int __init mtk_sysirq_of_init(struct device_node *node,
+ struct device_node *parent)
+{
+ struct irq_domain *domain, *domain_parent;
+ struct mtk_sysirq_chip_data *chip_data;
+ int ret = 0;
+
+ domain_parent = irq_find_host(parent);
+ if (!domain_parent) {
+ pr_err("mtk_sysirq: interrupt-parent not found\n");
+ return -EINVAL;
+ }
+
+ chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
+ if (!chip_data)
+ return -ENOMEM;
+
+ chip_data->intpol_base = of_io_request_and_map(node, 0, "intpol");
+ if (!chip_data->intpol_base) {
+ pr_err("mtk_sysirq: unable to map sysirq register\n");
+ ret = -ENOMEM;
+ goto out_free;
+ }
+
+ domain = irq_domain_add_hierarchy(domain_parent, 0,
+ MT6577_SYS_INTPOL_NUM, node,
+ &sysirq_domain_ops, chip_data);
+ if (!domain) {
+ ret = -ENOMEM;
+ goto out_unmap;
+ }
+ spin_lock_init(&chip_data->lock);
+
+ return 0;
+
+out_unmap:
+ iounmap(chip_data->intpol_base);
+out_free:
+ kfree(chip_data);
+ return ret;
+}
+IRQCHIP_DECLARE(mtk_sysirq, "mediatek,mt6577-sysirq", mtk_sysirq_of_init);
--
1.8.1.1.dirty

2014-11-25 08:05:01

by Yingjoe Chen

[permalink] [raw]
Subject: [PATCH v8 3/4] ARM: mediatek: Add sysirq in mt6589/mt8135/mt8127 dtsi

Add sysirq settings for mt6589/mt8135/mt8127
This also correct timer interrupt flag. The old setting works
because boot loader already set polarity for timer interrupt.
Without intpol support, the setting was not changed so gic
can get the irq correctly.

Signed-off-by: Yingjoe Chen <[email protected]>
---
arch/arm/boot/dts/mt6589.dtsi | 14 ++++++++++++--
arch/arm/boot/dts/mt8127.dtsi | 14 ++++++++++++--
arch/arm/boot/dts/mt8135.dtsi | 14 ++++++++++++--
3 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/mt6589.dtsi b/arch/arm/boot/dts/mt6589.dtsi
index e3c7600..c91b2a9 100644
--- a/arch/arm/boot/dts/mt6589.dtsi
+++ b/arch/arm/boot/dts/mt6589.dtsi
@@ -19,7 +19,7 @@

/ {
compatible = "mediatek,mt6589";
- interrupt-parent = <&gic>;
+ interrupt-parent = <&sysirq>;

cpus {
#address-cells = <1>;
@@ -76,15 +76,25 @@
timer: timer@10008000 {
compatible = "mediatek,mt6577-timer";
reg = <0x10008000 0x80>;
- interrupts = <GIC_SPI 113 IRQ_TYPE_EDGE_RISING>;
+ interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
clocks = <&system_clk>, <&rtc_clk>;
clock-names = "system-clk", "rtc-clk";
};

+ sysirq: interrupt-controller@10200100 {
+ compatible = "mediatek,mt6589-sysirq",
+ "mediatek,mt6577-sysirq";
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ interrupt-parent = <&gic>;
+ reg = <0x10200100 0x1c>;
+ };
+
gic: interrupt-controller@10211000 {
compatible = "arm,cortex-a7-gic";
interrupt-controller;
#interrupt-cells = <3>;
+ interrupt-parent = <&gic>;
reg = <0x10211000 0x1000>,
<0x10212000 0x1000>,
<0x10214000 0x2000>,
diff --git a/arch/arm/boot/dts/mt8127.dtsi b/arch/arm/boot/dts/mt8127.dtsi
index c3ee060..49f5976 100644
--- a/arch/arm/boot/dts/mt8127.dtsi
+++ b/arch/arm/boot/dts/mt8127.dtsi
@@ -18,7 +18,7 @@

/ {
compatible = "mediatek,mt8127";
- interrupt-parent = <&gic>;
+ interrupt-parent = <&sysirq>;

cpus {
#address-cells = <1>;
@@ -82,15 +82,25 @@
compatible = "mediatek,mt8127-timer",
"mediatek,mt6577-timer";
reg = <0 0x10008000 0 0x80>;
- interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_LOW>;
clocks = <&system_clk>, <&rtc_clk>;
clock-names = "system-clk", "rtc-clk";
};

+ sysirq: interrupt-controller@10200100 {
+ compatible = "mediatek,mt8127-sysirq",
+ "mediatek,mt6577-sysirq";
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ interrupt-parent = <&gic>;
+ reg = <0 0x10200100 0 0x1c>;
+ };
+
gic: interrupt-controller@10211000 {
compatible = "arm,cortex-a7-gic";
interrupt-controller;
#interrupt-cells = <3>;
+ interrupt-parent = <&gic>;
reg = <0 0x10211000 0 0x1000>,
<0 0x10212000 0 0x1000>,
<0 0x10214000 0 0x2000>,
diff --git a/arch/arm/boot/dts/mt8135.dtsi b/arch/arm/boot/dts/mt8135.dtsi
index 5faae6e..60338d9 100644
--- a/arch/arm/boot/dts/mt8135.dtsi
+++ b/arch/arm/boot/dts/mt8135.dtsi
@@ -18,7 +18,7 @@

/ {
compatible = "mediatek,mt8135";
- interrupt-parent = <&gic>;
+ interrupt-parent = <&sysirq>;

cpu-map {
cluster0 {
@@ -105,15 +105,25 @@
compatible = "mediatek,mt8135-timer",
"mediatek,mt6577-timer";
reg = <0 0x10008000 0 0x80>;
- interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_HIGH>;
+ interrupts = <GIC_SPI 113 IRQ_TYPE_LEVEL_LOW>;
clocks = <&system_clk>, <&rtc_clk>;
clock-names = "system-clk", "rtc-clk";
};

+ sysirq: interrupt-controller@10200030 {
+ compatible = "mediatek,mt8135-sysirq",
+ "mediatek,mt6577-sysirq";
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ interrupt-parent = <&gic>;
+ reg = <0 0x10200030 0 0x1c>;
+ };
+
gic: interrupt-controller@10211000 {
compatible = "arm,cortex-a15-gic";
interrupt-controller;
#interrupt-cells = <3>;
+ interrupt-parent = <&gic>;
reg = <0 0x10211000 0 0x1000>,
<0 0x10212000 0 0x1000>,
<0 0x10214000 0 0x2000>,
--
1.8.1.1.dirty

2014-11-25 08:04:59

by Yingjoe Chen

[permalink] [raw]
Subject: [PATCH v8 4/4] dt-bindings: add bindings for mediatek sysirq

Add binding documentation for Mediatek SoC SYSIRQ.

Signed-off-by: Yingjoe Chen <[email protected]>
---
.../bindings/arm/mediatek/mediatek,sysirq.txt | 28 ++++++++++++++++++++++
1 file changed, 28 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,sysirq.txt

diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,sysirq.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,sysirq.txt
new file mode 100644
index 0000000..d680b07
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,sysirq.txt
@@ -0,0 +1,28 @@
+Mediatek 65xx/81xx sysirq
+
+Mediatek SOCs sysirq support controllable irq inverter for each GIC SPI
+interrupt.
+
+Required properties:
+- compatible: should be one of:
+ "mediatek,mt8135-sysirq"
+ "mediatek,mt8127-sysirq"
+ "mediatek,mt6589-sysirq"
+ "mediatek,mt6582-sysirq"
+ "mediatek,mt6577-sysirq"
+- interrupt-controller : Identifies the node as an interrupt controller
+- #interrupt-cells : Use the same format as specified by GIC in
+ Documentation/devicetree/bindings/arm/gic.txt
+- interrupt-parent: phandle of irq parent for sysirq. The parent must
+ use the same interrupt-cells format as GIC.
+- reg: Physical base address of the intpol registers and length of memory
+ mapped region.
+
+Example:
+ sysirq: interrupt-controller@10200100 {
+ compatible = "mediatek,mt6589-sysirq", "mediatek,mt6577-sysirq";
+ interrupt-controller;
+ #interrupt-cells = <3>;
+ interrupt-parent = <&gic>;
+ reg = <0 0x10200100 0 0x1c>;
+ };
--
1.8.1.1.dirty

2014-11-25 11:27:42

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v8 1/4] irqchip: gic: Support hierarchy irq domain.

On 25/11/14 08:04, Yingjoe Chen wrote:
> Add support to use gic as a parent for stacked irq domain.
>
> Signed-off-by: Yingjoe Chen <[email protected]>

Looks good to me.

Acked-by: Marc Zyngier <[email protected]>

M.
--
Jazz is not dead. It just smells funny...

2014-11-26 08:08:49

by Jason Cooper

[permalink] [raw]
Subject: Re: [PATCH v8 0/4] ARM: mediatek: Add support for interrupt polarity

Yingjoe,

On Tue, Nov 25, 2014 at 04:04:18PM +0800, Yingjoe Chen wrote:
> This series is 8th version of interrupt polarity support for MediaTek SoCs.
>
> I rebased previous version to latest tip/irq/irqdomain, and fix issues
> raised by Marc & Mark. The interrupt & irq free work fine on mt8135 board.
>
> Simplified block diagram for interrupt on my system:
>
> +-------+ +-------+
> ---| SYSIRQ|------|ARM GIC|
> ---| |------| |
> ---| |------| |
> ---| |------| |
> ---| |------| |
> +-------+ +-------+
>
> In device tree, interrupt-parent for other devices is sysirq, child of gic.
> This describe HW better and allow device to specify polarity as it is sent
> by the device.
>
> When using hierarchy irq domain, gic will use irq_domain_add_linear to
> create irqdomain and all interrupt numbers must come from device tree. My
> /proc/interrupts looks like this now:
>
> # cat /proc/interrupts
> CPU0
> 16: 149578 MT_SYSIRQ 113 mtk_timer
> 20: 1082 MT_SYSIRQ 54 serial
>
> Changes in v8:
> - Rebased to new tip/irq/irqdomain
> - Update mediatek,sysirq.txt bindinds to document interrupt-cells format
> and interrupt-parents
> - Change interrupt-cells format check in mtk_sysirq.
> - Minor coding style fix
>
> Changes in v7:
> - Discussed in [1]
> - Rebased to tip/irq/irqdomain
> - In mtk_sysirq_domain_alloc, use copy of irq_data to workaound of_node
> check in gic_irq_domain_xlate.
>
> Changes in v6:
> - Discussed in [2]
> - Rebased to tip/irq/irqdomain
>
> Changes in v5:
> - Discussed in [3]
> - Fix bug on mt6589 reported by Matthias
> - Fix bug for irq_find_mapping in irq_create_of_mapping
> - Merge Marc's change to proper handle non-DT case in gic_init_bases
>
> Changes in v4:
> - Discussed in [4]
> - Remove arm,hierarchy-irq-domain. When GIC is probed by DT, it will
> support hierarchy irqdomain.
>
> Changes in v3:
> - Discussed in [5]
> - First implementation using hierarchy irqdomain
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/304119.html
> [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-November/302221.html
> [3] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/298161.html
> [4] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/296911.html
> [5] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/293766.html
>
> Yingjoe Chen (4):
> irqchip: gic: Support hierarchy irq domain.
> ARM: mediatek: Add sysirq interrupt polarity support
> ARM: mediatek: Add sysirq in mt6589/mt8135/mt8127 dtsi
> dt-bindings: add bindings for mediatek sysirq
>
> .../bindings/arm/mediatek/mediatek,sysirq.txt | 28 ++++
> arch/arm/boot/dts/mt6589.dtsi | 14 +-
> arch/arm/boot/dts/mt8127.dtsi | 14 +-
> arch/arm/boot/dts/mt8135.dtsi | 14 +-
> drivers/irqchip/Kconfig | 1 +
> drivers/irqchip/Makefile | 1 +
> drivers/irqchip/irq-gic.c | 77 +++++++---
> drivers/irqchip/irq-mtk-sysirq.c | 163 +++++++++++++++++++++
> 8 files changed, 282 insertions(+), 30 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,sysirq.txt
> create mode 100644 drivers/irqchip/irq-mtk-sysirq.c

Patches 1, 2, and 4 applied to irqchip/core with a dependency on
tip/irq/irqdomain with a few small subject line tweaks.

thx,

Jason.

2014-11-27 13:31:18

by Yingjoe Chen

[permalink] [raw]
Subject: Re: [PATCH v8 3/4] ARM: mediatek: Add sysirq in mt6589/mt8135/mt8127 dtsi

On Tue, 2014-11-25 at 16:04 +0800, Yingjoe Chen wrote:
> Add sysirq settings for mt6589/mt8135/mt8127
> This also correct timer interrupt flag. The old setting works
> because boot loader already set polarity for timer interrupt.
> Without intpol support, the setting was not changed so gic
> can get the irq correctly.

Hi Matthias,

I think this patch should go through you. Would you take a look at this
patch? Also, mt8135/mt8127 uart support[1] depends on this, should we
send it again?

Thanks.

Joe.C

[1]
http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/296147.html

2014-11-29 17:43:11

by Beniamino Galvani

[permalink] [raw]
Subject: Re: [PATCH v8 2/4] ARM: mediatek: Add sysirq interrupt polarity support

On Tue, Nov 25, 2014 at 04:04:20PM +0800, Yingjoe Chen wrote:
> Mediatek SoCs have interrupt polarity support in sysirq which
> allows to invert polarity for given interrupt. Add this support
> using hierarchy irq domain.
>
> [...]
>
> +static int __init mtk_sysirq_of_init(struct device_node *node,
> + struct device_node *parent)
> +{
> + struct irq_domain *domain, *domain_parent;
> + struct mtk_sysirq_chip_data *chip_data;
> + int ret = 0;
> +
> + domain_parent = irq_find_host(parent);
> + if (!domain_parent) {
> + pr_err("mtk_sysirq: interrupt-parent not found\n");
> + return -EINVAL;
> + }
> +
> + chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
> + if (!chip_data)
> + return -ENOMEM;
> +
> + chip_data->intpol_base = of_io_request_and_map(node, 0, "intpol");
> + if (!chip_data->intpol_base) {

Hi,

you should use IS_ERR() to check the return value here.

Beniamino

2014-12-01 14:09:38

by Yingjoe Chen

[permalink] [raw]
Subject: Re: [PATCH v8 2/4] ARM: mediatek: Add sysirq interrupt polarity support

On Sat, 2014-11-29 at 18:40 +0100, Beniamino Galvani wrote:
> On Tue, Nov 25, 2014 at 04:04:20PM +0800, Yingjoe Chen wrote:
> > Mediatek SoCs have interrupt polarity support in sysirq which
> > allows to invert polarity for given interrupt. Add this support
> > using hierarchy irq domain.
> >
> > [...]
> >
> > +static int __init mtk_sysirq_of_init(struct device_node *node,
> > + struct device_node *parent)
> > +{
> > + struct irq_domain *domain, *domain_parent;
> > + struct mtk_sysirq_chip_data *chip_data;
> > + int ret = 0;
> > +
> > + domain_parent = irq_find_host(parent);
> > + if (!domain_parent) {
> > + pr_err("mtk_sysirq: interrupt-parent not found\n");
> > + return -EINVAL;
> > + }
> > +
> > + chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
> > + if (!chip_data)
> > + return -ENOMEM;
> > +
> > + chip_data->intpol_base = of_io_request_and_map(node, 0, "intpol");
> > + if (!chip_data->intpol_base) {
>
> Hi,
>
> you should use IS_ERR() to check the return value here.

Thanks for catching this. I think this is merged, so I'll prepare a new
patch to fix this.

Joe.C

2014-12-02 13:28:02

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v8 1/4] irqchip: gic: Support hierarchy irq domain.

On Tue, Nov 25, 2014 at 9:04 AM, Yingjoe Chen <[email protected]> wrote:
> Add support to use gic as a parent for stacked irq domain.
>
> Signed-off-by: Yingjoe Chen <[email protected]>

This change (which is now in -next as commit 9a1091ef0017c40a) breaks
booting on r8a7740/armadillo-legacy.

It hangs because the timers cannot get their interrupts:

console [tty0] enabled
sh-tmu.0: ch0: used for clock events
sh-tmu.0: ch0: used for periodic clock events
sh-tmu.0: ch0: failed to request irq 230
sh-tmu.0: ch1: used as clock source
sh-cmt-48.1: ch0: failed to request irq 90
sh-cmt-48.1: ch0: registration failed
earlytimer: unable to probe sh-cmt-48 early.
Calibrating delay loop...

> ---
> drivers/irqchip/Kconfig | 1 +
> drivers/irqchip/irq-gic.c | 77 ++++++++++++++++++++++++++++++++---------------
> 2 files changed, 54 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index b21f12f..7f34138 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -5,6 +5,7 @@ config IRQCHIP
> config ARM_GIC
> bool
> select IRQ_DOMAIN
> + select IRQ_DOMAIN_HIERARCHY
> select MULTI_IRQ_HANDLER
>
> config GIC_NON_BANKED
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index 38493ff..ab6069b 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -788,17 +788,16 @@ static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
> {
> if (hw < 32) {
> irq_set_percpu_devid(irq);
> - irq_set_chip_and_handler(irq, &gic_chip,
> - handle_percpu_devid_irq);
> + irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
> + handle_percpu_devid_irq, NULL, NULL);
> set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
> } else {
> - irq_set_chip_and_handler(irq, &gic_chip,
> - handle_fasteoi_irq);
> + irq_domain_set_info(d, irq, hw, &gic_chip, d->host_data,
> + handle_fasteoi_irq, NULL, NULL);
> set_irq_flags(irq, IRQF_VALID | IRQF_PROBE);
>
> gic_routable_irq_domain_ops->map(d, irq, hw);
> }
> - irq_set_chip_data(irq, d->host_data);
> return 0;
> }
>
> @@ -858,6 +857,31 @@ static struct notifier_block gic_cpu_notifier = {
> };
> #endif
>
> +static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> + unsigned int nr_irqs, void *arg)
> +{
> + int i, ret;
> + irq_hw_number_t hwirq;
> + unsigned int type = IRQ_TYPE_NONE;
> + struct of_phandle_args *irq_data = arg;
> +
> + ret = gic_irq_domain_xlate(domain, irq_data->np, irq_data->args,
> + irq_data->args_count, &hwirq, &type);
> + if (ret)
> + return ret;
> +
> + for (i = 0; i < nr_irqs; i++)
> + gic_irq_domain_map(domain, virq + i, hwirq + i);
> +
> + return 0;
> +}
> +
> +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = {
> + .xlate = gic_irq_domain_xlate,
> + .alloc = gic_irq_domain_alloc,
> + .free = irq_domain_free_irqs_top,
> +};
> +
> static const struct irq_domain_ops gic_irq_domain_ops = {
> .map = gic_irq_domain_map,
> .unmap = gic_irq_domain_unmap,
> @@ -948,18 +972,6 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> gic_cpu_map[i] = 0xff;
>
> /*
> - * For primary GICs, skip over SGIs.
> - * For secondary GICs, skip over PPIs, too.
> - */
> - if (gic_nr == 0 && (irq_start & 31) > 0) {
> - hwirq_base = 16;
> - if (irq_start != -1)
> - irq_start = (irq_start & ~31) + 16;
> - } else {
> - hwirq_base = 32;
> - }
> -
> - /*
> * Find out how many interrupts are supported.
> * The GIC only supports up to 1020 interrupt sources.
> */
> @@ -969,10 +981,31 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> gic_irqs = 1020;
> gic->gic_irqs = gic_irqs;
>
> - gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
> + if (node) { /* DT case */
> + const struct irq_domain_ops *ops = &gic_irq_domain_hierarchy_ops;
> +
> + if (!of_property_read_u32(node, "arm,routable-irqs",
> + &nr_routable_irqs)) {
> + ops = &gic_irq_domain_ops;
> + gic_irqs = nr_routable_irqs;
> + }
> +
> + gic->domain = irq_domain_add_linear(node, gic_irqs, ops, gic);
> + } else { /* Non-DT case */
> + /*
> + * For primary GICs, skip over SGIs.
> + * For secondary GICs, skip over PPIs, too.
> + */
> + if (gic_nr == 0 && (irq_start & 31) > 0) {
> + hwirq_base = 16;
> + if (irq_start != -1)
> + irq_start = (irq_start & ~31) + 16;
> + } else {
> + hwirq_base = 32;
> + }
> +
> + gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>
> - if (of_property_read_u32(node, "arm,routable-irqs",
> - &nr_routable_irqs)) {
> irq_base = irq_alloc_descs(irq_start, 16, gic_irqs,
> numa_node_id());
> if (IS_ERR_VALUE(irq_base)) {
> @@ -983,10 +1016,6 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>
> gic->domain = irq_domain_add_legacy(node, gic_irqs, irq_base,
> hwirq_base, &gic_irq_domain_ops, gic);
> - } else {
> - gic->domain = irq_domain_add_linear(node, nr_routable_irqs,
> - &gic_irq_domain_ops,
> - gic);
> }
>
> if (WARN_ON(!gic->domain))
> --
> 1.8.1.1.dirty

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2014-12-02 13:38:12

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v8 1/4] irqchip: gic: Support hierarchy irq domain.

On 02/12/14 13:27, Geert Uytterhoeven wrote:
> On Tue, Nov 25, 2014 at 9:04 AM, Yingjoe Chen <[email protected]> wrote:
>> Add support to use gic as a parent for stacked irq domain.
>>
>> Signed-off-by: Yingjoe Chen <[email protected]>
>
> This change (which is now in -next as commit 9a1091ef0017c40a) breaks
> booting on r8a7740/armadillo-legacy.
>
> It hangs because the timers cannot get their interrupts:
>
> console [tty0] enabled
> sh-tmu.0: ch0: used for clock events
> sh-tmu.0: ch0: used for periodic clock events
> sh-tmu.0: ch0: failed to request irq 230
> sh-tmu.0: ch1: used as clock source
> sh-cmt-48.1: ch0: failed to request irq 90
> sh-cmt-48.1: ch0: registration failed
> earlytimer: unable to probe sh-cmt-48 early.
> Calibrating delay loop...

Grmbl... Are you, by any (lack of) chance, using a setup where the GIC
is probed via DT, but the timers have their IRQs hardcoded?

M.
--
Jazz is not dead. It just smells funny...

2014-12-02 13:55:19

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v8 1/4] irqchip: gic: Support hierarchy irq domain.

Hi Marc,

On Tue, Dec 2, 2014 at 2:37 PM, Marc Zyngier <[email protected]> wrote:
> On 02/12/14 13:27, Geert Uytterhoeven wrote:
>> On Tue, Nov 25, 2014 at 9:04 AM, Yingjoe Chen <[email protected]> wrote:
>>> Add support to use gic as a parent for stacked irq domain.
>>>
>>> Signed-off-by: Yingjoe Chen <[email protected]>
>>
>> This change (which is now in -next as commit 9a1091ef0017c40a) breaks
>> booting on r8a7740/armadillo-legacy.
>>
>> It hangs because the timers cannot get their interrupts:
>>
>> console [tty0] enabled
>> sh-tmu.0: ch0: used for clock events
>> sh-tmu.0: ch0: used for periodic clock events
>> sh-tmu.0: ch0: failed to request irq 230
>> sh-tmu.0: ch1: used as clock source
>> sh-cmt-48.1: ch0: failed to request irq 90
>> sh-cmt-48.1: ch0: registration failed
>> earlytimer: unable to probe sh-cmt-48 early.
>> Calibrating delay loop...
>
> Grmbl... Are you, by any (lack of) chance, using a setup where the GIC
> is probed via DT, but the timers have their IRQs hardcoded?

Yes, the GIC is declared in arch/arm/boot/dts/r8a7740.dtsi:

gic: interrupt-controller@c2800000 {
compatible = "arm,cortex-a9-gic";
#interrupt-cells = <3>;
interrupt-controller;
reg = <0xc2800000 0x1000>,
<0xc2000000 0x1000>;
};

The timers come from legacy C board code in
arch/arm/mach-shmobile/setup-r8a7740.c:

/* CMT */
static struct sh_timer_config cmt1_platform_data = {
.channels_mask = 0x3f,
};

static struct resource cmt1_resources[] = {
DEFINE_RES_MEM(0xe6138000, 0x170),
DEFINE_RES_IRQ(gic_spi(58)),
};

static struct platform_device cmt1_device = {
.name = "sh-cmt-48",
.id = 1,
.dev = {
.platform_data = &cmt1_platform_data,
},
.resource = cmt1_resources,
.num_resources = ARRAY_SIZE(cmt1_resources),
};

/* TMU */
static struct sh_timer_config tmu0_platform_data = {
.channels_mask = 7,
};

static struct resource tmu0_resources[] = {
DEFINE_RES_MEM(0xfff80000, 0x2c),
DEFINE_RES_IRQ(gic_spi(198)),
DEFINE_RES_IRQ(gic_spi(199)),
DEFINE_RES_IRQ(gic_spi(200)),
};

static struct platform_device tmu0_device = {
.name = "sh-tmu",
.id = 0,
.dev = {
.platform_data = &tmu0_platform_data,
},
.resource = tmu0_resources,
.num_resources = ARRAY_SIZE(tmu0_resources),
};

The corresponding r8a7740/armadillo-multiplatform works fine.

Still, I'm wondering why sh73a0/kzm9g-legacy is not impacted by this
change...

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2014-12-02 14:12:22

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v8 1/4] irqchip: gic: Support hierarchy irq domain.

On 02/12/14 13:55, Geert Uytterhoeven wrote:
> Hi Marc,
>
> On Tue, Dec 2, 2014 at 2:37 PM, Marc Zyngier <[email protected]> wrote:
>> On 02/12/14 13:27, Geert Uytterhoeven wrote:
>>> On Tue, Nov 25, 2014 at 9:04 AM, Yingjoe Chen <[email protected]> wrote:
>>>> Add support to use gic as a parent for stacked irq domain.
>>>>
>>>> Signed-off-by: Yingjoe Chen <[email protected]>
>>>
>>> This change (which is now in -next as commit 9a1091ef0017c40a) breaks
>>> booting on r8a7740/armadillo-legacy.
>>>
>>> It hangs because the timers cannot get their interrupts:
>>>
>>> console [tty0] enabled
>>> sh-tmu.0: ch0: used for clock events
>>> sh-tmu.0: ch0: used for periodic clock events
>>> sh-tmu.0: ch0: failed to request irq 230
>>> sh-tmu.0: ch1: used as clock source
>>> sh-cmt-48.1: ch0: failed to request irq 90
>>> sh-cmt-48.1: ch0: registration failed
>>> earlytimer: unable to probe sh-cmt-48 early.
>>> Calibrating delay loop...
>>
>> Grmbl... Are you, by any (lack of) chance, using a setup where the GIC
>> is probed via DT, but the timers have their IRQs hardcoded?
>
> Yes, the GIC is declared in arch/arm/boot/dts/r8a7740.dtsi:
>
> gic: interrupt-controller@c2800000 {
> compatible = "arm,cortex-a9-gic";
> #interrupt-cells = <3>;
> interrupt-controller;
> reg = <0xc2800000 0x1000>,
> <0xc2000000 0x1000>;
> };
>
> The timers come from legacy C board code in
> arch/arm/mach-shmobile/setup-r8a7740.c:
>
> /* CMT */
> static struct sh_timer_config cmt1_platform_data = {
> .channels_mask = 0x3f,
> };
>
> static struct resource cmt1_resources[] = {
> DEFINE_RES_MEM(0xe6138000, 0x170),
> DEFINE_RES_IRQ(gic_spi(58)),
> };

Yup. This gic_spi() macro is completely doomed, as it computes a
hardware interrupt where the kernel expects a virtual one. The only way
to fix this is to init the GIC outside of DT, so that we stick to a
legacy irqdomain. You can still use information from DT, but you can't
rely on irqchip_init() to produce something that a legacy platform can use.

> The corresponding r8a7740/armadillo-multiplatform works fine.

OK, that's indeed the expected result.

> Still, I'm wondering why sh73a0/kzm9g-legacy is not impacted by this
> change...

I'm a bit lost looking at the shmobile code. But I agree, kzm9g should
die a painful death, as it has the same level of breakage as armadillo.

M.
--
Jazz is not dead. It just smells funny...

2014-12-25 02:11:21

by Yingjoe Chen

[permalink] [raw]
Subject: Re: [PATCH v8 3/4] ARM: mediatek: Add sysirq in mt6589/mt8135/mt8127 dtsi


Hi Matthias,

On Tue, 2014-11-25 at 16:04 +0800, Yingjoe Chen wrote:
> Add sysirq settings for mt6589/mt8135/mt8127
> This also correct timer interrupt flag. The old setting works
> because boot loader already set polarity for timer interrupt.
> Without intpol support, the setting was not changed so gic
> can get the irq correctly.
>
> Signed-off-by: Yingjoe Chen <[email protected]>
> ---
> arch/arm/boot/dts/mt6589.dtsi | 14 ++++++++++++--
> arch/arm/boot/dts/mt8127.dtsi | 14 ++++++++++++--
> arch/arm/boot/dts/mt8135.dtsi | 14 ++++++++++++--
> 3 files changed, 36 insertions(+), 6 deletions(-)

It seems this patch is not merged in 3.19-rc1. This do have dependency
on other patches, and all the other patches in this series are merged.

Is this planed to merge later or do we need to send additional pull
request for it?

Joe.C