[PATCH V2 1/7] irqchip/csky: Support csky,dh7k SOC intc driver

[PATCH V2 1/7] irqchip/csky: Support csky,dh7k SOC intc driver

[guoren]

From: Guo Ren <ren_guo@c-sky.com>

C-SKY dh7k SOC use simple APB interrupt controller and most of driver's
implementation codes could be reused in csky-apb-intc.c. So merge them
together.

Signed-off-by: Guo Ren <ren_guo@c-sky.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-csky-apb-intc.c | 95 +++++++++++++++++++++++++++++++++----
 1 file changed, 85 insertions(+), 10 deletions(-)

diff --git a/drivers/irqchip/irq-csky-apb-intc.c b/drivers/irqchip/irq-csky-apb-intc.c
index 5a2ec43..fcc5444 100644
--- a/drivers/irqchip/irq-csky-apb-intc.c
+++ b/drivers/irqchip/irq-csky-apb-intc.c
@@ -12,6 +12,7 @@
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <asm/irq.h>
+#include <asm/traps.h>
 
 #define INTC_IRQS		64
 
@@ -31,6 +32,17 @@
 #define GX_INTC_NMASK63_32	0x54
 #define GX_INTC_SOURCE		0x60
 
+#define DH_INTC_CLR		0x10
+#define DH_INTC_INIT		0x34
+#define DH_INTC_NMASK31_00	0x08
+#define DH_INTC_NMASK63_32	0x68
+#define DH_INTC_SOURCE31_00	0x14
+#define DH_INTC_SOURCE63_32	0x6c
+#define DH_INTC_EDGE31_00	0x00
+#define DH_INTC_EDGE63_32	0x60
+#define DH_INTC_POLL31_00	0x04
+#define DH_INTC_POLL63_32	0x64
+
 static void __iomem *reg_base;
 static struct irq_domain *root_domain;
 
@@ -58,15 +70,21 @@ static void irq_ck_mask_set_bit(struct irq_data *d)
 }
 
 static void __init ck_set_gc(struct device_node *node, void __iomem *reg_base,
-			     u32 mask_reg, u32 irq_base)
+				u32 en_off, u32 mask_off, u32 irq_base)
 {
 	struct irq_chip_generic *gc;
 
 	gc = irq_get_domain_generic_chip(root_domain, irq_base);
 	gc->reg_base = reg_base;
-	gc->chip_types[0].regs.mask = mask_reg;
-	gc->chip_types[0].chip.irq_mask = irq_gc_mask_clr_bit;
-	gc->chip_types[0].chip.irq_unmask = irq_gc_mask_set_bit;
+	if (en_off) {
+		gc->chip_types[0].regs.mask = en_off;
+		gc->chip_types[0].chip.irq_mask = irq_gc_mask_clr_bit;
+		gc->chip_types[0].chip.irq_unmask = irq_gc_mask_set_bit;
+	} else if (mask_off) {
+		gc->chip_types[0].regs.mask = mask_off;
+		gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit;
+		gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit;
+	}
 
 	if (of_find_property(node, "csky,support-pulse-signal", NULL))
 		gc->chip_types[0].chip.irq_unmask = irq_ck_mask_set_bit;
@@ -183,8 +201,8 @@ gx_intc_init(struct device_node *node, struct device_node *parent)
 
 	setup_irq_channel(0x03020100, reg_base + GX_INTC_SOURCE);
 
-	ck_set_gc(node, reg_base, GX_INTC_NEN31_00, 0);
-	ck_set_gc(node, reg_base, GX_INTC_NEN63_32, 32);
+	ck_set_gc(node, reg_base, GX_INTC_NEN31_00, 0, 0);
+	ck_set_gc(node, reg_base, GX_INTC_NEN63_32, 0, 32);
 
 	set_handle_irq(gx_irq_handler);
 
@@ -192,6 +210,63 @@ gx_intc_init(struct device_node *node, struct device_node *parent)
 }
 IRQCHIP_DECLARE(csky_gx6605s_intc, "csky,gx6605s-intc", gx_intc_init);
 
+static void dh_irq_handler(struct pt_regs *regs)
+{
+	u32 tmp;
+	unsigned long vector = (mfcr("psr") >> 16) & 0xff;
+
+	tmp = readl(reg_base + DH_INTC_CLR);
+	tmp |= BIT(2);
+	writel(tmp, reg_base + DH_INTC_CLR);
+
+	handle_domain_irq(root_domain, vector - 32, regs);
+}
+
+extern void csky_irq(void);
+
+static int __init
+dh_intc_init(struct device_node *node, struct device_node *parent)
+{
+	int ret, i;
+
+	ret = ck_intc_init_comm(node, parent);
+	if (ret)
+		return ret;
+
+	/* set default mode */
+	writel(0xffffffff, reg_base + DH_INTC_EDGE31_00);
+	writel(0xffffffff, reg_base + DH_INTC_EDGE63_32);
+	writel(0xffffffff, reg_base + DH_INTC_POLL31_00);
+	writel(0xffffffff, reg_base + DH_INTC_POLL63_32);
+
+	writel(BIT(1) | BIT(6), reg_base + DH_INTC_INIT);
+
+	/* Setup  0-31 channel slots */
+	for (i = 0; i < INTC_IRQS/2; i += 4)
+		writel(build_channel_val(i, 0x03020100) + 0x40404040,
+				reg_base + DH_INTC_SOURCE31_00 + i);
+
+	/* Setup 32-63 channel slots */
+	for (i = 0; i < INTC_IRQS/2; i += 4)
+		writel(build_channel_val(i, 0x03020100) + 0x40404040,
+				reg_base + DH_INTC_SOURCE63_32 + i);
+
+	/* mask all interrrupts */
+	writel(0xffffffff, reg_base + DH_INTC_NMASK31_00);
+	writel(0xffffffff, reg_base + DH_INTC_NMASK63_32);
+
+	ck_set_gc(node, reg_base, 0, DH_INTC_NMASK31_00, 0);
+	ck_set_gc(node, reg_base, 0, DH_INTC_NMASK63_32, 32);
+
+	for (i = 32; i < 128; i++)
+		VEC_INIT(i, csky_irq);
+
+	set_handle_irq(dh_irq_handler);
+
+	return 0;
+}
+IRQCHIP_DECLARE(csky_dh7k_intc, "csky,dh7k-intc", dh_intc_init);
+
 /*
  * C-SKY simple 64 irqs interrupt controller, dual-together could support 128
  * irqs.
@@ -243,8 +318,8 @@ ck_intc_init(struct device_node *node, struct device_node *parent)
 	/* Enable irq intc */
 	writel(BIT(31), reg_base + CK_INTC_ICR);
 
-	ck_set_gc(node, reg_base, CK_INTC_NEN31_00, 0);
-	ck_set_gc(node, reg_base, CK_INTC_NEN63_32, 32);
+	ck_set_gc(node, reg_base, CK_INTC_NEN31_00, 0, 0);
+	ck_set_gc(node, reg_base, CK_INTC_NEN63_32, 0, 32);
 
 	setup_irq_channel(0x00010203, reg_base + CK_INTC_SOURCE);
 
@@ -270,8 +345,8 @@ ck_dual_intc_init(struct device_node *node, struct device_node *parent)
 	writel(0, reg_base + CK_INTC_NEN31_00 + CK_INTC_DUAL_BASE);
 	writel(0, reg_base + CK_INTC_NEN63_32 + CK_INTC_DUAL_BASE);
 
-	ck_set_gc(node, reg_base + CK_INTC_DUAL_BASE, CK_INTC_NEN31_00, 64);
-	ck_set_gc(node, reg_base + CK_INTC_DUAL_BASE, CK_INTC_NEN63_32, 96);
+	ck_set_gc(node, reg_base + CK_INTC_DUAL_BASE, CK_INTC_NEN31_00, 0, 64);
+	ck_set_gc(node, reg_base + CK_INTC_DUAL_BASE, CK_INTC_NEN63_32, 0, 96);
 
 	setup_irq_channel(0x00010203,
 			  reg_base + CK_INTC_SOURCE + CK_INTC_DUAL_BASE);
-- 
2.7.4

[PATCH V2 2/7] dt-bindings: csky,apb-intc: Add dh7k SOC support

[guoren]

From: Guo Ren <ren_guo@c-sky.com>

Add C-SKY dh7k SOC description.

Signed-off-by: Guo Ren <ren_guo@c-sky.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
---
 Documentation/devicetree/bindings/interrupt-controller/csky,apb-intc.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/csky,apb-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/csky,apb-intc.txt
index 44286dc..63bc9dc 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/csky,apb-intc.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/csky,apb-intc.txt
@@ -23,6 +23,7 @@ intc node bindings definition
 		Definition: must be "csky,apb-intc"
 				    "csky,dual-apb-intc"
 				    "csky,gx6605s-intc"
+				    "csky,dh7k-intc"
 	- #interrupt-cells
 		Usage: required
 		Value type: <u32>
-- 
2.7.4

[PATCH V2 3/7] irqchip/csky: Optimize remove unnecessary loop irq handle

[guoren]

From: Guo Ren <ren_guo@c-sky.com>

Here is the previous interrupt processing flow:
while (pending) {
^^^^^^^^^^^^^^^ It's unnecessary!
	get irq
	handle_level/fasteoi_irq {
		mask irq
		driver irq handler
		unmask irq
	}
	irq_exit {
		preempt_count_sub(HARDIRQ_OFFSET);
		if (!in_interrupt() && local_softirq_pending())
			invoke_softirq();
Because:		^^^^^^^^^^^^^^^^ linux enable irq Here!
	}
}

