LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH V3 1/2] dt-bindings: interrupt-controller: Update csky mpintc
@ 2019-05-13  9:55 guoren
  2019-05-13  9:55 ` [PATCH V3 2/2] irqchip/irq-csky-mpintc: Add triger type and priority guoren
  2019-05-13 17:49 ` [PATCH V3 1/2] dt-bindings: interrupt-controller: Update csky mpintc Rob Herring
  0 siblings, 2 replies; 6+ messages in thread
From: guoren @ 2019-05-13  9:55 UTC (permalink / raw)
  To: marc.zyngier, robh+dt
  Cc: mark.rutland, linux-kernel, devicetree, jason, tglx, guoren, ren_guo

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

Add trigger type and priority 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.

Changes for V3:
 - Update commit msg about compatible

Changes for V2:
 - 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH V3 2/2] irqchip/irq-csky-mpintc: Add triger type and priority
  2019-05-13  9:55 [PATCH V3 1/2] dt-bindings: interrupt-controller: Update csky mpintc guoren
@ 2019-05-13  9:55 ` guoren
  2019-05-21  7:22   ` Guo Ren
  2019-05-13 17:49 ` [PATCH V3 1/2] dt-bindings: interrupt-controller: Update csky mpintc Rob Herring
  1 sibling, 1 reply; 6+ messages in thread
From: guoren @ 2019-05-13  9:55 UTC (permalink / raw)
  To: marc.zyngier, robh+dt
  Cc: mark.rutland, linux-kernel, devicetree, jason, tglx, guoren, ren_guo

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.

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

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

Changes for V2:
 - 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 | 113 +++++++++++++++++++++++++++++++++++---
 1 file changed, 106 insertions(+), 7 deletions(-)

diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
index c67c961..5bc0868 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,11 +30,13 @@ 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_HPPIR	0x68
 #define INTCL_RDYIR	0x6c
 #define INTCL_SENR	0xa0
 #define INTCL_CENR	0xa4
@@ -41,21 +44,66 @@ 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);
 
-	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)
 {
 	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);
 }
 
@@ -73,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,
@@ -105,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
@@ -125,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_LEVEL_HIGH;
+
+	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
@@ -161,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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V3 1/2] dt-bindings: interrupt-controller: Update csky mpintc
  2019-05-13  9:55 [PATCH V3 1/2] dt-bindings: interrupt-controller: Update csky mpintc guoren
  2019-05-13  9:55 ` [PATCH V3 2/2] irqchip/irq-csky-mpintc: Add triger type and priority guoren
@ 2019-05-13 17:49 ` Rob Herring
  1 sibling, 0 replies; 6+ messages in thread
From: Rob Herring @ 2019-05-13 17:49 UTC (permalink / raw)
  To: guoren
  Cc: marc.zyngier, robh+dt, mark.rutland, linux-kernel, devicetree,
	jason, tglx, guoren, ren_guo

On Mon, 13 May 2019 17:55:38 +0800, guoren@kernel.org wrote:
> From: Guo Ren <ren_guo@c-sky.com>
> 
> Add trigger type and priority 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.
> 
> Changes for V3:
>  - Update commit msg about compatible
> 
> Changes for V2:
>  - 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(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V3 2/2] irqchip/irq-csky-mpintc: Add triger type and priority
  2019-05-13  9:55 ` [PATCH V3 2/2] irqchip/irq-csky-mpintc: Add triger type and priority guoren
@ 2019-05-21  7:22   ` Guo Ren
  2019-05-21  8:54     ` Marc Zyngier
  0 siblings, 1 reply; 6+ messages in thread
From: Guo Ren @ 2019-05-21  7:22 UTC (permalink / raw)
  To: marc.zyngier, robh+dt
  Cc: mark.rutland, linux-kernel, devicetree, jason, tglx, Ren Guo

Hi Marc,
ping ... Any problem ?

Thx
 Guo Ren

