[PATCH v3 0/7] ARM: mediatek: Add support for interrupt polarity

[PATCH v3 0/7] ARM: mediatek: Add support for interrupt polarity

[Joe.C]

This series is 3rd version of interrupt polarity support for MediaTek
SoCs. Unlike previous attempts[1], this version is implemented with hierarchy
irqdomain. This is based on Jiang's hierarchy irqdomain v2 [2] and my
mediatek SoC basic support [3].

Simplified block diagram for interrupt:

    +-------+      +-------+
 ---| 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.

In order to use hierarchy irq domain, gic can't use legacy irq domain anymore.
When arm,hierarchy-irq-domain property is specified, GIC will work in hierarchy
way and all interrupt numbers must be set by device tree. My /proc/interrupts
looks like this:

# cat /proc/interrupts
           CPU0
 16:       1862  MT_SYSIRQ 113  mtk_timer
 17:         67  MT_SYSIRQ  54  mtk-uart

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/279052.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/290832.html
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/284553.html

Joe.C (7):
  irqdomain: Fix irq_domain_alloc_irqs return check.
  genirq: Add more helper functions to support stacked irq_chip
  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 irq domain parent binding
  dt-bindings: add bindings for mediatek sysirq

 Documentation/devicetree/bindings/arm/gic.txt      |   2 +
 .../bindings/arm/mediatek/mediatek,sysirq.txt      |  19 +++
 arch/arm/boot/dts/mt6589.dtsi                      |  14 +-
 arch/arm/boot/dts/mt8127.dtsi                      |  14 +-
 arch/arm/boot/dts/mt8135.dtsi                      |  14 +-
 arch/arm/mach-mediatek/Kconfig                     |   1 +
 drivers/irqchip/Makefile                           |   1 +
 drivers/irqchip/irq-gic.c                          |  56 ++++++-
 drivers/irqchip/irq-mt65xx-sysirq.c                | 170 +++++++++++++++++++++
 include/linux/irq.h                                |   5 +
 kernel/irq/chip.c                                  |  28 ++++
 kernel/irq/irqdomain.c                             |   2 +-
 12 files changed, 311 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,sysirq.txt
 create mode 100644 drivers/irqchip/irq-mt65xx-sysirq.c

[PATCH v3 0/7] ARM: mediatek: Add support for interrupt polarity

[Joe.C]

This series is 3rd version of interrupt polarity support for MediaTek
SoCs. Unlike previous attempts[1], this version is implemented with hierarchy
irqdomain. This is based on Jiang's hierarchy irqdomain v2 [2] and my
mediatek SoC basic support [3].

Simplified block diagram for interrupt:

    +-------+      +-------+
 ---| 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.

In order to use hierarchy irq domain, gic can't use legacy irq domain anymore.
When arm,hierarchy-irq-domain property is specified, GIC will work in hierarchy
way and all interrupt numbers must be set by device tree. My /proc/interrupts
looks like this:

# cat /proc/interrupts
           CPU0
 16:       1862  MT_SYSIRQ 113  mtk_timer
 17:         67  MT_SYSIRQ  54  mtk-uart

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/279052.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/290832.html
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-September/284553.html

Joe.C (7):
  irqdomain: Fix irq_domain_alloc_irqs return check.
  genirq: Add more helper functions to support stacked irq_chip
  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 irq domain parent binding
  dt-bindings: add bindings for mediatek sysirq

 Documentation/devicetree/bindings/arm/gic.txt      |   2 +
 .../bindings/arm/mediatek/mediatek,sysirq.txt      |  19 +++
 arch/arm/boot/dts/mt6589.dtsi                      |  14 +-
 arch/arm/boot/dts/mt8127.dtsi                      |  14 +-
 arch/arm/boot/dts/mt8135.dtsi                      |  14 +-
 arch/arm/mach-mediatek/Kconfig                     |   1 +
 drivers/irqchip/Makefile                           |   1 +
 drivers/irqchip/irq-gic.c                          |  56 ++++++-
 drivers/irqchip/irq-mt65xx-sysirq.c                | 170 +++++++++++++++++++++
 include/linux/irq.h                                |   5 +
 kernel/irq/chip.c                                  |  28 ++++
 kernel/irq/irqdomain.c                             |   2 +-
 12 files changed, 311 insertions(+), 15 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,sysirq.txt
 create mode 100644 drivers/irqchip/irq-mt65xx-sysirq.c

[PATCH v3 1/7] irqdomain: Fix irq_domain_alloc_irqs return check.

[Joe.C]

From: "Joe.C" <yingjoe.chen@mediatek.com>

Change virq type from unsigned int to int. Otherwise the return value
check for irq_domain_alloc_irqs will always pass.

Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
---
 kernel/irq/irqdomain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 584be46..dd8d3ab 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -469,7 +469,7 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data)
 	struct irq_domain *domain;
 	irq_hw_number_t hwirq;
 	unsigned int type = IRQ_TYPE_NONE;
-	unsigned int virq;
+	int virq;
 
 	domain = irq_data->np ? irq_find_host(irq_data->np) : irq_default_domain;
 	if (!domain) {
-- 
1.8.1.1.dirty

[PATCH v3 1/7] irqdomain: Fix irq_domain_alloc_irqs return check.

[Joe.C]

From: "Joe.C" <yingjoe.chen@mediatek.com>

Change virq type from unsigned int to int. Otherwise the return value
check for irq_domain_alloc_irqs will always pass.

Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
---
 kernel/irq/irqdomain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 584be46..dd8d3ab 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -469,7 +469,7 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data)
 	struct irq_domain *domain;
 	irq_hw_number_t hwirq;
 	unsigned int type = IRQ_TYPE_NONE;
-	unsigned int virq;
+	int virq;
 
 	domain = irq_data->np ? irq_find_host(irq_data->np) : irq_default_domain;
 	if (!domain) {
-- 
1.8.1.1.dirty

[PATCH v3 2/7] genirq: Add more helper functions to support stacked irq_chip

[Joe.C]

From: "Joe.C" <yingjoe.chen@mediatek.com>

Add more helper function for stacked irq_chip to just call parent's
function.

Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
---
 include/linux/irq.h |  5 +++++
 kernel/irq/chip.c   | 28 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 07abf5a..6159256 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -437,6 +437,11 @@ extern void handle_nested_irq(unsigned int irq);
 
 #ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
 extern void irq_chip_ack_parent(struct irq_data *data);
+extern void irq_chip_mask_parent(struct irq_data *data);
+extern void irq_chip_unmask_parent(struct irq_data *data);
+extern void irq_chip_eoi_parent(struct irq_data *data);
+extern int irq_chip_set_affinity_parent(struct irq_data *data,
+				const struct cpumask *dest, bool force);
 extern int irq_chip_retrigger_hierarchy(struct irq_data *data);
 #endif
 
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 58ed9ed..0ecc270 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -829,6 +829,34 @@ void irq_chip_ack_parent(struct irq_data *data)
 	data->chip->irq_ack(data);
 }
 
+void irq_chip_mask_parent(struct irq_data *data)
+{
+	data = data->parent_data;
+	data->chip->irq_mask(data);
+}
+
+void irq_chip_unmask_parent(struct irq_data *data)
+{
+	data = data->parent_data;
+	data->chip->irq_unmask(data);
+}
+
+void irq_chip_eoi_parent(struct irq_data *data)
+{
+	data = data->parent_data;
+	data->chip->irq_eoi(data);
+}
+
+int irq_chip_set_affinity_parent(struct irq_data *data,
+				 const struct cpumask *dest, bool force)
+{
+	data = data->parent_data;
+	if (data->chip->irq_set_affinity)
+		return data->chip->irq_set_affinity(data, dest, force);
+
+	return -ENOSYS;
+}
+
 int irq_chip_retrigger_hierarchy(struct irq_data *data)
 {
 	for (data = data->parent_data; data; data = data->parent_data)
-- 
1.8.1.1.dirty

[PATCH v3 2/7] genirq: Add more helper functions to support stacked irq_chip

[Joe.C]

From: "Joe.C" <yingjoe.chen@mediatek.com>

Add more helper function for stacked irq_chip to just call parent's
function.

Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
---
 include/linux/irq.h |  5 +++++
 kernel/irq/chip.c   | 28 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/include/linux/irq.h b/include/linux/irq.h
index 07abf5a..6159256 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -437,6 +437,11 @@ extern void handle_nested_irq(unsigned int irq);
 
 #ifdef	CONFIG_IRQ_DOMAIN_HIERARCHY
 extern void irq_chip_ack_parent(struct irq_data *data);
+extern void irq_chip_mask_parent(struct irq_data *data);
+extern void irq_chip_unmask_parent(struct irq_data *data);
+extern void irq_chip_eoi_parent(struct irq_data *data);
+extern int irq_chip_set_affinity_parent(struct irq_data *data,
+				const struct cpumask *dest, bool force);
 extern int irq_chip_retrigger_hierarchy(struct irq_data *data);
 #endif
 
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 58ed9ed..0ecc270 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -829,6 +829,34 @@ void irq_chip_ack_parent(struct irq_data *data)
 	data->chip->irq_ack(data);
 }
 