Because linux enable the irq in irq_exit() before ret, we needn't loop
read pending register again for next irq which is done during irq_exit().

Signed-off-by: Guo Ren <ren_guo@c-sky.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-csky-apb-intc.c | 36 ++++++++++++++++++------------------
 drivers/irqchip/irq-csky-mpintc.c   |  8 ++------
 2 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/drivers/irqchip/irq-csky-apb-intc.c b/drivers/irqchip/irq-csky-apb-intc.c
index fcc5444..ae4c59b 100644
--- a/drivers/irqchip/irq-csky-apb-intc.c
+++ b/drivers/irqchip/irq-csky-apb-intc.c
@@ -150,7 +150,7 @@ ck_intc_init_comm(struct device_node *node, struct device_node *parent)
 	return 0;
 }
 
-static inline bool handle_irq_perbit(struct pt_regs *regs, u32 hwirq,
+static inline bool handle_irq_onebit(struct pt_regs *regs, u32 hwirq,
 				     u32 irq_base)
 {
 	if (hwirq == 0)
@@ -166,16 +166,15 @@ static void gx_irq_handler(struct pt_regs *regs)
 {
 	bool ret;
 
-retry:
-	ret = handle_irq_perbit(regs,
+	ret = handle_irq_onebit(regs,
 			readl(reg_base + GX_INTC_PEN63_32), 32);
 	if (ret)
-		goto retry;
+		return;
 
-	ret = handle_irq_perbit(regs,
+	ret = handle_irq_onebit(regs,
 			readl(reg_base + GX_INTC_PEN31_00), 0);
-	if (ret)
-		goto retry;
+	if (!ret)
+		pr_err("%s: none irq pending!\n", __func__);
 }
 
 static int __init
@@ -277,29 +276,30 @@ static void ck_irq_handler(struct pt_regs *regs)
 	void __iomem *reg_pen_lo = reg_base + CK_INTC_PEN31_00;
 	void __iomem *reg_pen_hi = reg_base + CK_INTC_PEN63_32;
 
-retry:
 	/* handle 0 - 63 irqs */
-	ret = handle_irq_perbit(regs, readl(reg_pen_hi), 32);
+	ret = handle_irq_onebit(regs, readl(reg_pen_hi), 32);
 	if (ret)
-		goto retry;
+		return;
 
-	ret = handle_irq_perbit(regs, readl(reg_pen_lo), 0);
+	ret = handle_irq_onebit(regs, readl(reg_pen_lo), 0);
 	if (ret)
-		goto retry;
+		return;
 
-	if (nr_irq == INTC_IRQS)
+	if (nr_irq == INTC_IRQS) {
+		pr_err("%s: none irq pending!\n", __func__);
 		return;
+	}
 
 	/* handle 64 - 127 irqs */
-	ret = handle_irq_perbit(regs,
+	ret = handle_irq_onebit(regs,
 			readl(reg_pen_hi + CK_INTC_DUAL_BASE), 96);
 	if (ret)
-		goto retry;
+		return;
 
-	ret = handle_irq_perbit(regs,
+	ret = handle_irq_onebit(regs,
 			readl(reg_pen_lo + CK_INTC_DUAL_BASE), 64);
-	if (ret)
-		goto retry;
+	if (!ret)
+		pr_err("%s: none irq pending!\n", __func__);
 }
 
 static int __init
diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
index c67c961..99d3f3f 100644
--- a/drivers/irqchip/irq-csky-mpintc.c
+++ b/drivers/irqchip/irq-csky-mpintc.c
@@ -33,7 +33,6 @@ static void __iomem *INTCL_base;
 
 #define INTCL_PICTLR	0x0
 #define INTCL_SIGR	0x60
-#define INTCL_HPPIR	0x68
 #define INTCL_RDYIR	0x6c
 #define INTCL_SENR	0xa0
 #define INTCL_CENR	0xa4
@@ -45,11 +44,8 @@ static void csky_mpintc_handler(struct pt_regs *regs)
 {
 	void __iomem *reg_base = this_cpu_read(intcl_reg);
 
-	do {
-		handle_domain_irq(root_domain,
-				  readl_relaxed(reg_base + INTCL_RDYIR),
-				  regs);
-	} while (readl_relaxed(reg_base + INTCL_HPPIR) & BIT(31));
+	handle_domain_irq(root_domain,
+		readl_relaxed(reg_base + INTCL_RDYIR), regs);
 }
 
 static void csky_mpintc_enable(struct irq_data *d)
-- 
2.7.4

[PATCH V2 4/7] irqchip/irq-csky-mpintc: Add triger type and priority

[guoren]

From: Guo Ren <ren_guo@c-sky.com>

Support 4 triger types:
 - IRQ_TYPE_LEVEL_HIGH
 - IRQ_TYPE_LEVEL_LOW
 - IRQ_TYPE_EDGE_RISING
 - IRQ_TYPE_EDGE_FALLING

Support 0-255 priority setting for each irq.

Changelog:
 - Fixup this_cpu_read() preempted problem.
 - Optimize the coding style.

Signed-off-by: Guo Ren <ren_guo@c-sky.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-csky-mpintc.c | 105 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 104 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
index 99d3f3f..07a3752 100644
--- a/drivers/irqchip/irq-csky-mpintc.c
+++ b/drivers/irqchip/irq-csky-mpintc.c
@@ -17,6 +17,7 @@
 #include <asm/reg_ops.h>
 
 static struct irq_domain *root_domain;
+
 static void __iomem *INTCG_base;
 static void __iomem *INTCL_base;
 
@@ -29,9 +30,12 @@ static void __iomem *INTCL_base;
 
 #define INTCG_ICTLR	0x0
 #define INTCG_CICFGR	0x100
+#define INTCG_CIPRTR	0x200
 #define INTCG_CIDSTR	0x1000
 
 #define INTCL_PICTLR	0x0
+#define INTCL_CFGR	0x14
+#define INTCL_PRTR	0x20
 #define INTCL_SIGR	0x60
 #define INTCL_RDYIR	0x6c
 #define INTCL_SENR	0xa0
@@ -40,6 +44,51 @@ static void __iomem *INTCL_base;
 
 static DEFINE_PER_CPU(void __iomem *, intcl_reg);
 
+static unsigned long *__trigger;
+static unsigned long *__priority;
+
+#define IRQ_OFFSET(irq) ((irq < COMM_IRQ_BASE) ? irq : (irq - COMM_IRQ_BASE))
+
+#define TRIG_BYTE_OFFSET(i)	((((i) * 2) / 32) * 4)
+#define TRIG_BIT_OFFSET(i)	 (((i) * 2) % 32)
+
+#define PRI_BYTE_OFFSET(i)	((((i) * 8) / 32) * 4)
+#define PRI_BIT_OFFSET(i)	 (((i) * 8) % 32)
+
+#define TRIG_VAL(trigger, irq)	(trigger << TRIG_BIT_OFFSET(IRQ_OFFSET(irq)))
+#define TRIG_VAL_MSK(irq)	    (~(3 << TRIG_BIT_OFFSET(IRQ_OFFSET(irq))))
+#define PRI_VAL(priority, irq)	(priority << PRI_BIT_OFFSET(IRQ_OFFSET(irq)))
+#define PRI_VAL_MSK(irq)	  (~(0xff << PRI_BIT_OFFSET(IRQ_OFFSET(irq))))
+
+#define TRIG_BASE(irq) \
+	(TRIG_BYTE_OFFSET(IRQ_OFFSET(irq)) + ((irq < COMM_IRQ_BASE) ? \
+	(this_cpu_read(intcl_reg) + INTCL_CFGR) : (INTCG_base + INTCG_CICFGR)))
+
+#define PRI_BASE(irq) \
+	(PRI_BYTE_OFFSET(IRQ_OFFSET(irq)) + ((irq < COMM_IRQ_BASE) ? \
+	(this_cpu_read(intcl_reg) + INTCL_PRTR) : (INTCG_base + INTCG_CIPRTR)))
+
+static DEFINE_SPINLOCK(setup_lock);
+static void setup_trigger_priority(unsigned long irq, unsigned long trigger,
+				   unsigned long priority)
+{
+	unsigned int tmp;
+
+	spin_lock(&setup_lock);
+
+	/* setup trigger */
+	tmp = readl_relaxed(TRIG_BASE(irq)) & TRIG_VAL_MSK(irq);
+
+	writel_relaxed(tmp | TRIG_VAL(trigger, irq), TRIG_BASE(irq));
+
+	/* setup priority */
+	tmp = readl_relaxed(PRI_BASE(irq)) & PRI_VAL_MSK(irq);
+
+	writel_relaxed(tmp | PRI_VAL(priority, irq), PRI_BASE(irq));
+
+	spin_unlock(&setup_lock);
+}
+
 static void csky_mpintc_handler(struct pt_regs *regs)
 {
 	void __iomem *reg_base = this_cpu_read(intcl_reg);
@@ -52,6 +101,9 @@ static void csky_mpintc_enable(struct irq_data *d)
 {
 	void __iomem *reg_base = this_cpu_read(intcl_reg);
 
+	setup_trigger_priority(d->hwirq, __trigger[d->hwirq],
+				 __priority[d->hwirq]);
+
 	writel_relaxed(d->hwirq, reg_base + INTCL_SENR);
 }
 
@@ -69,6 +121,28 @@ static void csky_mpintc_eoi(struct irq_data *d)
 	writel_relaxed(d->hwirq, reg_base + INTCL_CACR);
 }
 
+static int csky_mpintc_set_type(struct irq_data *d, unsigned int type)
+{
+	switch (type & IRQ_TYPE_SENSE_MASK) {
+	case IRQ_TYPE_LEVEL_HIGH:
+		__trigger[d->hwirq] = 0;
+		break;
+	case IRQ_TYPE_LEVEL_LOW:
+		__trigger[d->hwirq] = 1;
+		break;
+	case IRQ_TYPE_EDGE_RISING:
+		__trigger[d->hwirq] = 2;
+		break;
+	case IRQ_TYPE_EDGE_FALLING:
+		__trigger[d->hwirq] = 3;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 #ifdef CONFIG_SMP
 static int csky_irq_set_affinity(struct irq_data *d,
 				 const struct cpumask *mask_val,
@@ -101,6 +175,7 @@ static struct irq_chip csky_irq_chip = {
 	.irq_eoi	= csky_mpintc_eoi,
 	.irq_enable	= csky_mpintc_enable,
 	.irq_disable	= csky_mpintc_disable,
+	.irq_set_type	= csky_mpintc_set_type,
 #ifdef CONFIG_SMP
 	.irq_set_affinity = csky_irq_set_affinity,
 #endif
@@ -121,9 +196,29 @@ static int csky_irqdomain_map(struct irq_domain *d, unsigned int irq,
 	return 0;
 }
 
+static int csky_irq_domain_xlate_cells(struct irq_domain *d,
+		struct device_node *ctrlr, const u32 *intspec,
+		unsigned int intsize, unsigned long *out_hwirq,
+		unsigned int *out_type)
+{
+	if (WARN_ON(intsize < 1))
+		return -EINVAL;
+
+	*out_hwirq = intspec[0];
+	if (intsize > 1)
+		*out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
+	else
+		*out_type = IRQ_TYPE_NONE;
+
+	if (intsize > 2)
+		__priority[*out_hwirq] = intspec[2];
+
+	return 0;
+}
+
 static const struct irq_domain_ops csky_irqdomain_ops = {
 	.map	= csky_irqdomain_map,
-	.xlate	= irq_domain_xlate_onecell,
+	.xlate	= csky_irq_domain_xlate_cells,
 };
 
 #ifdef CONFIG_SMP
@@ -157,6 +252,14 @@ csky_mpintc_init(struct device_node *node, struct device_node *parent)
 	if (ret < 0)
 		nr_irq = INTC_IRQS;
 
+	__priority = kcalloc(nr_irq, sizeof(unsigned long), GFP_KERNEL);
+	if (__priority == NULL)
+		return -ENXIO;
+
+	__trigger  = kcalloc(nr_irq, sizeof(unsigned long), GFP_KERNEL);
+	if (__trigger == NULL)
+		return -ENXIO;
+
 	if (INTCG_base == NULL) {
 		INTCG_base = ioremap(mfcr("cr<31, 14>"),
 				     INTCL_SIZE*nr_cpu_ids + INTCG_SIZE);
-- 
2.7.4

[PATCH V2 5/7] dt-bindings: interrupt-controller: Update csky mpintc

[guoren]

From: Guo Ren <ren_guo@c-sky.com>

Add trigger type and priority setting for csky,mpintc.

Changelog:
 - change #interrupt-cells to <3>

Signed-off-by: Guo Ren <ren_guo@c-sky.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
---
 .../bindings/interrupt-controller/csky,mpintc.txt   | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt b/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt
index ab921f1..dccd913 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt
@@ -6,11 +6,18 @@ C-SKY Multi-processors Interrupt Controller is designed for ck807/ck810/ck860
 SMP soc, and it also could be used in non-SMP system.
 
 Interrupt number definition:
-
   0-15  : software irq, and we use 15 as our IPI_IRQ.
  16-31  : private  irq, and we use 16 as the co-processor timer.
  31-1024: common irq for soc ip.
 
+Interrupt triger mode:
+ IRQ_TYPE_LEVEL_HIGH (default)
+ IRQ_TYPE_LEVEL_LOW
+ IRQ_TYPE_EDGE_RISING
+ IRQ_TYPE_EDGE_FALLING
+
+Interrupt priority range: 0-255
+
 =============================
 intc node bindings definition
 =============================
@@ -26,15 +33,21 @@ intc node bindings definition
 	- #interrupt-cells
 		Usage: required
 		Value type: <u32>
-		Definition: must be <1>
+		Definition: <3>
 	- interrupt-controller:
 		Usage: required
 
-Examples:
+Examples: ("interrupts = <irq_num IRQ_TYPE_XXX priority>")
 ---------
 
 	intc: interrupt-controller {
 		compatible = "csky,mpintc";
-		#interrupt-cells = <1>;
+		#interrupt-cells = <3>;
 		interrupt-controller;
 	};
+
+	device: device-example {
+		...
+		interrupts = <34 IRQ_TYPE_EDGE_RISING 254>;
+		interrupt-parent = <&intc>;
+	};
-- 
2.7.4

[PATCH V2 6/7] dt-bindings: csky,apb-intc: Add vector irq mode

[guoren]

From: Guo Ren <ren_guo@c-sky.com>

Add csky,support-vector-irq for irq-csky-apb-intc.

Signed-off-by: Guo Ren <ren_guo@c-sky.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Rob Herring <robh+dt@kernel.org>
---
 .../devicetree/bindings/interrupt-controller/csky,apb-intc.txt         | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/csky,apb-intc.txt b/Documentation/devicetree/bindings/interrupt-controller/csky,apb-intc.txt
index 63bc9dc..0f9fa88 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/csky,apb-intc.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/csky,apb-intc.txt
@@ -37,6 +37,9 @@ intc node bindings definition
 	- csky,support-pulse-signal:
 		Usage: select
 		Description: to support pulse signal flag
+	- csky,support-vector-irq:
+		Usage: select
+		Description: to support vector irq mode
 
 Examples:
 ---------
-- 
2.7.4

[PATCH V2 7/7] irqchip/csky: Add support-vector-irq for apb-intc

[guoren]

From: Guo Ren <ren_guo@c-sky.com>

Some socs don't provide pending registers, so we must make the intc
working at vector mode.
Add csky,support-vector-irq in dts.

Signed-off-by: Guo Ren <ren_guo@c-sky.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-csky-apb-intc.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-csky-apb-intc.c b/drivers/irqchip/irq-csky-apb-intc.c
index ae4c59b..14ef206 100644
--- a/drivers/irqchip/irq-csky-apb-intc.c
+++ b/drivers/irqchip/irq-csky-apb-intc.c
@@ -302,10 +302,17 @@ static void ck_irq_handler(struct pt_regs *regs)
 		pr_err("%s: none irq pending!\n", __func__);
 }
 
+static void ck_vec_irq_handler(struct pt_regs *regs)
+{
+	unsigned long vector = (mfcr("psr") >> 16) & 0xff;
+
+	handle_domain_irq(root_domain, vector - 32, regs);
+}
+
 static int __init
 ck_intc_init(struct device_node *node, struct device_node *parent)
 {
-	int ret;
+	int ret, i;
 
 	ret = ck_intc_init_comm(node, parent);
 	if (ret)
@@ -323,7 +330,16 @@ ck_intc_init(struct device_node *node, struct device_node *parent)
 
 	setup_irq_channel(0x00010203, reg_base + CK_INTC_SOURCE);
 
-	set_handle_irq(ck_irq_handler);
+	if (of_find_property(node, "csky,support-vector-irq", NULL)) {
+		set_handle_irq(ck_vec_irq_handler);
+
+		for (i = 32; i < 128; i++)
+			VEC_INIT(i, csky_irq);
+
+		writel(0, reg_base + CK_INTC_ICR);
+	} else {
+		set_handle_irq(ck_irq_handler);
+	}
 
 	return 0;
 }
-- 
2.7.4

Re: [PATCH V2 5/7] dt-bindings: interrupt-controller: Update csky mpintc

[Marc Zyngier]

On Mon, 18 Feb 2019 10:04:41 +0800
guoren@kernel.org wrote:

> From: Guo Ren <ren_guo@c-sky.com>
> 
> Add trigger type and priority setting for csky,mpintc.
> 
> Changelog:
>  - change #interrupt-cells to <3>
> 
> Signed-off-by: Guo Ren <ren_guo@c-sky.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> ---
>  .../bindings/interrupt-controller/csky,mpintc.txt   | 21 +++++++++++++++++----
>  1 file changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt b/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt
> index ab921f1..dccd913 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt
> +++ b/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt
> @@ -6,11 +6,18 @@ C-SKY Multi-processors Interrupt Controller is designed for ck807/ck810/ck860
>  SMP soc, and it also could be used in non-SMP system.
>  
>  Interrupt number definition:
> -
>    0-15  : software irq, and we use 15 as our IPI_IRQ.
>   16-31  : private  irq, and we use 16 as the co-processor timer.
>   31-1024: common irq for soc ip.
>  
> +Interrupt triger mode:
> + IRQ_TYPE_LEVEL_HIGH (default)
> + IRQ_TYPE_LEVEL_LOW
> + IRQ_TYPE_EDGE_RISING
> + IRQ_TYPE_EDGE_FALLING
> +
> +Interrupt priority range: 0-255
> +
>  =============================
>  intc node bindings definition
>  =============================
> @@ -26,15 +33,21 @@ intc node bindings definition
>  	- #interrupt-cells
>  		Usage: required
>  		Value type: <u32>
> -		Definition: must be <1>
> +		Definition: <3>

This seem to be invalidating all existing DTs. Is this an acceptable
thing to do? It will require an Ack from a DT maintainer.

>  	- interrupt-controller:
>  		Usage: required
>  
> -Examples:
> +Examples: ("interrupts = <irq_num IRQ_TYPE_XXX priority>")
>  ---------
>  
>  	intc: interrupt-controller {
>  		compatible = "csky,mpintc";
> -		#interrupt-cells = <1>;
> +		#interrupt-cells = <3>;
>  		interrupt-controller;
>  	};
> +
> +	device: device-example {
> +		...
> +		interrupts = <34 IRQ_TYPE_EDGE_RISING 254>;
> +		interrupt-parent = <&intc>;
> +	};

Thanks,

	M.
-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH V2 4/7] irqchip/irq-csky-mpintc: Add triger type and priority

[Marc Zyngier]

On Mon, 18 Feb 2019 10:04:40 +0800
guoren@kernel.org wrote:

> From: Guo Ren <ren_guo@c-sky.com>
> 
> Support 4 triger types:
>  - IRQ_TYPE_LEVEL_HIGH
>  - IRQ_TYPE_LEVEL_LOW
>  - IRQ_TYPE_EDGE_RISING
>  - IRQ_TYPE_EDGE_FALLING
> 
> Support 0-255 priority setting for each irq.
> 
> Changelog:
>  - Fixup this_cpu_read() preempted problem.
>  - Optimize the coding style.
> 
> Signed-off-by: Guo Ren <ren_guo@c-sky.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/irqchip/irq-csky-mpintc.c | 105 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 104 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
> index 99d3f3f..07a3752 100644
> --- a/drivers/irqchip/irq-csky-mpintc.c
> +++ b/drivers/irqchip/irq-csky-mpintc.c
> @@ -17,6 +17,7 @@
>  #include <asm/reg_ops.h>
>  
>  static struct irq_domain *root_domain;
> +
>  static void __iomem *INTCG_base;
>  static void __iomem *INTCL_base;
>  
> @@ -29,9 +30,12 @@ static void __iomem *INTCL_base;
>  
>  #define INTCG_ICTLR	0x0
>  #define INTCG_CICFGR	0x100
> +#define INTCG_CIPRTR	0x200
>  #define INTCG_CIDSTR	0x1000
>  
>  #define INTCL_PICTLR	0x0
> +#define INTCL_CFGR	0x14
> +#define INTCL_PRTR	0x20
>  #define INTCL_SIGR	0x60
>  #define INTCL_RDYIR	0x6c
>  #define INTCL_SENR	0xa0
> @@ -40,6 +44,51 @@ static void __iomem *INTCL_base;
>  
>  static DEFINE_PER_CPU(void __iomem *, intcl_reg);
>  
> +static unsigned long *__trigger;
> +static unsigned long *__priority;
> +
> +#define IRQ_OFFSET(irq) ((irq < COMM_IRQ_BASE) ? irq : (irq - COMM_IRQ_BASE))
> +
> +#define TRIG_BYTE_OFFSET(i)	((((i) * 2) / 32) * 4)
> +#define TRIG_BIT_OFFSET(i)	 (((i) * 2) % 32)
> +
> +#define PRI_BYTE_OFFSET(i)	((((i) * 8) / 32) * 4)
> +#define PRI_BIT_OFFSET(i)	 (((i) * 8) % 32)
> +
> +#define TRIG_VAL(trigger, irq)	(trigger << TRIG_BIT_OFFSET(IRQ_OFFSET(irq)))
> +#define TRIG_VAL_MSK(irq)	    (~(3 << TRIG_BIT_OFFSET(IRQ_OFFSET(irq))))
> +#define PRI_VAL(priority, irq)	(priority << PRI_BIT_OFFSET(IRQ_OFFSET(irq)))
> +#define PRI_VAL_MSK(irq)	  (~(0xff << PRI_BIT_OFFSET(IRQ_OFFSET(irq))))
> +
> +#define TRIG_BASE(irq) \
> +	(TRIG_BYTE_OFFSET(IRQ_OFFSET(irq)) + ((irq < COMM_IRQ_BASE) ? \
> +	(this_cpu_read(intcl_reg) + INTCL_CFGR) : (INTCG_base + INTCG_CICFGR)))
> +
> +#define PRI_BASE(irq) \
> +	(PRI_BYTE_OFFSET(IRQ_OFFSET(irq)) + ((irq < COMM_IRQ_BASE) ? \
> +	(this_cpu_read(intcl_reg) + INTCL_PRTR) : (INTCG_base + INTCG_CIPRTR)))
> +
> +static DEFINE_SPINLOCK(setup_lock);
> +static void setup_trigger_priority(unsigned long irq, unsigned long trigger,
> +				   unsigned long priority)
> +{
> +	unsigned int tmp;
> +
> +	spin_lock(&setup_lock);
> +
> +	/* setup trigger */
> +	tmp = readl_relaxed(TRIG_BASE(irq)) & TRIG_VAL_MSK(irq);
> +
> +	writel_relaxed(tmp | TRIG_VAL(trigger, irq), TRIG_BASE(irq));
> +
> +	/* setup priority */
> +	tmp = readl_relaxed(PRI_BASE(irq)) & PRI_VAL_MSK(irq);
> +
> +	writel_relaxed(tmp | PRI_VAL(priority, irq), PRI_BASE(irq));
> +
> +	spin_unlock(&setup_lock);
> +}
> +
>  static void csky_mpintc_handler(struct pt_regs *regs)
>  {
>  	void __iomem *reg_base = this_cpu_read(intcl_reg);
> @@ -52,6 +101,9 @@ static void csky_mpintc_enable(struct irq_data *d)
>  {
>  	void __iomem *reg_base = this_cpu_read(intcl_reg);
>  
> +	setup_trigger_priority(d->hwirq, __trigger[d->hwirq],
> +				 __priority[d->hwirq]);
> +
>  	writel_relaxed(d->hwirq, reg_base + INTCL_SENR);
>  }
>  
> @@ -69,6 +121,28 @@ static void csky_mpintc_eoi(struct irq_data *d)
>  	writel_relaxed(d->hwirq, reg_base + INTCL_CACR);
>  }
>  
> +static int csky_mpintc_set_type(struct irq_data *d, unsigned int type)
> +{
> +	switch (type & IRQ_TYPE_SENSE_MASK) {
> +	case IRQ_TYPE_LEVEL_HIGH:
> +		__trigger[d->hwirq] = 0;
> +		break;
> +	case IRQ_TYPE_LEVEL_LOW:
> +		__trigger[d->hwirq] = 1;
> +		break;
> +	case IRQ_TYPE_EDGE_RISING:
> +		__trigger[d->hwirq] = 2;
> +		break;
> +	case IRQ_TYPE_EDGE_FALLING:
> +		__trigger[d->hwirq] = 3;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  #ifdef CONFIG_SMP
>  static int csky_irq_set_affinity(struct irq_data *d,
>  				 const struct cpumask *mask_val,
> @@ -101,6 +175,7 @@ static struct irq_chip csky_irq_chip = {
>  	.irq_eoi	= csky_mpintc_eoi,
>  	.irq_enable	= csky_mpintc_enable,
>  	.irq_disable	= csky_mpintc_disable,
> +	.irq_set_type	= csky_mpintc_set_type,
>  #ifdef CONFIG_SMP
>  	.irq_set_affinity = csky_irq_set_affinity,
>  #endif
> @@ -121,9 +196,29 @@ static int csky_irqdomain_map(struct irq_domain *d, unsigned int irq,
>  	return 0;
>  }
>  
> +static int csky_irq_domain_xlate_cells(struct irq_domain *d,
> +		struct device_node *ctrlr, const u32 *intspec,
> +		unsigned int intsize, unsigned long *out_hwirq,
> +		unsigned int *out_type)
> +{
> +	if (WARN_ON(intsize < 1))
> +		return -EINVAL;
> +
> +	*out_hwirq = intspec[0];
> +	if (intsize > 1)
> +		*out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
> +	else
> +		*out_type = IRQ_TYPE_NONE;

What does IRQ_TYPE_NONE mean in this context? Shouldn't it actually be
whatever the HW defaults to? Or even better, whatever was expected in
the previous definition of the DT binding?

> +
> +	if (intsize > 2)
> +		__priority[*out_hwirq] = intspec[2];

And what is the used priority in this case?

> +
> +	return 0;
> +}
> +
>  static const struct irq_domain_ops csky_irqdomain_ops = {
>  	.map	= csky_irqdomain_map,
> -	.xlate	= irq_domain_xlate_onecell,
> +	.xlate	= csky_irq_domain_xlate_cells,
>  };
>  
>  #ifdef CONFIG_SMP
> @@ -157,6 +252,14 @@ csky_mpintc_init(struct device_node *node, struct device_node *parent)
>  	if (ret < 0)
>  		nr_irq = INTC_IRQS;
>  
> +	__priority = kcalloc(nr_irq, sizeof(unsigned long), GFP_KERNEL);
> +	if (__priority == NULL)
> +		return -ENXIO;
> +
> +	__trigger  = kcalloc(nr_irq, sizeof(unsigned long), GFP_KERNEL);
> +	if (__trigger == NULL)
> +		return -ENXIO;

Maybe you should consider initializing these arrays to something that
makes sense for the case where the DT doesn't carry this information
(which is 100% of the DTs up to this point).

> +
>  	if (INTCG_base == NULL) {
>  		INTCG_base = ioremap(mfcr("cr<31, 14>"),
>  				     INTCL_SIZE*nr_cpu_ids + INTCG_SIZE);


Not directly related to this patch: Please add a cover letter to you
patch series, and describe the goal of the whole series as well as the
changes you make along the way. It will definitely help the reviewers.

Thanks,

	M.
-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH V2 4/7] irqchip/irq-csky-mpintc: Add triger type and priority

[Guo Ren]

Thx Marc,

Sorry for late reply:

On Mon, Feb 18, 2019 at 02:38:23PM +0000, Marc Zyngier wrote:
> On Mon, 18 Feb 2019 10:04:40 +0800
> guoren@kernel.org wrote:
> 
> > From: Guo Ren <ren_guo@c-sky.com>
> > 
> > Support 4 triger types:
> >  - IRQ_TYPE_LEVEL_HIGH
> >  - IRQ_TYPE_LEVEL_LOW
> >  - IRQ_TYPE_EDGE_RISING
> >  - IRQ_TYPE_EDGE_FALLING
> > 
> > Support 0-255 priority setting for each irq.
> > 
> > Changelog:
> >  - Fixup this_cpu_read() preempted problem.
> >  - Optimize the coding style.
> > 
> > Signed-off-by: Guo Ren <ren_guo@c-sky.com>
> > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >  drivers/irqchip/irq-csky-mpintc.c | 105 +++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 104 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
> > index 99d3f3f..07a3752 100644
> > --- a/drivers/irqchip/irq-csky-mpintc.c
> > +++ b/drivers/irqchip/irq-csky-mpintc.c
> > @@ -17,6 +17,7 @@
> >  #include <asm/reg_ops.h>
> >  
> >  static struct irq_domain *root_domain;
> > +
> >  static void __iomem *INTCG_base;
> >  static void __iomem *INTCL_base;
> >  
> > @@ -29,9 +30,12 @@ static void __iomem *INTCL_base;
> >  
> >  #define INTCG_ICTLR	0x0
> >  #define INTCG_CICFGR	0x100
> > +#define INTCG_CIPRTR	0x200
> >  #define INTCG_CIDSTR	0x1000
> >  
> >  #define INTCL_PICTLR	0x0
> > +#define INTCL_CFGR	0x14
> > +#define INTCL_PRTR	0x20
> >  #define INTCL_SIGR	0x60
> >  #define INTCL_RDYIR	0x6c
> >  #define INTCL_SENR	0xa0
> > @@ -40,6 +44,51 @@ static void __iomem *INTCL_base;
> >  
> >  static DEFINE_PER_CPU(void __iomem *, intcl_reg);
> >  
> > +static unsigned long *__trigger;
> > +static unsigned long *__priority;
> > +
> > +#define IRQ_OFFSET(irq) ((irq < COMM_IRQ_BASE) ? irq : (irq - COMM_IRQ_BASE))
> > +
> > +#define TRIG_BYTE_OFFSET(i)	((((i) * 2) / 32) * 4)
> > +#define TRIG_BIT_OFFSET(i)	 (((i) * 2) % 32)
> > +
> > +#define PRI_BYTE_OFFSET(i)	((((i) * 8) / 32) * 4)
> > +#define PRI_BIT_OFFSET(i)	 (((i) * 8) % 32)
> > +
> > +#define TRIG_VAL(trigger, irq)	(trigger << TRIG_BIT_OFFSET(IRQ_OFFSET(irq)))
> > +#define TRIG_VAL_MSK(irq)	    (~(3 << TRIG_BIT_OFFSET(IRQ_OFFSET(irq))))
> > +#define PRI_VAL(priority, irq)	(priority << PRI_BIT_OFFSET(IRQ_OFFSET(irq)))
> > +#define PRI_VAL_MSK(irq)	  (~(0xff << PRI_BIT_OFFSET(IRQ_OFFSET(irq))))
> > +
> > +#define TRIG_BASE(irq) \
> > +	(TRIG_BYTE_OFFSET(IRQ_OFFSET(irq)) + ((irq < COMM_IRQ_BASE) ? \
> > +	(this_cpu_read(intcl_reg) + INTCL_CFGR) : (INTCG_base + INTCG_CICFGR)))
> > +
> > +#define PRI_BASE(irq) \
> > +	(PRI_BYTE_OFFSET(IRQ_OFFSET(irq)) + ((irq < COMM_IRQ_BASE) ? \
> > +	(this_cpu_read(intcl_reg) + INTCL_PRTR) : (INTCG_base + INTCG_CIPRTR)))
> > +
> > +static DEFINE_SPINLOCK(setup_lock);
> > +static void setup_trigger_priority(unsigned long irq, unsigned long trigger,
> > +				   unsigned long priority)
> > +{
> > +	unsigned int tmp;
> > +
> > +	spin_lock(&setup_lock);
> > +
> > +	/* setup trigger */
> > +	tmp = readl_relaxed(TRIG_BASE(irq)) & TRIG_VAL_MSK(irq);
> > +
> > +	writel_relaxed(tmp | TRIG_VAL(trigger, irq), TRIG_BASE(irq));
> > +
> > +	/* setup priority */
> > +	tmp = readl_relaxed(PRI_BASE(irq)) & PRI_VAL_MSK(irq);
> > +
> > +	writel_relaxed(tmp | PRI_VAL(priority, irq), PRI_BASE(irq));
> > +
> > +	spin_unlock(&setup_lock);
> > +}
> > +
> >  static void csky_mpintc_handler(struct pt_regs *regs)
> >  {
> >  	void __iomem *reg_base = this_cpu_read(intcl_reg);
> > @@ -52,6 +101,9 @@ static void csky_mpintc_enable(struct irq_data *d)
> >  {
> >  	void __iomem *reg_base = this_cpu_read(intcl_reg);
> >  
> > +	setup_trigger_priority(d->hwirq, __trigger[d->hwirq],
> > +				 __priority[d->hwirq]);
> > +
> >  	writel_relaxed(d->hwirq, reg_base + INTCL_SENR);
> >  }
> >  
> > @@ -69,6 +121,28 @@ static void csky_mpintc_eoi(struct irq_data *d)
> >  	writel_relaxed(d->hwirq, reg_base + INTCL_CACR);
> >  }
> >  
> > +static int csky_mpintc_set_type(struct irq_data *d, unsigned int type)
> > +{
> > +	switch (type & IRQ_TYPE_SENSE_MASK) {
> > +	case IRQ_TYPE_LEVEL_HIGH:
> > +		__trigger[d->hwirq] = 0;
> > +		break;
> > +	case IRQ_TYPE_LEVEL_LOW:
> > +		__trigger[d->hwirq] = 1;
> > +		break;
> > +	case IRQ_TYPE_EDGE_RISING:
> > +		__trigger[d->hwirq] = 2;
> > +		break;
> > +	case IRQ_TYPE_EDGE_FALLING:
> > +		__trigger[d->hwirq] = 3;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  #ifdef CONFIG_SMP
> >  static int csky_irq_set_affinity(struct irq_data *d,
> >  				 const struct cpumask *mask_val,
> > @@ -101,6 +175,7 @@ static struct irq_chip csky_irq_chip = {
> >  	.irq_eoi	= csky_mpintc_eoi,
> >  	.irq_enable	= csky_mpintc_enable,
> >  	.irq_disable	= csky_mpintc_disable,
> > +	.irq_set_type	= csky_mpintc_set_type,
> >  #ifdef CONFIG_SMP
> >  	.irq_set_affinity = csky_irq_set_affinity,
> >  #endif
> > @@ -121,9 +196,29 @@ static int csky_irqdomain_map(struct irq_domain *d, unsigned int irq,
> >  	return 0;
> >  }
> >  
> > +static int csky_irq_domain_xlate_cells(struct irq_domain *d,
> > +		struct device_node *ctrlr, const u32 *intspec,
> > +		unsigned int intsize, unsigned long *out_hwirq,
> > +		unsigned int *out_type)
> > +{
> > +	if (WARN_ON(intsize < 1))
> > +		return -EINVAL;
> > +
> > +	*out_hwirq = intspec[0];
> > +	if (intsize > 1)
> > +		*out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
> > +	else
> > +		*out_type = IRQ_TYPE_NONE;
> 
> What does IRQ_TYPE_NONE mean in this context? Shouldn't it actually be
> whatever the HW defaults to? Or even better, whatever was expected in
> the previous definition of the DT binding?
Yes, it shouldn't use IRQ_TYPE_NONE and I'll use IRQ_TYPE_LEVEL_HIGH.

> 
> > +
> > +	if (intsize > 2)
> > +		__priority[*out_hwirq] = intspec[2];
> 
> And what is the used priority in this case?
C-SKY MPINTC could support interrupt's priority and this will be set in
INTCG_CIPRTR register. It is set in csky_mpintc_enable function.

> 
> > +
> > +	return 0;
> > +}
> > +
> >  static const struct irq_domain_ops csky_irqdomain_ops = {
> >  	.map	= csky_irqdomain_map,
> > -	.xlate	= irq_domain_xlate_onecell,
> > +	.xlate	= csky_irq_domain_xlate_cells,
> >  };
> >  
> >  #ifdef CONFIG_SMP
> > @@ -157,6 +252,14 @@ csky_mpintc_init(struct device_node *node, struct device_node *parent)
> >  	if (ret < 0)
> >  		nr_irq = INTC_IRQS;
> >  
> > +	__priority = kcalloc(nr_irq, sizeof(unsigned long), GFP_KERNEL);
> > +	if (__priority == NULL)
> > +		return -ENXIO;
> > +
> > +	__trigger  = kcalloc(nr_irq, sizeof(unsigned long), GFP_KERNEL);
> > +	if (__trigger == NULL)
> > +		return -ENXIO;
> 
> Maybe you should consider initializing these arrays to something that
> makes sense for the case where the DT doesn't carry this information
> (which is 100% of the DTs up to this point).
Yes, and zero is enough.

/**
 * kcalloc - allocate memory for an array. The memory is set to zero.
 * @n: number of elements.
 * @size: element size.
 * @flags: the type of memory to allocate (see kmalloc).
 */
static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
{
	return kmalloc_array(n, size, flags | __GFP_ZERO);
}

> 
> > +
> >  	if (INTCG_base == NULL) {
> >  		INTCG_base = ioremap(mfcr("cr<31, 14>"),
> >  				     INTCL_SIZE*nr_cpu_ids + INTCG_SIZE);
> 
> 
> Not directly related to this patch: Please add a cover letter to you
> patch series, and describe the goal of the whole series as well as the
> changes you make along the way. It will definitely help the reviewers.
I'll seperate them first.

Best Regards
 Guo Ren

Re: [PATCH V2 5/7] dt-bindings: interrupt-controller: Update csky mpintc

[Guo Ren]

Thx Marc,

On Mon, Feb 18, 2019 at 02:28:45PM +0000, Marc Zyngier wrote:
> On Mon, 18 Feb 2019 10:04:41 +0800
> guoren@kernel.org wrote:
> 
> > From: Guo Ren <ren_guo@c-sky.com>
> > 
> > Add trigger type and priority setting for csky,mpintc.
> > 
> > Changelog:
> >  - change #interrupt-cells to <3>
> > 
> > Signed-off-by: Guo Ren <ren_guo@c-sky.com>
> > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > Cc: Rob Herring <robh+dt@kernel.org>
> > ---
> >  .../bindings/interrupt-controller/csky,mpintc.txt   | 21 +++++++++++++++++----
> >  1 file changed, 17 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt b/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt
> > index ab921f1..dccd913 100644
> > --- a/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt
> > +++ b/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt
> > @@ -6,11 +6,18 @@ C-SKY Multi-processors Interrupt Controller is designed for ck807/ck810/ck860
> >  SMP soc, and it also could be used in non-SMP system.
> >  
> >  Interrupt number definition:
> > -
> >    0-15  : software irq, and we use 15 as our IPI_IRQ.
> >   16-31  : private  irq, and we use 16 as the co-processor timer.
> >   31-1024: common irq for soc ip.
> >  
> > +Interrupt triger mode:
> > + IRQ_TYPE_LEVEL_HIGH (default)
> > + IRQ_TYPE_LEVEL_LOW
> > + IRQ_TYPE_EDGE_RISING
> > + IRQ_TYPE_EDGE_FALLING
> > +
> > +Interrupt priority range: 0-255
> > +
> >  =============================
> >  intc node bindings definition
> >  =============================
> > @@ -26,15 +33,21 @@ intc node bindings definition
> >  	- #interrupt-cells
> >  		Usage: required
> >  		Value type: <u32>
> > -		Definition: must be <1>
> > +		Definition: <3>
> 
> This seem to be invalidating all existing DTs. Is this an acceptable
> thing to do? It will require an Ack from a DT maintainer.
There is no DTs upstreamed and current implementation of driver could
support old format which isn't mentioned here. We just give a complete
format here.

Best Regards
 Guo Ren

Re: [PATCH V2 5/7] dt-bindings: interrupt-controller: Update csky mpintc

[Marc Zyngier]

On Fri, 10 May 2019 09:57:54 +0100,
Guo Ren <guoren@kernel.org> wrote:
> 
> Thx Marc,
> 
> On Mon, Feb 18, 2019 at 02:28:45PM +0000, Marc Zyngier wrote:
> > On Mon, 18 Feb 2019 10:04:41 +0800
> > guoren@kernel.org wrote:
> > 
> > > From: Guo Ren <ren_guo@c-sky.com>
> > > 
> > > Add trigger type and priority setting for csky,mpintc.
> > > 
> > > Changelog:
> > >  - change #interrupt-cells to <3>
> > > 
> > > Signed-off-by: Guo Ren <ren_guo@c-sky.com>
> > > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > > Cc: Rob Herring <robh+dt@kernel.org>
> > > ---
> > >  .../bindings/interrupt-controller/csky,mpintc.txt   | 21 +++++++++++++++++----
> > >  1 file changed, 17 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt b/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt
> > > index ab921f1..dccd913 100644
> > > --- a/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt
> > > +++ b/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt
> > > @@ -6,11 +6,18 @@ C-SKY Multi-processors Interrupt Controller is designed for ck807/ck810/ck860
> > >  SMP soc, and it also could be used in non-SMP system.
> > >  
> > >  Interrupt number definition:
> > > -
> > >    0-15  : software irq, and we use 15 as our IPI_IRQ.
> > >   16-31  : private  irq, and we use 16 as the co-processor timer.
> > >   31-1024: common irq for soc ip.
> > >  
> > > +Interrupt triger mode:
> > > + IRQ_TYPE_LEVEL_HIGH (default)
> > > + IRQ_TYPE_LEVEL_LOW
> > > + IRQ_TYPE_EDGE_RISING
> > > + IRQ_TYPE_EDGE_FALLING
> > > +
> > > +Interrupt priority range: 0-255
> > > +
> > >  =============================
> > >  intc node bindings definition
> > >  =============================
> > > @@ -26,15 +33,21 @@ intc node bindings definition
> > >  	- #interrupt-cells
> > >  		Usage: required
> > >  		Value type: <u32>
> > > -		Definition: must be <1>
> > > +		Definition: <3>
> > 
> > This seem to be invalidating all existing DTs. Is this an acceptable
> > thing to do? It will require an Ack from a DT maintainer.
> There is no DTs upstreamed and current implementation of driver could
> support old format which isn't mentioned here. We just give a complete
> format here.

It would be good to mention it anyway, if only for people to
understand what is happening with this changing binding.

Thanks,

	M.

-- 
Jazz is not dead, it just smell funny.

Re: [PATCH V2 4/7] irqchip/irq-csky-mpintc: Add triger type and priority

[Marc Zyngier]

On Fri, 10 May 2019 09:25:10 +0100,
Guo Ren <guoren@kernel.org> wrote:
> 
> Thx Marc,
> 
> Sorry for late reply:
> 
> On Mon, Feb 18, 2019 at 02:38:23PM +0000, Marc Zyngier wrote:
> > On Mon, 18 Feb 2019 10:04:40 +0800
> > guoren@kernel.org wrote:
> > 
> > > From: Guo Ren <ren_guo@c-sky.com>
> > > 
> > > Support 4 triger types:
> > >  - IRQ_TYPE_LEVEL_HIGH
> > >  - IRQ_TYPE_LEVEL_LOW
> > >  - IRQ_TYPE_EDGE_RISING
> > >  - IRQ_TYPE_EDGE_FALLING
> > > 
> > > Support 0-255 priority setting for each irq.
> > > 
> > > Changelog:
> > >  - Fixup this_cpu_read() preempted problem.
> > >  - Optimize the coding style.
> > > 
> > > Signed-off-by: Guo Ren <ren_guo@c-sky.com>
> > > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > > ---
> > >  drivers/irqchip/irq-csky-mpintc.c | 105 +++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 104 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
> > > index 99d3f3f..07a3752 100644
> > > --- a/drivers/irqchip/irq-csky-mpintc.c
> > > +++ b/drivers/irqchip/irq-csky-mpintc.c
> > > @@ -17,6 +17,7 @@
> > >  #include <asm/reg_ops.h>
> > >  
> > >  static struct irq_domain *root_domain;
> > > +
> > >  static void __iomem *INTCG_base;
> > >  static void __iomem *INTCL_base;
> > >  
> > > @@ -29,9 +30,12 @@ static void __iomem *INTCL_base;
> > >  
> > >  #define INTCG_ICTLR	0x0
> > >  #define INTCG_CICFGR	0x100
> > > +#define INTCG_CIPRTR	0x200
> > >  #define INTCG_CIDSTR	0x1000
> > >  
> > >  #define INTCL_PICTLR	0x0
> > > +#define INTCL_CFGR	0x14
> > > +#define INTCL_PRTR	0x20
> > >  #define INTCL_SIGR	0x60
> > >  #define INTCL_RDYIR	0x6c
> > >  #define INTCL_SENR	0xa0
> > > @@ -40,6 +44,51 @@ static void __iomem *INTCL_base;
> > >  
> > >  static DEFINE_PER_CPU(void __iomem *, intcl_reg);
> > >  
> > > +static unsigned long *__trigger;
> > > +static unsigned long *__priority;
> > > +
> > > +#define IRQ_OFFSET(irq) ((irq < COMM_IRQ_BASE) ? irq : (irq - COMM_IRQ_BASE))
> > > +
> > > +#define TRIG_BYTE_OFFSET(i)	((((i) * 2) / 32) * 4)
> > > +#define TRIG_BIT_OFFSET(i)	 (((i) * 2) % 32)
> > > +
> > > +#define PRI_BYTE_OFFSET(i)	((((i) * 8) / 32) * 4)
> > > +#define PRI_BIT_OFFSET(i)	 (((i) * 8) % 32)
> > > +
> > > +#define TRIG_VAL(trigger, irq)	(trigger << TRIG_BIT_OFFSET(IRQ_OFFSET(irq)))
> > > +#define TRIG_VAL_MSK(irq)	    (~(3 << TRIG_BIT_OFFSET(IRQ_OFFSET(irq))))
> > > +#define PRI_VAL(priority, irq)	(priority << PRI_BIT_OFFSET(IRQ_OFFSET(irq)))
> > > +#define PRI_VAL_MSK(irq)	  (~(0xff << PRI_BIT_OFFSET(IRQ_OFFSET(irq))))
> > > +
> > > +#define TRIG_BASE(irq) \
> > > +	(TRIG_BYTE_OFFSET(IRQ_OFFSET(irq)) + ((irq < COMM_IRQ_BASE) ? \
> > > +	(this_cpu_read(intcl_reg) + INTCL_CFGR) : (INTCG_base + INTCG_CICFGR)))
> > > +
> > > +#define PRI_BASE(irq) \
> > > +	(PRI_BYTE_OFFSET(IRQ_OFFSET(irq)) + ((irq < COMM_IRQ_BASE) ? \
> > > +	(this_cpu_read(intcl_reg) + INTCL_PRTR) : (INTCG_base + INTCG_CIPRTR)))
> > > +
> > > +static DEFINE_SPINLOCK(setup_lock);
> > > +static void setup_trigger_priority(unsigned long irq, unsigned long trigger,
> > > +				   unsigned long priority)
> > > +{
> > > +	unsigned int tmp;
> > > +
> > > +	spin_lock(&setup_lock);
> > > +
> > > +	/* setup trigger */
> > > +	tmp = readl_relaxed(TRIG_BASE(irq)) & TRIG_VAL_MSK(irq);
> > > +
> > > +	writel_relaxed(tmp | TRIG_VAL(trigger, irq), TRIG_BASE(irq));
> > > +
> > > +	/* setup priority */
> > > +	tmp = readl_relaxed(PRI_BASE(irq)) & PRI_VAL_MSK(irq);
> > > +
> > > +	writel_relaxed(tmp | PRI_VAL(priority, irq), PRI_BASE(irq));
> > > +
> > > +	spin_unlock(&setup_lock);
> > > +}
> > > +
> > >  static void csky_mpintc_handler(struct pt_regs *regs)
> > >  {
> > >  	void __iomem *reg_base = this_cpu_read(intcl_reg);
> > > @@ -52,6 +101,9 @@ static void csky_mpintc_enable(struct irq_data *d)
> > >  {
> > >  	void __iomem *reg_base = this_cpu_read(intcl_reg);
> > >  
> > > +	setup_trigger_priority(d->hwirq, __trigger[d->hwirq],
> > > +				 __priority[d->hwirq]);
> > > +
> > >  	writel_relaxed(d->hwirq, reg_base + INTCL_SENR);
> > >  }
> > >  
> > > @@ -69,6 +121,28 @@ static void csky_mpintc_eoi(struct irq_data *d)
> > >  	writel_relaxed(d->hwirq, reg_base + INTCL_CACR);
> > >  }
> > >  
> > > +static int csky_mpintc_set_type(struct irq_data *d, unsigned int type)
> > > +{
> > > +	switch (type & IRQ_TYPE_SENSE_MASK) {
> > > +	case IRQ_TYPE_LEVEL_HIGH:
> > > +		__trigger[d->hwirq] = 0;
> > > +		break;
> > > +	case IRQ_TYPE_LEVEL_LOW:
> > > +		__trigger[d->hwirq] = 1;
> > > +		break;
> > > +	case IRQ_TYPE_EDGE_RISING:
> > > +		__trigger[d->hwirq] = 2;
> > > +		break;
> > > +	case IRQ_TYPE_EDGE_FALLING:
> > > +		__trigger[d->hwirq] = 3;
> > > +		break;
> > > +	default:
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  #ifdef CONFIG_SMP
> > >  static int csky_irq_set_affinity(struct irq_data *d,
> > >  				 const struct cpumask *mask_val,
> > > @@ -101,6 +175,7 @@ static struct irq_chip csky_irq_chip = {
> > >  	.irq_eoi	= csky_mpintc_eoi,
> > >  	.irq_enable	= csky_mpintc_enable,
> > >  	.irq_disable	= csky_mpintc_disable,
> > > +	.irq_set_type	= csky_mpintc_set_type,
> > >  #ifdef CONFIG_SMP
> > >  	.irq_set_affinity = csky_irq_set_affinity,
> > >  #endif
> > > @@ -121,9 +196,29 @@ static int csky_irqdomain_map(struct irq_domain *d, unsigned int irq,
> > >  	return 0;
> > >  }
> > >  
> > > +static int csky_irq_domain_xlate_cells(struct irq_domain *d,
> > > +		struct device_node *ctrlr, const u32 *intspec,
> > > +		unsigned int intsize, unsigned long *out_hwirq,
> > > +		unsigned int *out_type)
> > > +{
> > > +	if (WARN_ON(intsize < 1))
> > > +		return -EINVAL;
> > > +
> > > +	*out_hwirq = intspec[0];
> > > +	if (intsize > 1)
> > > +		*out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
> > > +	else
> > > +		*out_type = IRQ_TYPE_NONE;
> > 
> > What does IRQ_TYPE_NONE mean in this context? Shouldn't it actually be
> > whatever the HW defaults to? Or even better, whatever was expected in
> > the previous definition of the DT binding?
> Yes, it shouldn't use IRQ_TYPE_NONE and I'll use
> > IRQ_TYPE_LEVEL_HIGH.

I think you should use what the DT gives you and nothing else, unless
there is some backward compatibility scheme you want to support.

> 
> > 
> > > +
> > > +	if (intsize > 2)
> > > +		__priority[*out_hwirq] = intspec[2];
> > 
> > And what is the used priority in this case?
> C-SKY MPINTC could support interrupt's priority and this will be set in
> INTCG_CIPRTR register. It is set in csky_mpintc_enable function.
>
> > 
> > > +
> > > +	return 0;
> > > +}
> > > +
> > >  static const struct irq_domain_ops csky_irqdomain_ops = {
> > >  	.map	= csky_irqdomain_map,
> > > -	.xlate	= irq_domain_xlate_onecell,
> > > +	.xlate	= csky_irq_domain_xlate_cells,
> > >  };
> > >  
> > >  #ifdef CONFIG_SMP
> > > @@ -157,6 +252,14 @@ csky_mpintc_init(struct device_node *node, struct device_node *parent)
> > >  	if (ret < 0)
> > >  		nr_irq = INTC_IRQS;
> > >  
> > > +	__priority = kcalloc(nr_irq, sizeof(unsigned long), GFP_KERNEL);
> > > +	if (__priority == NULL)
> > > +		return -ENXIO;
> > > +
> > > +	__trigger  = kcalloc(nr_irq, sizeof(unsigned long), GFP_KERNEL);
> > > +	if (__trigger == NULL)
> > > +		return -ENXIO;
> > 
> > Maybe you should consider initializing these arrays to something that
> > makes sense for the case where the DT doesn't carry this information
> > (which is 100% of the DTs up to this point).
> Yes, and zero is enough.
> 
> /**
>  * kcalloc - allocate memory for an array. The memory is set to zero.
>  * @n: number of elements.
>  * @size: element size.
>  * @flags: the type of memory to allocate (see kmalloc).
>  */
> static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
> {
> 	return kmalloc_array(n, size, flags | __GFP_ZERO);
> }

Trust me, I have a rather precise idea of how kcalloc works, and I
have a copy of the kernel source handy, so no need to paste it in an
email.

My question was about the default values: Everything gets a default
priority of zero, and nothing seem to set it to another value
either. So why do we have this allocation anyway?

	M.

-- 
Jazz is not dead, it just smell funny.

Re: [PATCH V2 4/7] irqchip/irq-csky-mpintc: Add triger type and priority

[Guo Ren]

Thx Marc,

On Fri, May 10, 2019 at 07:12:18PM +0100, Marc Zyngier wrote:
> On Fri, 10 May 2019 09:25:10 +0100,
> Guo Ren <guoren@kernel.org> wrote:
> > 
> > Thx Marc,
> > 
> > Sorry for late reply:
> > 
> > On Mon, Feb 18, 2019 at 02:38:23PM +0000, Marc Zyngier wrote:
> > > On Mon, 18 Feb 2019 10:04:40 +0800
> > > guoren@kernel.org wrote:
> > > 
> > > > From: Guo Ren <ren_guo@c-sky.com>
> > > > 
> > > > Support 4 triger types:
> > > >  - IRQ_TYPE_LEVEL_HIGH
> > > >  - IRQ_TYPE_LEVEL_LOW
> > > >  - IRQ_TYPE_EDGE_RISING
> > > >  - IRQ_TYPE_EDGE_FALLING
> > > > 
> > > > Support 0-255 priority setting for each irq.
> > > > 
> > > > Changelog:
> > > >  - Fixup this_cpu_read() preempted problem.
> > > >  - Optimize the coding style.
> > > > 
> > > > Signed-off-by: Guo Ren <ren_guo@c-sky.com>
> > > > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > > > ---
> > > >  drivers/irqchip/irq-csky-mpintc.c | 105 +++++++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 104 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
> > > > index 99d3f3f..07a3752 100644
> > > > --- a/drivers/irqchip/irq-csky-mpintc.c
> > > > +++ b/drivers/irqchip/irq-csky-mpintc.c
> > > > @@ -17,6 +17,7 @@
> > > >  #include <asm/reg_ops.h>
> > > >  
> > > >  static struct irq_domain *root_domain;
> > > > +
> > > >  static void __iomem *INTCG_base;
> > > >  static void __iomem *INTCL_base;
> > > >  
> > > > @@ -29,9 +30,12 @@ static void __iomem *INTCL_base;
> > > >  
> > > >  #define INTCG_ICTLR	0x0
> > > >  #define INTCG_CICFGR	0x100
> > > > +#define INTCG_CIPRTR	0x200
> > > >  #define INTCG_CIDSTR	0x1000
> > > >  
> > > >  #define INTCL_PICTLR	0x0
> > > > +#define INTCL_CFGR	0x14
> > > > +#define INTCL_PRTR	0x20
> > > >  #define INTCL_SIGR	0x60
> > > >  #define INTCL_RDYIR	0x6c
> > > >  #define INTCL_SENR	0xa0
> > > > @@ -40,6 +44,51 @@ static void __iomem *INTCL_base;
> > > >  
> > > >  static DEFINE_PER_CPU(void __iomem *, intcl_reg);
> > > >  
> > > > +static unsigned long *__trigger;
> > > > +static unsigned long *__priority;
> > > > +
> > > > +#define IRQ_OFFSET(irq) ((irq < COMM_IRQ_BASE) ? irq : (irq - COMM_IRQ_BASE))
> > > > +
> > > > +#define TRIG_BYTE_OFFSET(i)	((((i) * 2) / 32) * 4)
> > > > +#define TRIG_BIT_OFFSET(i)	 (((i) * 2) % 32)
> > > > +
> > > > +#define PRI_BYTE_OFFSET(i)	((((i) * 8) / 32) * 4)
> > > > +#define PRI_BIT_OFFSET(i)	 (((i) * 8) % 32)
> > > > +
> > > > +#define TRIG_VAL(trigger, irq)	(trigger << TRIG_BIT_OFFSET(IRQ_OFFSET(irq)))
> > > > +#define TRIG_VAL_MSK(irq)	    (~(3 << TRIG_BIT_OFFSET(IRQ_OFFSET(irq))))
> > > > +#define PRI_VAL(priority, irq)	(priority << PRI_BIT_OFFSET(IRQ_OFFSET(irq)))
> > > > +#define PRI_VAL_MSK(irq)	  (~(0xff << PRI_BIT_OFFSET(IRQ_OFFSET(irq))))
> > > > +
> > > > +#define TRIG_BASE(irq) \
> > > > +	(TRIG_BYTE_OFFSET(IRQ_OFFSET(irq)) + ((irq < COMM_IRQ_BASE) ? \
> > > > +	(this_cpu_read(intcl_reg) + INTCL_CFGR) : (INTCG_base + INTCG_CICFGR)))
> > > > +
> > > > +#define PRI_BASE(irq) \
> > > > +	(PRI_BYTE_OFFSET(IRQ_OFFSET(irq)) + ((irq < COMM_IRQ_BASE) ? \
> > > > +	(this_cpu_read(intcl_reg) + INTCL_PRTR) : (INTCG_base + INTCG_CIPRTR)))
> > > > +
> > > > +static DEFINE_SPINLOCK(setup_lock);
> > > > +static void setup_trigger_priority(unsigned long irq, unsigned long trigger,
> > > > +				   unsigned long priority)
> > > > +{
> > > > +	unsigned int tmp;
> > > > +
> > > > +	spin_lock(&setup_lock);
> > > > +
> > > > +	/* setup trigger */
> > > > +	tmp = readl_relaxed(TRIG_BASE(irq)) & TRIG_VAL_MSK(irq);
> > > > +
> > > > +	writel_relaxed(tmp | TRIG_VAL(trigger, irq), TRIG_BASE(irq));
> > > > +
> > > > +	/* setup priority */
> > > > +	tmp = readl_relaxed(PRI_BASE(irq)) & PRI_VAL_MSK(irq);
> > > > +
> > > > +	writel_relaxed(tmp | PRI_VAL(priority, irq), PRI_BASE(irq));
> > > > +
> > > > +	spin_unlock(&setup_lock);
> > > > +}
> > > > +
> > > >  static void csky_mpintc_handler(struct pt_regs *regs)
> > > >  {
> > > >  	void __iomem *reg_base = this_cpu_read(intcl_reg);
> > > > @@ -52,6 +101,9 @@ static void csky_mpintc_enable(struct irq_data *d)
> > > >  {
> > > >  	void __iomem *reg_base = this_cpu_read(intcl_reg);
> > > >  
> > > > +	setup_trigger_priority(d->hwirq, __trigger[d->hwirq],
> > > > +				 __priority[d->hwirq]);
> > > > +
> > > >  	writel_relaxed(d->hwirq, reg_base + INTCL_SENR);
> > > >  }
> > > >  
> > > > @@ -69,6 +121,28 @@ static void csky_mpintc_eoi(struct irq_data *d)
> > > >  	writel_relaxed(d->hwirq, reg_base + INTCL_CACR);
> > > >  }
> > > >  
> > > > +static int csky_mpintc_set_type(struct irq_data *d, unsigned int type)
> > > > +{
> > > > +	switch (type & IRQ_TYPE_SENSE_MASK) {
> > > > +	case IRQ_TYPE_LEVEL_HIGH:
> > > > +		__trigger[d->hwirq] = 0;
> > > > +		break;
> > > > +	case IRQ_TYPE_LEVEL_LOW:
> > > > +		__trigger[d->hwirq] = 1;
> > > > +		break;
> > > > +	case IRQ_TYPE_EDGE_RISING:
> > > > +		__trigger[d->hwirq] = 2;
> > > > +		break;
> > > > +	case IRQ_TYPE_EDGE_FALLING:
> > > > +		__trigger[d->hwirq] = 3;
> > > > +		break;
> > > > +	default:
> > > > +		return -EINVAL;
> > > > +	}
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > >  #ifdef CONFIG_SMP
> > > >  static int csky_irq_set_affinity(struct irq_data *d,
> > > >  				 const struct cpumask *mask_val,
> > > > @@ -101,6 +175,7 @@ static struct irq_chip csky_irq_chip = {
> > > >  	.irq_eoi	= csky_mpintc_eoi,
> > > >  	.irq_enable	= csky_mpintc_enable,
> > > >  	.irq_disable	= csky_mpintc_disable,
> > > > +	.irq_set_type	= csky_mpintc_set_type,
> > > >  #ifdef CONFIG_SMP
> > > >  	.irq_set_affinity = csky_irq_set_affinity,
> > > >  #endif
> > > > @@ -121,9 +196,29 @@ static int csky_irqdomain_map(struct irq_domain *d, unsigned int irq,
> > > >  	return 0;
> > > >  }
> > > >  
> > > > +static int csky_irq_domain_xlate_cells(struct irq_domain *d,
> > > > +		struct device_node *ctrlr, const u32 *intspec,
> > > > +		unsigned int intsize, unsigned long *out_hwirq,
> > > > +		unsigned int *out_type)
> > > > +{
> > > > +	if (WARN_ON(intsize < 1))
> > > > +		return -EINVAL;
> > > > +
> > > > +	*out_hwirq = intspec[0];
> > > > +	if (intsize > 1)
> > > > +		*out_type = intspec[1] & IRQ_TYPE_SENSE_MASK;
> > > > +	else
> > > > +		*out_type = IRQ_TYPE_NONE;
> > > 
> > > What does IRQ_TYPE_NONE mean in this context? Shouldn't it actually be
> > > whatever the HW defaults to? Or even better, whatever was expected in
> > > the previous definition of the DT binding?
> > Yes, it shouldn't use IRQ_TYPE_NONE and I'll use
> > > IRQ_TYPE_LEVEL_HIGH.
> 
> I think you should use what the DT gives you and nothing else, unless
> there is some backward compatibility scheme you want to support.
I want backward compatibliltiy scheme, eg:
	interrupts = <34>; (IRQ_TYPE_LEVEL_HIGH priority=0)
	interrupts = <34 IRQ_TYPE_EDGE_RISING>; (priority=0)
	interrupts = <34 IRQ_TYPE_EDGE_RISING 253>; (priority=253)

All of above DT settings could work.

> 
> > 
> > > 
> > > > +
> > > > +	if (intsize > 2)
> > > > +		__priority[*out_hwirq] = intspec[2];
> > > 
> > > And what is the used priority in this case?
> > C-SKY MPINTC could support interrupt's priority and this will be set in
> > INTCG_CIPRTR register. It is set in csky_mpintc_enable function.
> >
> > > 
> > > > +
> > > > +	return 0;
> > > > +}
> > > > +
> > > >  static const struct irq_domain_ops csky_irqdomain_ops = {
> > > >  	.map	= csky_irqdomain_map,
> > > > -	.xlate	= irq_domain_xlate_onecell,
> > > > +	.xlate	= csky_irq_domain_xlate_cells,
> > > >  };
> > > >  
> > > >  #ifdef CONFIG_SMP
> > > > @@ -157,6 +252,14 @@ csky_mpintc_init(struct device_node *node, struct device_node *parent)
> > > >  	if (ret < 0)
> > > >  		nr_irq = INTC_IRQS;
> > > >  
> > > > +	__priority = kcalloc(nr_irq, sizeof(unsigned long), GFP_KERNEL);
> > > > +	if (__priority == NULL)
> > > > +		return -ENXIO;
> > > > +
> > > > +	__trigger  = kcalloc(nr_irq, sizeof(unsigned long), GFP_KERNEL);
> > > > +	if (__trigger == NULL)
> > > > +		return -ENXIO;
> > > 
> > > Maybe you should consider initializing these arrays to something that
> > > makes sense for the case where the DT doesn't carry this information
> > > (which is 100% of the DTs up to this point).
> > Yes, and zero is enough.
> > 
> > /**
> >  * kcalloc - allocate memory for an array. The memory is set to zero.
> >  * @n: number of elements.
> >  * @size: element size.
> >  * @flags: the type of memory to allocate (see kmalloc).
> >  */
> > static inline void *kcalloc(size_t n, size_t size, gfp_t flags)
> > {
> > 	return kmalloc_array(n, size, flags | __GFP_ZERO);
> > }
> 
> Trust me, I have a rather precise idea of how kcalloc works, and I
> have a copy of the kernel source handy, so no need to paste it in an
> email.
> 
> My question was about the default values: Everything gets a default
> priority of zero, and nothing seem to set it to another value
> either. So why do we have this allocation anyway?
__priority and __trigger are used to setup interrupt controller's irq
number's priority and trigger type. (See previous answer)

Best Regards
 Guo Ren