[PATCH V4 0/4] irqchip/irq-csky-mpintc: Update some features

[PATCH V4 0/4] irqchip/irq-csky-mpintc: Update some features

[guoren]

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

Here are some patches for irq-csky-mpintc. Any feedback is welcome.

Changes for V4:
 - Remove priority setting
 - Add remove loop in csky_mpintc_handler in a seperate patch
 - Add auto irq deliver fixup patch
 - Add include file for devicetree document's example
 - Add --cover-letter

Changes for V3:
 - Use IRQ_TYPE_LEVEL_HIGH as default instead of IRQ_TYPE_NONE
 - Remove unnecessary loop in csky_mpintc_handler
 - Update commit log msg

Changes for V2:
 - Fixup this_cpu_read() preempted problem
 - Optimize the coding style
 - Optimize #interrupt-cells to one style

Guo Ren (4):
  irqchip/irq-csky-mpintc: Add triger type
  dt-bindings: interrupt-controller: Update csky mpintc
  irqchip/irq-csky-mpintc: Support auto irq deliver to all cpus
  irqchip/irq-csky-mpintc: Remove unnecessary loop in interrupt handler

 .../bindings/interrupt-controller/csky,mpintc.txt  |  20 +++-
 drivers/irqchip/irq-csky-mpintc.c                  | 102 +++++++++++++++++++--
 2 files changed, 109 insertions(+), 13 deletions(-)

-- 
2.7.4

[PATCH V4 1/4] irqchip/irq-csky-mpintc: Add triger type

[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

All of above could be set in DeviceTree file and it still compatible
with the old DeviceTree format.

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

diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
index c67c961..a451a07 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;
 
@@ -32,8 +33,8 @@ static void __iomem *INTCL_base;
 #define INTCG_CIDSTR	0x1000
 
 #define INTCL_PICTLR	0x0
+#define INTCL_CFGR	0x14
 #define INTCL_SIGR	0x60
-#define INTCL_HPPIR	0x68
 #define INTCL_RDYIR	0x6c
 #define INTCL_SENR	0xa0
 #define INTCL_CENR	0xa4
@@ -41,6 +42,35 @@ static void __iomem *INTCL_base;
 
 static DEFINE_PER_CPU(void __iomem *, intcl_reg);
 
+static unsigned long *__trigger;
+
+#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 TRIG_VAL(trigger, irq)	(trigger << TRIG_BIT_OFFSET(IRQ_OFFSET(irq)))
+#define TRIG_VAL_MSK(irq)	    (~(3 << TRIG_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)))
+
+static DEFINE_SPINLOCK(setup_lock);
+static void setup_trigger(unsigned long irq, unsigned long trigger)
+{
+	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));
+
+	spin_unlock(&setup_lock);
+}
+
 static void csky_mpintc_handler(struct pt_regs *regs)
 {
 	void __iomem *reg_base = this_cpu_read(intcl_reg);
@@ -56,6 +86,8 @@ static void csky_mpintc_enable(struct irq_data *d)
 {
 	void __iomem *reg_base = this_cpu_read(intcl_reg);
 
+	setup_trigger(d->hwirq, __trigger[d->hwirq]);
+
 	writel_relaxed(d->hwirq, reg_base + INTCL_SENR);
 }
 
@@ -73,6 +105,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,
@@ -105,6 +159,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
@@ -125,9 +180,26 @@ 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_LEVEL_HIGH;
+
+	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
@@ -161,6 +233,10 @@ csky_mpintc_init(struct device_node *node, struct device_node *parent)
 	if (ret < 0)
 		nr_irq = INTC_IRQS;
 
+	__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 V4 2/4] dt-bindings: interrupt-controller: Update csky mpintc

[guoren]

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

Add trigger type setting for csky,mpintc. The driver also could
support #interrupt-cells <1> and it wouldn't invalidate existing
DTs. Here we only show the complete format.