<guoren@kernel.org> 于2019年5月13日周一 下午5:55写道:
>
> 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.
>
> All of above could be set in DeviceTree file and it still compatible
> with the old DeviceTree format.
>
> Changes for V3:
>  - Use IRQ_TYPE_LEVEL_HIGH as default instead of IRQ_TYPE_NONE
>  - Remove unnecessary loop in csky_mpintc_handler
>
> Changes for V2:
>  - 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 | 113 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 106 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
> index c67c961..5bc0868 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,11 +30,13 @@ 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_HPPIR    0x68
>  #define INTCL_RDYIR    0x6c
>  #define INTCL_SENR     0xa0
>  #define INTCL_CENR     0xa4
> @@ -41,21 +44,66 @@ 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);
>
> -       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)
>  {
>         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);
>  }
>
> @@ -73,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,
> @@ -105,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
> @@ -125,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_LEVEL_HIGH;
> +
> +       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
> @@ -161,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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V3 2/2] irqchip/irq-csky-mpintc: Add triger type and priority
  2019-05-21  7:22   ` Guo Ren
@ 2019-05-21  8:54     ` Marc Zyngier
  2019-05-21  9:24       ` Guo Ren
  0 siblings, 1 reply; 6+ messages in thread
From: Marc Zyngier @ 2019-05-21  8:54 UTC (permalink / raw)
  To: Guo Ren, robh+dt
  Cc: mark.rutland, linux-kernel, devicetree, jason, tglx, Ren Guo

On 21/05/2019 08:22, Guo Ren wrote:
> Hi Marc,
> ping ... Any problem ?

Pinging me twice in four days for a patch posted in the middle of the
merge window is a problem, yes. In general, do not post such patches
during the merge window, unless this is an urgent fix. Everybody is busy
unbreaking the tree.

> 
> Thx
>  Guo Ren
> 
> <guoren@kernel.org> 于2019年5月13日周一 下午5:55写道:
>>
>> 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.
>>
>> All of above could be set in DeviceTree file and it still compatible
>> with the old DeviceTree format.
>>
>> Changes for V3:
>>  - Use IRQ_TYPE_LEVEL_HIGH as default instead of IRQ_TYPE_NONE
>>  - Remove unnecessary loop in csky_mpintc_handler
>>
>> Changes for V2:
>>  - 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 | 113 +++++++++++++++++++++++++++++++++++---
>>  1 file changed, 106 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
>> index c67c961..5bc0868 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,11 +30,13 @@ 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_HPPIR    0x68
>>  #define INTCL_RDYIR    0x6c
>>  #define INTCL_SENR     0xa0
>>  #define INTCL_CENR     0xa4
>> @@ -41,21 +44,66 @@ 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);
>>
>> -       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);

This seems to be an unrelated change.

>>  }
>>
>>  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);
>>  }
>>
>> @@ -73,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,
>> @@ -105,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
>> @@ -125,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_LEVEL_HIGH;
>> +
>> +       if (intsize > 2)
>> +               __priority[*out_hwirq] = intspec[2];

You keep doing this, and I keep objecting to it. Linux doesn't support
interrupt priorities, and yet you keep bringing them back. Why don't you
simply ignore that field and stick all priorities to some arbitrary value?

Thanks,

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V3 2/2] irqchip/irq-csky-mpintc: Add triger type and priority
  2019-05-21  8:54     ` Marc Zyngier
@ 2019-05-21  9:24       ` Guo Ren
  0 siblings, 0 replies; 6+ messages in thread
From: Guo Ren @ 2019-05-21  9:24 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: robh+dt, mark.rutland, linux-kernel, devicetree, jason, tglx, Ren Guo

Thx Marc,

On Tue, May 21, 2019 at 09:54:18AM +0100, Marc Zyngier wrote:
> On 21/05/2019 08:22, Guo Ren wrote:
> > Hi Marc,
> > ping ... Any problem ?
> 
> Pinging me twice in four days for a patch posted in the middle of the
> merge window is a problem, yes. In general, do not post such patches
> during the merge window, unless this is an urgent fix. Everybody is busy
> unbreaking the tree.
Ok, got it and I will avoid this situation next time.

> 
> > 
> > Thx
> >  Guo Ren
> > 
> > <guoren@kernel.org> 于2019年5月13日周一 下午5:55写道:
> >>
> >> 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.
> >>
> >> All of above could be set in DeviceTree file and it still compatible
> >> with the old DeviceTree format.
> >>
> >> Changes for V3:
> >>  - Use IRQ_TYPE_LEVEL_HIGH as default instead of IRQ_TYPE_NONE
> >>  - Remove unnecessary loop in csky_mpintc_handler
> >>
> >> Changes for V2:
> >>  - 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 | 113 +++++++++++++++++++++++++++++++++++---
> >>  1 file changed, 106 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/irqchip/irq-csky-mpintc.c b/drivers/irqchip/irq-csky-mpintc.c
> >> index c67c961..5bc0868 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,11 +30,13 @@ 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_HPPIR    0x68
> >>  #define INTCL_RDYIR    0x6c
> >>  #define INTCL_SENR     0xa0
> >>  #define INTCL_CENR     0xa4
> >> @@ -41,21 +44,66 @@ 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);
> >>
> >> -       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);
> 
> This seems to be an unrelated change.
Yes, I need seperate it to another patch.

> 
> >>  }
> >>
> >>  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);
> >>  }
> >>
> >> @@ -73,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,
> >> @@ -105,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
> >> @@ -125,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_LEVEL_HIGH;
> >> +
> >> +       if (intsize > 2)
> >> +               __priority[*out_hwirq] = intspec[2];
> 
> You keep doing this, and I keep objecting to it. Linux doesn't support
> interrupt priorities, and yet you keep bringing them back. Why don't you
> simply ignore that field and stick all priorities to some arbitrary value?
Em... Ok, I'll remove priorities.

Best Regards
 Guo Ren

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-05-21  9:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-13  9:55 [PATCH V3 1/2] dt-bindings: interrupt-controller: Update csky mpintc guoren
2019-05-13  9:55 ` [PATCH V3 2/2] irqchip/irq-csky-mpintc: Add triger type and priority guoren
2019-05-21  7:22   ` Guo Ren
2019-05-21  8:54     ` Marc Zyngier
2019-05-21  9:24       ` Guo Ren
2019-05-13 17:49 ` [PATCH V3 1/2] dt-bindings: interrupt-controller: Update csky mpintc Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).