+void irq_chip_mask_parent(struct irq_data *data)
+{
+	data = data->parent_data;
+	data->chip->irq_mask(data);
+}
+
+void irq_chip_unmask_parent(struct irq_data *data)
+{
+	data = data->parent_data;
+	data->chip->irq_unmask(data);
+}
+
+void irq_chip_eoi_parent(struct irq_data *data)
+{
+	data = data->parent_data;
+	data->chip->irq_eoi(data);
+}
+
+int irq_chip_set_affinity_parent(struct irq_data *data,
+				 const struct cpumask *dest, bool force)
+{
+	data = data->parent_data;
+	if (data->chip->irq_set_affinity)
+		return data->chip->irq_set_affinity(data, dest, force);
+
+	return -ENOSYS;
+}
+
 int irq_chip_retrigger_hierarchy(struct irq_data *data)
 {
 	for (data = data->parent_data; data; data = data->parent_data)
-- 
1.8.1.1.dirty

[PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Joe.C]

From: "Joe.C" <yingjoe.chen@mediatek.com>

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

Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
---
 drivers/irqchip/irq-gic.c | 56 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 48 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index dda6dbc..17f5aa6 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -767,19 +767,17 @@ void __init gic_init_physaddr(struct device_node *node)
 static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
 				irq_hw_number_t hw)
 {
+	irq_domain_set_hwirq_and_chip(d, irq, hw, &gic_chip, d->host_data);
 	if (hw < 32) {
 		irq_set_percpu_devid(irq);
-		irq_set_chip_and_handler(irq, &gic_chip,
-					 handle_percpu_devid_irq);
+		irq_set_handler(irq, handle_percpu_devid_irq);
 		set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
 	} else {
-		irq_set_chip_and_handler(irq, &gic_chip,
-					 handle_fasteoi_irq);
+		irq_set_handler(irq, handle_fasteoi_irq);
 		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;
 }
 
@@ -795,8 +793,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
 {
 	unsigned long ret = 0;
 
-	if (d->of_node != controller)
-		return -EINVAL;
 	if (intsize < 3)
 		return -EINVAL;
 
@@ -839,6 +835,46 @@ static struct notifier_block gic_cpu_notifier = {
 };
 #endif
 
+
+#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
+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 void gic_irq_domain_free(struct irq_domain *domain, unsigned int virq,
+				unsigned int nr_irqs)
+{
+	int i;
+
+	for (i = 0; i < nr_irqs; i++) {
+		irq_set_handler(virq + i, NULL);
+		irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
+	}
+}
+
+static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = {
+	.alloc = gic_irq_domain_alloc,
+	.free = gic_irq_domain_free,
+};
+#else
+#define gic_irq_domain_hierarchy_ops 0
+#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
+
 static const struct irq_domain_ops gic_irq_domain_ops = {
 	.map = gic_irq_domain_map,
 	.unmap = gic_irq_domain_unmap,
@@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
 
 	gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
 
-	if (of_property_read_u32(node, "arm,routable-irqs",
+	if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
+		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
+		gic->domain = irq_domain_add_linear(node, gic_irqs,
+					&gic_irq_domain_hierarchy_ops, gic);
+	else 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());
-- 
1.8.1.1.dirty

[PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Joe.C]

From: "Joe.C" <yingjoe.chen@mediatek.com>

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

Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
---
 drivers/irqchip/irq-gic.c | 56 ++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 48 insertions(+), 8 deletions(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index dda6dbc..17f5aa6 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -767,19 +767,17 @@ void __init gic_init_physaddr(struct device_node *node)
 static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
 				irq_hw_number_t hw)
 {
+	irq_domain_set_hwirq_and_chip(d, irq, hw, &gic_chip, d->host_data);
 	if (hw < 32) {
 		irq_set_percpu_devid(irq);
-		irq_set_chip_and_handler(irq, &gic_chip,
-					 handle_percpu_devid_irq);
+		irq_set_handler(irq, handle_percpu_devid_irq);
 		set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
 	} else {
-		irq_set_chip_and_handler(irq, &gic_chip,
-					 handle_fasteoi_irq);
+		irq_set_handler(irq, handle_fasteoi_irq);
 		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;
 }
 
@@ -795,8 +793,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
 {
 	unsigned long ret = 0;
 
-	if (d->of_node != controller)
-		return -EINVAL;
 	if (intsize < 3)
 		return -EINVAL;
 
@@ -839,6 +835,46 @@ static struct notifier_block gic_cpu_notifier = {
 };
 #endif
 
+
+#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
+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 void gic_irq_domain_free(struct irq_domain *domain, unsigned int virq,
+				unsigned int nr_irqs)
+{
+	int i;
+
+	for (i = 0; i < nr_irqs; i++) {
+		irq_set_handler(virq + i, NULL);
+		irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
+	}
+}
+
+static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = {
+	.alloc = gic_irq_domain_alloc,
+	.free = gic_irq_domain_free,
+};
+#else
+#define gic_irq_domain_hierarchy_ops 0
+#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
+
 static const struct irq_domain_ops gic_irq_domain_ops = {
 	.map = gic_irq_domain_map,
 	.unmap = gic_irq_domain_unmap,
@@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
 
 	gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
 
-	if (of_property_read_u32(node, "arm,routable-irqs",
+	if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
+		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
+		gic->domain = irq_domain_add_linear(node, gic_irqs,
+					&gic_irq_domain_hierarchy_ops, gic);
+	else 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());
-- 
1.8.1.1.dirty

[PATCH v3 4/7] ARM: mediatek: Add sysirq interrupt polarity support

[Joe.C]

From: "Joe.C" <yingjoe.chen@mediatek.com>

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

Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
---
 arch/arm/mach-mediatek/Kconfig      |   1 +
 drivers/irqchip/Makefile            |   1 +
 drivers/irqchip/irq-mt65xx-sysirq.c | 170 ++++++++++++++++++++++++++++++++++++
 3 files changed, 172 insertions(+)
 create mode 100644 drivers/irqchip/irq-mt65xx-sysirq.c

diff --git a/arch/arm/mach-mediatek/Kconfig b/arch/arm/mach-mediatek/Kconfig
index 2c043a2..7093859 100644
--- a/arch/arm/mach-mediatek/Kconfig
+++ b/arch/arm/mach-mediatek/Kconfig
@@ -2,5 +2,6 @@ config ARCH_MEDIATEK
 	bool "Mediatek MT6589 SoC" if ARCH_MULTI_V7
 	select ARM_GIC
 	select MTK_TIMER
+	select IRQ_DOMAIN_HIERARCHY
 	help
 	  Support for Mediatek Cortex-A7 Quad-Core-SoC MT6589.
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 73052ba..809c9d5 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -34,3 +34,4 @@ obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
 obj-$(CONFIG_XTENSA_MX)			+= irq-xtensa-mx.o
 obj-$(CONFIG_IRQ_CROSSBAR)		+= irq-crossbar.o
 obj-$(CONFIG_BRCMSTB_L2_IRQ)		+= irq-brcmstb-l2.o
+obj-$(CONFIG_ARCH_MEDIATEK)		+= irq-mt65xx-sysirq.o
diff --git a/drivers/irqchip/irq-mt65xx-sysirq.c b/drivers/irqchip/irq-mt65xx-sysirq.c
new file mode 100644
index 0000000..9e0eee5
--- /dev/null
+++ b/drivers/irqchip/irq-mt65xx-sysirq.c
@@ -0,0 +1,170 @@
+/*
+ * Copyright (c) 2014 MediaTek Inc.
+ * Author: Joe.C <yingjoe.chen@mediatek.com>
+ *
+ * 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 mt_sysirq_chip_data {
+	spinlock_t lock;
+	void __iomem *intpol_base;
+};
+
+
+static int mt_sysirq_set_type(struct irq_data *data, unsigned int type)
+{
+	irq_hw_number_t hwirq = data->hwirq;
+	struct mt_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 mt_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		= mt_sysirq_set_type,
+	.irq_retrigger		= irq_chip_retrigger_hierarchy,
+	.irq_set_affinity	= irq_chip_set_affinity_parent,
+};
+
+static int mt_sysirq_domain_alloc(struct irq_domain *domain, unsigned int virq,
+				unsigned int nr_irqs, void *arg)
+{
+	int i, ret;
+	irq_hw_number_t hwirq;
+	struct of_phandle_args *irq_data = arg;
+	unsigned int type;
+
+	if (irq_data->args_count != 3)
+		return -EINVAL;
+
+	hwirq = irq_data->args[1];
+	if (irq_find_mapping(domain, hwirq) > 0)
+		return -EEXIST;
+
+	for (i = 0; i < nr_irqs; i++)
+		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+					&mt_sysirq_chip, domain->host_data);
+
+	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
+	if (ret < 0)
+		return ret;
+
+	type = irq_data->args[2] & IRQ_TYPE_SENSE_MASK;
+
+	/* Set type if specified and different than the current one */
+	if (type != IRQ_TYPE_NONE &&
+	    type != irq_get_trigger_type(virq))
+		irq_set_irq_type(virq, type);
+
+	return 0;
+}
+
+static void mt_sysirq_domain_free(struct irq_domain *domain, unsigned int virq,
+				unsigned int nr_irqs)
+{
+	int i;
+
+	for (i = 0; i < nr_irqs; i++) {
+		irq_set_handler(virq + i, NULL);
+		irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
+	}
+	irq_domain_free_irqs_parent(domain, virq, nr_irqs);
+}
+
+static struct irq_domain_ops sysirq_domain_ops = {
+	.alloc = mt_sysirq_domain_alloc,
+	.free = mt_sysirq_domain_free,
+};
+
+static int __init mtk_sysirq_of_init(struct device_node *node,
+				   struct device_node *parent)
+{
+	struct device_node *parent_node;
+	struct irq_domain *domain, *domain_parent = NULL;
+	struct mt_sysirq_chip_data *chip_data;
+	int ret = 0;
+
+	parent_node = of_irq_find_parent(node);
+	if (parent_node) {
+		domain_parent = irq_find_host(parent_node);
+		of_node_put(parent_node);
+	}
+
+	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_linear(node, MT6577_SYS_INTPOL_NUM,
+				&sysirq_domain_ops, chip_data);
+	if (!domain) {
+		ret = -ENOMEM;
+		goto out_unmap;
+	}
+	domain->parent = domain_parent;
+	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

[PATCH v3 4/7] ARM: mediatek: Add sysirq interrupt polarity support

[Joe.C]

From: "Joe.C" <yingjoe.chen@mediatek.com>

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

Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
---
 arch/arm/mach-mediatek/Kconfig      |   1 +
 drivers/irqchip/Makefile            |   1 +
 drivers/irqchip/irq-mt65xx-sysirq.c | 170 ++++++++++++++++++++++++++++++++++++
 3 files changed, 172 insertions(+)
 create mode 100644 drivers/irqchip/irq-mt65xx-sysirq.c

diff --git a/arch/arm/mach-mediatek/Kconfig b/arch/arm/mach-mediatek/Kconfig
index 2c043a2..7093859 100644
--- a/arch/arm/mach-mediatek/Kconfig
+++ b/arch/arm/mach-mediatek/Kconfig
@@ -2,5 +2,6 @@ config ARCH_MEDIATEK
 	bool "Mediatek MT6589 SoC" if ARCH_MULTI_V7
 	select ARM_GIC
 	select MTK_TIMER
+	select IRQ_DOMAIN_HIERARCHY
 	help
 	  Support for Mediatek Cortex-A7 Quad-Core-SoC MT6589.
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 73052ba..809c9d5 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -34,3 +34,4 @@ obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
 obj-$(CONFIG_XTENSA_MX)			+= irq-xtensa-mx.o
 obj-$(CONFIG_IRQ_CROSSBAR)		+= irq-crossbar.o
 obj-$(CONFIG_BRCMSTB_L2_IRQ)		+= irq-brcmstb-l2.o
+obj-$(CONFIG_ARCH_MEDIATEK)		+= irq-mt65xx-sysirq.o
diff --git a/drivers/irqchip/irq-mt65xx-sysirq.c b/drivers/irqchip/irq-mt65xx-sysirq.c
new file mode 100644
index 0000000..9e0eee5
--- /dev/null
+++ b/drivers/irqchip/irq-mt65xx-sysirq.c
@@ -0,0 +1,170 @@
+/*
+ * Copyright (c) 2014 MediaTek Inc.
+ * Author: Joe.C <yingjoe.chen@mediatek.com>
+ *
+ * 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 mt_sysirq_chip_data {
+	spinlock_t lock;
+	void __iomem *intpol_base;
+};
+
+
+static int mt_sysirq_set_type(struct irq_data *data, unsigned int type)
+{
+	irq_hw_number_t hwirq = data->hwirq;
+	struct mt_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 mt_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		= mt_sysirq_set_type,
+	.irq_retrigger		= irq_chip_retrigger_hierarchy,
+	.irq_set_affinity	= irq_chip_set_affinity_parent,
+};
+
+static int mt_sysirq_domain_alloc(struct irq_domain *domain, unsigned int virq,
+				unsigned int nr_irqs, void *arg)
+{
+	int i, ret;
+	irq_hw_number_t hwirq;
+	struct of_phandle_args *irq_data = arg;
+	unsigned int type;
+
+	if (irq_data->args_count != 3)
+		return -EINVAL;
+
+	hwirq = irq_data->args[1];
+	if (irq_find_mapping(domain, hwirq) > 0)
+		return -EEXIST;
+
+	for (i = 0; i < nr_irqs; i++)
+		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
+					&mt_sysirq_chip, domain->host_data);
+
+	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
+	if (ret < 0)
+		return ret;
+
+	type = irq_data->args[2] & IRQ_TYPE_SENSE_MASK;
+
+	/* Set type if specified and different than the current one */
+	if (type != IRQ_TYPE_NONE &&
+	    type != irq_get_trigger_type(virq))
+		irq_set_irq_type(virq, type);
+
+	return 0;
+}
+
+static void mt_sysirq_domain_free(struct irq_domain *domain, unsigned int virq,
+				unsigned int nr_irqs)
+{
+	int i;
+
+	for (i = 0; i < nr_irqs; i++) {
+		irq_set_handler(virq + i, NULL);
+		irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
+	}
+	irq_domain_free_irqs_parent(domain, virq, nr_irqs);
+}
+
+static struct irq_domain_ops sysirq_domain_ops = {
+	.alloc = mt_sysirq_domain_alloc,
+	.free = mt_sysirq_domain_free,
+};
+
+static int __init mtk_sysirq_of_init(struct device_node *node,
+				   struct device_node *parent)
+{
+	struct device_node *parent_node;
+	struct irq_domain *domain, *domain_parent = NULL;
+	struct mt_sysirq_chip_data *chip_data;
+	int ret = 0;
+
+	parent_node = of_irq_find_parent(node);
+	if (parent_node) {
+		domain_parent = irq_find_host(parent_node);
+		of_node_put(parent_node);
+	}
+
+	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_linear(node, MT6577_SYS_INTPOL_NUM,
+				&sysirq_domain_ops, chip_data);
+	if (!domain) {
+		ret = -ENOMEM;
+		goto out_unmap;
+	}
+	domain->parent = domain_parent;
+	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

[PATCH v3 5/7] ARM: mediatek: Add sysirq in mt6589/mt8135/mt8127 dtsi

[Joe.C]

From: "Joe.C" <yingjoe.chen@mediatek.com>

Add sysirq settings for mt6589/mt8135/mt8127
This also correct timer interrupt flag. The old flag 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: Joe.C <yingjoe.chen@mediatek.com>
---
 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..9f3e6b9 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 = <0 0x10200100 0 0x1c>;
+		};
+
 		gic: interrupt-controller@10211000 {
 			compatible = "arm,cortex-a7-gic";
 			interrupt-controller;
 			#interrupt-cells = <3>;
+			arm,irq-domain-hierarchy;
+			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 25c9f69..b2a17ba 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>;
@@ -75,15 +75,25 @@
 		timer: timer@10008000 {
 			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>;
+			arm,irq-domain-hierarchy;
+			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 90a56ad..bc2e520 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 {
@@ -97,15 +97,25 @@
 		timer: timer@10008000 {
 			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>;
+			arm,irq-domain-hierarchy;
+			interrupt-parent = <&gic>;
 			reg = <0 0x10211000 0 0x1000>,
 			      <0 0x10212000 0 0x1000>,
 			      <0 0x10214000 0 0x2000>,
-- 
1.8.1.1.dirty

[PATCH v3 5/7] ARM: mediatek: Add sysirq in mt6589/mt8135/mt8127 dtsi

[Joe.C]

From: "Joe.C" <yingjoe.chen@mediatek.com>

Add sysirq settings for mt6589/mt8135/mt8127
This also correct timer interrupt flag. The old flag 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: Joe.C <yingjoe.chen@mediatek.com>
---
 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..9f3e6b9 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 at 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 at 10200100 {
+			compatible = "mediatek,mt6589-sysirq", "mediatek,mt6577-sysirq";
+			interrupt-controller;
+			#interrupt-cells = <3>;
+			interrupt-parent = <&gic>;
+			reg = <0 0x10200100 0 0x1c>;
+		};
+
 		gic: interrupt-controller at 10211000 {
 			compatible = "arm,cortex-a7-gic";
 			interrupt-controller;
 			#interrupt-cells = <3>;
+			arm,irq-domain-hierarchy;
+			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 25c9f69..b2a17ba 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>;
@@ -75,15 +75,25 @@
 		timer: timer at 10008000 {
 			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 at 10200100 {
+			compatible = "mediatek,mt8127-sysirq", "mediatek,mt6577-sysirq";
+			interrupt-controller;
+			#interrupt-cells = <3>;
+			interrupt-parent = <&gic>;
+			reg = <0 0x10200100 0 0x1c>;
+		};
+
 		gic: interrupt-controller at 10211000 {
 			compatible = "arm,cortex-a7-gic";
 			interrupt-controller;
 			#interrupt-cells = <3>;
+			arm,irq-domain-hierarchy;
+			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 90a56ad..bc2e520 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 {
@@ -97,15 +97,25 @@
 		timer: timer at 10008000 {
 			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 at 10200030 {
+			compatible = "mediatek,mt8135-sysirq", "mediatek,mt6577-sysirq";
+			interrupt-controller;
+			#interrupt-cells = <3>;
+			interrupt-parent = <&gic>;
+			reg = <0 0x10200030 0 0x1c>;
+		};
+
 		gic: interrupt-controller at 10211000 {
 			compatible = "arm,cortex-a15-gic";
 			interrupt-controller;
 			#interrupt-cells = <3>;
+			arm,irq-domain-hierarchy;
+			interrupt-parent = <&gic>;
 			reg = <0 0x10211000 0 0x1000>,
 			      <0 0x10212000 0 0x1000>,
 			      <0 0x10214000 0 0x2000>,
-- 
1.8.1.1.dirty

[PATCH v3 6/7] dt-bindings: add irq domain parent binding

[Joe.C]

From: "Joe.C" <yingjoe.chen@mediatek.com>

Add "arm,irq-domain-hierarchy" optional property.

Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
---
 Documentation/devicetree/bindings/arm/gic.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
index c7d2fa1..f0bb759 100644
--- a/Documentation/devicetree/bindings/arm/gic.txt
+++ b/Documentation/devicetree/bindings/arm/gic.txt
@@ -68,6 +68,8 @@ Example:
 		      <0xfff10100 0x100>;
 	};
 
+- arm,irq-domain-hierarchy : When this property exists, gic will be used as
+  parent for hierarchy irq domain.
 
 * GIC virtualization extensions (VGIC)
 
-- 
1.8.1.1.dirty

[PATCH v3 6/7] dt-bindings: add irq domain parent binding

[Joe.C]

From: "Joe.C" <yingjoe.chen@mediatek.com>

Add "arm,irq-domain-hierarchy" optional property.

Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
---
 Documentation/devicetree/bindings/arm/gic.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
index c7d2fa1..f0bb759 100644
--- a/Documentation/devicetree/bindings/arm/gic.txt
+++ b/Documentation/devicetree/bindings/arm/gic.txt
@@ -68,6 +68,8 @@ Example:
 		      <0xfff10100 0x100>;
 	};
 
+- arm,irq-domain-hierarchy : When this property exists, gic will be used as
+  parent for hierarchy irq domain.
 
 * GIC virtualization extensions (VGIC)
 
-- 
1.8.1.1.dirty

[PATCH v3 7/7] dt-bindings: add bindings for mediatek sysirq

[Joe.C]

From: "Joe.C" <yingjoe.chen@mediatek.com>

Add binding documentation for Mediatek SoC SYSIRQ.

Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
---
 .../bindings/arm/mediatek/mediatek,sysirq.txt         | 19 +++++++++++++++++++
 1 file changed, 19 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..63d8fa1
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,sysirq.txt
@@ -0,0 +1,19 @@
+Mediatek 65xx/81xx sysirq
+
+Mediatek SOCs sysirq support controllable irq inverter for each GIC SPI
+interrupt.
+
+Required properties:
+- compatible: Compatible property value should be "mediatek,mt6577-sysirq"
+- interrupt-parent: phandle of irq domain parent for sysirq.
+- 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

[PATCH v3 7/7] dt-bindings: add bindings for mediatek sysirq

[Joe.C]

From: "Joe.C" <yingjoe.chen@mediatek.com>

Add binding documentation for Mediatek SoC SYSIRQ.

Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
---
 .../bindings/arm/mediatek/mediatek,sysirq.txt         | 19 +++++++++++++++++++
 1 file changed, 19 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..63d8fa1
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,sysirq.txt
@@ -0,0 +1,19 @@
+Mediatek 65xx/81xx sysirq
+
+Mediatek SOCs sysirq support controllable irq inverter for each GIC SPI
+interrupt.
+
+Required properties:
+- compatible: Compatible property value should be "mediatek,mt6577-sysirq"
+- interrupt-parent: phandle of irq domain parent for sysirq.
+- reg: Physical base address of the intpol registers and length of memory
+  mapped region.
+
+Example:
+	sysirq: interrupt-controller at 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

Re: [PATCH v3 4/7] ARM: mediatek: Add sysirq interrupt polarity support

[Arnd Bergmann]

On Thursday 09 October 2014 22:29:37 Joe. C wrote:

> +static int __init mtk_sysirq_of_init(struct device_node *node,
> +				   struct device_node *parent)
> +{
> +	struct device_node *parent_node;
> +	struct irq_domain *domain, *domain_parent = NULL;
> +	struct mt_sysirq_chip_data *chip_data;
> +	int ret = 0;
> +
> +	parent_node = of_irq_find_parent(node);
> +	if (parent_node) {
> +		domain_parent = irq_find_host(parent_node);
> +		of_node_put(parent_node);
> +	}

Just a small comment: I think the 'parent' argument to the function
already contains the device node you are looking up here, so
no need for the second variable or _find_parent call.

This means you can also drop the  = NULL assignment for the
domain.

	Arnd

Re: [PATCH v3 4/7] ARM: mediatek: Add sysirq interrupt polarity support

[Arnd Bergmann]

On Thursday 09 October 2014 22:29:37 Joe. C wrote:

> +static int __init mtk_sysirq_of_init(struct device_node *node,
> +				   struct device_node *parent)
> +{
> +	struct device_node *parent_node;
> +	struct irq_domain *domain, *domain_parent = NULL;
> +	struct mt_sysirq_chip_data *chip_data;
> +	int ret = 0;
> +
> +	parent_node = of_irq_find_parent(node);
> +	if (parent_node) {
> +		domain_parent = irq_find_host(parent_node);
> +		of_node_put(parent_node);
> +	}

Just a small comment: I think the 'parent' argument to the function
already contains the device node you are looking up here, so
no need for the second variable or _find_parent call.

This means you can also drop the  = NULL assignment for the
domain.

	Arnd

[PATCH v3 4/7] ARM: mediatek: Add sysirq interrupt polarity support

[Arnd Bergmann]

On Thursday 09 October 2014 22:29:37 Joe. C wrote:

> +static int __init mtk_sysirq_of_init(struct device_node *node,
> +				   struct device_node *parent)
> +{
> +	struct device_node *parent_node;
> +	struct irq_domain *domain, *domain_parent = NULL;
> +	struct mt_sysirq_chip_data *chip_data;
> +	int ret = 0;
> +
> +	parent_node = of_irq_find_parent(node);
> +	if (parent_node) {
> +		domain_parent = irq_find_host(parent_node);
> +		of_node_put(parent_node);
> +	}

Just a small comment: I think the 'parent' argument to the function
already contains the device node you are looking up here, so
no need for the second variable or _find_parent call.

This means you can also drop the  = NULL assignment for the
domain.

	Arnd

Re: [PATCH v3 4/7] ARM: mediatek: Add sysirq interrupt polarity support

[Joe.C]

On Thu, 2014-10-09 at 16:37 +0200, Arnd Bergmann wrote:
> On Thursday 09 October 2014 22:29:37 Joe. C wrote:
> 
> > +static int __init mtk_sysirq_of_init(struct device_node *node,
> > +				   struct device_node *parent)
> > +{
> > +	struct device_node *parent_node;
> > +	struct irq_domain *domain, *domain_parent = NULL;
> > +	struct mt_sysirq_chip_data *chip_data;
> > +	int ret = 0;
> > +
> > +	parent_node = of_irq_find_parent(node);
> > +	if (parent_node) {
> > +		domain_parent = irq_find_host(parent_node);
> > +		of_node_put(parent_node);
> > +	}
> 
> Just a small comment: I think the 'parent' argument to the function
> already contains the device node you are looking up here, so
> no need for the second variable or _find_parent call.
> 
> This means you can also drop the  = NULL assignment for the
> domain.
> 
> 	Arnd

Thanks for point this out. I'll fix this in the next version.

Joe.C

[PATCH v3 4/7] ARM: mediatek: Add sysirq interrupt polarity support

[Joe.C]

On Thu, 2014-10-09 at 16:37 +0200, Arnd Bergmann wrote:
> On Thursday 09 October 2014 22:29:37 Joe. C wrote:
> 
> > +static int __init mtk_sysirq_of_init(struct device_node *node,
> > +				   struct device_node *parent)
> > +{
> > +	struct device_node *parent_node;
> > +	struct irq_domain *domain, *domain_parent = NULL;
> > +	struct mt_sysirq_chip_data *chip_data;
> > +	int ret = 0;
> > +
> > +	parent_node = of_irq_find_parent(node);
> > +	if (parent_node) {
> > +		domain_parent = irq_find_host(parent_node);
> > +		of_node_put(parent_node);
> > +	}
> 
> Just a small comment: I think the 'parent' argument to the function
> already contains the device node you are looking up here, so
> no need for the second variable or _find_parent call.
> 
> This means you can also drop the  = NULL assignment for the
> domain.
> 
> 	Arnd

Thanks for point this out. I'll fix this in the next version.

Joe.C

Re: [PATCH v3 6/7] dt-bindings: add irq domain parent binding

[Mark Rutland]

Hi,

On Thu, Oct 09, 2014 at 03:29:39PM +0100, Joe.C wrote:
> From: "Joe.C" <yingjoe.chen@mediatek.com>
> 
> Add "arm,irq-domain-hierarchy" optional property.
> 
> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
> ---
>  Documentation/devicetree/bindings/arm/gic.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> index c7d2fa1..f0bb759 100644
> --- a/Documentation/devicetree/bindings/arm/gic.txt
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -68,6 +68,8 @@ Example:
>  		      <0xfff10100 0x100>;
>  	};
>  
> +- arm,irq-domain-hierarchy : When this property exists, gic will be used as
> +  parent for hierarchy irq domain.

IRQ domains are a Linux internal detail, and their description doesn't
belong in the DT. So this property does not look like it should be
described in the DT.

I don't see a description of any relationship between devices here, so
it's not clear to me what this property is intended to imply.

Why do you think you need this?

Thanks,
Mark.

Re: [PATCH v3 6/7] dt-bindings: add irq domain parent binding

[Mark Rutland]

Hi,

On Thu, Oct 09, 2014 at 03:29:39PM +0100, Joe.C wrote:
> From: "Joe.C" <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> 
> Add "arm,irq-domain-hierarchy" optional property.
> 
> Signed-off-by: Joe.C <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/arm/gic.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> index c7d2fa1..f0bb759 100644
> --- a/Documentation/devicetree/bindings/arm/gic.txt
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -68,6 +68,8 @@ Example:
>  		      <0xfff10100 0x100>;
>  	};
>  
> +- arm,irq-domain-hierarchy : When this property exists, gic will be used as
> +  parent for hierarchy irq domain.

IRQ domains are a Linux internal detail, and their description doesn't
belong in the DT. So this property does not look like it should be
described in the DT.

I don't see a description of any relationship between devices here, so
it's not clear to me what this property is intended to imply.

Why do you think you need this?

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 6/7] dt-bindings: add irq domain parent binding

[Mark Rutland]

Hi,

On Thu, Oct 09, 2014 at 03:29:39PM +0100, Joe.C wrote:
> From: "Joe.C" <yingjoe.chen@mediatek.com>
> 
> Add "arm,irq-domain-hierarchy" optional property.
> 
> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
> ---
>  Documentation/devicetree/bindings/arm/gic.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/gic.txt b/Documentation/devicetree/bindings/arm/gic.txt
> index c7d2fa1..f0bb759 100644
> --- a/Documentation/devicetree/bindings/arm/gic.txt
> +++ b/Documentation/devicetree/bindings/arm/gic.txt
> @@ -68,6 +68,8 @@ Example:
>  		      <0xfff10100 0x100>;
>  	};
>  
> +- arm,irq-domain-hierarchy : When this property exists, gic will be used as
> +  parent for hierarchy irq domain.

IRQ domains are a Linux internal detail, and their description doesn't
belong in the DT. So this property does not look like it should be
described in the DT.

I don't see a description of any relationship between devices here, so
it's not clear to me what this property is intended to imply.

Why do you think you need this?

Thanks,
Mark.

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Marc Zyngier]

On 09/10/14 15:29, Joe.C wrote:
> From: "Joe.C" <yingjoe.chen@mediatek.com>
> 
> Add support to use gic as a parent for stacked irq domain.
> 
> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
> ---
>  drivers/irqchip/irq-gic.c | 56 ++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 48 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index dda6dbc..17f5aa6 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -767,19 +767,17 @@ void __init gic_init_physaddr(struct device_node *node)
>  static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
>  				irq_hw_number_t hw)
>  {
> +	irq_domain_set_hwirq_and_chip(d, irq, hw, &gic_chip, d->host_data);
>  	if (hw < 32) {
>  		irq_set_percpu_devid(irq);
> -		irq_set_chip_and_handler(irq, &gic_chip,
> -					 handle_percpu_devid_irq);
> +		irq_set_handler(irq, handle_percpu_devid_irq);
>  		set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
>  	} else {
> -		irq_set_chip_and_handler(irq, &gic_chip,
> -					 handle_fasteoi_irq);
> +		irq_set_handler(irq, handle_fasteoi_irq);
>  		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;
>  }
>  
> @@ -795,8 +793,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
>  {
>  	unsigned long ret = 0;
>  
> -	if (d->of_node != controller)
> -		return -EINVAL;
>  	if (intsize < 3)
>  		return -EINVAL;
>  
> @@ -839,6 +835,46 @@ static struct notifier_block gic_cpu_notifier = {
>  };
>  #endif
>  
> +
> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
> +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 void gic_irq_domain_free(struct irq_domain *domain, unsigned int virq,
> +				unsigned int nr_irqs)
> +{
> +	int i;
> +
> +	for (i = 0; i < nr_irqs; i++) {
> +		irq_set_handler(virq + i, NULL);
> +		irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
> +	}
> +}
> +
> +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = {
> +	.alloc = gic_irq_domain_alloc,
> +	.free = gic_irq_domain_free,
> +};
> +#else
> +#define gic_irq_domain_hierarchy_ops 0
> +#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
> +
>  static const struct irq_domain_ops gic_irq_domain_ops = {
>  	.map = gic_irq_domain_map,
>  	.unmap = gic_irq_domain_unmap,
> @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>  
>  	gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>  
> -	if (of_property_read_u32(node, "arm,routable-irqs",
> +	if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
> +		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
> +		gic->domain = irq_domain_add_linear(node, gic_irqs,
> +					&gic_irq_domain_hierarchy_ops, gic);

I really think that looking for a property is the wrong thing to do. If
"node" is non-NULL, then we're pretty sure that we're initializing from
DT, and that a pure linear domain should be the right thing, leaving the
legacy stuff for the few non-DT platforms that are still around.

Thanks,

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

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Marc Zyngier]

On 09/10/14 15:29, Joe.C wrote:
> From: "Joe.C" <yingjoe.chen@mediatek.com>
> 
> Add support to use gic as a parent for stacked irq domain.
> 
> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
> ---
>  drivers/irqchip/irq-gic.c | 56 ++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 48 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index dda6dbc..17f5aa6 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -767,19 +767,17 @@ void __init gic_init_physaddr(struct device_node *node)
>  static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
>  				irq_hw_number_t hw)
>  {
> +	irq_domain_set_hwirq_and_chip(d, irq, hw, &gic_chip, d->host_data);
>  	if (hw < 32) {
>  		irq_set_percpu_devid(irq);
> -		irq_set_chip_and_handler(irq, &gic_chip,
> -					 handle_percpu_devid_irq);
> +		irq_set_handler(irq, handle_percpu_devid_irq);
>  		set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
>  	} else {
> -		irq_set_chip_and_handler(irq, &gic_chip,
> -					 handle_fasteoi_irq);
> +		irq_set_handler(irq, handle_fasteoi_irq);
>  		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;
>  }
>  
> @@ -795,8 +793,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
>  {
>  	unsigned long ret = 0;
>  
> -	if (d->of_node != controller)
> -		return -EINVAL;
>  	if (intsize < 3)
>  		return -EINVAL;
>  
> @@ -839,6 +835,46 @@ static struct notifier_block gic_cpu_notifier = {
>  };
>  #endif
>  
> +
> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
> +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 void gic_irq_domain_free(struct irq_domain *domain, unsigned int virq,
> +				unsigned int nr_irqs)
> +{
> +	int i;
> +
> +	for (i = 0; i < nr_irqs; i++) {
> +		irq_set_handler(virq + i, NULL);
> +		irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
> +	}
> +}
> +
> +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = {
> +	.alloc = gic_irq_domain_alloc,
> +	.free = gic_irq_domain_free,
> +};
> +#else
> +#define gic_irq_domain_hierarchy_ops 0
> +#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
> +
>  static const struct irq_domain_ops gic_irq_domain_ops = {
>  	.map = gic_irq_domain_map,
>  	.unmap = gic_irq_domain_unmap,
> @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>  
>  	gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>  
> -	if (of_property_read_u32(node, "arm,routable-irqs",
> +	if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
> +		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
> +		gic->domain = irq_domain_add_linear(node, gic_irqs,
> +					&gic_irq_domain_hierarchy_ops, gic);

I really think that looking for a property is the wrong thing to do. If
"node" is non-NULL, then we're pretty sure that we're initializing from
DT, and that a pure linear domain should be the right thing, leaving the
legacy stuff for the few non-DT platforms that are still around.

Thanks,

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

[PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Marc Zyngier]

On 09/10/14 15:29, Joe.C wrote:
> From: "Joe.C" <yingjoe.chen@mediatek.com>
> 
> Add support to use gic as a parent for stacked irq domain.
> 
> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
> ---
>  drivers/irqchip/irq-gic.c | 56 ++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 48 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index dda6dbc..17f5aa6 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -767,19 +767,17 @@ void __init gic_init_physaddr(struct device_node *node)
>  static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
>  				irq_hw_number_t hw)
>  {
> +	irq_domain_set_hwirq_and_chip(d, irq, hw, &gic_chip, d->host_data);
>  	if (hw < 32) {
>  		irq_set_percpu_devid(irq);
> -		irq_set_chip_and_handler(irq, &gic_chip,
> -					 handle_percpu_devid_irq);
> +		irq_set_handler(irq, handle_percpu_devid_irq);
>  		set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
>  	} else {
> -		irq_set_chip_and_handler(irq, &gic_chip,
> -					 handle_fasteoi_irq);
> +		irq_set_handler(irq, handle_fasteoi_irq);
>  		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;
>  }
>  
> @@ -795,8 +793,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
>  {
>  	unsigned long ret = 0;
>  
> -	if (d->of_node != controller)
> -		return -EINVAL;
>  	if (intsize < 3)
>  		return -EINVAL;
>  
> @@ -839,6 +835,46 @@ static struct notifier_block gic_cpu_notifier = {
>  };
>  #endif
>  
> +
> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
> +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 void gic_irq_domain_free(struct irq_domain *domain, unsigned int virq,
> +				unsigned int nr_irqs)
> +{
> +	int i;
> +
> +	for (i = 0; i < nr_irqs; i++) {
> +		irq_set_handler(virq + i, NULL);
> +		irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
> +	}
> +}
> +
> +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = {
> +	.alloc = gic_irq_domain_alloc,
> +	.free = gic_irq_domain_free,
> +};
> +#else
> +#define gic_irq_domain_hierarchy_ops 0
> +#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
> +
>  static const struct irq_domain_ops gic_irq_domain_ops = {
>  	.map = gic_irq_domain_map,
>  	.unmap = gic_irq_domain_unmap,
> @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>  
>  	gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>  
> -	if (of_property_read_u32(node, "arm,routable-irqs",
> +	if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
> +		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
> +		gic->domain = irq_domain_add_linear(node, gic_irqs,
> +					&gic_irq_domain_hierarchy_ops, gic);

I really think that looking for a property is the wrong thing to do. If
"node" is non-NULL, then we're pretty sure that we're initializing from
DT, and that a pure linear domain should be the right thing, leaving the
legacy stuff for the few non-DT platforms that are still around.

Thanks,

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

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Arnd Bergmann]

On Thursday 09 October 2014 17:59:56 Marc Zyngier wrote:
> > @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> >  
> >       gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
> >  
> > -     if (of_property_read_u32(node, "arm,routable-irqs",
> > +     if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
> > +             of_find_property(node, "arm,irq-domain-hierarchy", NULL))
> > +             gic->domain = irq_domain_add_linear(node, gic_irqs,
> > +                                     &gic_irq_domain_hierarchy_ops, gic);
> 
> I really think that looking for a property is the wrong thing to do. If
> "node" is non-NULL, then we're pretty sure that we're initializing from
> DT, and that a pure linear domain should be the right thing, leaving the
> legacy stuff for the few non-DT platforms that are still around.

FWIW, these are the remaining ones that use gic:

arch/arm/mach-cns3xxx/core.c:   gic_init(0, 29, IOMEM(CNS3XXX_TC11MP_GIC_DIST_BASE_VIRT),
arch/arm/mach-realview/realview_eb.c:           gic_init(0, 29, __io_address(REALVIEW_EB11MP_GIC_DIST_BASE),
arch/arm/mach-realview/realview_eb.c:           gic_init(1, 96, __io_address(REALVIEW_EB_GIC_DIST_BASE),
arch/arm/mach-realview/realview_eb.c:           gic_init(0, 29, __io_address(REALVIEW_EB_GIC_DIST_BASE),
arch/arm/mach-realview/realview_pb1176.c:       gic_init(0, IRQ_DC1176_GIC_START,
arch/arm/mach-realview/realview_pb1176.c:       gic_init(1, IRQ_PB1176_GIC_START,
arch/arm/mach-realview/realview_pb11mp.c:       gic_init(0, 29, __io_address(REALVIEW_TC11MP_GIC_DIST_BASE),
arch/arm/mach-realview/realview_pb11mp.c:       gic_init(1, IRQ_PB11MP_GIC_START,
arch/arm/mach-realview/realview_pba8.c: gic_init(0, IRQ_PBA8_GIC_START,
arch/arm/mach-realview/realview_pbx.c:          gic_init(0, 29, __io_address(REALVIEW_PBX_TILE_GIC_DIST_BASE),
arch/arm/mach-realview/realview_pbx.c:          gic_init(0, IRQ_PBX_GIC_START,
arch/arm/mach-shmobile/intc-sh73a0.c:   gic_init(0, 29, gic_dist_base, gic_cpu_base);
arch/arm/mach-tegra/irq.c:              gic_init(0, 29, distbase,
arch/arm/mach-vexpress/ct-ca9x4.c:      gic_init(0, 29, ioremap(A9_MPCORE_GIC_DIST, SZ_4K),

Linus Walleij is working on realview, Pawel already has a patch for vexpress, the tegra one
can just be removed and the Renesas team will deal with shmobile.

This basically leaves cns3xxx, which now has a new maintainer but I suspect we won't
have DT-only support for it too soon anyway.

	Arnd

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Arnd Bergmann]

On Thursday 09 October 2014 17:59:56 Marc Zyngier wrote:
> > @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> >  
> >       gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
> >  
> > -     if (of_property_read_u32(node, "arm,routable-irqs",
> > +     if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
> > +             of_find_property(node, "arm,irq-domain-hierarchy", NULL))
> > +             gic->domain = irq_domain_add_linear(node, gic_irqs,
> > +                                     &gic_irq_domain_hierarchy_ops, gic);
> 
> I really think that looking for a property is the wrong thing to do. If
> "node" is non-NULL, then we're pretty sure that we're initializing from
> DT, and that a pure linear domain should be the right thing, leaving the
> legacy stuff for the few non-DT platforms that are still around.

FWIW, these are the remaining ones that use gic:

arch/arm/mach-cns3xxx/core.c:   gic_init(0, 29, IOMEM(CNS3XXX_TC11MP_GIC_DIST_BASE_VIRT),
arch/arm/mach-realview/realview_eb.c:           gic_init(0, 29, __io_address(REALVIEW_EB11MP_GIC_DIST_BASE),
arch/arm/mach-realview/realview_eb.c:           gic_init(1, 96, __io_address(REALVIEW_EB_GIC_DIST_BASE),
arch/arm/mach-realview/realview_eb.c:           gic_init(0, 29, __io_address(REALVIEW_EB_GIC_DIST_BASE),
arch/arm/mach-realview/realview_pb1176.c:       gic_init(0, IRQ_DC1176_GIC_START,
arch/arm/mach-realview/realview_pb1176.c:       gic_init(1, IRQ_PB1176_GIC_START,
arch/arm/mach-realview/realview_pb11mp.c:       gic_init(0, 29, __io_address(REALVIEW_TC11MP_GIC_DIST_BASE),
arch/arm/mach-realview/realview_pb11mp.c:       gic_init(1, IRQ_PB11MP_GIC_START,
arch/arm/mach-realview/realview_pba8.c: gic_init(0, IRQ_PBA8_GIC_START,
arch/arm/mach-realview/realview_pbx.c:          gic_init(0, 29, __io_address(REALVIEW_PBX_TILE_GIC_DIST_BASE),
arch/arm/mach-realview/realview_pbx.c:          gic_init(0, IRQ_PBX_GIC_START,
arch/arm/mach-shmobile/intc-sh73a0.c:   gic_init(0, 29, gic_dist_base, gic_cpu_base);
arch/arm/mach-tegra/irq.c:              gic_init(0, 29, distbase,
arch/arm/mach-vexpress/ct-ca9x4.c:      gic_init(0, 29, ioremap(A9_MPCORE_GIC_DIST, SZ_4K),

Linus Walleij is working on realview, Pawel already has a patch for vexpress, the tegra one
can just be removed and the Renesas team will deal with shmobile.

This basically leaves cns3xxx, which now has a new maintainer but I suspect we won't
have DT-only support for it too soon anyway.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Arnd Bergmann]

On Thursday 09 October 2014 17:59:56 Marc Zyngier wrote:
> > @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> >  
> >       gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
> >  
> > -     if (of_property_read_u32(node, "arm,routable-irqs",
> > +     if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
> > +             of_find_property(node, "arm,irq-domain-hierarchy", NULL))
> > +             gic->domain = irq_domain_add_linear(node, gic_irqs,
> > +                                     &gic_irq_domain_hierarchy_ops, gic);
> 
> I really think that looking for a property is the wrong thing to do. If
> "node" is non-NULL, then we're pretty sure that we're initializing from
> DT, and that a pure linear domain should be the right thing, leaving the
> legacy stuff for the few non-DT platforms that are still around.

FWIW, these are the remaining ones that use gic:

arch/arm/mach-cns3xxx/core.c:   gic_init(0, 29, IOMEM(CNS3XXX_TC11MP_GIC_DIST_BASE_VIRT),
arch/arm/mach-realview/realview_eb.c:           gic_init(0, 29, __io_address(REALVIEW_EB11MP_GIC_DIST_BASE),
arch/arm/mach-realview/realview_eb.c:           gic_init(1, 96, __io_address(REALVIEW_EB_GIC_DIST_BASE),
arch/arm/mach-realview/realview_eb.c:           gic_init(0, 29, __io_address(REALVIEW_EB_GIC_DIST_BASE),
arch/arm/mach-realview/realview_pb1176.c:       gic_init(0, IRQ_DC1176_GIC_START,
arch/arm/mach-realview/realview_pb1176.c:       gic_init(1, IRQ_PB1176_GIC_START,
arch/arm/mach-realview/realview_pb11mp.c:       gic_init(0, 29, __io_address(REALVIEW_TC11MP_GIC_DIST_BASE),
arch/arm/mach-realview/realview_pb11mp.c:       gic_init(1, IRQ_PB11MP_GIC_START,
arch/arm/mach-realview/realview_pba8.c: gic_init(0, IRQ_PBA8_GIC_START,
arch/arm/mach-realview/realview_pbx.c:          gic_init(0, 29, __io_address(REALVIEW_PBX_TILE_GIC_DIST_BASE),
arch/arm/mach-realview/realview_pbx.c:          gic_init(0, IRQ_PBX_GIC_START,
arch/arm/mach-shmobile/intc-sh73a0.c:   gic_init(0, 29, gic_dist_base, gic_cpu_base);
arch/arm/mach-tegra/irq.c:              gic_init(0, 29, distbase,
arch/arm/mach-vexpress/ct-ca9x4.c:      gic_init(0, 29, ioremap(A9_MPCORE_GIC_DIST, SZ_4K),

Linus Walleij is working on realview, Pawel already has a patch for vexpress, the tegra one
can just be removed and the Renesas team will deal with shmobile.

This basically leaves cns3xxx, which now has a new maintainer but I suspect we won't
have DT-only support for it too soon anyway.

	Arnd

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Marc Zyngier]

On 09/10/14 15:29, Joe.C wrote:
> From: "Joe.C" <yingjoe.chen@mediatek.com>
> 
> Add support to use gic as a parent for stacked irq domain.
> 
> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
> ---
>  drivers/irqchip/irq-gic.c | 56 ++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 48 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index dda6dbc..17f5aa6 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -767,19 +767,17 @@ void __init gic_init_physaddr(struct device_node *node)
>  static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
>  				irq_hw_number_t hw)
>  {
> +	irq_domain_set_hwirq_and_chip(d, irq, hw, &gic_chip, d->host_data);
>  	if (hw < 32) {
>  		irq_set_percpu_devid(irq);
> -		irq_set_chip_and_handler(irq, &gic_chip,
> -					 handle_percpu_devid_irq);
> +		irq_set_handler(irq, handle_percpu_devid_irq);
>  		set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
>  	} else {
> -		irq_set_chip_and_handler(irq, &gic_chip,
> -					 handle_fasteoi_irq);
> +		irq_set_handler(irq, handle_fasteoi_irq);
>  		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;
>  }
>  
> @@ -795,8 +793,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
>  {
>  	unsigned long ret = 0;
>  
> -	if (d->of_node != controller)
> -		return -EINVAL;
>  	if (intsize < 3)
>  		return -EINVAL;
>  
> @@ -839,6 +835,46 @@ static struct notifier_block gic_cpu_notifier = {
>  };
>  #endif
>  
> +
> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
> +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 void gic_irq_domain_free(struct irq_domain *domain, unsigned int virq,
> +				unsigned int nr_irqs)
> +{
> +	int i;
> +
> +	for (i = 0; i < nr_irqs; i++) {
> +		irq_set_handler(virq + i, NULL);
> +		irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
> +	}
> +}
> +
> +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = {
> +	.alloc = gic_irq_domain_alloc,
> +	.free = gic_irq_domain_free,
> +};
> +#else
> +#define gic_irq_domain_hierarchy_ops 0
> +#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
> +
>  static const struct irq_domain_ops gic_irq_domain_ops = {
>  	.map = gic_irq_domain_map,
>  	.unmap = gic_irq_domain_unmap,
> @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>  
>  	gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>  
> -	if (of_property_read_u32(node, "arm,routable-irqs",
> +	if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
> +		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
> +		gic->domain = irq_domain_add_linear(node, gic_irqs,
> +					&gic_irq_domain_hierarchy_ops, gic);
> +	else 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());
> 

So I've been playing with this over the weekend (with quite a few
tweaks), and I'm hitting a not-so-nice effect of the automatic platform
device creation from the device tree.

What happens is the following:
- Kernel starts
- GIC gets initialized with a linear domain supporting hierarchy
- per-cpu timers are up and running
- platform devices get created from the device tree:
  - irq_of_parse_and_map()
  - irq_create_of_mapping()
  - irq_domain_alloc_irqs()

Here, we start re-allocating interrupts that have already been allocated
(the timer interrupts). This has a side effect of nuking the
percpu_dev_id, and everything explodes on the next timer tick. Grmbl.

The main issue here is that we have a single path that:
- translates the interrupt from DT to HW
- configures the interrupts
and that we use it more than once.

The non-hierarchy path works because the the "map" operation takes place
only once, and virtual interrupts are allocated upfront.

When we switch to this more dynamic way of doing things, the fact that
the virqs only available at allocation time is screwing us up.

I can see a way out of this, which would involve having a way of
detecting that a hwirq has already been allocated (which requires having
the xlate callback instantiated). Something like this (not even
compile-tested):

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index dd8d3ab..6a45821 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -479,6 +479,21 @@ unsigned int irq_create_of_mapping(struct
of_phandle_args *irq_data)
 	}

 	if (irq_domain_is_hierarchy(domain)) {
+		if (domain->ops->xlate) {
+			/*
+			 * If we've already configured this interrupt,
+			 * don't do it again, or hell will break loose.
+			 */
+			if (domain->ops->xlate(domain, irq_data->np,
+					       irq_data->args,
+					       irq_data->args_count,
+					       &hwirq, &type))
+				return 0;
+
+			virq = irq_find_mapping(domain, hwirq);
+			if (virq)
+				return virq;
+		}
 		virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, irq_data);
 		return virq <= 0 ? 0 : virq;
 	}

Thoughts?

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

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Marc Zyngier]

On 09/10/14 15:29, Joe.C wrote:
> From: "Joe.C" <yingjoe.chen@mediatek.com>
> 
> Add support to use gic as a parent for stacked irq domain.
> 
> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
> ---
>  drivers/irqchip/irq-gic.c | 56 ++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 48 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index dda6dbc..17f5aa6 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -767,19 +767,17 @@ void __init gic_init_physaddr(struct device_node *node)
>  static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
>  				irq_hw_number_t hw)
>  {
> +	irq_domain_set_hwirq_and_chip(d, irq, hw, &gic_chip, d->host_data);
>  	if (hw < 32) {
>  		irq_set_percpu_devid(irq);
> -		irq_set_chip_and_handler(irq, &gic_chip,
> -					 handle_percpu_devid_irq);
> +		irq_set_handler(irq, handle_percpu_devid_irq);
>  		set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
>  	} else {
> -		irq_set_chip_and_handler(irq, &gic_chip,
> -					 handle_fasteoi_irq);
> +		irq_set_handler(irq, handle_fasteoi_irq);
>  		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;
>  }
>  
> @@ -795,8 +793,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
>  {
>  	unsigned long ret = 0;
>  
> -	if (d->of_node != controller)
> -		return -EINVAL;
>  	if (intsize < 3)
>  		return -EINVAL;
>  
> @@ -839,6 +835,46 @@ static struct notifier_block gic_cpu_notifier = {
>  };
>  #endif
>  
> +
> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
> +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 void gic_irq_domain_free(struct irq_domain *domain, unsigned int virq,
> +				unsigned int nr_irqs)
> +{
> +	int i;
> +
> +	for (i = 0; i < nr_irqs; i++) {
> +		irq_set_handler(virq + i, NULL);
> +		irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
> +	}
> +}
> +
> +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = {
> +	.alloc = gic_irq_domain_alloc,
> +	.free = gic_irq_domain_free,
> +};
> +#else
> +#define gic_irq_domain_hierarchy_ops 0
> +#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
> +
>  static const struct irq_domain_ops gic_irq_domain_ops = {
>  	.map = gic_irq_domain_map,
>  	.unmap = gic_irq_domain_unmap,
> @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>  
>  	gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>  
> -	if (of_property_read_u32(node, "arm,routable-irqs",
> +	if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
> +		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
> +		gic->domain = irq_domain_add_linear(node, gic_irqs,
> +					&gic_irq_domain_hierarchy_ops, gic);
> +	else 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());
> 

So I've been playing with this over the weekend (with quite a few
tweaks), and I'm hitting a not-so-nice effect of the automatic platform
device creation from the device tree.

What happens is the following:
- Kernel starts
- GIC gets initialized with a linear domain supporting hierarchy
- per-cpu timers are up and running
- platform devices get created from the device tree:
  - irq_of_parse_and_map()
  - irq_create_of_mapping()
  - irq_domain_alloc_irqs()

Here, we start re-allocating interrupts that have already been allocated
(the timer interrupts). This has a side effect of nuking the
percpu_dev_id, and everything explodes on the next timer tick. Grmbl.

The main issue here is that we have a single path that:
- translates the interrupt from DT to HW
- configures the interrupts
and that we use it more than once.

The non-hierarchy path works because the the "map" operation takes place
only once, and virtual interrupts are allocated upfront.

When we switch to this more dynamic way of doing things, the fact that
the virqs only available at allocation time is screwing us up.

I can see a way out of this, which would involve having a way of
detecting that a hwirq has already been allocated (which requires having
the xlate callback instantiated). Something like this (not even
compile-tested):

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index dd8d3ab..6a45821 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -479,6 +479,21 @@ unsigned int irq_create_of_mapping(struct
of_phandle_args *irq_data)
 	}

 	if (irq_domain_is_hierarchy(domain)) {
+		if (domain->ops->xlate) {
+			/*
+			 * If we've already configured this interrupt,
+			 * don't do it again, or hell will break loose.
+			 */
+			if (domain->ops->xlate(domain, irq_data->np,
+					       irq_data->args,
+					       irq_data->args_count,
+					       &hwirq, &type))
+				return 0;
+
+			virq = irq_find_mapping(domain, hwirq);
+			if (virq)
+				return virq;
+		}
 		virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, irq_data);
 		return virq <= 0 ? 0 : virq;
 	}

Thoughts?

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

[PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Marc Zyngier]

On 09/10/14 15:29, Joe.C wrote:
> From: "Joe.C" <yingjoe.chen@mediatek.com>
> 
> Add support to use gic as a parent for stacked irq domain.
> 
> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
> ---
>  drivers/irqchip/irq-gic.c | 56 ++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 48 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
> index dda6dbc..17f5aa6 100644
> --- a/drivers/irqchip/irq-gic.c
> +++ b/drivers/irqchip/irq-gic.c
> @@ -767,19 +767,17 @@ void __init gic_init_physaddr(struct device_node *node)
>  static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
>  				irq_hw_number_t hw)
>  {
> +	irq_domain_set_hwirq_and_chip(d, irq, hw, &gic_chip, d->host_data);
>  	if (hw < 32) {
>  		irq_set_percpu_devid(irq);
> -		irq_set_chip_and_handler(irq, &gic_chip,
> -					 handle_percpu_devid_irq);
> +		irq_set_handler(irq, handle_percpu_devid_irq);
>  		set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
>  	} else {
> -		irq_set_chip_and_handler(irq, &gic_chip,
> -					 handle_fasteoi_irq);
> +		irq_set_handler(irq, handle_fasteoi_irq);
>  		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;
>  }
>  
> @@ -795,8 +793,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
>  {
>  	unsigned long ret = 0;
>  
> -	if (d->of_node != controller)
> -		return -EINVAL;
>  	if (intsize < 3)
>  		return -EINVAL;
>  
> @@ -839,6 +835,46 @@ static struct notifier_block gic_cpu_notifier = {
>  };
>  #endif
>  
> +
> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
> +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 void gic_irq_domain_free(struct irq_domain *domain, unsigned int virq,
> +				unsigned int nr_irqs)
> +{
> +	int i;
> +
> +	for (i = 0; i < nr_irqs; i++) {
> +		irq_set_handler(virq + i, NULL);
> +		irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
> +	}
> +}
> +
> +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = {
> +	.alloc = gic_irq_domain_alloc,
> +	.free = gic_irq_domain_free,
> +};
> +#else
> +#define gic_irq_domain_hierarchy_ops 0
> +#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
> +
>  static const struct irq_domain_ops gic_irq_domain_ops = {
>  	.map = gic_irq_domain_map,
>  	.unmap = gic_irq_domain_unmap,
> @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>  
>  	gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>  
> -	if (of_property_read_u32(node, "arm,routable-irqs",
> +	if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
> +		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
> +		gic->domain = irq_domain_add_linear(node, gic_irqs,
> +					&gic_irq_domain_hierarchy_ops, gic);
> +	else 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());
> 

So I've been playing with this over the weekend (with quite a few
tweaks), and I'm hitting a not-so-nice effect of the automatic platform
device creation from the device tree.

What happens is the following:
- Kernel starts
- GIC gets initialized with a linear domain supporting hierarchy
- per-cpu timers are up and running
- platform devices get created from the device tree:
  - irq_of_parse_and_map()
  - irq_create_of_mapping()
  - irq_domain_alloc_irqs()

Here, we start re-allocating interrupts that have already been allocated
(the timer interrupts). This has a side effect of nuking the
percpu_dev_id, and everything explodes on the next timer tick. Grmbl.

The main issue here is that we have a single path that:
- translates the interrupt from DT to HW
- configures the interrupts
and that we use it more than once.

The non-hierarchy path works because the the "map" operation takes place
only once, and virtual interrupts are allocated upfront.

When we switch to this more dynamic way of doing things, the fact that
the virqs only available at allocation time is screwing us up.

I can see a way out of this, which would involve having a way of
detecting that a hwirq has already been allocated (which requires having
the xlate callback instantiated). Something like this (not even
compile-tested):

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index dd8d3ab..6a45821 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -479,6 +479,21 @@ unsigned int irq_create_of_mapping(struct
of_phandle_args *irq_data)
 	}

 	if (irq_domain_is_hierarchy(domain)) {
+		if (domain->ops->xlate) {
+			/*
+			 * If we've already configured this interrupt,
+			 * don't do it again, or hell will break loose.
+			 */
+			if (domain->ops->xlate(domain, irq_data->np,
+					       irq_data->args,
+					       irq_data->args_count,
+					       &hwirq, &type))
+				return 0;
+
+			virq = irq_find_mapping(domain, hwirq);
+			if (virq)
+				return virq;
+		}
 		virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, irq_data);
 		return virq <= 0 ? 0 : virq;
 	}

Thoughts?

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

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Marc Zyngier]

On 13/10/14 09:56, Marc Zyngier wrote:
> On 09/10/14 15:29, Joe.C wrote:
>> From: "Joe.C" <yingjoe.chen@mediatek.com>
>>
>> Add support to use gic as a parent for stacked irq domain.
>>
>> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
>> ---
>>  drivers/irqchip/irq-gic.c | 56 ++++++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 48 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>> index dda6dbc..17f5aa6 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -767,19 +767,17 @@ void __init gic_init_physaddr(struct device_node *node)
>>  static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
>>  				irq_hw_number_t hw)
>>  {
>> +	irq_domain_set_hwirq_and_chip(d, irq, hw, &gic_chip, d->host_data);
>>  	if (hw < 32) {
>>  		irq_set_percpu_devid(irq);
>> -		irq_set_chip_and_handler(irq, &gic_chip,
>> -					 handle_percpu_devid_irq);
>> +		irq_set_handler(irq, handle_percpu_devid_irq);
>>  		set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
>>  	} else {
>> -		irq_set_chip_and_handler(irq, &gic_chip,
>> -					 handle_fasteoi_irq);
>> +		irq_set_handler(irq, handle_fasteoi_irq);
>>  		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;
>>  }
>>  
>> @@ -795,8 +793,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
>>  {
>>  	unsigned long ret = 0;
>>  
>> -	if (d->of_node != controller)
>> -		return -EINVAL;
>>  	if (intsize < 3)
>>  		return -EINVAL;
>>  
>> @@ -839,6 +835,46 @@ static struct notifier_block gic_cpu_notifier = {
>>  };
>>  #endif
>>  
>> +
>> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
>> +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 void gic_irq_domain_free(struct irq_domain *domain, unsigned int virq,
>> +				unsigned int nr_irqs)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < nr_irqs; i++) {
>> +		irq_set_handler(virq + i, NULL);
>> +		irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
>> +	}
>> +}
>> +
>> +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = {
>> +	.alloc = gic_irq_domain_alloc,
>> +	.free = gic_irq_domain_free,
>> +};
>> +#else
>> +#define gic_irq_domain_hierarchy_ops 0
>> +#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
>> +
>>  static const struct irq_domain_ops gic_irq_domain_ops = {
>>  	.map = gic_irq_domain_map,
>>  	.unmap = gic_irq_domain_unmap,
>> @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>  
>>  	gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>>  
>> -	if (of_property_read_u32(node, "arm,routable-irqs",
>> +	if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
>> +		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
>> +		gic->domain = irq_domain_add_linear(node, gic_irqs,
>> +					&gic_irq_domain_hierarchy_ops, gic);
>> +	else 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());
>>
> 
> So I've been playing with this over the weekend (with quite a few
> tweaks), and I'm hitting a not-so-nice effect of the automatic platform
> device creation from the device tree.
> 
> What happens is the following:
> - Kernel starts
> - GIC gets initialized with a linear domain supporting hierarchy
> - per-cpu timers are up and running
> - platform devices get created from the device tree:
>   - irq_of_parse_and_map()
>   - irq_create_of_mapping()
>   - irq_domain_alloc_irqs()
> 
> Here, we start re-allocating interrupts that have already been allocated
> (the timer interrupts). This has a side effect of nuking the
> percpu_dev_id, and everything explodes on the next timer tick. Grmbl.
> 
> The main issue here is that we have a single path that:
> - translates the interrupt from DT to HW
> - configures the interrupts
> and that we use it more than once.
> 
> The non-hierarchy path works because the the "map" operation takes place
> only once, and virtual interrupts are allocated upfront.
> 
> When we switch to this more dynamic way of doing things, the fact that
> the virqs only available at allocation time is screwing us up.
> 
> I can see a way out of this, which would involve having a way of
> detecting that a hwirq has already been allocated (which requires having
> the xlate callback instantiated). Something like this (not even
> compile-tested):
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index dd8d3ab..6a45821 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -479,6 +479,21 @@ unsigned int irq_create_of_mapping(struct
> of_phandle_args *irq_data)
>  	}
> 
>  	if (irq_domain_is_hierarchy(domain)) {
> +		if (domain->ops->xlate) {
> +			/*
> +			 * If we've already configured this interrupt,
> +			 * don't do it again, or hell will break loose.
> +			 */
> +			if (domain->ops->xlate(domain, irq_data->np,
> +					       irq_data->args,
> +					       irq_data->args_count,
> +					       &hwirq, &type))
> +				return 0;
> +
> +			virq = irq_find_mapping(domain, hwirq);
> +			if (virq)
> +				return virq;
> +		}
>  		virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, irq_data);
>  		return virq <= 0 ? 0 : virq;
>  	}
> 
> Thoughts?

For the record, I just booted a pure DT system with the above patch, and
everything seems to be fine. Haven't completely convinced myself this is
the right fix though.

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

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Marc Zyngier]

On 13/10/14 09:56, Marc Zyngier wrote:
> On 09/10/14 15:29, Joe.C wrote:
>> From: "Joe.C" <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>>
>> Add support to use gic as a parent for stacked irq domain.
>>
>> Signed-off-by: Joe.C <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>> ---
>>  drivers/irqchip/irq-gic.c | 56 ++++++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 48 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>> index dda6dbc..17f5aa6 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -767,19 +767,17 @@ void __init gic_init_physaddr(struct device_node *node)
>>  static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
>>  				irq_hw_number_t hw)
>>  {
>> +	irq_domain_set_hwirq_and_chip(d, irq, hw, &gic_chip, d->host_data);
>>  	if (hw < 32) {
>>  		irq_set_percpu_devid(irq);
>> -		irq_set_chip_and_handler(irq, &gic_chip,
>> -					 handle_percpu_devid_irq);
>> +		irq_set_handler(irq, handle_percpu_devid_irq);
>>  		set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
>>  	} else {
>> -		irq_set_chip_and_handler(irq, &gic_chip,
>> -					 handle_fasteoi_irq);
>> +		irq_set_handler(irq, handle_fasteoi_irq);
>>  		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;
>>  }
>>  
>> @@ -795,8 +793,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
>>  {
>>  	unsigned long ret = 0;
>>  
>> -	if (d->of_node != controller)
>> -		return -EINVAL;
>>  	if (intsize < 3)
>>  		return -EINVAL;
>>  
>> @@ -839,6 +835,46 @@ static struct notifier_block gic_cpu_notifier = {
>>  };
>>  #endif
>>  
>> +
>> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
>> +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 void gic_irq_domain_free(struct irq_domain *domain, unsigned int virq,
>> +				unsigned int nr_irqs)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < nr_irqs; i++) {
>> +		irq_set_handler(virq + i, NULL);
>> +		irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
>> +	}
>> +}
>> +
>> +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = {
>> +	.alloc = gic_irq_domain_alloc,
>> +	.free = gic_irq_domain_free,
>> +};
>> +#else
>> +#define gic_irq_domain_hierarchy_ops 0
>> +#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
>> +
>>  static const struct irq_domain_ops gic_irq_domain_ops = {
>>  	.map = gic_irq_domain_map,
>>  	.unmap = gic_irq_domain_unmap,
>> @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>  
>>  	gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>>  
>> -	if (of_property_read_u32(node, "arm,routable-irqs",
>> +	if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
>> +		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
>> +		gic->domain = irq_domain_add_linear(node, gic_irqs,
>> +					&gic_irq_domain_hierarchy_ops, gic);
>> +	else 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());
>>
> 
> So I've been playing with this over the weekend (with quite a few
> tweaks), and I'm hitting a not-so-nice effect of the automatic platform
> device creation from the device tree.
> 
> What happens is the following:
> - Kernel starts
> - GIC gets initialized with a linear domain supporting hierarchy
> - per-cpu timers are up and running
> - platform devices get created from the device tree:
>   - irq_of_parse_and_map()
>   - irq_create_of_mapping()
>   - irq_domain_alloc_irqs()
> 
> Here, we start re-allocating interrupts that have already been allocated
> (the timer interrupts). This has a side effect of nuking the
> percpu_dev_id, and everything explodes on the next timer tick. Grmbl.
> 
> The main issue here is that we have a single path that:
> - translates the interrupt from DT to HW
> - configures the interrupts
> and that we use it more than once.
> 
> The non-hierarchy path works because the the "map" operation takes place
> only once, and virtual interrupts are allocated upfront.
> 
> When we switch to this more dynamic way of doing things, the fact that
> the virqs only available at allocation time is screwing us up.
> 
> I can see a way out of this, which would involve having a way of
> detecting that a hwirq has already been allocated (which requires having
> the xlate callback instantiated). Something like this (not even
> compile-tested):
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index dd8d3ab..6a45821 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -479,6 +479,21 @@ unsigned int irq_create_of_mapping(struct
> of_phandle_args *irq_data)
>  	}
> 
>  	if (irq_domain_is_hierarchy(domain)) {
> +		if (domain->ops->xlate) {
> +			/*
> +			 * If we've already configured this interrupt,
> +			 * don't do it again, or hell will break loose.
> +			 */
> +			if (domain->ops->xlate(domain, irq_data->np,
> +					       irq_data->args,
> +					       irq_data->args_count,
> +					       &hwirq, &type))
> +				return 0;
> +
> +			virq = irq_find_mapping(domain, hwirq);
> +			if (virq)
> +				return virq;
> +		}
>  		virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, irq_data);
>  		return virq <= 0 ? 0 : virq;
>  	}
> 
> Thoughts?

For the record, I just booted a pure DT system with the above patch, and
everything seems to be fine. Haven't completely convinced myself this is
the right fix though.

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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Marc Zyngier]

On 13/10/14 09:56, Marc Zyngier wrote:
> On 09/10/14 15:29, Joe.C wrote:
>> From: "Joe.C" <yingjoe.chen@mediatek.com>
>>
>> Add support to use gic as a parent for stacked irq domain.
>>
>> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
>> ---
>>  drivers/irqchip/irq-gic.c | 56 ++++++++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 48 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
>> index dda6dbc..17f5aa6 100644
>> --- a/drivers/irqchip/irq-gic.c
>> +++ b/drivers/irqchip/irq-gic.c
>> @@ -767,19 +767,17 @@ void __init gic_init_physaddr(struct device_node *node)
>>  static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
>>  				irq_hw_number_t hw)
>>  {
>> +	irq_domain_set_hwirq_and_chip(d, irq, hw, &gic_chip, d->host_data);
>>  	if (hw < 32) {
>>  		irq_set_percpu_devid(irq);
>> -		irq_set_chip_and_handler(irq, &gic_chip,
>> -					 handle_percpu_devid_irq);
>> +		irq_set_handler(irq, handle_percpu_devid_irq);
>>  		set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
>>  	} else {
>> -		irq_set_chip_and_handler(irq, &gic_chip,
>> -					 handle_fasteoi_irq);
>> +		irq_set_handler(irq, handle_fasteoi_irq);
>>  		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;
>>  }
>>  
>> @@ -795,8 +793,6 @@ static int gic_irq_domain_xlate(struct irq_domain *d,
>>  {
>>  	unsigned long ret = 0;
>>  
>> -	if (d->of_node != controller)
>> -		return -EINVAL;
>>  	if (intsize < 3)
>>  		return -EINVAL;
>>  
>> @@ -839,6 +835,46 @@ static struct notifier_block gic_cpu_notifier = {
>>  };
>>  #endif
>>  
>> +
>> +#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
>> +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 void gic_irq_domain_free(struct irq_domain *domain, unsigned int virq,
>> +				unsigned int nr_irqs)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < nr_irqs; i++) {
>> +		irq_set_handler(virq + i, NULL);
>> +		irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
>> +	}
>> +}
>> +
>> +static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = {
>> +	.alloc = gic_irq_domain_alloc,
>> +	.free = gic_irq_domain_free,
>> +};
>> +#else
>> +#define gic_irq_domain_hierarchy_ops 0
>> +#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
>> +
>>  static const struct irq_domain_ops gic_irq_domain_ops = {
>>  	.map = gic_irq_domain_map,
>>  	.unmap = gic_irq_domain_unmap,
>> @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>  
>>  	gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>>  
>> -	if (of_property_read_u32(node, "arm,routable-irqs",
>> +	if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
>> +		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
>> +		gic->domain = irq_domain_add_linear(node, gic_irqs,
>> +					&gic_irq_domain_hierarchy_ops, gic);
>> +	else 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());
>>
> 
> So I've been playing with this over the weekend (with quite a few
> tweaks), and I'm hitting a not-so-nice effect of the automatic platform
> device creation from the device tree.
> 
> What happens is the following:
> - Kernel starts
> - GIC gets initialized with a linear domain supporting hierarchy
> - per-cpu timers are up and running
> - platform devices get created from the device tree:
>   - irq_of_parse_and_map()
>   - irq_create_of_mapping()
>   - irq_domain_alloc_irqs()
> 
> Here, we start re-allocating interrupts that have already been allocated
> (the timer interrupts). This has a side effect of nuking the
> percpu_dev_id, and everything explodes on the next timer tick. Grmbl.
> 
> The main issue here is that we have a single path that:
> - translates the interrupt from DT to HW
> - configures the interrupts
> and that we use it more than once.
> 
> The non-hierarchy path works because the the "map" operation takes place
> only once, and virtual interrupts are allocated upfront.
> 
> When we switch to this more dynamic way of doing things, the fact that
> the virqs only available at allocation time is screwing us up.
> 
> I can see a way out of this, which would involve having a way of
> detecting that a hwirq has already been allocated (which requires having
> the xlate callback instantiated). Something like this (not even
> compile-tested):
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index dd8d3ab..6a45821 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -479,6 +479,21 @@ unsigned int irq_create_of_mapping(struct
> of_phandle_args *irq_data)
>  	}
> 
>  	if (irq_domain_is_hierarchy(domain)) {
> +		if (domain->ops->xlate) {
> +			/*
> +			 * If we've already configured this interrupt,
> +			 * don't do it again, or hell will break loose.
> +			 */
> +			if (domain->ops->xlate(domain, irq_data->np,
> +					       irq_data->args,
> +					       irq_data->args_count,
> +					       &hwirq, &type))
> +				return 0;
> +
> +			virq = irq_find_mapping(domain, hwirq);
> +			if (virq)
> +				return virq;
> +		}
>  		virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, irq_data);
>  		return virq <= 0 ? 0 : virq;
>  	}
> 
> Thoughts?

For the record, I just booted a pure DT system with the above patch, and
everything seems to be fine. Haven't completely convinced myself this is
the right fix though.

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

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Arnd Bergmann]

On Monday 13 October 2014 09:56:20 Marc Zyngier wrote:
>         if (irq_domain_is_hierarchy(domain)) {
> +               if (domain->ops->xlate) {
> +                       /*
> +                        * If we've already configured this interrupt,
> +                        * don't do it again, or hell will break loose.
> +                        */
> +                       if (domain->ops->xlate(domain, irq_data->np,
> +                                              irq_data->args,
> +                                              irq_data->args_count,
> +                                              &hwirq, &type))
> +                               return 0;
> +
> +                       virq = irq_find_mapping(domain, hwirq);
> +                       if (virq)
> +                               return virq;
> +               }
>                 virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, irq_data);
>                 return virq <= 0 ? 0 : virq;
>         }
> 
> Thoughts?

Using irq_find_mapping() first is probably the right approach, that
is what irq_create_mapping() does too, and I suppose we want those
to be symmetric.

mt_sysirq_domain_alloc() in patch 4 has the irq_find_domain check
in it, which I guess we can remove when it has moved to the common
code.

I don't see irq_domain_alloc_irqs() in linux-next or older kernels, where
does that get introduced?

	Arnd

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Arnd Bergmann]

On Monday 13 October 2014 09:56:20 Marc Zyngier wrote:
>         if (irq_domain_is_hierarchy(domain)) {
> +               if (domain->ops->xlate) {
> +                       /*
> +                        * If we've already configured this interrupt,
> +                        * don't do it again, or hell will break loose.
> +                        */
> +                       if (domain->ops->xlate(domain, irq_data->np,
> +                                              irq_data->args,
> +                                              irq_data->args_count,
> +                                              &hwirq, &type))
> +                               return 0;
> +
> +                       virq = irq_find_mapping(domain, hwirq);
> +                       if (virq)
> +                               return virq;
> +               }
>                 virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, irq_data);
>                 return virq <= 0 ? 0 : virq;
>         }
> 
> Thoughts?

Using irq_find_mapping() first is probably the right approach, that
is what irq_create_mapping() does too, and I suppose we want those
to be symmetric.

mt_sysirq_domain_alloc() in patch 4 has the irq_find_domain check
in it, which I guess we can remove when it has moved to the common
code.

I don't see irq_domain_alloc_irqs() in linux-next or older kernels, where
does that get introduced?

	Arnd

[PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Arnd Bergmann]

On Monday 13 October 2014 09:56:20 Marc Zyngier wrote:
>         if (irq_domain_is_hierarchy(domain)) {
> +               if (domain->ops->xlate) {
> +                       /*
> +                        * If we've already configured this interrupt,
> +                        * don't do it again, or hell will break loose.
> +                        */
> +                       if (domain->ops->xlate(domain, irq_data->np,
> +                                              irq_data->args,
> +                                              irq_data->args_count,
> +                                              &hwirq, &type))
> +                               return 0;
> +
> +                       virq = irq_find_mapping(domain, hwirq);
> +                       if (virq)
> +                               return virq;
> +               }
>                 virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, irq_data);
>                 return virq <= 0 ? 0 : virq;
>         }
> 
> Thoughts?

Using irq_find_mapping() first is probably the right approach, that
is what irq_create_mapping() does too, and I suppose we want those
to be symmetric.

mt_sysirq_domain_alloc() in patch 4 has the irq_find_domain check
in it, which I guess we can remove when it has moved to the common
code.

I don't see irq_domain_alloc_irqs() in linux-next or older kernels, where
does that get introduced?

	Arnd

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Marc Zyngier]

On 13/10/14 10:27, Arnd Bergmann wrote:
> On Monday 13 October 2014 09:56:20 Marc Zyngier wrote:
>>         if (irq_domain_is_hierarchy(domain)) {
>> +               if (domain->ops->xlate) {
>> +                       /*
>> +                        * If we've already configured this interrupt,
>> +                        * don't do it again, or hell will break loose.
>> +                        */
>> +                       if (domain->ops->xlate(domain, irq_data->np,
>> +                                              irq_data->args,
>> +                                              irq_data->args_count,
>> +                                              &hwirq, &type))
>> +                               return 0;
>> +
>> +                       virq = irq_find_mapping(domain, hwirq);
>> +                       if (virq)
>> +                               return virq;
>> +               }
>>                 virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, irq_data);
>>                 return virq <= 0 ? 0 : virq;
>>         }
>>
>> Thoughts?
> 
> Using irq_find_mapping() first is probably the right approach, that
> is what irq_create_mapping() does too, and I suppose we want those
> to be symmetric.

Ah, good point. I somehow missed that.

> mt_sysirq_domain_alloc() in patch 4 has the irq_find_domain check
> in it, which I guess we can remove when it has moved to the common
> code.
> 
> I don't see irq_domain_alloc_irqs() in linux-next or older kernels, where
> does that get introduced?

This is part of Jiang's domain hierarchy series:
https://patchwork.ozlabs.org/patch/388279/

which I plan to use to get rid of the ugly gic_extn hack that only Tegra
uses (but that everyone tries to abuse), and also for the GICv2m support.

Thanks,

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

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Marc Zyngier]

On 13/10/14 10:27, Arnd Bergmann wrote:
> On Monday 13 October 2014 09:56:20 Marc Zyngier wrote:
>>         if (irq_domain_is_hierarchy(domain)) {
>> +               if (domain->ops->xlate) {
>> +                       /*
>> +                        * If we've already configured this interrupt,
>> +                        * don't do it again, or hell will break loose.
>> +                        */
>> +                       if (domain->ops->xlate(domain, irq_data->np,
>> +                                              irq_data->args,
>> +                                              irq_data->args_count,
>> +                                              &hwirq, &type))
>> +                               return 0;
>> +
>> +                       virq = irq_find_mapping(domain, hwirq);
>> +                       if (virq)
>> +                               return virq;
>> +               }
>>                 virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, irq_data);
>>                 return virq <= 0 ? 0 : virq;
>>         }
>>
>> Thoughts?
> 
> Using irq_find_mapping() first is probably the right approach, that
> is what irq_create_mapping() does too, and I suppose we want those
> to be symmetric.

Ah, good point. I somehow missed that.

> mt_sysirq_domain_alloc() in patch 4 has the irq_find_domain check
> in it, which I guess we can remove when it has moved to the common
> code.
> 
> I don't see irq_domain_alloc_irqs() in linux-next or older kernels, where
> does that get introduced?

This is part of Jiang's domain hierarchy series:
https://patchwork.ozlabs.org/patch/388279/

which I plan to use to get rid of the ugly gic_extn hack that only Tegra
uses (but that everyone tries to abuse), and also for the GICv2m support.

Thanks,

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

[PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Marc Zyngier]

On 13/10/14 10:27, Arnd Bergmann wrote:
> On Monday 13 October 2014 09:56:20 Marc Zyngier wrote:
>>         if (irq_domain_is_hierarchy(domain)) {
>> +               if (domain->ops->xlate) {
>> +                       /*
>> +                        * If we've already configured this interrupt,
>> +                        * don't do it again, or hell will break loose.
>> +                        */
>> +                       if (domain->ops->xlate(domain, irq_data->np,
>> +                                              irq_data->args,
>> +                                              irq_data->args_count,
>> +                                              &hwirq, &type))
>> +                               return 0;
>> +
>> +                       virq = irq_find_mapping(domain, hwirq);
>> +                       if (virq)
>> +                               return virq;
>> +               }
>>                 virq = irq_domain_alloc_irqs(domain, 1, NUMA_NO_NODE, irq_data);
>>                 return virq <= 0 ? 0 : virq;
>>         }
>>
>> Thoughts?
> 
> Using irq_find_mapping() first is probably the right approach, that
> is what irq_create_mapping() does too, and I suppose we want those
> to be symmetric.

Ah, good point. I somehow missed that.

> mt_sysirq_domain_alloc() in patch 4 has the irq_find_domain check
> in it, which I guess we can remove when it has moved to the common
> code.
> 
> I don't see irq_domain_alloc_irqs() in linux-next or older kernels, where
> does that get introduced?

This is part of Jiang's domain hierarchy series:
https://patchwork.ozlabs.org/patch/388279/

which I plan to use to get rid of the ugly gic_extn hack that only Tegra
uses (but that everyone tries to abuse), and also for the GICv2m support.

Thanks,

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

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Joe.C]

On Thu, 2014-10-09 at 17:59 +0100, Marc Zyngier wrote:
> On 09/10/14 15:29, Joe.C wrote
> > @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> >  
> >  	gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
> >  
> > -	if (of_property_read_u32(node, "arm,routable-irqs",
> > +	if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
> > +		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
> > +		gic->domain = irq_domain_add_linear(node, gic_irqs,
> > +					&gic_irq_domain_hierarchy_ops, gic);
> 
> I really think that looking for a property is the wrong thing to do. If
> "node" is non-NULL, then we're pretty sure that we're initializing from
> DT, and that a pure linear domain should be the right thing, leaving the
> legacy stuff for the few non-DT platforms that are still around.
> 
> Thanks,
> 
> 	M.

The only reason I introduce "arm,irq-domain-hierarchy" property is
trying to keep original behavior when hierarchy irq domain is not used.
Without this, when a board init GIC with DT, all driver will have to use
devicetree. I'm not sure we want to break things like this. I will
remove this and just use linear for all DT in my next version.

Joe.C

[PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Joe.C]

On Thu, 2014-10-09 at 17:59 +0100, Marc Zyngier wrote:
> On 09/10/14 15:29, Joe.C wrote
> > @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> >  
> >  	gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
> >  
> > -	if (of_property_read_u32(node, "arm,routable-irqs",
> > +	if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
> > +		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
> > +		gic->domain = irq_domain_add_linear(node, gic_irqs,
> > +					&gic_irq_domain_hierarchy_ops, gic);
> 
> I really think that looking for a property is the wrong thing to do. If
> "node" is non-NULL, then we're pretty sure that we're initializing from
> DT, and that a pure linear domain should be the right thing, leaving the
> legacy stuff for the few non-DT platforms that are still around.
> 
> Thanks,
> 
> 	M.

The only reason I introduce "arm,irq-domain-hierarchy" property is
trying to keep original behavior when hierarchy irq domain is not used.
Without this, when a board init GIC with DT, all driver will have to use
devicetree. I'm not sure we want to break things like this. I will
remove this and just use linear for all DT in my next version.

Joe.C

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Marc Zyngier]

[-- Attachment #1: Type: text/plain, Size: 1612 bytes --]

On 13/10/14 11:43, Joe.C wrote:
> On Thu, 2014-10-09 at 17:59 +0100, Marc Zyngier wrote:
>> On 09/10/14 15:29, Joe.C wrote
>>> @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>>  
>>>  	gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>>>  
>>> -	if (of_property_read_u32(node, "arm,routable-irqs",
>>> +	if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
>>> +		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
>>> +		gic->domain = irq_domain_add_linear(node, gic_irqs,
>>> +					&gic_irq_domain_hierarchy_ops, gic);
>>
>> I really think that looking for a property is the wrong thing to do. If
>> "node" is non-NULL, then we're pretty sure that we're initializing from
>> DT, and that a pure linear domain should be the right thing, leaving the
>> legacy stuff for the few non-DT platforms that are still around.
>>
>> Thanks,
>>
>> 	M.
> 
> The only reason I introduce "arm,irq-domain-hierarchy" property is
> trying to keep original behavior when hierarchy irq domain is not used.
> Without this, when a board init GIC with DT, all driver will have to use
> devicetree. I'm not sure we want to break things like this.

I don't think we want to support a "middle of the road" setup, where the
GIC is probed by DT, but some devices have hardcoded interrupts.

> I will remove this and just use linear for all DT in my next version.

I came up with the attached patch, which allows me to boot my test
platform (together with the other fix I posted earlier).

Thanks,

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-fixup-irqchip-gic-Support-hierarchy-irq-domain.patch --]
[-- Type: text/x-diff;  name=0001-fixup-irqchip-gic-Support-hierarchy-irq-domain.patch, Size: 3828 bytes --]

>From 97d4ea1f0922fb47dd1b09cd2694b7fa5b519db9 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Mon, 13 Oct 2014 10:57:28 +0100
Subject: [PATCH] fixup! irqchip: gic: Support hierarchy irq domain.

---
 drivers/irqchip/Kconfig   |  1 +
 drivers/irqchip/irq-gic.c | 53 ++++++++++++++++++++++-------------------------
 2 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index b8632bf..2a48e0a 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 17f5aa6..a99c211 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -835,8 +835,6 @@ static struct notifier_block gic_cpu_notifier = {
 };
 #endif
 
-
-#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
 static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
 				unsigned int nr_irqs, void *arg)
 {
@@ -870,10 +868,8 @@ static void gic_irq_domain_free(struct irq_domain *domain, unsigned int virq,
 static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = {
 	.alloc = gic_irq_domain_alloc,
 	.free = gic_irq_domain_free,
+	.xlate = gic_irq_domain_xlate,
 };
-#else
-#define gic_irq_domain_hierarchy_ops 0
-#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
 
 static const struct irq_domain_ops gic_irq_domain_ops = {
 	.map = gic_irq_domain_map,
@@ -965,18 +961,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.
 	 */
@@ -986,14 +970,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 (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
-		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
-		gic->domain = irq_domain_add_linear(node, gic_irqs,
-					&gic_irq_domain_hierarchy_ops, gic);
-	else 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)) {
@@ -1004,10 +1005,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))
-- 
2.0.4

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Marc Zyngier]

[-- Attachment #1: Type: text/plain, Size: 1612 bytes --]

On 13/10/14 11:43, Joe.C wrote:
> On Thu, 2014-10-09 at 17:59 +0100, Marc Zyngier wrote:
>> On 09/10/14 15:29, Joe.C wrote
>>> @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>>  
>>>  	gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>>>  
>>> -	if (of_property_read_u32(node, "arm,routable-irqs",
>>> +	if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
>>> +		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
>>> +		gic->domain = irq_domain_add_linear(node, gic_irqs,
>>> +					&gic_irq_domain_hierarchy_ops, gic);
>>
>> I really think that looking for a property is the wrong thing to do. If
>> "node" is non-NULL, then we're pretty sure that we're initializing from
>> DT, and that a pure linear domain should be the right thing, leaving the
>> legacy stuff for the few non-DT platforms that are still around.
>>
>> Thanks,
>>
>> 	M.
> 
> The only reason I introduce "arm,irq-domain-hierarchy" property is
> trying to keep original behavior when hierarchy irq domain is not used.
> Without this, when a board init GIC with DT, all driver will have to use
> devicetree. I'm not sure we want to break things like this.

I don't think we want to support a "middle of the road" setup, where the
GIC is probed by DT, but some devices have hardcoded interrupts.

> I will remove this and just use linear for all DT in my next version.

I came up with the attached patch, which allows me to boot my test
platform (together with the other fix I posted earlier).

Thanks,

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-fixup-irqchip-gic-Support-hierarchy-irq-domain.patch --]
[-- Type: text/x-diff; name=0001-fixup-irqchip-gic-Support-hierarchy-irq-domain.patch, Size: 3828 bytes --]

>From 97d4ea1f0922fb47dd1b09cd2694b7fa5b519db9 Mon Sep 17 00:00:00 2001
From: Marc Zyngier <marc.zyngier@arm.com>
Date: Mon, 13 Oct 2014 10:57:28 +0100
Subject: [PATCH] fixup! irqchip: gic: Support hierarchy irq domain.

---
 drivers/irqchip/Kconfig   |  1 +
 drivers/irqchip/irq-gic.c | 53 ++++++++++++++++++++++-------------------------
 2 files changed, 26 insertions(+), 28 deletions(-)

diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index b8632bf..2a48e0a 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 17f5aa6..a99c211 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -835,8 +835,6 @@ static struct notifier_block gic_cpu_notifier = {
 };
 #endif
 
-
-#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
 static int gic_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
 				unsigned int nr_irqs, void *arg)
 {
@@ -870,10 +868,8 @@ static void gic_irq_domain_free(struct irq_domain *domain, unsigned int virq,
 static const struct irq_domain_ops gic_irq_domain_hierarchy_ops = {
 	.alloc = gic_irq_domain_alloc,
 	.free = gic_irq_domain_free,
+	.xlate = gic_irq_domain_xlate,
 };
-#else
-#define gic_irq_domain_hierarchy_ops 0
-#endif /* CONFIG_IRQ_DOMAIN_HIERARCHY */
 
 static const struct irq_domain_ops gic_irq_domain_ops = {
 	.map = gic_irq_domain_map,
@@ -965,18 +961,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.
 	 */
@@ -986,14 +970,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 (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
-		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
-		gic->domain = irq_domain_add_linear(node, gic_irqs,
-					&gic_irq_domain_hierarchy_ops, gic);
-	else 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)) {
@@ -1004,10 +1005,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))
-- 
2.0.4

[PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Marc Zyngier]

On 13/10/14 11:43, Joe.C wrote:
> On Thu, 2014-10-09 at 17:59 +0100, Marc Zyngier wrote:
>> On 09/10/14 15:29, Joe.C wrote
>>> @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
>>>  
>>>  	gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
>>>  
>>> -	if (of_property_read_u32(node, "arm,routable-irqs",
>>> +	if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
>>> +		of_find_property(node, "arm,irq-domain-hierarchy", NULL))
>>> +		gic->domain = irq_domain_add_linear(node, gic_irqs,
>>> +					&gic_irq_domain_hierarchy_ops, gic);
>>
>> I really think that looking for a property is the wrong thing to do. If
>> "node" is non-NULL, then we're pretty sure that we're initializing from
>> DT, and that a pure linear domain should be the right thing, leaving the
>> legacy stuff for the few non-DT platforms that are still around.
>>
>> Thanks,
>>
>> 	M.
> 
> The only reason I introduce "arm,irq-domain-hierarchy" property is
> trying to keep original behavior when hierarchy irq domain is not used.
> Without this, when a board init GIC with DT, all driver will have to use
> devicetree. I'm not sure we want to break things like this.

I don't think we want to support a "middle of the road" setup, where the
GIC is probed by DT, but some devices have hardcoded interrupts.

> I will remove this and just use linear for all DT in my next version.

I came up with the attached patch, which allows me to boot my test
platform (together with the other fix I posted earlier).

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-fixup-irqchip-gic-Support-hierarchy-irq-domain.patch
Type: text/x-diff
Size: 3710 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141013/af3ac3cf/attachment.bin>

Re: [PATCH v3 1/7] irqdomain: Fix irq_domain_alloc_irqs return check.

[Marc Zyngier]

On 09/10/14 15:29, Joe.C wrote:
> From: "Joe.C" <yingjoe.chen@mediatek.com>
> 
> Change virq type from unsigned int to int. Otherwise the return value
> check for irq_domain_alloc_irqs will always pass.
> 
> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
> ---
>  kernel/irq/irqdomain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 584be46..dd8d3ab 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -469,7 +469,7 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data)
>  	struct irq_domain *domain;
>  	irq_hw_number_t hwirq;
>  	unsigned int type = IRQ_TYPE_NONE;
> -	unsigned int virq;
> +	int virq;
>  
>  	domain = irq_data->np ? irq_find_host(irq_data->np) : irq_default_domain;
>  	if (!domain) {
> 

I'd expect this patch to get folded into Jiang's series.

Thanks,

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

Re: [PATCH v3 1/7] irqdomain: Fix irq_domain_alloc_irqs return check.

[Marc Zyngier]

On 09/10/14 15:29, Joe.C wrote:
> From: "Joe.C" <yingjoe.chen@mediatek.com>
> 
> Change virq type from unsigned int to int. Otherwise the return value
> check for irq_domain_alloc_irqs will always pass.
> 
> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
> ---
>  kernel/irq/irqdomain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 584be46..dd8d3ab 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -469,7 +469,7 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data)
>  	struct irq_domain *domain;
>  	irq_hw_number_t hwirq;
>  	unsigned int type = IRQ_TYPE_NONE;
> -	unsigned int virq;
> +	int virq;
>  
>  	domain = irq_data->np ? irq_find_host(irq_data->np) : irq_default_domain;
>  	if (!domain) {
> 

I'd expect this patch to get folded into Jiang's series.

Thanks,

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

[PATCH v3 1/7] irqdomain: Fix irq_domain_alloc_irqs return check.

[Marc Zyngier]

On 09/10/14 15:29, Joe.C wrote:
> From: "Joe.C" <yingjoe.chen@mediatek.com>
> 
> Change virq type from unsigned int to int. Otherwise the return value
> check for irq_domain_alloc_irqs will always pass.
> 
> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
> ---
>  kernel/irq/irqdomain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 584be46..dd8d3ab 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -469,7 +469,7 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data)
>  	struct irq_domain *domain;
>  	irq_hw_number_t hwirq;
>  	unsigned int type = IRQ_TYPE_NONE;
> -	unsigned int virq;
> +	int virq;
>  
>  	domain = irq_data->np ? irq_find_host(irq_data->np) : irq_default_domain;
>  	if (!domain) {
> 

I'd expect this patch to get folded into Jiang's series.

Thanks,

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

Re: [PATCH v3 4/7] ARM: mediatek: Add sysirq interrupt polarity support

[Matthias Brugger]

On 09/10/14 16:29, Joe.C wrote:
> From: "Joe.C" <yingjoe.chen@mediatek.com>
> 
> Mediatek SoCs have interrupt polarity in sysirq which allows
> to swap the polarity for given interrupts. Add this support
> using hierarchy irq domain.
> 
> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
> ---
>  arch/arm/mach-mediatek/Kconfig      |   1 +
>  drivers/irqchip/Makefile            |   1 +
>  drivers/irqchip/irq-mt65xx-sysirq.c | 170 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 172 insertions(+)
>  create mode 100644 drivers/irqchip/irq-mt65xx-sysirq.c
> 
> diff --git a/arch/arm/mach-mediatek/Kconfig b/arch/arm/mach-mediatek/Kconfig
> index 2c043a2..7093859 100644
> --- a/arch/arm/mach-mediatek/Kconfig
> +++ b/arch/arm/mach-mediatek/Kconfig
> @@ -2,5 +2,6 @@ config ARCH_MEDIATEK
>  	bool "Mediatek MT6589 SoC" if ARCH_MULTI_V7
>  	select ARM_GIC
>  	select MTK_TIMER
> +	select IRQ_DOMAIN_HIERARCHY
>  	help
>  	  Support for Mediatek Cortex-A7 Quad-Core-SoC MT6589.
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 73052ba..809c9d5 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -34,3 +34,4 @@ obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
>  obj-$(CONFIG_XTENSA_MX)			+= irq-xtensa-mx.o
>  obj-$(CONFIG_IRQ_CROSSBAR)		+= irq-crossbar.o
>  obj-$(CONFIG_BRCMSTB_L2_IRQ)		+= irq-brcmstb-l2.o
> +obj-$(CONFIG_ARCH_MEDIATEK)		+= irq-mt65xx-sysirq.o
> diff --git a/drivers/irqchip/irq-mt65xx-sysirq.c b/drivers/irqchip/irq-mt65xx-sysirq.c
> new file mode 100644
> index 0000000..9e0eee5
> --- /dev/null
> +++ b/drivers/irqchip/irq-mt65xx-sysirq.c
> @@ -0,0 +1,170 @@
> +/*
> + * Copyright (c) 2014 MediaTek Inc.
> + * Author: Joe.C <yingjoe.chen@mediatek.com>
> + *
> + * 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 mt_sysirq_chip_data {
> +	spinlock_t lock;
> +	void __iomem *intpol_base;
> +};
> +
> +
> +static int mt_sysirq_set_type(struct irq_data *data, unsigned int type)

Are the mt_sysirq_ prefixed functions special to mt65xx processors? In
this case you should prefix them mt65xx_sysirq_
If not, please rename to mtk_sysirq_ to have a consistency in the kernel.

> +{
> +	irq_hw_number_t hwirq = data->hwirq;
> +	struct mt_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 mt_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		= mt_sysirq_set_type,
> +	.irq_retrigger		= irq_chip_retrigger_hierarchy,
> +	.irq_set_affinity	= irq_chip_set_affinity_parent,
> +};
> +
> +static int mt_sysirq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> +				unsigned int nr_irqs, void *arg)

Same here.

> +{
> +	int i, ret;
> +	irq_hw_number_t hwirq;
> +	struct of_phandle_args *irq_data = arg;
> +	unsigned int type;
> +
> +	if (irq_data->args_count != 3)
> +		return -EINVAL;
> +
> +	hwirq = irq_data->args[1];
> +	if (irq_find_mapping(domain, hwirq) > 0)
> +		return -EEXIST;
> +
> +	for (i = 0; i < nr_irqs; i++)
> +		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
> +					&mt_sysirq_chip, domain->host_data);
> +
> +	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
> +	if (ret < 0)
> +		return ret;
> +
> +	type = irq_data->args[2] & IRQ_TYPE_SENSE_MASK;
> +
> +	/* Set type if specified and different than the current one */
> +	if (type != IRQ_TYPE_NONE &&
> +	    type != irq_get_trigger_type(virq))
> +		irq_set_irq_type(virq, type);
> +
> +	return 0;
> +}
> +
> +static void mt_sysirq_domain_free(struct irq_domain *domain, unsigned int virq,
> +				unsigned int nr_irqs)

Same here.

> +{
> +	int i;
> +
> +	for (i = 0; i < nr_irqs; i++) {
> +		irq_set_handler(virq + i, NULL);
> +		irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
> +	}
> +	irq_domain_free_irqs_parent(domain, virq, nr_irqs);
> +}
> +
> +static struct irq_domain_ops sysirq_domain_ops = {
> +	.alloc = mt_sysirq_domain_alloc,
> +	.free = mt_sysirq_domain_free,
> +};
> +
> +static int __init mtk_sysirq_of_init(struct device_node *node,
> +				   struct device_node *parent)
> +{
> +	struct device_node *parent_node;
> +	struct irq_domain *domain, *domain_parent = NULL;
> +	struct mt_sysirq_chip_data *chip_data;
> +	int ret = 0;
> +
> +	parent_node = of_irq_find_parent(node);
> +	if (parent_node) {
> +		domain_parent = irq_find_host(parent_node);
> +		of_node_put(parent_node);
> +	}
> +
> +	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_linear(node, MT6577_SYS_INTPOL_NUM,
> +				&sysirq_domain_ops, chip_data);
> +	if (!domain) {
> +		ret = -ENOMEM;
> +		goto out_unmap;
> +	}
> +	domain->parent = domain_parent;
> +	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);

If this is compatible to all mt65xx from mt6577 upwards, we should
rename the file to irq-mt6577-sysirq.c to have consistency in the naming.

> 

[PATCH v3 4/7] ARM: mediatek: Add sysirq interrupt polarity support

[Matthias Brugger]

On 09/10/14 16:29, Joe.C wrote:
> From: "Joe.C" <yingjoe.chen@mediatek.com>
> 
> Mediatek SoCs have interrupt polarity in sysirq which allows
> to swap the polarity for given interrupts. Add this support
> using hierarchy irq domain.
> 
> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
> ---
>  arch/arm/mach-mediatek/Kconfig      |   1 +
>  drivers/irqchip/Makefile            |   1 +
>  drivers/irqchip/irq-mt65xx-sysirq.c | 170 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 172 insertions(+)
>  create mode 100644 drivers/irqchip/irq-mt65xx-sysirq.c
> 
> diff --git a/arch/arm/mach-mediatek/Kconfig b/arch/arm/mach-mediatek/Kconfig
> index 2c043a2..7093859 100644
> --- a/arch/arm/mach-mediatek/Kconfig
> +++ b/arch/arm/mach-mediatek/Kconfig
> @@ -2,5 +2,6 @@ config ARCH_MEDIATEK
>  	bool "Mediatek MT6589 SoC" if ARCH_MULTI_V7
>  	select ARM_GIC
>  	select MTK_TIMER
> +	select IRQ_DOMAIN_HIERARCHY
>  	help
>  	  Support for Mediatek Cortex-A7 Quad-Core-SoC MT6589.
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index 73052ba..809c9d5 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -34,3 +34,4 @@ obj-$(CONFIG_XTENSA)			+= irq-xtensa-pic.o
>  obj-$(CONFIG_XTENSA_MX)			+= irq-xtensa-mx.o
>  obj-$(CONFIG_IRQ_CROSSBAR)		+= irq-crossbar.o
>  obj-$(CONFIG_BRCMSTB_L2_IRQ)		+= irq-brcmstb-l2.o
> +obj-$(CONFIG_ARCH_MEDIATEK)		+= irq-mt65xx-sysirq.o
> diff --git a/drivers/irqchip/irq-mt65xx-sysirq.c b/drivers/irqchip/irq-mt65xx-sysirq.c
> new file mode 100644
> index 0000000..9e0eee5
> --- /dev/null
> +++ b/drivers/irqchip/irq-mt65xx-sysirq.c
> @@ -0,0 +1,170 @@
> +/*
> + * Copyright (c) 2014 MediaTek Inc.
> + * Author: Joe.C <yingjoe.chen@mediatek.com>
> + *
> + * 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 mt_sysirq_chip_data {
> +	spinlock_t lock;
> +	void __iomem *intpol_base;
> +};
> +
> +
> +static int mt_sysirq_set_type(struct irq_data *data, unsigned int type)

Are the mt_sysirq_ prefixed functions special to mt65xx processors? In
this case you should prefix them mt65xx_sysirq_
If not, please rename to mtk_sysirq_ to have a consistency in the kernel.

> +{
> +	irq_hw_number_t hwirq = data->hwirq;
> +	struct mt_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 mt_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		= mt_sysirq_set_type,
> +	.irq_retrigger		= irq_chip_retrigger_hierarchy,
> +	.irq_set_affinity	= irq_chip_set_affinity_parent,
> +};
> +
> +static int mt_sysirq_domain_alloc(struct irq_domain *domain, unsigned int virq,
> +				unsigned int nr_irqs, void *arg)

Same here.

> +{
> +	int i, ret;
> +	irq_hw_number_t hwirq;
> +	struct of_phandle_args *irq_data = arg;
> +	unsigned int type;
> +
> +	if (irq_data->args_count != 3)
> +		return -EINVAL;
> +
> +	hwirq = irq_data->args[1];
> +	if (irq_find_mapping(domain, hwirq) > 0)
> +		return -EEXIST;
> +
> +	for (i = 0; i < nr_irqs; i++)
> +		irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
> +					&mt_sysirq_chip, domain->host_data);
> +
> +	ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
> +	if (ret < 0)
> +		return ret;
> +
> +	type = irq_data->args[2] & IRQ_TYPE_SENSE_MASK;
> +
> +	/* Set type if specified and different than the current one */
> +	if (type != IRQ_TYPE_NONE &&
> +	    type != irq_get_trigger_type(virq))
> +		irq_set_irq_type(virq, type);
> +
> +	return 0;
> +}
> +
> +static void mt_sysirq_domain_free(struct irq_domain *domain, unsigned int virq,
> +				unsigned int nr_irqs)

Same here.

> +{
> +	int i;
> +
> +	for (i = 0; i < nr_irqs; i++) {
> +		irq_set_handler(virq + i, NULL);
> +		irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
> +	}
> +	irq_domain_free_irqs_parent(domain, virq, nr_irqs);
> +}
> +
> +static struct irq_domain_ops sysirq_domain_ops = {
> +	.alloc = mt_sysirq_domain_alloc,
> +	.free = mt_sysirq_domain_free,
> +};
> +
> +static int __init mtk_sysirq_of_init(struct device_node *node,
> +				   struct device_node *parent)
> +{
> +	struct device_node *parent_node;
> +	struct irq_domain *domain, *domain_parent = NULL;
> +	struct mt_sysirq_chip_data *chip_data;
> +	int ret = 0;
> +
> +	parent_node = of_irq_find_parent(node);
> +	if (parent_node) {
> +		domain_parent = irq_find_host(parent_node);
> +		of_node_put(parent_node);
> +	}
> +
> +	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_linear(node, MT6577_SYS_INTPOL_NUM,
> +				&sysirq_domain_ops, chip_data);
> +	if (!domain) {
> +		ret = -ENOMEM;
> +		goto out_unmap;
> +	}
> +	domain->parent = domain_parent;
> +	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);

If this is compatible to all mt65xx from mt6577 upwards, we should
rename the file to irq-mt6577-sysirq.c to have consistency in the naming.

> 

Re: [PATCH v3 1/7] irqdomain: Fix irq_domain_alloc_irqs return check.

[Jiang Liu]

On 2014/10/13 20:11, Marc Zyngier wrote:
> On 09/10/14 15:29, Joe.C wrote:
>> From: "Joe.C" <yingjoe.chen@mediatek.com>
>>
>> Change virq type from unsigned int to int. Otherwise the return value
>> check for irq_domain_alloc_irqs will always pass.
>>
>> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
>> ---
>>  kernel/irq/irqdomain.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>> index 584be46..dd8d3ab 100644
>> --- a/kernel/irq/irqdomain.c
>> +++ b/kernel/irq/irqdomain.c
>> @@ -469,7 +469,7 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data)
>>  	struct irq_domain *domain;
>>  	irq_hw_number_t hwirq;
>>  	unsigned int type = IRQ_TYPE_NONE;
>> -	unsigned int virq;
>> +	int virq;
>>  
>>  	domain = irq_data->np ? irq_find_host(irq_data->np) : irq_default_domain;
>>  	if (!domain) {
>>
> 
> I'd expect this patch to get folded into Jiang's series.
Hi Mark and Joe,
	Yes, my next version includes this fixup.
Regards!
Gerry
> 
> Thanks,
> 
> 	M.
> 

Re: [PATCH v3 1/7] irqdomain: Fix irq_domain_alloc_irqs return check.

[Jiang Liu]

On 2014/10/13 20:11, Marc Zyngier wrote:
> On 09/10/14 15:29, Joe.C wrote:
>> From: "Joe.C" <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>>
>> Change virq type from unsigned int to int. Otherwise the return value
>> check for irq_domain_alloc_irqs will always pass.
>>
>> Signed-off-by: Joe.C <yingjoe.chen-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>> ---
>>  kernel/irq/irqdomain.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>> index 584be46..dd8d3ab 100644
>> --- a/kernel/irq/irqdomain.c
>> +++ b/kernel/irq/irqdomain.c
>> @@ -469,7 +469,7 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data)
>>  	struct irq_domain *domain;
>>  	irq_hw_number_t hwirq;
>>  	unsigned int type = IRQ_TYPE_NONE;
>> -	unsigned int virq;
>> +	int virq;
>>  
>>  	domain = irq_data->np ? irq_find_host(irq_data->np) : irq_default_domain;
>>  	if (!domain) {
>>
> 
> I'd expect this patch to get folded into Jiang's series.
Hi Mark and Joe,
	Yes, my next version includes this fixup.
Regards!
Gerry
> 
> Thanks,
> 
> 	M.
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3 1/7] irqdomain: Fix irq_domain_alloc_irqs return check.

[Jiang Liu]

On 2014/10/13 20:11, Marc Zyngier wrote:
> On 09/10/14 15:29, Joe.C wrote:
>> From: "Joe.C" <yingjoe.chen@mediatek.com>
>>
>> Change virq type from unsigned int to int. Otherwise the return value
>> check for irq_domain_alloc_irqs will always pass.
>>
>> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
>> ---
>>  kernel/irq/irqdomain.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
>> index 584be46..dd8d3ab 100644
>> --- a/kernel/irq/irqdomain.c
>> +++ b/kernel/irq/irqdomain.c
>> @@ -469,7 +469,7 @@ unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data)
>>  	struct irq_domain *domain;
>>  	irq_hw_number_t hwirq;
>>  	unsigned int type = IRQ_TYPE_NONE;
>> -	unsigned int virq;
>> +	int virq;
>>  
>>  	domain = irq_data->np ? irq_find_host(irq_data->np) : irq_default_domain;
>>  	if (!domain) {
>>
> 
> I'd expect this patch to get folded into Jiang's series.
Hi Mark and Joe,
	Yes, my next version includes this fixup.
Regards!
Gerry
> 
> Thanks,
> 
> 	M.
> 

Re: [PATCH v3 4/7] ARM: mediatek: Add sysirq interrupt polarity support

[Matthias Brugger]

2014-10-13 15:43 GMT+02:00 Matthias Brugger <matthias.bgg@gmail.com>:
>
>
> On 09/10/14 16:29, Joe.C wrote:
>> From: "Joe.C" <yingjoe.chen@mediatek.com>
>>
>> Mediatek SoCs have interrupt polarity in sysirq which allows
>> to swap the polarity for given interrupts. Add this support
>> using hierarchy irq domain.
>>
>> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
>> ---
>>  arch/arm/mach-mediatek/Kconfig      |   1 +
>>  drivers/irqchip/Makefile            |   1 +
>>  drivers/irqchip/irq-mt65xx-sysirq.c | 170 ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 172 insertions(+)
>>  create mode 100644 drivers/irqchip/irq-mt65xx-sysirq.c
>>
>> diff --git a/arch/arm/mach-mediatek/Kconfig b/arch/arm/mach-mediatek/Kconfig
>> index 2c043a2..7093859 100644
>> --- a/arch/arm/mach-mediatek/Kconfig
>> +++ b/arch/arm/mach-mediatek/Kconfig
>> @@ -2,5 +2,6 @@ config ARCH_MEDIATEK
>>       bool "Mediatek MT6589 SoC" if ARCH_MULTI_V7
>>       select ARM_GIC
>>       select MTK_TIMER
>> +     select IRQ_DOMAIN_HIERARCHY
>>       help
>>         Support for Mediatek Cortex-A7 Quad-Core-SoC MT6589.
>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>> index 73052ba..809c9d5 100644
>> --- a/drivers/irqchip/Makefile
>> +++ b/drivers/irqchip/Makefile
>> @@ -34,3 +34,4 @@ obj-$(CONFIG_XTENSA)                        += irq-xtensa-pic.o
>>  obj-$(CONFIG_XTENSA_MX)                      += irq-xtensa-mx.o
>>  obj-$(CONFIG_IRQ_CROSSBAR)           += irq-crossbar.o
>>  obj-$(CONFIG_BRCMSTB_L2_IRQ)         += irq-brcmstb-l2.o
>> +obj-$(CONFIG_ARCH_MEDIATEK)          += irq-mt65xx-sysirq.o
>> diff --git a/drivers/irqchip/irq-mt65xx-sysirq.c b/drivers/irqchip/irq-mt65xx-sysirq.c
>> new file mode 100644
>> index 0000000..9e0eee5
>> --- /dev/null
>> +++ b/drivers/irqchip/irq-mt65xx-sysirq.c
>> @@ -0,0 +1,170 @@
>> +/*
>> + * Copyright (c) 2014 MediaTek Inc.
>> + * Author: Joe.C <yingjoe.chen@mediatek.com>
>> + *
>> + * 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 mt_sysirq_chip_data {
>> +     spinlock_t lock;
>> +     void __iomem *intpol_base;
>> +};
>> +
>> +
>> +static int mt_sysirq_set_type(struct irq_data *data, unsigned int type)
>
> Are the mt_sysirq_ prefixed functions special to mt65xx processors? In
> this case you should prefix them mt65xx_sysirq_
> If not, please rename to mtk_sysirq_ to have a consistency in the kernel.
>
>> +{
>> +     irq_hw_number_t hwirq = data->hwirq;
>> +     struct mt_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 mt_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           = mt_sysirq_set_type,
>> +     .irq_retrigger          = irq_chip_retrigger_hierarchy,
>> +     .irq_set_affinity       = irq_chip_set_affinity_parent,
>> +};
>> +
>> +static int mt_sysirq_domain_alloc(struct irq_domain *domain, unsigned int virq,
>> +                             unsigned int nr_irqs, void *arg)
>
> Same here.
>
>> +{
>> +     int i, ret;
>> +     irq_hw_number_t hwirq;
>> +     struct of_phandle_args *irq_data = arg;
>> +     unsigned int type;
>> +
>> +     if (irq_data->args_count != 3)
>> +             return -EINVAL;
>> +
>> +     hwirq = irq_data->args[1];
>> +     if (irq_find_mapping(domain, hwirq) > 0)
>> +             return -EEXIST;
>> +
>> +     for (i = 0; i < nr_irqs; i++)
>> +             irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
>> +                                     &mt_sysirq_chip, domain->host_data);
>> +
>> +     ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     type = irq_data->args[2] & IRQ_TYPE_SENSE_MASK;
>> +
>> +     /* Set type if specified and different than the current one */
>> +     if (type != IRQ_TYPE_NONE &&
>> +         type != irq_get_trigger_type(virq))
>> +             irq_set_irq_type(virq, type);
>> +
>> +     return 0;
>> +}
>> +
>> +static void mt_sysirq_domain_free(struct irq_domain *domain, unsigned int virq,
>> +                             unsigned int nr_irqs)
>
> Same here.
>
>> +{
>> +     int i;
>> +
>> +     for (i = 0; i < nr_irqs; i++) {
>> +             irq_set_handler(virq + i, NULL);
>> +             irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
>> +     }
>> +     irq_domain_free_irqs_parent(domain, virq, nr_irqs);
>> +}
>> +
>> +static struct irq_domain_ops sysirq_domain_ops = {
>> +     .alloc = mt_sysirq_domain_alloc,
>> +     .free = mt_sysirq_domain_free,
>> +};
>> +
>> +static int __init mtk_sysirq_of_init(struct device_node *node,
>> +                                struct device_node *parent)
>> +{
>> +     struct device_node *parent_node;
>> +     struct irq_domain *domain, *domain_parent = NULL;
>> +     struct mt_sysirq_chip_data *chip_data;
>> +     int ret = 0;
>> +
>> +     parent_node = of_irq_find_parent(node);
>> +     if (parent_node) {
>> +             domain_parent = irq_find_host(parent_node);
>> +             of_node_put(parent_node);
>> +     }
>> +
>> +     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_linear(node, MT6577_SYS_INTPOL_NUM,
>> +                             &sysirq_domain_ops, chip_data);
>> +     if (!domain) {
>> +             ret = -ENOMEM;
>> +             goto out_unmap;
>> +     }
>> +     domain->parent = domain_parent;
>> +     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);
>
> If this is compatible to all mt65xx from mt6577 upwards, we should
> rename the file to irq-mt6577-sysirq.c to have consistency in the naming.

After reviewing the patches, I see that this is compatible to mt6589
as well as to mt81xx, right?
In this case please rename the file to irq-mtk-sysirq.c
Although I think al the mt_ prefixed functions should be renamed using
mtk_ prefix.

Thanks,
Matthias

>
>>



-- 
motzblog.wordpress.com

Re: [PATCH v3 4/7] ARM: mediatek: Add sysirq interrupt polarity support

[Matthias Brugger]

2014-10-13 15:43 GMT+02:00 Matthias Brugger <matthias.bgg@gmail.com>:
>
>
> On 09/10/14 16:29, Joe.C wrote:
>> From: "Joe.C" <yingjoe.chen@mediatek.com>
>>
>> Mediatek SoCs have interrupt polarity in sysirq which allows
>> to swap the polarity for given interrupts. Add this support
>> using hierarchy irq domain.
>>
>> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
>> ---
>>  arch/arm/mach-mediatek/Kconfig      |   1 +
>>  drivers/irqchip/Makefile            |   1 +
>>  drivers/irqchip/irq-mt65xx-sysirq.c | 170 ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 172 insertions(+)
>>  create mode 100644 drivers/irqchip/irq-mt65xx-sysirq.c
>>
>> diff --git a/arch/arm/mach-mediatek/Kconfig b/arch/arm/mach-mediatek/Kconfig
>> index 2c043a2..7093859 100644
>> --- a/arch/arm/mach-mediatek/Kconfig
>> +++ b/arch/arm/mach-mediatek/Kconfig
>> @@ -2,5 +2,6 @@ config ARCH_MEDIATEK
>>       bool "Mediatek MT6589 SoC" if ARCH_MULTI_V7
>>       select ARM_GIC
>>       select MTK_TIMER
>> +     select IRQ_DOMAIN_HIERARCHY
>>       help
>>         Support for Mediatek Cortex-A7 Quad-Core-SoC MT6589.
>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>> index 73052ba..809c9d5 100644
>> --- a/drivers/irqchip/Makefile
>> +++ b/drivers/irqchip/Makefile
>> @@ -34,3 +34,4 @@ obj-$(CONFIG_XTENSA)                        += irq-xtensa-pic.o
>>  obj-$(CONFIG_XTENSA_MX)                      += irq-xtensa-mx.o
>>  obj-$(CONFIG_IRQ_CROSSBAR)           += irq-crossbar.o
>>  obj-$(CONFIG_BRCMSTB_L2_IRQ)         += irq-brcmstb-l2.o
>> +obj-$(CONFIG_ARCH_MEDIATEK)          += irq-mt65xx-sysirq.o
>> diff --git a/drivers/irqchip/irq-mt65xx-sysirq.c b/drivers/irqchip/irq-mt65xx-sysirq.c
>> new file mode 100644
>> index 0000000..9e0eee5
>> --- /dev/null
>> +++ b/drivers/irqchip/irq-mt65xx-sysirq.c
>> @@ -0,0 +1,170 @@
>> +/*
>> + * Copyright (c) 2014 MediaTek Inc.
>> + * Author: Joe.C <yingjoe.chen@mediatek.com>
>> + *
>> + * 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 mt_sysirq_chip_data {
>> +     spinlock_t lock;
>> +     void __iomem *intpol_base;
>> +};
>> +
>> +
>> +static int mt_sysirq_set_type(struct irq_data *data, unsigned int type)
>
> Are the mt_sysirq_ prefixed functions special to mt65xx processors? In
> this case you should prefix them mt65xx_sysirq_
> If not, please rename to mtk_sysirq_ to have a consistency in the kernel.
>
>> +{
>> +     irq_hw_number_t hwirq = data->hwirq;
>> +     struct mt_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 mt_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           = mt_sysirq_set_type,
>> +     .irq_retrigger          = irq_chip_retrigger_hierarchy,
>> +     .irq_set_affinity       = irq_chip_set_affinity_parent,
>> +};
>> +
>> +static int mt_sysirq_domain_alloc(struct irq_domain *domain, unsigned int virq,
>> +                             unsigned int nr_irqs, void *arg)
>
> Same here.
>
>> +{
>> +     int i, ret;
>> +     irq_hw_number_t hwirq;
>> +     struct of_phandle_args *irq_data = arg;
>> +     unsigned int type;
>> +
>> +     if (irq_data->args_count != 3)
>> +             return -EINVAL;
>> +
>> +     hwirq = irq_data->args[1];
>> +     if (irq_find_mapping(domain, hwirq) > 0)
>> +             return -EEXIST;
>> +
>> +     for (i = 0; i < nr_irqs; i++)
>> +             irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
>> +                                     &mt_sysirq_chip, domain->host_data);
>> +
>> +     ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     type = irq_data->args[2] & IRQ_TYPE_SENSE_MASK;
>> +
>> +     /* Set type if specified and different than the current one */
>> +     if (type != IRQ_TYPE_NONE &&
>> +         type != irq_get_trigger_type(virq))
>> +             irq_set_irq_type(virq, type);
>> +
>> +     return 0;
>> +}
>> +
>> +static void mt_sysirq_domain_free(struct irq_domain *domain, unsigned int virq,
>> +                             unsigned int nr_irqs)
>
> Same here.
>
>> +{
>> +     int i;
>> +
>> +     for (i = 0; i < nr_irqs; i++) {
>> +             irq_set_handler(virq + i, NULL);
>> +             irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
>> +     }
>> +     irq_domain_free_irqs_parent(domain, virq, nr_irqs);
>> +}
>> +
>> +static struct irq_domain_ops sysirq_domain_ops = {
>> +     .alloc = mt_sysirq_domain_alloc,
>> +     .free = mt_sysirq_domain_free,
>> +};
>> +
>> +static int __init mtk_sysirq_of_init(struct device_node *node,
>> +                                struct device_node *parent)
>> +{
>> +     struct device_node *parent_node;
>> +     struct irq_domain *domain, *domain_parent = NULL;
>> +     struct mt_sysirq_chip_data *chip_data;
>> +     int ret = 0;
>> +
>> +     parent_node = of_irq_find_parent(node);
>> +     if (parent_node) {
>> +             domain_parent = irq_find_host(parent_node);
>> +             of_node_put(parent_node);
>> +     }
>> +
>> +     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_linear(node, MT6577_SYS_INTPOL_NUM,
>> +                             &sysirq_domain_ops, chip_data);
>> +     if (!domain) {
>> +             ret = -ENOMEM;
>> +             goto out_unmap;
>> +     }
>> +     domain->parent = domain_parent;
>> +     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);
>
> If this is compatible to all mt65xx from mt6577 upwards, we should
> rename the file to irq-mt6577-sysirq.c to have consistency in the naming.

After reviewing the patches, I see that this is compatible to mt6589
as well as to mt81xx, right?
In this case please rename the file to irq-mtk-sysirq.c
Although I think al the mt_ prefixed functions should be renamed using
mtk_ prefix.

Thanks,
Matthias

>
>>



-- 
motzblog.wordpress.com

[PATCH v3 4/7] ARM: mediatek: Add sysirq interrupt polarity support

[Matthias Brugger]

2014-10-13 15:43 GMT+02:00 Matthias Brugger <matthias.bgg@gmail.com>:
>
>
> On 09/10/14 16:29, Joe.C wrote:
>> From: "Joe.C" <yingjoe.chen@mediatek.com>
>>
>> Mediatek SoCs have interrupt polarity in sysirq which allows
>> to swap the polarity for given interrupts. Add this support
>> using hierarchy irq domain.
>>
>> Signed-off-by: Joe.C <yingjoe.chen@mediatek.com>
>> ---
>>  arch/arm/mach-mediatek/Kconfig      |   1 +
>>  drivers/irqchip/Makefile            |   1 +
>>  drivers/irqchip/irq-mt65xx-sysirq.c | 170 ++++++++++++++++++++++++++++++++++++
>>  3 files changed, 172 insertions(+)
>>  create mode 100644 drivers/irqchip/irq-mt65xx-sysirq.c
>>
>> diff --git a/arch/arm/mach-mediatek/Kconfig b/arch/arm/mach-mediatek/Kconfig
>> index 2c043a2..7093859 100644
>> --- a/arch/arm/mach-mediatek/Kconfig
>> +++ b/arch/arm/mach-mediatek/Kconfig
>> @@ -2,5 +2,6 @@ config ARCH_MEDIATEK
>>       bool "Mediatek MT6589 SoC" if ARCH_MULTI_V7
>>       select ARM_GIC
>>       select MTK_TIMER
>> +     select IRQ_DOMAIN_HIERARCHY
>>       help
>>         Support for Mediatek Cortex-A7 Quad-Core-SoC MT6589.
>> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
>> index 73052ba..809c9d5 100644
>> --- a/drivers/irqchip/Makefile
>> +++ b/drivers/irqchip/Makefile
>> @@ -34,3 +34,4 @@ obj-$(CONFIG_XTENSA)                        += irq-xtensa-pic.o
>>  obj-$(CONFIG_XTENSA_MX)                      += irq-xtensa-mx.o
>>  obj-$(CONFIG_IRQ_CROSSBAR)           += irq-crossbar.o
>>  obj-$(CONFIG_BRCMSTB_L2_IRQ)         += irq-brcmstb-l2.o
>> +obj-$(CONFIG_ARCH_MEDIATEK)          += irq-mt65xx-sysirq.o
>> diff --git a/drivers/irqchip/irq-mt65xx-sysirq.c b/drivers/irqchip/irq-mt65xx-sysirq.c
>> new file mode 100644
>> index 0000000..9e0eee5
>> --- /dev/null
>> +++ b/drivers/irqchip/irq-mt65xx-sysirq.c
>> @@ -0,0 +1,170 @@
>> +/*
>> + * Copyright (c) 2014 MediaTek Inc.
>> + * Author: Joe.C <yingjoe.chen@mediatek.com>
>> + *
>> + * 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 mt_sysirq_chip_data {
>> +     spinlock_t lock;
>> +     void __iomem *intpol_base;
>> +};
>> +
>> +
>> +static int mt_sysirq_set_type(struct irq_data *data, unsigned int type)
>
> Are the mt_sysirq_ prefixed functions special to mt65xx processors? In
> this case you should prefix them mt65xx_sysirq_
> If not, please rename to mtk_sysirq_ to have a consistency in the kernel.
>
>> +{
>> +     irq_hw_number_t hwirq = data->hwirq;
>> +     struct mt_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 mt_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           = mt_sysirq_set_type,
>> +     .irq_retrigger          = irq_chip_retrigger_hierarchy,
>> +     .irq_set_affinity       = irq_chip_set_affinity_parent,
>> +};
>> +
>> +static int mt_sysirq_domain_alloc(struct irq_domain *domain, unsigned int virq,
>> +                             unsigned int nr_irqs, void *arg)
>
> Same here.
>
>> +{
>> +     int i, ret;
>> +     irq_hw_number_t hwirq;
>> +     struct of_phandle_args *irq_data = arg;
>> +     unsigned int type;
>> +
>> +     if (irq_data->args_count != 3)
>> +             return -EINVAL;
>> +
>> +     hwirq = irq_data->args[1];
>> +     if (irq_find_mapping(domain, hwirq) > 0)
>> +             return -EEXIST;
>> +
>> +     for (i = 0; i < nr_irqs; i++)
>> +             irq_domain_set_hwirq_and_chip(domain, virq + i, hwirq + i,
>> +                                     &mt_sysirq_chip, domain->host_data);
>> +
>> +     ret = irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, arg);
>> +     if (ret < 0)
>> +             return ret;
>> +
>> +     type = irq_data->args[2] & IRQ_TYPE_SENSE_MASK;
>> +
>> +     /* Set type if specified and different than the current one */
>> +     if (type != IRQ_TYPE_NONE &&
>> +         type != irq_get_trigger_type(virq))
>> +             irq_set_irq_type(virq, type);
>> +
>> +     return 0;
>> +}
>> +
>> +static void mt_sysirq_domain_free(struct irq_domain *domain, unsigned int virq,
>> +                             unsigned int nr_irqs)
>
> Same here.
>
>> +{
>> +     int i;
>> +
>> +     for (i = 0; i < nr_irqs; i++) {
>> +             irq_set_handler(virq + i, NULL);
>> +             irq_domain_set_hwirq_and_chip(domain, virq + i, 0, NULL, NULL);
>> +     }
>> +     irq_domain_free_irqs_parent(domain, virq, nr_irqs);
>> +}
>> +
>> +static struct irq_domain_ops sysirq_domain_ops = {
>> +     .alloc = mt_sysirq_domain_alloc,
>> +     .free = mt_sysirq_domain_free,
>> +};
>> +
>> +static int __init mtk_sysirq_of_init(struct device_node *node,
>> +                                struct device_node *parent)
>> +{
>> +     struct device_node *parent_node;
>> +     struct irq_domain *domain, *domain_parent = NULL;
>> +     struct mt_sysirq_chip_data *chip_data;
>> +     int ret = 0;
>> +
>> +     parent_node = of_irq_find_parent(node);
>> +     if (parent_node) {
>> +             domain_parent = irq_find_host(parent_node);
>> +             of_node_put(parent_node);
>> +     }
>> +
>> +     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_linear(node, MT6577_SYS_INTPOL_NUM,
>> +                             &sysirq_domain_ops, chip_data);
>> +     if (!domain) {
>> +             ret = -ENOMEM;
>> +             goto out_unmap;
>> +     }
>> +     domain->parent = domain_parent;
>> +     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);
>
> If this is compatible to all mt65xx from mt6577 upwards, we should
> rename the file to irq-mt6577-sysirq.c to have consistency in the naming.

After reviewing the patches, I see that this is compatible to mt6589
as well as to mt81xx, right?
In this case please rename the file to irq-mtk-sysirq.c
Although I think al the mt_ prefixed functions should be renamed using
mtk_ prefix.

Thanks,
Matthias

>
>>



-- 
motzblog.wordpress.com

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Arnd Bergmann]

On Monday 13 October 2014 13:10:32 Marc Zyngier wrote:
> On 13/10/14 11:43, Joe.C wrote:
> > On Thu, 2014-10-09 at 17:59 +0100, Marc Zyngier wrote:
> >> On 09/10/14 15:29, Joe.C wrote
> >>> @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> >>>  
> >>>     gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
> >>>  
> >>> -   if (of_property_read_u32(node, "arm,routable-irqs",
> >>> +   if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
> >>> +           of_find_property(node, "arm,irq-domain-hierarchy", NULL))
> >>> +           gic->domain = irq_domain_add_linear(node, gic_irqs,
> >>> +                                   &gic_irq_domain_hierarchy_ops, gic);
> >>
> >> I really think that looking for a property is the wrong thing to do. If
> >> "node" is non-NULL, then we're pretty sure that we're initializing from
> >> DT, and that a pure linear domain should be the right thing, leaving the
> >> legacy stuff for the few non-DT platforms that are still around.
> >>
> >> Thanks,
> >>
> >>      M.
> > 
> > The only reason I introduce "arm,irq-domain-hierarchy" property is
> > trying to keep original behavior when hierarchy irq domain is not used.
> > Without this, when a board init GIC with DT, all driver will have to use
> > devicetree. I'm not sure we want to break things like this.
> 
> I don't think we want to support a "middle of the road" setup, where the
> GIC is probed by DT, but some devices have hardcoded interrupts.

Agreed. We should work on making GIC DT-only by converting the few remaining
users instead, and certainly should not add any new board files that
might use the domain hierarchy code.

	Arnd

Re: [PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Arnd Bergmann]

On Monday 13 October 2014 13:10:32 Marc Zyngier wrote:
> On 13/10/14 11:43, Joe.C wrote:
> > On Thu, 2014-10-09 at 17:59 +0100, Marc Zyngier wrote:
> >> On 09/10/14 15:29, Joe.C wrote
> >>> @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> >>>  
> >>>     gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
> >>>  
> >>> -   if (of_property_read_u32(node, "arm,routable-irqs",
> >>> +   if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
> >>> +           of_find_property(node, "arm,irq-domain-hierarchy", NULL))
> >>> +           gic->domain = irq_domain_add_linear(node, gic_irqs,
> >>> +                                   &gic_irq_domain_hierarchy_ops, gic);
> >>
> >> I really think that looking for a property is the wrong thing to do. If
> >> "node" is non-NULL, then we're pretty sure that we're initializing from
> >> DT, and that a pure linear domain should be the right thing, leaving the
> >> legacy stuff for the few non-DT platforms that are still around.
> >>
> >> Thanks,
> >>
> >>      M.
> > 
> > The only reason I introduce "arm,irq-domain-hierarchy" property is
> > trying to keep original behavior when hierarchy irq domain is not used.
> > Without this, when a board init GIC with DT, all driver will have to use
> > devicetree. I'm not sure we want to break things like this.
> 
> I don't think we want to support a "middle of the road" setup, where the
> GIC is probed by DT, but some devices have hardcoded interrupts.

Agreed. We should work on making GIC DT-only by converting the few remaining
users instead, and certainly should not add any new board files that
might use the domain hierarchy code.

	Arnd

[PATCH v3 3/7] irqchip: gic: Support hierarchy irq domain.

[Arnd Bergmann]

On Monday 13 October 2014 13:10:32 Marc Zyngier wrote:
> On 13/10/14 11:43, Joe.C wrote:
> > On Thu, 2014-10-09 at 17:59 +0100, Marc Zyngier wrote:
> >> On 09/10/14 15:29, Joe.C wrote
> >>> @@ -952,7 +988,11 @@ void __init gic_init_bases(unsigned int gic_nr, int irq_start,
> >>>  
> >>>     gic_irqs -= hwirq_base; /* calculate # of irqs to allocate */
> >>>  
> >>> -   if (of_property_read_u32(node, "arm,routable-irqs",
> >>> +   if (IS_ENABLED(CONFIG_IRQ_DOMAIN_HIERARCHY) &&
> >>> +           of_find_property(node, "arm,irq-domain-hierarchy", NULL))
> >>> +           gic->domain = irq_domain_add_linear(node, gic_irqs,
> >>> +                                   &gic_irq_domain_hierarchy_ops, gic);
> >>
> >> I really think that looking for a property is the wrong thing to do. If
> >> "node" is non-NULL, then we're pretty sure that we're initializing from
> >> DT, and that a pure linear domain should be the right thing, leaving the
> >> legacy stuff for the few non-DT platforms that are still around.
> >>
> >> Thanks,
> >>
> >>      M.
> > 
> > The only reason I introduce "arm,irq-domain-hierarchy" property is
> > trying to keep original behavior when hierarchy irq domain is not used.
> > Without this, when a board init GIC with DT, all driver will have to use
> > devicetree. I'm not sure we want to break things like this.
> 
> I don't think we want to support a "middle of the road" setup, where the
> GIC is probed by DT, but some devices have hardcoded interrupts.

Agreed. We should work on making GIC DT-only by converting the few remaining
users instead, and certainly should not add any new board files that
might use the domain hierarchy code.

	Arnd