Signed-off-by: Guo Ren <ren_guo@c-sky.com>
Reviewed-by: Rob Herring <robh+dt@kernel.org>
Cc: Marc Zyngier <marc.zyngier@arm.com>
---
 .../bindings/interrupt-controller/csky,mpintc.txt    | 20 ++++++++++++++++----
 1 file changed, 16 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..e134053 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt
+++ b/Documentation/devicetree/bindings/interrupt-controller/csky,mpintc.txt
@@ -6,11 +6,16 @@ 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: (Defined in dt-bindings/interrupt-controller/irq.h)
+ IRQ_TYPE_LEVEL_HIGH (default)
+ IRQ_TYPE_LEVEL_LOW
+ IRQ_TYPE_EDGE_RISING
+ IRQ_TYPE_EDGE_FALLING
+
 =============================
 intc node bindings definition
 =============================
@@ -26,15 +31,22 @@ intc node bindings definition
 	- #interrupt-cells
 		Usage: required
 		Value type: <u32>
-		Definition: must be <1>
+		Definition: <2>
 	- interrupt-controller:
 		Usage: required
 
-Examples:
+Examples: ("interrupts = <irq_num IRQ_TYPE_XXX>")
 ---------
+#include <dt-bindings/interrupt-controller/irq.h>
 
 	intc: interrupt-controller {
 		compatible = "csky,mpintc";
-		#interrupt-cells = <1>;
+		#interrupt-cells = <2>;
 		interrupt-controller;
 	};
+
+	device: device-example {
+		...
+		interrupts = <34 IRQ_TYPE_EDGE_RISING>;
+		interrupt-parent = <&intc>;
+	};
-- 
2.7.4

[PATCH V4 3/4] irqchip/irq-csky-mpintc: Support auto irq deliver to all cpus

[guoren]

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

The csky,mpintc could deliver a external irq to one cpu or all cpus, but
it couldn't deliver a external irq to a group of cpus with cpu_mask. So
we only use auto deliver mode when affinity mask_val is equal to
cpu_present_mask.

There is no limitation for only two cpus in SMP system.

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

diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
index a451a07..2740dd5 100644
--- a/drivers/irqchip/irq-csky-mpintc.c
+++ b/drivers/irqchip/irq-csky-mpintc.c
@@ -143,8 +143,19 @@ static int csky_irq_set_affinity(struct irq_data *d,
 	if (cpu >= nr_cpu_ids)
 		return -EINVAL;
 
-	/* Enable interrupt destination */
-	cpu |= BIT(31);
+	/*
+	 * The csky,mpintc could support auto irq deliver, but it only
+	 * could deliver external irq to one cpu or all cpus. So it
+	 * doesn't support deliver external irq to a group of cpus
+	 * with cpu_mask.
+	 * SO we only use auto deliver mode when affinity mask_val is
+	 * equal to cpu_present_mask.
+	 *
+	 */
+	if (cpumask_equal(mask_val, cpu_present_mask))
+		cpu = 0;
+	else
+		cpu |= BIT(31);
 
 	writel_relaxed(cpu, INTCG_base + INTCG_CIDSTR + offset);
 
-- 
2.7.4

[PATCH V4 4/4] irqchip/irq-csky-mpintc: Remove unnecessary loop in interrupt handler

[guoren]

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

csky_mpintc_handler()
  ->handle_domain_irq()
    ->irq_exit()
      ->invoke_softirq()
        ->__do_softirq()
          ->local_irq_enable()

If new interrupt coming, it'll get into interrupt trap before return to
csky_mpintc_handler(). So there is no need loop in csky_mpintc_handler.

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

diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
index 2740dd5..122cd43 100644
--- a/drivers/irqchip/irq-csky-mpintc.c
+++ b/drivers/irqchip/irq-csky-mpintc.c
@@ -75,11 +75,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

Re: [PATCH V4 3/4] irqchip/irq-csky-mpintc: Support auto irq deliver to all cpus

[Marc Zyngier]

On 04/06/2019 12:05, guoren@kernel.org wrote:
> From: Guo Ren <ren_guo@c-sky.com>
> 
> The csky,mpintc could deliver a external irq to one cpu or all cpus, but
> it couldn't deliver a external irq to a group of cpus with cpu_mask. So
> we only use auto deliver mode when affinity mask_val is equal to
> cpu_present_mask.
> 
> There is no limitation for only two cpus in SMP system.
> 
> Signed-off-by: Guo Ren <ren_guo@c-sky.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> ---
>  drivers/irqchip/irq-csky-mpintc.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
> index a451a07..2740dd5 100644
> --- a/drivers/irqchip/irq-csky-mpintc.c
> +++ b/drivers/irqchip/irq-csky-mpintc.c
> @@ -143,8 +143,19 @@ static int csky_irq_set_affinity(struct irq_data *d,
>  	if (cpu >= nr_cpu_ids)
>  		return -EINVAL;
>  
> -	/* Enable interrupt destination */
> -	cpu |= BIT(31);
> +	/*
> +	 * The csky,mpintc could support auto irq deliver, but it only
> +	 * could deliver external irq to one cpu or all cpus. So it
> +	 * doesn't support deliver external irq to a group of cpus
> +	 * with cpu_mask.
> +	 * SO we only use auto deliver mode when affinity mask_val is
> +	 * equal to cpu_present_mask.
> +	 *
> +	 */
> +	if (cpumask_equal(mask_val, cpu_present_mask))
> +		cpu = 0;
> +	else
> +		cpu |= BIT(31);
>  
>  	writel_relaxed(cpu, INTCG_base + INTCG_CIDSTR + offset);
>  
> 

Isn't that the same patch as [1]? In which case, I've queued it as a fix
already.

Thanks,

	M.

[1] https://lkml.org/lkml/2019/5/21/174
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH V4 3/4] irqchip/irq-csky-mpintc: Support auto irq deliver to all cpus

[Guo Ren]

Hello Marc,

On Tue, Jun 4, 2019 at 7:54 PM Marc Zyngier <marc.zyngier@arm.com> wrote:
>
> On 04/06/2019 12:05, guoren@kernel.org wrote:
> > From: Guo Ren <ren_guo@c-sky.com>
> >
> > The csky,mpintc could deliver a external irq to one cpu or all cpus, but
> > it couldn't deliver a external irq to a group of cpus with cpu_mask. So
> > we only use auto deliver mode when affinity mask_val is equal to
> > cpu_present_mask.
> >
> > There is no limitation for only two cpus in SMP system.
> >
> > Signed-off-by: Guo Ren <ren_guo@c-sky.com>
> > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > ---
> >  drivers/irqchip/irq-csky-mpintc.c | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
> > index a451a07..2740dd5 100644
> > --- a/drivers/irqchip/irq-csky-mpintc.c
> > +++ b/drivers/irqchip/irq-csky-mpintc.c
> > @@ -143,8 +143,19 @@ static int csky_irq_set_affinity(struct irq_data *d,
> >       if (cpu >= nr_cpu_ids)
> >               return -EINVAL;
> >
> > -     /* Enable interrupt destination */
> > -     cpu |= BIT(31);
> > +     /*
> > +      * The csky,mpintc could support auto irq deliver, but it only
> > +      * could deliver external irq to one cpu or all cpus. So it
> > +      * doesn't support deliver external irq to a group of cpus
> > +      * with cpu_mask.
> > +      * SO we only use auto deliver mode when affinity mask_val is
> > +      * equal to cpu_present_mask.
> > +      *
> > +      */
> > +     if (cpumask_equal(mask_val, cpu_present_mask))
> > +             cpu = 0;
> > +     else
> > +             cpu |= BIT(31);
> >
> >       writel_relaxed(cpu, INTCG_base + INTCG_CIDSTR + offset);
> >
> >
>
> Isn't that the same patch as [1]? In which case, I've queued it as a fix
> already.
Yes, they are the same. It's duplicate.

I think you've queued this one:
https://lore.kernel.org/lkml/1558425245-20995-1-git-send-email-guoren@kernel.org/

That's OK.

Best Regards
 Guo Ren