changelog

PATCH Superseded Oleksij Rempel 4 participants 39 messages Sep 18, 2015 – Oct 14, 2015

Subsystems (2)

Arm i.MX and Layerscape irqchip drivers

lore

Share & tools

lore

Oleksij Rempel

author

2015-09-18 09:19 UTC #0 lore raw

v2 cover changelog

v2:
- move some parts from patch 2 to 1
- make separate irq_chip for ICOLL and ASM9260

Oleksij Rempel (2):
ARM: irqchip: mxs: prepare driver for HW with different offsets
ARM: irqchip: mxs: add Alpascale ASM9260 support

 drivers/irqchip/Kconfig                    |   5 +
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 +++++++++++++++++
 drivers/irqchip/irq-mxs.c                  | 188 ++++++++++++++++++++++++++---
 4 files changed, 286 insertions(+), 18 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

Signature · 1 line

2.5.0

Oleksij Rempel

author

2015-09-18 09:19 UTC #1 lore raw

PATCH v2 1/2 ARM: irqchip: mxs: prepare driver for HW with different offsets

Some HW has similar functionality but different register offsets. Make sure we can change offsets dynamically.

Signed-off-by Oleksij Rempel <[email protected]>

---
 drivers/irqchip/irq-mxs.c | 83 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 67 insertions(+), 16 deletions(-)

drivers/irqchip/irq-mxs.c +67 ~~-16~~

index 1faf812..14374de 100644
@@ -28,18 +28,39 @@
 #include <linux/stmp_device.h>
 #include <asm/exception.h>
 
+/*
+ * this device provide 4 offsets for each register:
+ * 0x0 - plain read write mode
+ * 0x4 - set mode, OR logic.
+ * 0x8 - clr mode, XOR logic.
+ * 0xc - togle mode.
+ */
+#define SET_REG 4
+#define CLR_REG 8
+
 #define HW_ICOLL_VECTOR				0x0000
 #define HW_ICOLL_LEVELACK			0x0010
 #define HW_ICOLL_CTRL				0x0020
 #define HW_ICOLL_STAT_OFFSET			0x0070
-#define HW_ICOLL_INTERRUPTn_SET(n)		(0x0124 + (n) * 0x10)
-#define HW_ICOLL_INTERRUPTn_CLR(n)		(0x0128 + (n) * 0x10)
-#define BM_ICOLL_INTERRUPTn_ENABLE		0x00000004
+#define HW_ICOLL_INTERRUPT0			0x0120
+#define HW_ICOLL_INTERRUPTn(n)			((n) * 0x10)
+#define BM_ICOLL_INTR_ENABLE			BIT(2)
 #define BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0	0x1
 
 #define ICOLL_NUM_IRQS		128
 
-static void __iomem *icoll_base;
+struct icoll_priv {
+	void __iomem *vector;
+	void __iomem *levelack;
+	void __iomem *ctrl;
+	void __iomem *stat;
+	void __iomem *intr;
+	/* number of interrupts per register */
+	int intr_per_reg;
+	void __iomem *clear;
+};
+
+static struct icoll_priv icoll_priv;
 static struct irq_domain *icoll_domain;
 
 static void icoll_ack_irq(struct irq_data *d)

@@ -50,19 +71,19 @@ static void icoll_ack_irq(struct irq_data *d)
 	 * BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0 unconditionally.
 	 */
 	__raw_writel(BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0,
-			icoll_base + HW_ICOLL_LEVELACK);
+			icoll_priv.levelack);
 }
 
 static void icoll_mask_irq(struct irq_data *d)
 {
-	__raw_writel(BM_ICOLL_INTERRUPTn_ENABLE,
-			icoll_base + HW_ICOLL_INTERRUPTn_CLR(d->hwirq));
+	__raw_writel(BM_ICOLL_INTR_ENABLE,
+			icoll_priv.intr + CLR_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
 static void icoll_unmask_irq(struct irq_data *d)
 {
-	__raw_writel(BM_ICOLL_INTERRUPTn_ENABLE,
-			icoll_base + HW_ICOLL_INTERRUPTn_SET(d->hwirq));
+	__raw_writel(BM_ICOLL_INTR_ENABLE,
+			icoll_priv.intr + SET_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
 static struct irq_chip mxs_icoll_chip = {

@@ -75,8 +96,8 @@ asmlinkage void __exception_irq_entry icoll_handle_irq(struct pt_regs *regs)
 {
 	u32 irqnr;
 
-	irqnr = __raw_readl(icoll_base + HW_ICOLL_STAT_OFFSET);
-	__raw_writel(irqnr, icoll_base + HW_ICOLL_VECTOR);
+	irqnr = __raw_readl(icoll_priv.stat);
+	__raw_writel(irqnr, icoll_priv.vector);
 	handle_domain_irq(icoll_domain, irqnr, regs);
 }

@@ -94,20 +115,50 @@ static const struct irq_domain_ops icoll_irq_domain_ops = {
 	.xlate = irq_domain_xlate_onecell,
 };
 
+static void __init icoll_add_domain(struct device_node *np,
+			  int num)
+{
+	icoll_domain = irq_domain_add_linear(np, num,
+					     &icoll_irq_domain_ops, NULL);
+
+	if (!icoll_domain)
+		panic("%s: unable add irq domain", np->full_name);
+	irq_set_default_host(icoll_domain);
+	set_handle_irq(icoll_handle_irq);
+}
+
+static void __iomem * __init icoll_init_iobase(struct device_node *np)
+{
+	void __iomem *icoll_base;
+
+	icoll_base = of_io_request_and_map(np, 0, np->name);
+	if (!icoll_base)
+		panic("%s: unable to map resource", np->full_name);
+	return icoll_base;
+}
+
 static int __init icoll_of_init(struct device_node *np,
 			  struct device_node *interrupt_parent)
 {
-	icoll_base = of_iomap(np, 0);
-	WARN_ON(!icoll_base);
+	void __iomem *icoll_base;
+
+	icoll_base		= icoll_init_iobase(np);
+	icoll_priv.vector	= icoll_base + HW_ICOLL_VECTOR;
+	icoll_priv.levelack	= icoll_base + HW_ICOLL_LEVELACK;
+	icoll_priv.ctrl		= icoll_base + HW_ICOLL_CTRL;
+	icoll_priv.stat		= icoll_base + HW_ICOLL_STAT_OFFSET;
+	icoll_priv.intr		= icoll_base + HW_ICOLL_INTERRUPT0;
+	icoll_priv.intr_per_reg	= 1;
+	icoll_priv.clear	= NULL;
 
 	/*
 	 * Interrupt Collector reset, which initializes the priority
 	 * for each irq to level 0.
 	 */
-	stmp_reset_block(icoll_base + HW_ICOLL_CTRL);
+	stmp_reset_block(icoll_priv.ctrl);
+
+	icoll_add_domain(np, ICOLL_NUM_IRQS);
 
-	icoll_domain = irq_domain_add_linear(np, ICOLL_NUM_IRQS,
-					     &icoll_irq_domain_ops, NULL);
 	return icoll_domain ? 0 : -ENODEV;
 }
 IRQCHIP_DECLARE(mxs, "fsl,icoll", icoll_of_init);

Signature · 1 line

2.5.0

Thomas Gleixner

2015-09-18 10:21 UTC #2 lore raw

Re: [PATCH v2 1/2] ARM: irqchip: mxs: prepare driver for HW with different offsets

Quoted 2 lines · Oleksij Rempel

Some HW has similar functionality but different register offsets. Make sure we can change offsets dynamically.

You forgot to mention the other preparatory changes.

Thanks,

tglx

Oleksij Rempel

author

2015-09-18 09:19 UTC #3 lore raw

PATCH v2 2/2 ARM: irqchip: mxs: add Alpascale ASM9260 support

In reply to Oleksij Rempel

From: Oleksij Rempel <[email protected]>

Freescale iMX23/iMX28 and Alphascale ASM9260 have similar interrupt collectors. It makes easy to reuse irq-mxs code for ASM9260. Differences between this devices are fallowing:
- different register offsets
- different count of intterupt lines per register
- ASM9260 don't provide reset bit
- ASM9260 don't support FIQ.

Signed-off-by Oleksij Rempel <[email protected]>

---
 drivers/irqchip/Kconfig                    |   5 ++
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 +++++++++++++++++++++++++++++
 drivers/irqchip/irq-mxs.c                  | 106 +++++++++++++++++++++++++++-
 4 files changed, 220 insertions(+), 2 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

drivers/irqchip/Kconfig +5 -0

index 27b52c8..0dfd840 100644
@@ -187,3 +187,8 @@ config IMX_GPCV2
 	select IRQ_DOMAIN
 	help
 	  Enables the wakeup IRQs for IMX platforms with GPCv2 block
+
+config IRQ_MXS
+	def_bool y if MACH_ASM9260 || ARCH_MXS
+	select IRQ_DOMAIN
+	select STMP_DEVICE

drivers/irqchip/Makefile +1 -1

index bb3048f..177f78f 100644
@@ -6,7 +6,7 @@ obj-$(CONFIG_ARCH_EXYNOS)		+= exynos-combiner.o
 obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
 obj-$(CONFIG_ARCH_MMP)			+= irq-mmp.o
 obj-$(CONFIG_ARCH_MVEBU)		+= irq-armada-370-xp.o
-obj-$(CONFIG_ARCH_MXS)			+= irq-mxs.o
+obj-$(CONFIG_IRQ_MXS)			+= irq-mxs.o
 obj-$(CONFIG_ARCH_TEGRA)		+= irq-tegra.o
 obj-$(CONFIG_ARCH_S3C24XX)		+= irq-s3c24xx.o
 obj-$(CONFIG_DW_APB_ICTL)		+= irq-dw-apb-ictl.o

drivers/irqchip/alphascale_asm9260-icoll.h new file +109 -0

109 lines not on this page · raw message

drivers/irqchip/irq-mxs.c +105 -1

160 lines not on this page · raw message

Signature · 1 line

2.5.0

Marc Zyngier

2015-09-18 10:43 UTC #4 lore raw

Re: Possible Spam [PATCH v2 2/2] ARM: irqchip: mxs: add Alpascale ASM9260 support

Quoted 3 lines · On Fri, 18 Sep 2015 11:18:42 +0200 Oleksij Rempel <[email protected]>…

From: Oleksij Rempel <[email protected]>
Freescale iMX23/iMX28 and Alphascale ASM9260 have similar

Is it Alphascale or Alpascale? You may need to fix the patch title.

Quoted 16 lines

interrupt collectors. It makes easy to reuse irq-mxs code for ASM9260. Differences between this devices are fallowing:
- different register offsets
- different count of intterupt lines per register
- ASM9260 don't provide reset bit
- ASM9260 don't support FIQ.
Signed-off-by Oleksij Rempel <[email protected]>
---
 drivers/irqchip/Kconfig                    |   5 ++
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 +++++++++++++++++++++++++++++
 drivers/irqchip/irq-mxs.c                  | 106 +++++++++++++++++++++++++++-
 4 files changed, 220 insertions(+), 2 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

[...]

Quoted 59 lines

drivers/irqchip/irq-mxs.c +27 -0

index 14374de..1470087 100644
@@ -1,5 +1,7 @@
 /*
  * Copyright (C) 2009-2010 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2014 Oleksij Rempel <[email protected]>
+ *	Add Alphascale ASM9260 support.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by

@@ -28,6 +30,8 @@
 #include <linux/stmp_device.h>
 #include <asm/exception.h>
 
+#include "alphascale_asm9260-icoll.h"
+
 /*
  * this device provide 4 offsets for each register:
  * 0x0 - plain read write mode

@@ -49,6 +53,11 @@
 
 #define ICOLL_NUM_IRQS		128
 
+enum icoll_type {
+	ICOLL,
+	ASM9260_ICOLL,
+};
+
 struct icoll_priv {
 	void __iomem *vector;
 	void __iomem *levelack;

@@ -58,10 +67,38 @@ struct icoll_priv {
 	/* number of interrupts per register */
 	int ;
 	void __iomem *clear;
+	enum icoll_type type;
 };
 
 static struct icoll_priv icoll_priv;
 static struct irq_domain *icoll_domain;
+static DEFINE_RAW_SPINLOCK(icoll_lock);
+
+/* calculate bit offset depending on number of intterupt per register */
+static u32 icoll_intr_bitshift(struct irq_data *d, u32 bit)
+{
+	/*
+	 * We expect intr_per_reg to be 4 or 1, it means
+	 * "n" will be 3 or 0.
+	 */
+	int n = icoll_priv.intr_per_reg - 1;
+
+	/*
+	 * If n = 0, "bit" is never shifted.
+	 * If n = 3, mask lower part of hwirq to convert it
+	 * in 0, 1, 2 or 3 and then multiply it by 8 (or shift by 3)
+	 */
+	return bit << ((d->hwirq & n) << n);

drivers/irqchip/irq-mxs.c struct icoll_priv { · line 101

Quoted 10 lines

drivers/irqchip/irq-mxs.c +10 -0

+}
+
+/* calculate mem offset depending on number of intterupt per register */
+static void __iomem *icoll_intr_reg(struct irq_data *d)
+{
+	int n = icoll_priv.intr_per_reg >> 1;
+
+	/* offset = hwirq / intr_per_reg * 0x10 */
+	return icoll_priv.intr + ((d->hwirq >> n) * 0x10);
+}

Please correct me if I'm wrong, but it looks like these function are only useful when used on ams9260. So why do we need intr_per_reg at all? MXS doesn't need it (always 1), and ams9260 always need it (always 4). Save yourself some previous cycles and simplify the whole thing.

Quoted 16 lines

drivers/irqchip/irq-mxs.c +9 -0

 static void icoll_ack_irq(struct irq_data *d)
 {

@@ -86,12 +123,38 @@ static void icoll_unmask_irq(struct irq_data *d)
 			icoll_priv.intr + SET_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
+static void asm9260_mask_irq(struct irq_data *d)
+{
+	raw_spin_lock(&icoll_lock);
+	__raw_writel(icoll_intr_bitshift(d, BM_ICOLL_INTR_ENABLE),
+			icoll_intr_reg(d) + CLR_REG);
+	raw_spin_unlock(&icoll_lock);
+}
+
+static void asm9260_unmask_irq(struct irq_data *d)

drivers/irqchip/irq-mxs.c static void icoll_unmask_irq(struct irq_data *d) · line 144

Quoted 10 lines

drivers/irqchip/irq-mxs.c +10 -0

+{
+	raw_spin_lock(&icoll_lock);
+	__raw_writel(ASM9260_BM_CLEAR_BIT(d->hwirq),
+		     icoll_priv.clear +
+		     ASM9260_HW_ICOLL_CLEARn(d->hwirq));
+
+	__raw_writel(icoll_intr_bitshift(d, BM_ICOLL_INTR_ENABLE),
+			icoll_intr_reg(d) + SET_REG);
+	raw_spin_unlock(&icoll_lock);
+}

Can you please explain the rational for this lock? mask/unmask use different registers, and it is not obvious to me what race you are trying to avoid here.

Thanks,

M.

Signature · 1 line

Jazz is not dead. It just smells funny.

Oleksij Rempel

author

2015-09-19 05:53 UTC #5 lore raw

Re: Possible Spam [PATCH v2 2/2] ARM: irqchip: mxs: add Alpascale ASM9260 support

Am 18.09.2015 um 12:42 schrieb Marc Zyngier:

Quoted 8 lines

Quoted 3 lines · On Fri, 18 Sep 2015 11:18:42 +0200 Oleksij Rempel <[email protected]>…
From: Oleksij Rempel <[email protected]>
Freescale iMX23/iMX28 and Alphascale ASM9260 have similar
Is it Alphascale or Alpascale? You may need to fix the patch title.

ok.

Quoted 86 lines

Quoted 17 lines

interrupt collectors. It makes easy to reuse irq-mxs code for ASM9260. Differences between this devices are fallowing:
- different register offsets
- different count of intterupt lines per register
- ASM9260 don't provide reset bit
- ASM9260 don't support FIQ.
Signed-off-by Oleksij Rempel <[email protected]>
---
 drivers/irqchip/Kconfig                    |   5 ++
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 +++++++++++++++++++++++++++++
 drivers/irqchip/irq-mxs.c                  | 106 +++++++++++++++++++++++++++-
 4 files changed, 220 insertions(+), 2 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

[...]

Quoted 66 lines

drivers/irqchip/irq-mxs.c +34 -0

index 14374de..1470087 100644
@@ -1,5 +1,7 @@
 /*
  * Copyright (C) 2009-2010 Freescale Semiconductor, Inc. All Rights Reserved.
+ * Copyright (C) 2014 Oleksij Rempel <[email protected]>
+ *	Add Alphascale ASM9260 support.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by

@@ -28,6 +30,8 @@
 #include <linux/stmp_device.h>
 #include <asm/exception.h>
 
+#include "alphascale_asm9260-icoll.h"
+
 /*
  * this device provide 4 offsets for each register:
  * 0x0 - plain read write mode

@@ -49,6 +53,11 @@
 
 #define ICOLL_NUM_IRQS		128
 
+enum icoll_type {
+	ICOLL,
+	ASM9260_ICOLL,
+};
+
 struct icoll_priv {
 	void __iomem *vector;
 	void __iomem *levelack;

@@ -58,10 +67,38 @@ struct icoll_priv {
 	/* number of interrupts per register */
 	int ;
 	void __iomem *clear;
+	enum icoll_type type;
 };
 
 static struct icoll_priv icoll_priv;
 static struct irq_domain *icoll_domain;
+static DEFINE_RAW_SPINLOCK(icoll_lock);
+
+/* calculate bit offset depending on number of intterupt per register */
+static u32 icoll_intr_bitshift(struct irq_data *d, u32 bit)
+{
+	/*
+	 * We expect intr_per_reg to be 4 or 1, it means
+	 * "n" will be 3 or 0.
+	 */
+	int n = icoll_priv.intr_per_reg - 1;
+
+	/*
+	 * If n = 0, "bit" is never shifted.
+	 * If n = 3, mask lower part of hwirq to convert it
+	 * in 0, 1, 2 or 3 and then multiply it by 8 (or shift by 3)
+	 */
+	return bit << ((d->hwirq & n) << n);
+}
+
+/* calculate mem offset depending on number of intterupt per register */
+static void __iomem *icoll_intr_reg(struct irq_data *d)
+{
+	int n = icoll_priv.intr_per_reg >> 1;
+

Quoted 8 lines

drivers/irqchip/irq-mxs.c struct icoll_priv { · line 101
Quoted 3 lines
drivers/irqchip/irq-mxs.c +3 -0
+	/* offset = hwirq / intr_per_reg * 0x10 */
+	return icoll_priv.intr + ((d->hwirq >> n) * 0x10);
+}
Please correct me if I'm wrong, but it looks like these function are only useful when used on ams9260. So why do we need intr_per_reg at all? MXS doesn't need it (always 1), and ams9260 always need it (always 4). Save yourself some previous cycles and simplify the whole thing.

ok.

Quoted 22 lines

drivers/irqchip/irq-mxs.c +15 -0

 static void icoll_ack_irq(struct irq_data *d)
 {

@@ -86,12 +123,38 @@ static void icoll_unmask_irq(struct irq_data *d)
 			icoll_priv.intr + SET_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
+static void asm9260_mask_irq(struct irq_data *d)
+{
+	raw_spin_lock(&icoll_lock);
+	__raw_writel(icoll_intr_bitshift(d, BM_ICOLL_INTR_ENABLE),
+			icoll_intr_reg(d) + CLR_REG);
+	raw_spin_unlock(&icoll_lock);
+}
+
+static void asm9260_unmask_irq(struct irq_data *d)
+{
+	raw_spin_lock(&icoll_lock);
+	__raw_writel(ASM9260_BM_CLEAR_BIT(d->hwirq),
+		     icoll_priv.clear +
+		     ASM9260_HW_ICOLL_CLEARn(d->hwirq));
+

Quoted 8 lines

drivers/irqchip/irq-mxs.c static void icoll_unmask_irq(struct irq_data *d) · line 144
Quoted 4 lines
drivers/irqchip/irq-mxs.c +4 -0
+	__raw_writel(icoll_intr_bitshift(d, BM_ICOLL_INTR_ENABLE),
+			icoll_intr_reg(d) + SET_REG);
+	raw_spin_unlock(&icoll_lock);
+}
Can you please explain the rational for this lock? mask/unmask use different registers, and it is not obvious to me what race you are trying to avoid here.

Uff... in one of earliest reviews i was asked to add lock.. I also was asked to add asm9260 to some existing driver. Not sure if it is still making sense.

Signature · 2 lines

Regards,
Oleksij

Marc Zyngier

2015-09-20 11:28 UTC #6 lore raw

Re: [PATCH v2 2/2] ARM: irqchip: mxs: add Alpascale ASM9260 support

Quoted 4 lines · On Sat, 19 Sep 2015 07:53:34 +0200 Oleksij Rempel <[email protected]>…

Am 18.09.2015 um 12:42 schrieb Marc Zyngier:
Quoted 2 lines
On Fri, 18 Sep 2015 11:18:42 +0200
Oleksij Rempel <[email protected]> wrote:

[...]

Quoted 24 lines

Quoted 15 lines
Quoted 15 lines
drivers/irqchip/irq-mxs.c +15 -0
+static void asm9260_mask_irq(struct irq_data *d)
+{
+	raw_spin_lock(&icoll_lock);
+	__raw_writel(icoll_intr_bitshift(d, BM_ICOLL_INTR_ENABLE),
+			icoll_intr_reg(d) + CLR_REG);
+	raw_spin_unlock(&icoll_lock);
+}
+
+static void asm9260_unmask_irq(struct irq_data *d)
+{
+	raw_spin_lock(&icoll_lock);
+	__raw_writel(ASM9260_BM_CLEAR_BIT(d->hwirq),
+		     icoll_priv.clear +
+		     ASM9260_HW_ICOLL_CLEARn(d->hwirq));
+
Quoted 8 lines
drivers/irqchip/irq-mxs.c static void icoll_unmask_irq(struct irq_data *d) · line 144
Quoted 4 lines
drivers/irqchip/irq-mxs.c +4 -0
+	__raw_writel(icoll_intr_bitshift(d, BM_ICOLL_INTR_ENABLE),
+			icoll_intr_reg(d) + SET_REG);
+	raw_spin_unlock(&icoll_lock);
+}
Can you please explain the rational for this lock? mask/unmask use different registers, and it is not obvious to me what race you are trying to avoid here.

Quoted 3 lines

Uff... in one of earliest reviews i was asked to add lock.. I also was asked to add asm9260 to some existing driver. Not sure if it is still making sense.

Adding or removing a lock is not about what people ask you to do. It is about requirements dictated by either the HW (you need to perform a given sequence atomically with respect to another code sequence), the kernel, or both. So I'd like to understand what is the underlying reason for this lock. It is not disabling interrupts, so it could end up being called in an interrupt context -> deadlock.

So either the HW requires it and you have the wrong accessors, or it doesn't, and you can remove it. Either way, we need to know.

M.

Signature · 1 line

Jazz is not dead. It just smells funny.

Oleksij Rempel

author

2015-09-20 13:50 UTC #7 lore raw

Re: [PATCH v2 2/2] ARM: irqchip: mxs: add Alpascale ASM9260 support

Am 20.09.2015 um 13:28 schrieb Marc Zyngier:

Quoted 38 lines

Quoted 4 lines · On Sat, 19 Sep 2015 07:53:34 +0200 Oleksij Rempel <[email protected]>…
Am 18.09.2015 um 12:42 schrieb Marc Zyngier:
Quoted 2 lines
On Fri, 18 Sep 2015 11:18:42 +0200
Oleksij Rempel <[email protected]> wrote:
[...]
Quoted 24 lines
Quoted 15 lines
Quoted 15 lines
drivers/irqchip/irq-mxs.c +15 -0
+static void asm9260_mask_irq(struct irq_data *d)
+{
+	raw_spin_lock(&icoll_lock);
+	__raw_writel(icoll_intr_bitshift(d, BM_ICOLL_INTR_ENABLE),
+			icoll_intr_reg(d) + CLR_REG);
+	raw_spin_unlock(&icoll_lock);
+}
+
+static void asm9260_unmask_irq(struct irq_data *d)
+{
+	raw_spin_lock(&icoll_lock);
+	__raw_writel(ASM9260_BM_CLEAR_BIT(d->hwirq),
+		     icoll_priv.clear +
+		     ASM9260_HW_ICOLL_CLEARn(d->hwirq));
+
Quoted 8 lines
drivers/irqchip/irq-mxs.c static void icoll_unmask_irq(struct irq_data *d) · line 144
Quoted 4 lines
drivers/irqchip/irq-mxs.c +4 -0
+	__raw_writel(icoll_intr_bitshift(d, BM_ICOLL_INTR_ENABLE),
+			icoll_intr_reg(d) + SET_REG);
+	raw_spin_unlock(&icoll_lock);
+}
Can you please explain the rational for this lock? mask/unmask use different registers, and it is not obvious to me what race you are trying to avoid here.
Quoted 3 lines
Uff... in one of earliest reviews i was asked to add lock.. I also was asked to add asm9260 to some existing driver. Not sure if it is still making sense.

Quoted 6 lines

Adding or removing a lock is not about what people ask you to do. It is about requirements dictated by either the HW (you need to perform a given sequence atomically with respect to another code sequence), the kernel, or both. So I'd like to understand what is the underlying reason for this lock. It is not disabling interrupts, so it could end up being called in an interrupt context -> deadlock.

You right.

Quoted 4 lines

So either the HW requires it and you have the wrong accessors, or it doesn't, and you can remove it. Either way, we need to know.
	M.

I don't see any need to lock it.

Signature · 2 lines

Regards,
Oleksij

Thomas Gleixner

2015-09-21 09:07 UTC #8 lore raw

Re: [PATCH v2 2/2] ARM: irqchip: mxs: add Alpascale ASM9260 support

In reply to Marc Zyngier

Quoted 10 lines · Marc Zyngier

Quoted 3 lines
Uff... in one of earliest reviews i was asked to add lock.. I also was asked to add asm9260 to some existing driver. Not sure if it is still making sense.
Adding or removing a lock is not about what people ask you to do. It is about requirements dictated by either the HW (you need to perform a given sequence atomically with respect to another code sequence), the kernel, or both. So I'd like to understand what is the underlying reason for this lock. It is not disabling interrupts, so it could end up being called in an interrupt context -> deadlock.

The callback is always called with interrupts disabled, so that's a non issue. But still, the rationale for that lock is unclear.

Thanks,

tglx

Oleksij Rempel

author

2015-09-21 11:53 UTC #9 lore raw

v3 cover Add support for ASM9260 interrupt controller

In reply to Marc Zyngier

v2:
- move icoll_init_iobasea and icoll_add_domain to preparation patch
- add enum icoll_type
- make separate irq_chip for ICOLL and ASM9260

v3:
- fix typo in subject: Alp_h_ascale
- remove "intr_per_reg", it is not used any more. It will also save some CPU cycles
- remove spin locks. It was a bad idea.

Oleksij Rempel (2):
ARM: irqchip: mxs: prepare driver for HW with different offsets
ARM: irqchip: mxs: add Alphascale ASM9260 support

 drivers/irqchip/Kconfig                    |   5 +
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 ++++++++++++++++++
 drivers/irqchip/irq-mxs.c                  | 171 ++++++++++++++++++++++++++---
 4 files changed, 269 insertions(+), 18 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

Signature · 1 line

2.5.0

Oleksij Rempel

author

2015-09-21 11:54 UTC #10 lore raw

PATCH v3 1/2 ARM: irqchip: mxs: prepare driver for HW with different offsets

Some HW has similar functionality but different register offsets. Make sure we can change offsets dynamically.

Signed-off-by Oleksij Rempel <[email protected]>

---
 drivers/irqchip/irq-mxs.c | 83 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 67 insertions(+), 16 deletions(-)

drivers/irqchip/irq-mxs.c +67 ~~-16~~

index 1faf812..14374de 100644
@@ -28,18 +28,39 @@
 #include <linux/stmp_device.h>
 #include <asm/exception.h>
 
+/*
+ * this device provide 4 offsets for each register:
+ * 0x0 - plain read write mode
+ * 0x4 - set mode, OR logic.
+ * 0x8 - clr mode, XOR logic.
+ * 0xc - togle mode.
+ */
+#define SET_REG 4
+#define CLR_REG 8
+
 #define HW_ICOLL_VECTOR				0x0000
 #define HW_ICOLL_LEVELACK			0x0010
 #define HW_ICOLL_CTRL				0x0020
 #define HW_ICOLL_STAT_OFFSET			0x0070
-#define HW_ICOLL_INTERRUPTn_SET(n)		(0x0124 + (n) * 0x10)
-#define HW_ICOLL_INTERRUPTn_CLR(n)		(0x0128 + (n) * 0x10)
-#define BM_ICOLL_INTERRUPTn_ENABLE		0x00000004
+#define HW_ICOLL_INTERRUPT0			0x0120
+#define HW_ICOLL_INTERRUPTn(n)			((n) * 0x10)
+#define BM_ICOLL_INTR_ENABLE			BIT(2)
 #define BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0	0x1
 
 #define ICOLL_NUM_IRQS		128
 
-static void __iomem *icoll_base;
+struct icoll_priv {
+	void __iomem *vector;
+	void __iomem *levelack;
+	void __iomem *ctrl;
+	void __iomem *stat;
+	void __iomem *intr;
+	/* number of interrupts per register */
+	int intr_per_reg;
+	void __iomem *clear;
+};
+
+static struct icoll_priv icoll_priv;
 static struct irq_domain *icoll_domain;
 
 static void icoll_ack_irq(struct irq_data *d)

@@ -50,19 +71,19 @@ static void icoll_ack_irq(struct irq_data *d)
 	 * BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0 unconditionally.
 	 */
 	__raw_writel(BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0,
-			icoll_base + HW_ICOLL_LEVELACK);
+			icoll_priv.levelack);
 }
 
 static void icoll_mask_irq(struct irq_data *d)
 {
-	__raw_writel(BM_ICOLL_INTERRUPTn_ENABLE,
-			icoll_base + HW_ICOLL_INTERRUPTn_CLR(d->hwirq));
+	__raw_writel(BM_ICOLL_INTR_ENABLE,
+			icoll_priv.intr + CLR_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
 static void icoll_unmask_irq(struct irq_data *d)
 {
-	__raw_writel(BM_ICOLL_INTERRUPTn_ENABLE,
-			icoll_base + HW_ICOLL_INTERRUPTn_SET(d->hwirq));
+	__raw_writel(BM_ICOLL_INTR_ENABLE,
+			icoll_priv.intr + SET_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
 static struct irq_chip mxs_icoll_chip = {

@@ -75,8 +96,8 @@ asmlinkage void __exception_irq_entry icoll_handle_irq(struct pt_regs *regs)
 {
 	u32 irqnr;
 
-	irqnr = __raw_readl(icoll_base + HW_ICOLL_STAT_OFFSET);
-	__raw_writel(irqnr, icoll_base + HW_ICOLL_VECTOR);
+	irqnr = __raw_readl(icoll_priv.stat);
+	__raw_writel(irqnr, icoll_priv.vector);
 	handle_domain_irq(icoll_domain, irqnr, regs);
 }

@@ -94,20 +115,50 @@ static const struct irq_domain_ops icoll_irq_domain_ops = {
 	.xlate = irq_domain_xlate_onecell,
 };
 
+static void __init icoll_add_domain(struct device_node *np,
+			  int num)
+{
+	icoll_domain = irq_domain_add_linear(np, num,
+					     &icoll_irq_domain_ops, NULL);
+
+	if (!icoll_domain)
+		panic("%s: unable add irq domain", np->full_name);
+	irq_set_default_host(icoll_domain);
+	set_handle_irq(icoll_handle_irq);
+}
+
+static void __iomem * __init icoll_init_iobase(struct device_node *np)
+{
+	void __iomem *icoll_base;
+
+	icoll_base = of_io_request_and_map(np, 0, np->name);
+	if (!icoll_base)
+		panic("%s: unable to map resource", np->full_name);
+	return icoll_base;
+}
+
 static int __init icoll_of_init(struct device_node *np,
 			  struct device_node *interrupt_parent)
 {
-	icoll_base = of_iomap(np, 0);
-	WARN_ON(!icoll_base);
+	void __iomem *icoll_base;
+
+	icoll_base		= icoll_init_iobase(np);
+	icoll_priv.vector	= icoll_base + HW_ICOLL_VECTOR;
+	icoll_priv.levelack	= icoll_base + HW_ICOLL_LEVELACK;
+	icoll_priv.ctrl		= icoll_base + HW_ICOLL_CTRL;
+	icoll_priv.stat		= icoll_base + HW_ICOLL_STAT_OFFSET;
+	icoll_priv.intr		= icoll_base + HW_ICOLL_INTERRUPT0;
+	icoll_priv.intr_per_reg	= 1;
+	icoll_priv.clear	= NULL;
 
 	/*
 	 * Interrupt Collector reset, which initializes the priority
 	 * for each irq to level 0.
 	 */
-	stmp_reset_block(icoll_base + HW_ICOLL_CTRL);
+	stmp_reset_block(icoll_priv.ctrl);
+
+	icoll_add_domain(np, ICOLL_NUM_IRQS);
 
-	icoll_domain = irq_domain_add_linear(np, ICOLL_NUM_IRQS,
-					     &icoll_irq_domain_ops, NULL);
 	return icoll_domain ? 0 : -ENODEV;
 }
 IRQCHIP_DECLARE(mxs, "fsl,icoll", icoll_of_init);

Signature · 1 line

2.5.0

Oleksij Rempel

author

2015-09-21 11:54 UTC #11 lore raw

PATCH v3 2/2 ARM: irqchip: mxs: add Alphascale ASM9260 support

In reply to Oleksij Rempel

From: Oleksij Rempel <[email protected]>

Signed-off-by Oleksij Rempel <[email protected]>

---
 drivers/irqchip/Kconfig                    |   5 ++
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 +++++++++++++++++++++++++++++
 drivers/irqchip/irq-mxs.c                  |  94 +++++++++++++++++++++++--
 4 files changed, 205 insertions(+), 5 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

drivers/irqchip/Kconfig +5 -0

index 27b52c8..0dfd840 100644
@@ -187,3 +187,8 @@ config IMX_GPCV2
 	select IRQ_DOMAIN
 	help
 	  Enables the wakeup IRQs for IMX platforms with GPCv2 block
+
+config IRQ_MXS
+	def_bool y if MACH_ASM9260 || ARCH_MXS
+	select IRQ_DOMAIN
+	select STMP_DEVICE

drivers/irqchip/Makefile +1 -1

index bb3048f..177f78f 100644
@@ -6,7 +6,7 @@ obj-$(CONFIG_ARCH_EXYNOS)		+= exynos-combiner.o
 obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
 obj-$(CONFIG_ARCH_MMP)			+= irq-mmp.o
 obj-$(CONFIG_ARCH_MVEBU)		+= irq-armada-370-xp.o
-obj-$(CONFIG_ARCH_MXS)			+= irq-mxs.o
+obj-$(CONFIG_IRQ_MXS)			+= irq-mxs.o
 obj-$(CONFIG_ARCH_TEGRA)		+= irq-tegra.o
 obj-$(CONFIG_ARCH_S3C24XX)		+= irq-s3c24xx.o
 obj-$(CONFIG_DW_APB_ICTL)		+= irq-dw-apb-ictl.o

drivers/irqchip/alphascale_asm9260-icoll.h new file +109 -0

109 lines not on this page · raw message

drivers/irqchip/irq-mxs.c +90 -4

156 lines not on this page · raw message

Signature · 1 line

2.5.0

Thomas Gleixner

2015-09-22 10:35 UTC #12 lore raw

Re: [PATCH v3 2/2] ARM: irqchip: mxs: add Alphascale ASM9260 support

drivers/irqchip/irq-mxs.c asmlinkage void __exception_irq_entry icoll_handle_irq(struct pt_regs *regs) · line 162

Quoted 13 lines · Oleksij Rempel

static int icoll_irq_domain_map(struct irq_domain *d, unsigned int virq,

drivers/irqchip/irq-mxs.c +8 -1

 				irq_hw_number_t hw)
 {
-	irq_set_chip_and_handler(virq, &mxs_icoll_chip, handle_level_irq);
+	struct irq_chip *chip;
+
+	if (icoll_priv.type == ICOLL)
+		chip = &mxs_icoll_chip;
+	else
+		chip = &asm9260_icoll_chip;
+
+	irq_set_chip_and_handler(virq, chip, handle_level_irq);
 	set_irq_flags(virq, IRQF_VALID);

set_irq_flags() is gone. Please redo against 4.3-rc2

Thanks,

tglx

Oleksij Rempel

author

2015-09-23 16:20 UTC #13 lore raw

v4 cover Add support for ASM9260 interrupt controller

v2:
- move icoll_init_iobasea and icoll_add_domain to preparation patch
- add enum icoll_type
- make separate irq_chip for ICOLL and ASM9260

v3:
- fix typo in subject: Alp_h_ascale
- remove "intr_per_reg", it is not used any more. It will also save some CPU cycles
- remove spin locks. It was a bad idea.

v4:
- remove set_irq_flags()

Oleksij Rempel (2):
ARM: irqchip: mxs: prepare driver for HW with different offsets
ARM: irqchip: mxs: add Alphascale ASM9260 support

 drivers/irqchip/Kconfig                    |   5 +
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 ++++++++++++++++++
 drivers/irqchip/irq-mxs.c                  | 171 ++++++++++++++++++++++++++---
 4 files changed, 269 insertions(+), 18 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

Signature · 1 line

2.5.0

Oleksij Rempel

author

2015-09-23 16:20 UTC #14 lore raw

PATCH v4 1/2 ARM: irqchip: mxs: prepare driver for HW with different offsets

Some HW has similar functionality but different register offsets. Make sure we can change offsets dynamically.

Signed-off-by Oleksij Rempel <[email protected]>

---
 drivers/irqchip/irq-mxs.c | 83 ++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 67 insertions(+), 16 deletions(-)

drivers/irqchip/irq-mxs.c +67 ~~-16~~

index 604df63..a8e14c3 100644
@@ -28,18 +28,39 @@
 #include <linux/stmp_device.h>
 #include <asm/exception.h>
 
+/*
+ * this device provide 4 offsets for each register:
+ * 0x0 - plain read write mode
+ * 0x4 - set mode, OR logic.
+ * 0x8 - clr mode, XOR logic.
+ * 0xc - togle mode.
+ */
+#define SET_REG 4
+#define CLR_REG 8
+
 #define HW_ICOLL_VECTOR				0x0000
 #define HW_ICOLL_LEVELACK			0x0010
 #define HW_ICOLL_CTRL				0x0020
 #define HW_ICOLL_STAT_OFFSET			0x0070
-#define HW_ICOLL_INTERRUPTn_SET(n)		(0x0124 + (n) * 0x10)
-#define HW_ICOLL_INTERRUPTn_CLR(n)		(0x0128 + (n) * 0x10)
-#define BM_ICOLL_INTERRUPTn_ENABLE		0x00000004
+#define HW_ICOLL_INTERRUPT0			0x0120
+#define HW_ICOLL_INTERRUPTn(n)			((n) * 0x10)
+#define BM_ICOLL_INTR_ENABLE			BIT(2)
 #define BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0	0x1
 
 #define ICOLL_NUM_IRQS		128
 
-static void __iomem *icoll_base;
+struct icoll_priv {
+	void __iomem *vector;
+	void __iomem *levelack;
+	void __iomem *ctrl;
+	void __iomem *stat;
+	void __iomem *intr;
+	/* number of interrupts per register */
+	int intr_per_reg;
+	void __iomem *clear;
+};
+
+static struct icoll_priv icoll_priv;
 static struct irq_domain *icoll_domain;
 
 static void icoll_ack_irq(struct irq_data *d)

@@ -50,19 +71,19 @@ static void icoll_ack_irq(struct irq_data *d)
 	 * BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0 unconditionally.
 	 */
 	__raw_writel(BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0,
-			icoll_base + HW_ICOLL_LEVELACK);
+			icoll_priv.levelack);
 }
 
 static void icoll_mask_irq(struct irq_data *d)
 {
-	__raw_writel(BM_ICOLL_INTERRUPTn_ENABLE,
-			icoll_base + HW_ICOLL_INTERRUPTn_CLR(d->hwirq));
+	__raw_writel(BM_ICOLL_INTR_ENABLE,
+			icoll_priv.intr + CLR_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
 static void icoll_unmask_irq(struct irq_data *d)
 {
-	__raw_writel(BM_ICOLL_INTERRUPTn_ENABLE,
-			icoll_base + HW_ICOLL_INTERRUPTn_SET(d->hwirq));
+	__raw_writel(BM_ICOLL_INTR_ENABLE,
+			icoll_priv.intr + SET_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
 static struct irq_chip mxs_icoll_chip = {

@@ -75,8 +96,8 @@ asmlinkage void __exception_irq_entry icoll_handle_irq(struct pt_regs *regs)
 {
 	u32 irqnr;
 
-	irqnr = __raw_readl(icoll_base + HW_ICOLL_STAT_OFFSET);
-	__raw_writel(irqnr, icoll_base + HW_ICOLL_VECTOR);
+	irqnr = __raw_readl(icoll_priv.stat);
+	__raw_writel(irqnr, icoll_priv.vector);
 	handle_domain_irq(icoll_domain, irqnr, regs);
 }

@@ -93,20 +114,50 @@ static const struct irq_domain_ops icoll_irq_domain_ops = {
 	.xlate = irq_domain_xlate_onecell,
 };
 
+static void __init icoll_add_domain(struct device_node *np,
+			  int num)
+{
+	icoll_domain = irq_domain_add_linear(np, num,
+					     &icoll_irq_domain_ops, NULL);
+
+	if (!icoll_domain)
+		panic("%s: unable add irq domain", np->full_name);
+	irq_set_default_host(icoll_domain);
+	set_handle_irq(icoll_handle_irq);
+}
+
+static void __iomem * __init icoll_init_iobase(struct device_node *np)
+{
+	void __iomem *icoll_base;
+
+	icoll_base = of_io_request_and_map(np, 0, np->name);
+	if (!icoll_base)
+		panic("%s: unable to map resource", np->full_name);
+	return icoll_base;
+}
+
 static int __init icoll_of_init(struct device_node *np,
 			  struct device_node *interrupt_parent)
 {
-	icoll_base = of_iomap(np, 0);
-	WARN_ON(!icoll_base);
+	void __iomem *icoll_base;
+
+	icoll_base		= icoll_init_iobase(np);
+	icoll_priv.vector	= icoll_base + HW_ICOLL_VECTOR;
+	icoll_priv.levelack	= icoll_base + HW_ICOLL_LEVELACK;
+	icoll_priv.ctrl		= icoll_base + HW_ICOLL_CTRL;
+	icoll_priv.stat		= icoll_base + HW_ICOLL_STAT_OFFSET;
+	icoll_priv.intr		= icoll_base + HW_ICOLL_INTERRUPT0;
+	icoll_priv.intr_per_reg	= 1;
+	icoll_priv.clear	= NULL;
 
 	/*
 	 * Interrupt Collector reset, which initializes the priority
 	 * for each irq to level 0.
 	 */
-	stmp_reset_block(icoll_base + HW_ICOLL_CTRL);
+	stmp_reset_block(icoll_priv.ctrl);
+
+	icoll_add_domain(np, ICOLL_NUM_IRQS);
 
-	icoll_domain = irq_domain_add_linear(np, ICOLL_NUM_IRQS,
-					     &icoll_irq_domain_ops, NULL);
 	return icoll_domain ? 0 : -ENODEV;
 }
 IRQCHIP_DECLARE(mxs, "fsl,icoll", icoll_of_init);

Signature · 1 line

2.5.0

Thomas Gleixner

2015-09-29 16:07 UTC #15 lore raw

Re: [PATCH v4 1/2] ARM: irqchip: mxs: prepare driver for HW with different offsets

On Wed, 23 Sep 2015, Oleksij Rempel wrote:

Subject: irqchip/mxs: ..... Please

drivers/irqchip/irq-mxs.c line 59

Quoted 8 lines

drivers/irqchip/irq-mxs.c +8 -0

+struct icoll_priv {
+	void __iomem *vector;
+	void __iomem *levelack;
+	void __iomem *ctrl;
+	void __iomem *stat;
+	void __iomem *intr;
+	/* number of interrupts per register */
+	int intr_per_reg;

Why are you adding this here just to remove it in the next patch?

drivers/irqchip/irq-mxs.c line 60

Quoted 1 line

drivers/irqchip/irq-mxs.c +1 -0
+	void __iomem *clear;

Please do not add members which are not required for the preparatory step.

Thanks,

tglx

Oleksij Rempel

author

2015-10-05 19:00 UTC #16 lore raw

v5 cover Add support for ASM9260 interrupt controller

v2:
- move icoll_init_iobasea and icoll_add_domain to preparation patch
- add enum icoll_type
- make separate irq_chip for ICOLL and ASM9260

v3:
- fix typo in subject: Alp_h_ascale
- remove "intr_per_reg", it is not used any more. It will also save some CPU cycles
- remove spin locks. It was a bad idea.

v4:
- remove set_irq_flags()

v5:
- remove intr_per_reg from first patch
- move *clear to second patch

Oleksij Rempel (2):
ARM: irqchip: mxs: prepare driver for HW with different offsets
ARM: irqchip: mxs: add Alphascale ASM9260 support

 drivers/irqchip/Kconfig                    |   5 +
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 ++++++++++++++++++
 drivers/irqchip/irq-mxs.c                  | 171 ++++++++++++++++++++++++++---
 4 files changed, 269 insertions(+), 18 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

Signature · 1 line

2.5.0

Oleksij Rempel

author

2015-10-05 19:00 UTC #17 lore raw

PATCH v5 1/2 ARM: irqchip: mxs: prepare driver for HW with different offsets

Some HW has similar functionality but different register offsets. Make sure we can change offsets dynamically.

Signed-off-by Oleksij Rempel <[email protected]>

---
 drivers/irqchip/irq-mxs.c | 78 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 62 insertions(+), 16 deletions(-)

drivers/irqchip/irq-mxs.c +62 ~~-16~~

index 1faf812..1ed759b 100644
@@ -28,18 +28,36 @@
 #include <linux/stmp_device.h>
 #include <asm/exception.h>
 
+/*
+ * this device provide 4 offsets for each register:
+ * 0x0 - plain read write mode
+ * 0x4 - set mode, OR logic.
+ * 0x8 - clr mode, XOR logic.
+ * 0xc - togle mode.
+ */
+#define SET_REG 4
+#define CLR_REG 8
+
 #define HW_ICOLL_VECTOR				0x0000
 #define HW_ICOLL_LEVELACK			0x0010
 #define HW_ICOLL_CTRL				0x0020
 #define HW_ICOLL_STAT_OFFSET			0x0070
-#define HW_ICOLL_INTERRUPTn_SET(n)		(0x0124 + (n) * 0x10)
-#define HW_ICOLL_INTERRUPTn_CLR(n)		(0x0128 + (n) * 0x10)
-#define BM_ICOLL_INTERRUPTn_ENABLE		0x00000004
+#define HW_ICOLL_INTERRUPT0			0x0120
+#define HW_ICOLL_INTERRUPTn(n)			((n) * 0x10)
+#define BM_ICOLL_INTR_ENABLE			BIT(2)
 #define BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0	0x1
 
 #define ICOLL_NUM_IRQS		128
 
-static void __iomem *icoll_base;
+struct icoll_priv {
+	void __iomem *vector;
+	void __iomem *levelack;
+	void __iomem *ctrl;
+	void __iomem *stat;
+	void __iomem *intr;
+};
+
+static struct icoll_priv icoll_priv;
 static struct irq_domain *icoll_domain;
 
 static void icoll_ack_irq(struct irq_data *d)

@@ -50,19 +68,19 @@ static void icoll_ack_irq(struct irq_data *d)
 	 * BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0 unconditionally.
 	 */
 	__raw_writel(BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0,
-			icoll_base + HW_ICOLL_LEVELACK);
+			icoll_priv.levelack);
 }
 
 static void icoll_mask_irq(struct irq_data *d)
 {
-	__raw_writel(BM_ICOLL_INTERRUPTn_ENABLE,
-			icoll_base + HW_ICOLL_INTERRUPTn_CLR(d->hwirq));
+	__raw_writel(BM_ICOLL_INTR_ENABLE,
+			icoll_priv.intr + CLR_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
 static void icoll_unmask_irq(struct irq_data *d)
 {
-	__raw_writel(BM_ICOLL_INTERRUPTn_ENABLE,
-			icoll_base + HW_ICOLL_INTERRUPTn_SET(d->hwirq));
+	__raw_writel(BM_ICOLL_INTR_ENABLE,
+			icoll_priv.intr + SET_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
 static struct irq_chip mxs_icoll_chip = {

@@ -75,8 +93,8 @@ asmlinkage void __exception_irq_entry icoll_handle_irq(struct pt_regs *regs)
 {
 	u32 irqnr;
 
-	irqnr = __raw_readl(icoll_base + HW_ICOLL_STAT_OFFSET);
-	__raw_writel(irqnr, icoll_base + HW_ICOLL_VECTOR);
+	irqnr = __raw_readl(icoll_priv.stat);
+	__raw_writel(irqnr, icoll_priv.vector);
 	handle_domain_irq(icoll_domain, irqnr, regs);
 }

@@ -94,20 +112,48 @@ static const struct irq_domain_ops icoll_irq_domain_ops = {
 	.xlate = irq_domain_xlate_onecell,
 };
 
+static void __init icoll_add_domain(struct device_node *np,
+			  int num)
+{
+	icoll_domain = irq_domain_add_linear(np, num,
+					     &icoll_irq_domain_ops, NULL);
+
+	if (!icoll_domain)
+		panic("%s: unable add irq domain", np->full_name);
+	irq_set_default_host(icoll_domain);
+	set_handle_irq(icoll_handle_irq);
+}
+
+static void __iomem * __init icoll_init_iobase(struct device_node *np)
+{
+	void __iomem *icoll_base;
+
+	icoll_base = of_io_request_and_map(np, 0, np->name);
+	if (!icoll_base)
+		panic("%s: unable to map resource", np->full_name);
+	return icoll_base;
+}
+
 static int __init icoll_of_init(struct device_node *np,
 			  struct device_node *interrupt_parent)
 {
-	icoll_base = of_iomap(np, 0);
-	WARN_ON(!icoll_base);
+	void __iomem *icoll_base;
+
+	icoll_base		= icoll_init_iobase(np);
+	icoll_priv.vector	= icoll_base + HW_ICOLL_VECTOR;
+	icoll_priv.levelack	= icoll_base + HW_ICOLL_LEVELACK;
+	icoll_priv.ctrl		= icoll_base + HW_ICOLL_CTRL;
+	icoll_priv.stat		= icoll_base + HW_ICOLL_STAT_OFFSET;
+	icoll_priv.intr		= icoll_base + HW_ICOLL_INTERRUPT0;
 
 	/*
 	 * Interrupt Collector reset, which initializes the priority
 	 * for each irq to level 0.
 	 */
-	stmp_reset_block(icoll_base + HW_ICOLL_CTRL);
+	stmp_reset_block(icoll_priv.ctrl);
+
+	icoll_add_domain(np, ICOLL_NUM_IRQS);
 
-	icoll_domain = irq_domain_add_linear(np, ICOLL_NUM_IRQS,
-					     &icoll_irq_domain_ops, NULL);
 	return icoll_domain ? 0 : -ENODEV;
 }
 IRQCHIP_DECLARE(mxs, "fsl,icoll", icoll_of_init);

Signature · 1 line

2.5.0

Oleksij Rempel

author

2015-10-05 19:00 UTC #18 lore raw

PATCH v5 2/2 ARM: irqchip: mxs: add Alphascale ASM9260 support

In reply to Oleksij Rempel

Signed-off-by Oleksij Rempel <[email protected]>

---
 drivers/irqchip/Kconfig                    |   5 ++
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 +++++++++++++++++++++++++++++
 drivers/irqchip/irq-mxs.c                  |  93 +++++++++++++++++++++++-
 4 files changed, 207 insertions(+), 2 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

drivers/irqchip/Kconfig +5 -0

index 27b52c8..0dfd840 100644
@@ -187,3 +187,8 @@ config IMX_GPCV2
 	select IRQ_DOMAIN
 	help
 	  Enables the wakeup IRQs for IMX platforms with GPCv2 block
+
+config IRQ_MXS
+	def_bool y if MACH_ASM9260 || ARCH_MXS
+	select IRQ_DOMAIN
+	select STMP_DEVICE

drivers/irqchip/Makefile +1 -1

index bb3048f..177f78f 100644
@@ -6,7 +6,7 @@ obj-$(CONFIG_ARCH_EXYNOS)		+= exynos-combiner.o
 obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
 obj-$(CONFIG_ARCH_MMP)			+= irq-mmp.o
 obj-$(CONFIG_ARCH_MVEBU)		+= irq-armada-370-xp.o
-obj-$(CONFIG_ARCH_MXS)			+= irq-mxs.o
+obj-$(CONFIG_IRQ_MXS)			+= irq-mxs.o
 obj-$(CONFIG_ARCH_TEGRA)		+= irq-tegra.o
 obj-$(CONFIG_ARCH_S3C24XX)		+= irq-s3c24xx.o
 obj-$(CONFIG_DW_APB_ICTL)		+= irq-dw-apb-ictl.o

drivers/irqchip/alphascale_asm9260-icoll.h new file +109 -0

109 lines not on this page · raw message

drivers/irqchip/irq-mxs.c +92 -1

154 lines not on this page · raw message

Signature · 1 line

2.5.0

Thomas Gleixner

2015-10-09 14:12 UTC #19 lore raw

Re: [PATCH v5 2/2] ARM: irqchip: mxs: add Alphascale ASM9260 support

Quoted 4 lines · Oleksij Rempel

drivers/irqchip/irq-mxs.c +0 -0

@@ -101,7 +151,14 @@ asmlinkage void __exception_irq_entry icoll_handle_irq(struct pt_regs *regs)
 static int icoll_irq_domain_map(struct irq_domain *d, unsigned int virq,
 				irq_hw_number_t hw)
 {

drivers/irqchip/irq-mxs.c asmlinkage void __exception_irq_entry icoll_handle_irq(struct pt_regs *regs) · line 162

Quoted 10 lines

drivers/irqchip/irq-mxs.c +8 -1

-	irq_set_chip_and_handler(virq, &mxs_icoll_chip, handle_level_irq);
+	struct irq_chip *chip;
+
+	if (icoll_priv.type == ICOLL)
+		chip = &mxs_icoll_chip;
+	else
+		chip = &asm9260_icoll_chip;
+
+	irq_set_chip_and_handler(virq, chip, handle_level_irq);
 	set_irq_flags(virq, IRQF_VALID);

This does neither apply on mainline nor on tip/irq/core. Please prepare your patches against current trees not against something stale.

Thanks,

tglx

Oleksij Rempel

author

2015-10-10 13:08 UTC #20 lore raw

v6 cover Add support for ASM9260 interrupt controller

v2:
- move icoll_init_iobasea and icoll_add_domain to preparation patch
- add enum icoll_type
- make separate irq_chip for ICOLL and ASM9260

v3:
- fix typo in subject: Alp_h_ascale
- remove "intr_per_reg", it is not used any more. It will also save some CPU cycles
- remove spin locks. It was a bad idea.

v4:
- remove set_irq_flags()

v5:
- remove intr_per_reg from first patch
- move *clear to second patch

v6:
- update against latest torvalds/linux.git master v4.3-rc4

Oleksij Rempel (2):
ARM: irqchip: mxs: prepare driver for HW with different offsets
ARM: irqchip: mxs: add Alphascale ASM9260 support

 drivers/irqchip/Kconfig                    |   5 +
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 ++++++++++++++++++
 drivers/irqchip/irq-mxs.c                  | 171 ++++++++++++++++++++++++++---
 4 files changed, 269 insertions(+), 18 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

Signature · 1 line

2.1.4

Oleksij Rempel

author

2015-10-10 13:09 UTC #21 lore raw

PATCH v6 1/2 ARM: irqchip: mxs: prepare driver for HW with different offsets

Some HW has similar functionality but different register offsets. Make sure we can change offsets dynamically.

Signed-off-by Oleksij Rempel <[email protected]>

---
 drivers/irqchip/irq-mxs.c | 78 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 62 insertions(+), 16 deletions(-)

drivers/irqchip/irq-mxs.c +62 ~~-16~~

index 604df63..86dec9a 100644
@@ -28,18 +28,36 @@
 #include <linux/stmp_device.h>
 #include <asm/exception.h>
 
+/*
+ * this device provide 4 offsets for each register:
+ * 0x0 - plain read write mode
+ * 0x4 - set mode, OR logic.
+ * 0x8 - clr mode, XOR logic.
+ * 0xc - togle mode.
+ */
+#define SET_REG 4
+#define CLR_REG 8
+
 #define HW_ICOLL_VECTOR				0x0000
 #define HW_ICOLL_LEVELACK			0x0010
 #define HW_ICOLL_CTRL				0x0020
 #define HW_ICOLL_STAT_OFFSET			0x0070
-#define HW_ICOLL_INTERRUPTn_SET(n)		(0x0124 + (n) * 0x10)
-#define HW_ICOLL_INTERRUPTn_CLR(n)		(0x0128 + (n) * 0x10)
-#define BM_ICOLL_INTERRUPTn_ENABLE		0x00000004
+#define HW_ICOLL_INTERRUPT0			0x0120
+#define HW_ICOLL_INTERRUPTn(n)			((n) * 0x10)
+#define BM_ICOLL_INTR_ENABLE			BIT(2)
 #define BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0	0x1
 
 #define ICOLL_NUM_IRQS		128
 
-static void __iomem *icoll_base;
+struct icoll_priv {
+	void __iomem *vector;
+	void __iomem *levelack;
+	void __iomem *ctrl;
+	void __iomem *stat;
+	void __iomem *intr;
+};
+
+static struct icoll_priv icoll_priv;
 static struct irq_domain *icoll_domain;
 
 static void icoll_ack_irq(struct irq_data *d)

@@ -50,19 +68,19 @@ static void icoll_ack_irq(struct irq_data *d)
 	 * BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0 unconditionally.
 	 */
 	__raw_writel(BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0,
-			icoll_base + HW_ICOLL_LEVELACK);
+			icoll_priv.levelack);
 }
 
 static void icoll_mask_irq(struct irq_data *d)
 {
-	__raw_writel(BM_ICOLL_INTERRUPTn_ENABLE,
-			icoll_base + HW_ICOLL_INTERRUPTn_CLR(d->hwirq));
+	__raw_writel(BM_ICOLL_INTR_ENABLE,
+			icoll_priv.intr + CLR_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
 static void icoll_unmask_irq(struct irq_data *d)
 {
-	__raw_writel(BM_ICOLL_INTERRUPTn_ENABLE,
-			icoll_base + HW_ICOLL_INTERRUPTn_SET(d->hwirq));
+	__raw_writel(BM_ICOLL_INTR_ENABLE,
+			icoll_priv.intr + SET_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
 static struct irq_chip mxs_icoll_chip = {

@@ -75,8 +93,8 @@ asmlinkage void __exception_irq_entry icoll_handle_irq(struct pt_regs *regs)
 {
 	u32 irqnr;
 
-	irqnr = __raw_readl(icoll_base + HW_ICOLL_STAT_OFFSET);
-	__raw_writel(irqnr, icoll_base + HW_ICOLL_VECTOR);
+	irqnr = __raw_readl(icoll_priv.stat);
+	__raw_writel(irqnr, icoll_priv.vector);
 	handle_domain_irq(icoll_domain, irqnr, regs);
 }

@@ -93,20 +111,48 @@ static const struct irq_domain_ops icoll_irq_domain_ops = {
 	.xlate = irq_domain_xlate_onecell,
 };
 
+static void __init icoll_add_domain(struct device_node *np,
+			  int num)
+{
+	icoll_domain = irq_domain_add_linear(np, num,
+					     &icoll_irq_domain_ops, NULL);
+
+	if (!icoll_domain)
+		panic("%s: unable add irq domain", np->full_name);
+	irq_set_default_host(icoll_domain);
+	set_handle_irq(icoll_handle_irq);
+}
+
+static void __iomem * __init icoll_init_iobase(struct device_node *np)
+{
+	void __iomem *icoll_base;
+
+	icoll_base = of_io_request_and_map(np, 0, np->name);
+	if (!icoll_base)
+		panic("%s: unable to map resource", np->full_name);
+	return icoll_base;
+}
+
 static int __init icoll_of_init(struct device_node *np,
 			  struct device_node *interrupt_parent)
 {
-	icoll_base = of_iomap(np, 0);
-	WARN_ON(!icoll_base);
+	void __iomem *icoll_base;
+
+	icoll_base		= icoll_init_iobase(np);
+	icoll_priv.vector	= icoll_base + HW_ICOLL_VECTOR;
+	icoll_priv.levelack	= icoll_base + HW_ICOLL_LEVELACK;
+	icoll_priv.ctrl		= icoll_base + HW_ICOLL_CTRL;
+	icoll_priv.stat		= icoll_base + HW_ICOLL_STAT_OFFSET;
+	icoll_priv.intr		= icoll_base + HW_ICOLL_INTERRUPT0;
 
 	/*
 	 * Interrupt Collector reset, which initializes the priority
 	 * for each irq to level 0.
 	 */
-	stmp_reset_block(icoll_base + HW_ICOLL_CTRL);
+	stmp_reset_block(icoll_priv.ctrl);
+
+	icoll_add_domain(np, ICOLL_NUM_IRQS);
 
-	icoll_domain = irq_domain_add_linear(np, ICOLL_NUM_IRQS,
-					     &icoll_irq_domain_ops, NULL);
 	return icoll_domain ? 0 : -ENODEV;
 }
 IRQCHIP_DECLARE(mxs, "fsl,icoll", icoll_of_init);

Signature · 1 line

2.1.4

Thomas Gleixner

2015-10-11 17:59 UTC #22 lore raw

Re: [PATCH v6 1/2] ARM: irqchip: mxs: prepare driver for HW with different offsets

Oleksij,

On Sat, 10 Oct 2015, Oleksij Rempel wrote:

The proper subject line starts with:

irqchip/mxs:

Quoted 2 lines

Some HW has similar functionality but different register offsets. Make sure we can change offsets dynamically.

The patch does way more than that. I told you in V2 already:

Quoted 1 line

Quoted 1 line
You forgot to mention the other preparatory changes.

There is still nothing in the changelog.

drivers/irqchip/irq-mxs.c static const struct irq_domain_ops icoll_irq_domain_ops = { · line 121

Quoted 8 lines

drivers/irqchip/irq-mxs.c +8 -0

+static void __init icoll_add_domain(struct device_node *np,
+			  int num)
+{
+	icoll_domain = irq_domain_add_linear(np, num,
+					     &icoll_irq_domain_ops, NULL);
+
+	if (!icoll_domain)
+		panic("%s: unable add irq domain", np->full_name);

This splitout should be a separate patch with an explanation why you add

drivers/irqchip/irq-mxs.c static const struct irq_domain_ops icoll_irq_domain_ops = { · line 122

Quoted 1 line

drivers/irqchip/irq-mxs.c +1 -0
+	irq_set_default_host(icoll_domain);

and

drivers/irqchip/irq-mxs.c static const struct irq_domain_ops icoll_irq_domain_ops = { · line 123

Quoted 1 line

drivers/irqchip/irq-mxs.c +1 -0
+	set_handle_irq(icoll_handle_irq);

The latter is already done via the DT_MACHINE_START magic. So you should it remove there, because otherwise that call is just pointless. See the implementation of set_handle_irq.

drivers/irqchip/irq-mxs.c static const struct irq_domain_ops icoll_irq_domain_ops = { · line 134

Quoted 11 lines

drivers/irqchip/irq-mxs.c +11 -0

+}
+
+static void __iomem * __init icoll_init_iobase(struct device_node *np)
+{
+	void __iomem *icoll_base;
+
+	icoll_base = of_io_request_and_map(np, 0, np->name);
+	if (!icoll_base)
+		panic("%s: unable to map resource", np->full_name);
+	return icoll_base;
+}

The panic() is actually a bug fix, because the old code had a WARN_ON() and then happily dereferenced the NULL pointer. So this wants to go into a separate patch as well.

drivers/irqchip/irq-mxs.c static const struct irq_domain_ops icoll_irq_domain_ops = { · line 156

Quoted 5 lines

drivers/irqchip/irq-mxs.c +1 -2

+	icoll_add_domain(np, ICOLL_NUM_IRQS);
 
-	icoll_domain = irq_domain_add_linear(np, ICOLL_NUM_IRQS,
-					     &icoll_irq_domain_ops, NULL);
 	return icoll_domain ? 0 : -ENODEV;

In case of !icoll_domain this return is not reached as you paniced already. So why would we still check icoll_domain?

Thanks,

tglx

Oleksij Rempel

author

2015-10-12 09:29 UTC #23 lore raw

Re: [PATCH v6 1/2] ARM: irqchip: mxs: prepare driver for HW with different offsets

Am 11.10.2015 um 19:58 schrieb Thomas Gleixner:

Quoted 17 lines

Oleksij,
On Sat, 10 Oct 2015, Oleksij Rempel wrote:
The proper subject line starts with:
irqchip/mxs:
Quoted 2 lines
Some HW has similar functionality but different register offsets. Make sure we can change offsets dynamically.
The patch does way more than that. I told you in V2 already:
Quoted 1 line
Quoted 1 line
You forgot to mention the other preparatory changes.
There is still nothing in the changelog.

Quoted 11 lines

drivers/irqchip/irq-mxs.c static const struct irq_domain_ops icoll_irq_domain_ops = { · line 121
Quoted 8 lines
drivers/irqchip/irq-mxs.c +8 -0
+static void __init icoll_add_domain(struct device_node *np,
+			  int num)
+{
+	icoll_domain = irq_domain_add_linear(np, num,
+					     &icoll_irq_domain_ops, NULL);
+
+	if (!icoll_domain)
+		panic("%s: unable add irq domain", np->full_name);
This splitout should be a separate patch with an explanation why you add

I assume i can remove this both lines. irq_set_default_host is an artefact from old code.

Quoted 26 lines

drivers/irqchip/irq-mxs.c static const struct irq_domain_ops icoll_irq_domain_ops = { · line 122
Quoted 1 line
drivers/irqchip/irq-mxs.c +1 -0
+	irq_set_default_host(icoll_domain);
and
drivers/irqchip/irq-mxs.c static const struct irq_domain_ops icoll_irq_domain_ops = { · line 123
Quoted 1 line
drivers/irqchip/irq-mxs.c +1 -0
+	set_handle_irq(icoll_handle_irq);
The latter is already done via the DT_MACHINE_START magic. So you should it remove there, because otherwise that call is just pointless. See the implementation of set_handle_irq.
drivers/irqchip/irq-mxs.c static const struct irq_domain_ops icoll_irq_domain_ops = { · line 134
Quoted 11 lines
drivers/irqchip/irq-mxs.c +11 -0
+}
+
+static void __iomem * __init icoll_init_iobase(struct device_node *np)
+{
+	void __iomem *icoll_base;
+
+	icoll_base = of_io_request_and_map(np, 0, np->name);
+	if (!icoll_base)
+		panic("%s: unable to map resource", np->full_name);
+	return icoll_base;
+}
The panic() is actually a bug fix, because the old code had a WARN_ON() and then happily dereferenced the NULL pointer. So this wants to go into a separate patch as well.

Quoted 8 lines

drivers/irqchip/irq-mxs.c static const struct irq_domain_ops icoll_irq_domain_ops = { · line 156
Quoted 5 lines
drivers/irqchip/irq-mxs.c +1 -2
+	icoll_add_domain(np, ICOLL_NUM_IRQS);
 
-	icoll_domain = irq_domain_add_linear(np, ICOLL_NUM_IRQS,
-					     &icoll_irq_domain_ops, NULL);
 	return icoll_domain ? 0 : -ENODEV;
In case of !icoll_domain this return is not reached as you paniced already. So why would we still check icoll_domain?

I'm paniced on !icoll_base, icoll_domain will give only warning.
http://lxr.free-electrons.com/source/kernel/irq/irqdomain.c#L52

Or do i miss some thing?

Quoted 5 lines

Thanks,
tglx

Signature · 2 lines

Regards,
Oleksij

Thomas Gleixner

2015-10-12 09:33 UTC #24 lore raw

Re: [PATCH v6 1/2] ARM: irqchip: mxs: prepare driver for HW with different offsets

Quoted 10 lines · Oleksij Rempel

Am 11.10.2015 um 19:58 schrieb Thomas Gleixner:
Quoted 8 lines
drivers/irqchip/irq-mxs.c static const struct irq_domain_ops icoll_irq_domain_ops = { · line 156
Quoted 5 lines
drivers/irqchip/irq-mxs.c +1 -2
+	icoll_add_domain(np, ICOLL_NUM_IRQS);
 
-	icoll_domain = irq_domain_add_linear(np, ICOLL_NUM_IRQS,
-					     &icoll_irq_domain_ops, NULL);
 	return icoll_domain ? 0 : -ENODEV;
In case of !icoll_domain this return is not reached as you paniced already. So why would we still check icoll_domain?

Quoted 4 lines

I'm paniced on !icoll_base, icoll_domain will give only warning.
http://lxr.free-electrons.com/source/kernel/irq/irqdomain.c#L52
Or do i miss some thing?

Yes. Your patch makes that:

Quoted 2 lines

drivers/irqchip/irq-mxs.c static const struct irq_domain_ops icoll_irq_domain_ops = { · line 121
Quoted 2 lines
drivers/irqchip/irq-mxs.c +2 -0
+    if (!icoll_domain)
+            panic("%s: unable add irq domain", np->full_name);

Thanks,

tglx

Oleksij Rempel

author

2015-10-12 19:15 UTC #25 lore raw

v7 cover Add support for ASM9260 interrupt controller

v2:
- move icoll_init_iobasea and icoll_add_domain to preparation patch
- add enum icoll_type
- make separate irq_chip for ICOLL and ASM9260

v3:
- fix typo in subject: Alp_h_ascale
- remove "intr_per_reg", it is not used any more. It will also save some CPU cycles
- remove spin locks. It was a bad idea.

v4:
- remove set_irq_flags()

v5:
- remove intr_per_reg from first patch
- move *clear to second patch

v6:
- update against latest torvalds/linux.git master v4.3-rc4

v7:
- remove irq_set_default_host and set_handle_irq from patch 1/2. they are artifacts from old code.
- remove "return icoll_domain ? 0 : -ENODEV;" checking from both patches.
- create separate patch to replace WARN_ON(!icoll_base), sinse it is separate bugfix

Oleksij Rempel (3):
ARM: irqchip: mxs: do panic if icoll_base == NULL
ARM: irqchip: mxs: prepare driver for HW with different offsets
ARM: irqchip: mxs: add Alphascale ASM9260 support

 drivers/irqchip/Kconfig                    |   5 +
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 ++++++++++++++++++
 drivers/irqchip/irq-mxs.c                  | 171 ++++++++++++++++++++++++++---
 4 files changed, 268 insertions(+), 19 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

Signature · 1 line

2.1.4

Oleksij Rempel

author

2015-10-12 19:15 UTC #26 lore raw

PATCH v6 1/2 ARM: irqchip: mxs: prepare driver for HW with different offsets

Some HW has similar functionality but different register offsets. Make sure we can change offsets dynamically.

Signed-off-by Oleksij Rempel <[email protected]>

---
 drivers/irqchip/irq-mxs.c | 78 +++++++++++++++++++++++++++++++++++++----------
 1 file changed, 62 insertions(+), 16 deletions(-)

drivers/irqchip/irq-mxs.c +62 ~~-16~~

index 604df63..86dec9a 100644
@@ -28,18 +28,36 @@
 #include <linux/stmp_device.h>
 #include <asm/exception.h>
 
+/*
+ * this device provide 4 offsets for each register:
+ * 0x0 - plain read write mode
+ * 0x4 - set mode, OR logic.
+ * 0x8 - clr mode, XOR logic.
+ * 0xc - togle mode.
+ */
+#define SET_REG 4
+#define CLR_REG 8
+
 #define HW_ICOLL_VECTOR				0x0000
 #define HW_ICOLL_LEVELACK			0x0010
 #define HW_ICOLL_CTRL				0x0020
 #define HW_ICOLL_STAT_OFFSET			0x0070
-#define HW_ICOLL_INTERRUPTn_SET(n)		(0x0124 + (n) * 0x10)
-#define HW_ICOLL_INTERRUPTn_CLR(n)		(0x0128 + (n) * 0x10)
-#define BM_ICOLL_INTERRUPTn_ENABLE		0x00000004
+#define HW_ICOLL_INTERRUPT0			0x0120
+#define HW_ICOLL_INTERRUPTn(n)			((n) * 0x10)
+#define BM_ICOLL_INTR_ENABLE			BIT(2)
 #define BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0	0x1
 
 #define ICOLL_NUM_IRQS		128
 
-static void __iomem *icoll_base;
+struct icoll_priv {
+	void __iomem *vector;
+	void __iomem *levelack;
+	void __iomem *ctrl;
+	void __iomem *stat;
+	void __iomem *intr;
+};
+
+static struct icoll_priv icoll_priv;
 static struct irq_domain *icoll_domain;
 
 static void icoll_ack_irq(struct irq_data *d)

@@ -50,19 +68,19 @@ static void icoll_ack_irq(struct irq_data *d)
 	 * BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0 unconditionally.
 	 */
 	__raw_writel(BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0,
-			icoll_base + HW_ICOLL_LEVELACK);
+			icoll_priv.levelack);
 }
 
 static void icoll_mask_irq(struct irq_data *d)
 {
-	__raw_writel(BM_ICOLL_INTERRUPTn_ENABLE,
-			icoll_base + HW_ICOLL_INTERRUPTn_CLR(d->hwirq));
+	__raw_writel(BM_ICOLL_INTR_ENABLE,
+			icoll_priv.intr + CLR_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
 static void icoll_unmask_irq(struct irq_data *d)
 {
-	__raw_writel(BM_ICOLL_INTERRUPTn_ENABLE,
-			icoll_base + HW_ICOLL_INTERRUPTn_SET(d->hwirq));
+	__raw_writel(BM_ICOLL_INTR_ENABLE,
+			icoll_priv.intr + SET_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
 static struct irq_chip mxs_icoll_chip = {

@@ -75,8 +93,8 @@ asmlinkage void __exception_irq_entry icoll_handle_irq(struct pt_regs *regs)
 {
 	u32 irqnr;
 
-	irqnr = __raw_readl(icoll_base + HW_ICOLL_STAT_OFFSET);
-	__raw_writel(irqnr, icoll_base + HW_ICOLL_VECTOR);
+	irqnr = __raw_readl(icoll_priv.stat);
+	__raw_writel(irqnr, icoll_priv.vector);
 	handle_domain_irq(icoll_domain, irqnr, regs);
 }

@@ -93,20 +111,48 @@ static const struct irq_domain_ops icoll_irq_domain_ops = {
 	.xlate = irq_domain_xlate_onecell,
 };
 
+static void __init icoll_add_domain(struct device_node *np,
+			  int num)
+{
+	icoll_domain = irq_domain_add_linear(np, num,
+					     &icoll_irq_domain_ops, NULL);
+
+	if (!icoll_domain)
+		panic("%s: unable add irq domain", np->full_name);
+	irq_set_default_host(icoll_domain);
+	set_handle_irq(icoll_handle_irq);
+}
+
+static void __iomem * __init icoll_init_iobase(struct device_node *np)
+{
+	void __iomem *icoll_base;
+
+	icoll_base = of_io_request_and_map(np, 0, np->name);
+	if (!icoll_base)
+		panic("%s: unable to map resource", np->full_name);
+	return icoll_base;
+}
+
 static int __init icoll_of_init(struct device_node *np,
 			  struct device_node *interrupt_parent)
 {
-	icoll_base = of_iomap(np, 0);
-	WARN_ON(!icoll_base);
+	void __iomem *icoll_base;
+
+	icoll_base		= icoll_init_iobase(np);
+	icoll_priv.vector	= icoll_base + HW_ICOLL_VECTOR;
+	icoll_priv.levelack	= icoll_base + HW_ICOLL_LEVELACK;
+	icoll_priv.ctrl		= icoll_base + HW_ICOLL_CTRL;
+	icoll_priv.stat		= icoll_base + HW_ICOLL_STAT_OFFSET;
+	icoll_priv.intr		= icoll_base + HW_ICOLL_INTERRUPT0;
 
 	/*
 	 * Interrupt Collector reset, which initializes the priority
 	 * for each irq to level 0.
 	 */
-	stmp_reset_block(icoll_base + HW_ICOLL_CTRL);
+	stmp_reset_block(icoll_priv.ctrl);
+
+	icoll_add_domain(np, ICOLL_NUM_IRQS);
 
-	icoll_domain = irq_domain_add_linear(np, ICOLL_NUM_IRQS,
-					     &icoll_irq_domain_ops, NULL);
 	return icoll_domain ? 0 : -ENODEV;
 }
 IRQCHIP_DECLARE(mxs, "fsl,icoll", icoll_of_init);

Signature · 1 line

2.1.4

Oleksij Rempel

author

2015-10-12 19:17 UTC #27 lore raw

PATCH v7 1/3 ARM: irqchip: mxs: do panic if icoll_base == NULL

In reply to Oleksij Rempel

Current code will only WARN_ON() and dereference the NULL pointer. In this case it is better to panic and stop execution.

Signed-off-by Oleksij Rempel <[email protected]>

---
 drivers/irqchip/irq-mxs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/irqchip/irq-mxs.c +2 -1

index 604df63..ce69860 100644
@@ -97,7 +97,8 @@ static int __init icoll_of_init(struct device_node *np,
 			  struct device_node *interrupt_parent)
 {
 	icoll_base = of_iomap(np, 0);
-	WARN_ON(!icoll_base);
+	if (!icoll_base)
+		panic("%s: unable to map resource", np->full_name);
 
 	/*
 	 * Interrupt Collector reset, which initializes the priority

Signature · 1 line

2.1.4

tip-bot for Oleksij Rempel

merge bot

2015-10-14 07:42 UTC #28 lore raw

[tip:irq/core] irqchip/mxs: Panic if ioremap or domain creation fails

irqchip/mxs: Panic if ioremap or domain creation fails

Commit-ID:  e59a8451be1162d5a10a33e40092f1796cb8fdca
Gitweb:     http://git.kernel.org/tip/e59a8451be1162d5a10a33e40092f1796cb8fdca
Author:     Oleksij Rempel <[email protected]>
AuthorDate: Mon, 12 Oct 2015 21:15:30 +0200
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Wed, 14 Oct 2015 09:37:47 +0200

irqchip/mxs: Panic if ioremap or domain creation fails

Current code will only warn and then dereference the NULL pointer or continue, which results in a fatal NULL pointer dereference later.

If the initialization fails, the machine is unusable, so panic right away.

[ tglx: Massaged changelog and picked the irqdomain panic from the
next patch]

Signed-off-by Oleksij Rempel <[email protected]>

Tested-by Shawn Guo <[email protected]>

Cc Sascha Hauer <[email protected]>

Cc [email protected]

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Thomas Gleixner <[email protected]>

---
 drivers/irqchip/irq-mxs.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

drivers/irqchip/irq-mxs.c +6 -2

index 604df63..96148a7 100644
@@ -97,7 +97,8 @@ static int __init icoll_of_init(struct device_node *np,
 			  struct device_node *interrupt_parent)
 {
 	icoll_base = of_iomap(np, 0);
-	WARN_ON(!icoll_base);
+	if (!icoll_base)
+		panic("%s: unable to map resource", np->full_name);
 
 	/*
 	 * Interrupt Collector reset, which initializes the priority

@@ -107,6 +108,9 @@ static int __init icoll_of_init(struct device_node *np,
 
 	icoll_domain = irq_domain_add_linear(np, ICOLL_NUM_IRQS,
 					     &icoll_irq_domain_ops, NULL);
-	return icoll_domain ? 0 : -ENODEV;
+	if (!icoll_domain)
+		panic("%s: unable to create irqdomain", np->full_name);
+
+	return 0;
 }
 IRQCHIP_DECLARE(mxs, "fsl,icoll", icoll_of_init);

Oleksij Rempel

author

2015-10-12 19:15 UTC #29 lore raw

PATCH v7 2/3 ARM: irqchip: mxs: prepare driver for HW with different offsets

In reply to Oleksij Rempel

Alphascale asm9260 has similar functionality but different register offsets. This patch should prepare irq-mxs code to reused for asm9260 too.

Fallowing changes was made:
- Define SET_REG and CLR_REG. This controllers support separate CLR and SET offsets for each register.
- Re define HW_ICOLL_INTERRUPT with use of SET_REG and CLR_REG to make it readable for both cases.
- Create struct icoll_priv, to change register offsets on load.
- Creat function icoll_init_iobase and icoll_add_domain for shared init code.
- Don't check icoll_domain on return. We will call panic on each error case any way.

Signed-off-by Oleksij Rempel <[email protected]>

---
 drivers/irqchip/irq-mxs.c | 79 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 61 insertions(+), 18 deletions(-)

drivers/irqchip/irq-mxs.c +61 ~~-18~~

index ce69860..d5cf208 100644
@@ -28,18 +28,36 @@
 #include <linux/stmp_device.h>
 #include <asm/exception.h>
 
+/*
+ * this device provide 4 offsets for each register:
+ * 0x0 - plain read write mode
+ * 0x4 - set mode, OR logic.
+ * 0x8 - clr mode, XOR logic.
+ * 0xc - togle mode.
+ */
+#define SET_REG 4
+#define CLR_REG 8
+
 #define HW_ICOLL_VECTOR				0x0000
 #define HW_ICOLL_LEVELACK			0x0010
 #define HW_ICOLL_CTRL				0x0020
 #define HW_ICOLL_STAT_OFFSET			0x0070
-#define HW_ICOLL_INTERRUPTn_SET(n)		(0x0124 + (n) * 0x10)
-#define HW_ICOLL_INTERRUPTn_CLR(n)		(0x0128 + (n) * 0x10)
-#define BM_ICOLL_INTERRUPTn_ENABLE		0x00000004
+#define HW_ICOLL_INTERRUPT0			0x0120
+#define HW_ICOLL_INTERRUPTn(n)			((n) * 0x10)
+#define BM_ICOLL_INTR_ENABLE			BIT(2)
 #define BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0	0x1
 
 #define ICOLL_NUM_IRQS		128
 
-static void __iomem *icoll_base;
+struct icoll_priv {
+	void __iomem *vector;
+	void __iomem *levelack;
+	void __iomem *ctrl;
+	void __iomem *stat;
+	void __iomem *intr;
+};
+
+static struct icoll_priv icoll_priv;
 static struct irq_domain *icoll_domain;
 
 static void icoll_ack_irq(struct irq_data *d)

@@ -50,19 +68,19 @@ static void icoll_ack_irq(struct irq_data *d)
 	 * BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0 unconditionally.
 	 */
 	__raw_writel(BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0,
-			icoll_base + HW_ICOLL_LEVELACK);
+			icoll_priv.levelack);
 }
 
 static void icoll_mask_irq(struct irq_data *d)
 {
-	__raw_writel(BM_ICOLL_INTERRUPTn_ENABLE,
-			icoll_base + HW_ICOLL_INTERRUPTn_CLR(d->hwirq));
+	__raw_writel(BM_ICOLL_INTR_ENABLE,
+			icoll_priv.intr + CLR_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
 static void icoll_unmask_irq(struct irq_data *d)
 {
-	__raw_writel(BM_ICOLL_INTERRUPTn_ENABLE,
-			icoll_base + HW_ICOLL_INTERRUPTn_SET(d->hwirq));
+	__raw_writel(BM_ICOLL_INTR_ENABLE,
+			icoll_priv.intr + SET_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
 static struct irq_chip mxs_icoll_chip = {

@@ -75,8 +93,8 @@ asmlinkage void __exception_irq_entry icoll_handle_irq(struct pt_regs *regs)
 {
 	u32 irqnr;
 
-	irqnr = __raw_readl(icoll_base + HW_ICOLL_STAT_OFFSET);
-	__raw_writel(irqnr, icoll_base + HW_ICOLL_VECTOR);
+	irqnr = __raw_readl(icoll_priv.stat);
+	__raw_writel(irqnr, icoll_priv.vector);
 	handle_domain_irq(icoll_domain, irqnr, regs);
 }

@@ -93,21 +111,46 @@ static const struct irq_domain_ops icoll_irq_domain_ops = {
 	.xlate = irq_domain_xlate_onecell,
 };
 
-static int __init icoll_of_init(struct device_node *np,
-			  struct device_node *interrupt_parent)
+static void __init icoll_add_domain(struct device_node *np,
+			  int num)
+{
+	icoll_domain = irq_domain_add_linear(np, num,
+					     &icoll_irq_domain_ops, NULL);
+
+	if (!icoll_domain)
+		panic("%s: unable add irq domain", np->full_name);
+}
+
+static void __iomem * __init icoll_init_iobase(struct device_node *np)
 {
-	icoll_base = of_iomap(np, 0);
+	void __iomem *icoll_base;
+
+	icoll_base = of_io_request_and_map(np, 0, np->name);
 	if (!icoll_base)
 		panic("%s: unable to map resource", np->full_name);
+	return icoll_base;
+}
+
+static int __init icoll_of_init(struct device_node *np,
+			  struct device_node *interrupt_parent)
+{
+	void __iomem *icoll_base;
+
+	icoll_base		= icoll_init_iobase(np);
+	icoll_priv.vector	= icoll_base + HW_ICOLL_VECTOR;
+	icoll_priv.levelack	= icoll_base + HW_ICOLL_LEVELACK;
+	icoll_priv.ctrl		= icoll_base + HW_ICOLL_CTRL;
+	icoll_priv.stat		= icoll_base + HW_ICOLL_STAT_OFFSET;
+	icoll_priv.intr		= icoll_base + HW_ICOLL_INTERRUPT0;
 
 	/*
 	 * Interrupt Collector reset, which initializes the priority
 	 * for each irq to level 0.
 	 */
-	stmp_reset_block(icoll_base + HW_ICOLL_CTRL);
+	stmp_reset_block(icoll_priv.ctrl);
 
-	icoll_domain = irq_domain_add_linear(np, ICOLL_NUM_IRQS,
-					     &icoll_irq_domain_ops, NULL);
-	return icoll_domain ? 0 : -ENODEV;
+	icoll_add_domain(np, ICOLL_NUM_IRQS);
+
+	return 0;
 }
 IRQCHIP_DECLARE(mxs, "fsl,icoll", icoll_of_init);

Signature · 1 line

2.1.4

tip-bot for Oleksij Rempel

merge bot

2015-10-14 07:43 UTC #30 lore raw

[tip:irq/core] irqchip/mxs: Prepare driver for hardware with different offsets

irqchip/mxs: Prepare driver for hardware with different offsets

Commit-ID:  25e34b44313b61d7a87819498ccfd0129441604a
Gitweb:     http://git.kernel.org/tip/25e34b44313b61d7a87819498ccfd0129441604a
Author:     Oleksij Rempel <[email protected]>
AuthorDate: Mon, 12 Oct 2015 21:15:33 +0200
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Wed, 14 Oct 2015 09:37:47 +0200

irqchip/mxs: Prepare driver for hardware with different offsets

Alphascale asm9260 has similar functionality but different register offsets. To support asm9260 in the mxs driver we need to rework the hardcoded access mechanisms.

- Define SET_REG and CLR_REG. These controllers support seperate CLR and SET offsets for each register.

- Reimplement HW_ICOLL_INTERRUPT with SET_REG and CLR_REG to make it usable for both cases.

- Instead of using icoll_base and adding the offsets at runtime, create a new data structure which contains base pointers to all required regitsters and use it.

- Split out functionality, which is required for the init code of mxs and asm9260, into helper functions

[ tglx: Massaged changelog and moved the return value change to the
previous patch ]

Signed-off-by Oleksij Rempel <[email protected]>

Tested-by Shawn Guo <[email protected]>

Cc Sascha Hauer <[email protected]>

Cc [email protected]

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Thomas Gleixner <[email protected]>

---
 drivers/irqchip/irq-mxs.c | 78 +++++++++++++++++++++++++++++++++++------------
 1 file changed, 59 insertions(+), 19 deletions(-)

drivers/irqchip/irq-mxs.c +59 ~~-19~~

index 96148a7..eaab5a6 100644
@@ -28,18 +28,36 @@
 #include <linux/stmp_device.h>
 #include <asm/exception.h>
 
+/*
+ * this device provide 4 offsets for each register:
+ * 0x0 - plain read write mode
+ * 0x4 - set mode, OR logic.
+ * 0x8 - clr mode, XOR logic.
+ * 0xc - togle mode.
+ */
+#define SET_REG 4
+#define CLR_REG 8
+
 #define HW_ICOLL_VECTOR				0x0000
 #define HW_ICOLL_LEVELACK			0x0010
 #define HW_ICOLL_CTRL				0x0020
 #define HW_ICOLL_STAT_OFFSET			0x0070
-#define HW_ICOLL_INTERRUPTn_SET(n)		(0x0124 + (n) * 0x10)
-#define HW_ICOLL_INTERRUPTn_CLR(n)		(0x0128 + (n) * 0x10)
-#define BM_ICOLL_INTERRUPTn_ENABLE		0x00000004
+#define HW_ICOLL_INTERRUPT0			0x0120
+#define HW_ICOLL_INTERRUPTn(n)			((n) * 0x10)
+#define BM_ICOLL_INTR_ENABLE			BIT(2)
 #define BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0	0x1
 
 #define ICOLL_NUM_IRQS		128
 
-static void __iomem *icoll_base;
+struct icoll_priv {
+	void __iomem *vector;
+	void __iomem *levelack;
+	void __iomem *ctrl;
+	void __iomem *stat;
+	void __iomem *intr;
+};
+
+static struct icoll_priv icoll_priv;
 static struct irq_domain *icoll_domain;
 
 static void icoll_ack_irq(struct irq_data *d)

@@ -50,19 +68,19 @@ static void icoll_ack_irq(struct irq_data *d)
 	 * BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0 unconditionally.
 	 */
 	__raw_writel(BV_ICOLL_LEVELACK_IRQLEVELACK__LEVEL0,
-			icoll_base + HW_ICOLL_LEVELACK);
+			icoll_priv.levelack);
 }
 
 static void icoll_mask_irq(struct irq_data *d)
 {
-	__raw_writel(BM_ICOLL_INTERRUPTn_ENABLE,
-			icoll_base + HW_ICOLL_INTERRUPTn_CLR(d->hwirq));
+	__raw_writel(BM_ICOLL_INTR_ENABLE,
+			icoll_priv.intr + CLR_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
 static void icoll_unmask_irq(struct irq_data *d)
 {
-	__raw_writel(BM_ICOLL_INTERRUPTn_ENABLE,
-			icoll_base + HW_ICOLL_INTERRUPTn_SET(d->hwirq));
+	__raw_writel(BM_ICOLL_INTR_ENABLE,
+			icoll_priv.intr + SET_REG + HW_ICOLL_INTERRUPTn(d->hwirq));
 }
 
 static struct irq_chip mxs_icoll_chip = {

@@ -75,8 +93,8 @@ asmlinkage void __exception_irq_entry icoll_handle_irq(struct pt_regs *regs)
 {
 	u32 irqnr;
 
-	irqnr = __raw_readl(icoll_base + HW_ICOLL_STAT_OFFSET);
-	__raw_writel(irqnr, icoll_base + HW_ICOLL_VECTOR);
+	irqnr = __raw_readl(icoll_priv.stat);
+	__raw_writel(irqnr, icoll_priv.vector);
 	handle_domain_irq(icoll_domain, irqnr, regs);
 }

@@ -93,23 +111,45 @@ static const struct irq_domain_ops icoll_irq_domain_ops = {
 	.xlate = irq_domain_xlate_onecell,
 };
 
-static int __init icoll_of_init(struct device_node *np,
-			  struct device_node *interrupt_parent)
+static void __init icoll_add_domain(struct device_node *np,
+			  int num)
+{
+	icoll_domain = irq_domain_add_linear(np, num,
+					     &icoll_irq_domain_ops, NULL);
+
+	if (!icoll_domain)
+		panic("%s: unable to create irq domain", np->full_name);
+}
+
+static void __iomem * __init icoll_init_iobase(struct device_node *np)
 {
-	icoll_base = of_iomap(np, 0);
+	void __iomem *icoll_base;
+
+	icoll_base = of_io_request_and_map(np, 0, np->name);
 	if (!icoll_base)
 		panic("%s: unable to map resource", np->full_name);
+	return icoll_base;
+}
+
+static int __init icoll_of_init(struct device_node *np,
+			  struct device_node *interrupt_parent)
+{
+	void __iomem *icoll_base;
+
+	icoll_base		= icoll_init_iobase(np);
+	icoll_priv.vector	= icoll_base + HW_ICOLL_VECTOR;
+	icoll_priv.levelack	= icoll_base + HW_ICOLL_LEVELACK;
+	icoll_priv.ctrl		= icoll_base + HW_ICOLL_CTRL;
+	icoll_priv.stat		= icoll_base + HW_ICOLL_STAT_OFFSET;
+	icoll_priv.intr		= icoll_base + HW_ICOLL_INTERRUPT0;
 
 	/*
 	 * Interrupt Collector reset, which initializes the priority
 	 * for each irq to level 0.
 	 */
-	stmp_reset_block(icoll_base + HW_ICOLL_CTRL);
+	stmp_reset_block(icoll_priv.ctrl);
 
-	icoll_domain = irq_domain_add_linear(np, ICOLL_NUM_IRQS,
-					     &icoll_irq_domain_ops, NULL);
-	if (!icoll_domain)
-		panic("%s: unable to create irqdomain", np->full_name);
+	icoll_add_domain(np, ICOLL_NUM_IRQS);
 
 	return 0;
 }

Oleksij Rempel

author

2015-10-12 19:16 UTC #31 lore raw

PATCH v6 2/2 ARM: irqchip: mxs: add Alphascale ASM9260 support

In reply to Oleksij Rempel

Signed-off-by Oleksij Rempel <[email protected]>

---
 drivers/irqchip/Kconfig                    |   5 ++
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 +++++++++++++++++++++++++++++
 drivers/irqchip/irq-mxs.c                  |  93 +++++++++++++++++++++++-
 4 files changed, 207 insertions(+), 2 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

drivers/irqchip/Kconfig +5 -0

index 27b52c8..0dfd840 100644
@@ -187,3 +187,8 @@ config IMX_GPCV2
 	select IRQ_DOMAIN
 	help
 	  Enables the wakeup IRQs for IMX platforms with GPCv2 block
+
+config IRQ_MXS
+	def_bool y if MACH_ASM9260 || ARCH_MXS
+	select IRQ_DOMAIN
+	select STMP_DEVICE

drivers/irqchip/Makefile +1 -1

index bb3048f..177f78f 100644
@@ -6,7 +6,7 @@ obj-$(CONFIG_ARCH_EXYNOS)		+= exynos-combiner.o
 obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
 obj-$(CONFIG_ARCH_MMP)			+= irq-mmp.o
 obj-$(CONFIG_ARCH_MVEBU)		+= irq-armada-370-xp.o
-obj-$(CONFIG_ARCH_MXS)			+= irq-mxs.o
+obj-$(CONFIG_IRQ_MXS)			+= irq-mxs.o
 obj-$(CONFIG_ARCH_TEGRA)		+= irq-tegra.o
 obj-$(CONFIG_ARCH_S3C24XX)		+= irq-s3c24xx.o
 obj-$(CONFIG_DW_APB_ICTL)		+= irq-dw-apb-ictl.o

drivers/irqchip/alphascale_asm9260-icoll.h new file +109 -0

109 lines not on this page · raw message

drivers/irqchip/irq-mxs.c +92 -1

154 lines not on this page · raw message

Signature · 1 line

2.1.4

Oleksij Rempel

author

2015-10-12 19:15 UTC #32 lore raw

PATCH v7 3/3 ARM: irqchip: mxs: add Alphascale ASM9260 support

In reply to Oleksij Rempel

Signed-off-by Oleksij Rempel <[email protected]>

---
 drivers/irqchip/Kconfig                    |   5 ++
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 +++++++++++++++++++++++++++++
 drivers/irqchip/irq-mxs.c                  |  93 +++++++++++++++++++++++-
 4 files changed, 207 insertions(+), 2 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

drivers/irqchip/Kconfig +5 -0

index 27b52c8..0dfd840 100644
@@ -187,3 +187,8 @@ config IMX_GPCV2
 	select IRQ_DOMAIN
 	help
 	  Enables the wakeup IRQs for IMX platforms with GPCv2 block
+
+config IRQ_MXS
+	def_bool y if MACH_ASM9260 || ARCH_MXS
+	select IRQ_DOMAIN
+	select STMP_DEVICE

drivers/irqchip/Makefile +1 -1

index bb3048f..177f78f 100644
@@ -6,7 +6,7 @@ obj-$(CONFIG_ARCH_EXYNOS)		+= exynos-combiner.o
 obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
 obj-$(CONFIG_ARCH_MMP)			+= irq-mmp.o
 obj-$(CONFIG_ARCH_MVEBU)		+= irq-armada-370-xp.o
-obj-$(CONFIG_ARCH_MXS)			+= irq-mxs.o
+obj-$(CONFIG_IRQ_MXS)			+= irq-mxs.o
 obj-$(CONFIG_ARCH_TEGRA)		+= irq-tegra.o
 obj-$(CONFIG_ARCH_S3C24XX)		+= irq-s3c24xx.o
 obj-$(CONFIG_DW_APB_ICTL)		+= irq-dw-apb-ictl.o

drivers/irqchip/alphascale_asm9260-icoll.h new file +109 -0

109 lines not on this page · raw message

drivers/irqchip/irq-mxs.c +92 -1

154 lines not on this page · raw message

Signature · 1 line

2.1.4

tip-bot for Oleksij Rempel

merge bot

2015-10-14 07:43 UTC #33 lore raw

[tip:irq/core] irqchip/mxs: Add Alphascale ASM9260 support

irqchip/mxs: Add Alphascale ASM9260 support

Commit-ID:  7e4ac676ee468108886f12a20e25795f1c330939
Gitweb:     http://git.kernel.org/tip/7e4ac676ee468108886f12a20e25795f1c330939
Author:     Oleksij Rempel <[email protected]>
AuthorDate: Mon, 12 Oct 2015 21:15:34 +0200
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Wed, 14 Oct 2015 09:37:47 +0200

irqchip/mxs: Add Alphascale ASM9260 support

Freescale iMX23/iMX28 and Alphascale ASM9260 have similar interrupt collectors. We already prepared the mxs driver to handle a different register layout. Add the actual ASM9260 support.

Differences between these devices:
- Different register offsets
- Different count of interupt lines per register
- ASM9260 does not provide reset bit
- ASM9260 does not support FIQ.

Signed-off-by Oleksij Rempel <[email protected]>

Tested-by Shawn Guo <[email protected]>

Cc Sascha Hauer <[email protected]>

Cc [email protected]

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Thomas Gleixner <[email protected]>

---
 drivers/irqchip/Kconfig                    |   5 ++
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 +++++++++++++++++++++++++++++
 drivers/irqchip/irq-mxs.c                  |  93 +++++++++++++++++++++++-
 4 files changed, 207 insertions(+), 2 deletions(-)

drivers/irqchip/Kconfig +5 -0

index 67d8027..4d7294e 100644
@@ -188,3 +188,8 @@ config IMX_GPCV2
 	select IRQ_DOMAIN
 	help
 	  Enables the wakeup IRQs for IMX platforms with GPCv2 block
+
+config IRQ_MXS
+	def_bool y if MACH_ASM9260 || ARCH_MXS
+	select IRQ_DOMAIN
+	select STMP_DEVICE

drivers/irqchip/Makefile +1 -1

index bb3048f..177f78f 100644
@@ -6,7 +6,7 @@ obj-$(CONFIG_ARCH_EXYNOS)		+= exynos-combiner.o
 obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
 obj-$(CONFIG_ARCH_MMP)			+= irq-mmp.o
 obj-$(CONFIG_ARCH_MVEBU)		+= irq-armada-370-xp.o
-obj-$(CONFIG_ARCH_MXS)			+= irq-mxs.o
+obj-$(CONFIG_IRQ_MXS)			+= irq-mxs.o
 obj-$(CONFIG_ARCH_TEGRA)		+= irq-tegra.o
 obj-$(CONFIG_ARCH_S3C24XX)		+= irq-s3c24xx.o
 obj-$(CONFIG_DW_APB_ICTL)		+= irq-dw-apb-ictl.o

drivers/irqchip/alphascale_asm9260-icoll.h new file +109 -0

109 lines not on this page · raw message

drivers/irqchip/irq-mxs.c +92 -1

154 lines not on this page · raw message

Oleksij Rempel

author

2015-10-10 13:09 UTC #34 lore raw

PATCH v6 2/2 ARM: irqchip: mxs: add Alphascale ASM9260 support

In reply to Oleksij Rempel

Signed-off-by Oleksij Rempel <[email protected]>

---
 drivers/irqchip/Kconfig                    |   5 ++
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 +++++++++++++++++++++++++++++
 drivers/irqchip/irq-mxs.c                  |  93 +++++++++++++++++++++++-
 4 files changed, 207 insertions(+), 2 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

drivers/irqchip/Kconfig +5 -0

index 27b52c8..0dfd840 100644
@@ -187,3 +187,8 @@ config IMX_GPCV2
 	select IRQ_DOMAIN
 	help
 	  Enables the wakeup IRQs for IMX platforms with GPCv2 block
+
+config IRQ_MXS
+	def_bool y if MACH_ASM9260 || ARCH_MXS
+	select IRQ_DOMAIN
+	select STMP_DEVICE

drivers/irqchip/Makefile +1 -1

index bb3048f..177f78f 100644
@@ -6,7 +6,7 @@ obj-$(CONFIG_ARCH_EXYNOS)		+= exynos-combiner.o
 obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
 obj-$(CONFIG_ARCH_MMP)			+= irq-mmp.o
 obj-$(CONFIG_ARCH_MVEBU)		+= irq-armada-370-xp.o
-obj-$(CONFIG_ARCH_MXS)			+= irq-mxs.o
+obj-$(CONFIG_IRQ_MXS)			+= irq-mxs.o
 obj-$(CONFIG_ARCH_TEGRA)		+= irq-tegra.o
 obj-$(CONFIG_ARCH_S3C24XX)		+= irq-s3c24xx.o
 obj-$(CONFIG_DW_APB_ICTL)		+= irq-dw-apb-ictl.o

drivers/irqchip/alphascale_asm9260-icoll.h new file +109 -0

109 lines not on this page · raw message

drivers/irqchip/irq-mxs.c +92 -1

154 lines not on this page · raw message

Signature · 1 line

2.1.4

Oleksij Rempel

author

2015-09-23 16:21 UTC #35 lore raw

PATCH v4 2/2 ARM: irqchip: mxs: add Alphascale ASM9260 support

In reply to Oleksij Rempel

Signed-off-by Oleksij Rempel <[email protected]>

---
 drivers/irqchip/Kconfig                    |   5 ++
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 +++++++++++++++++++++++++++++
 drivers/irqchip/irq-mxs.c                  |  94 +++++++++++++++++++++++--
 4 files changed, 205 insertions(+), 5 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

drivers/irqchip/Kconfig +5 -0

index 27b52c8..0dfd840 100644
@@ -187,3 +187,8 @@ config IMX_GPCV2
 	select IRQ_DOMAIN
 	help
 	  Enables the wakeup IRQs for IMX platforms with GPCv2 block
+
+config IRQ_MXS
+	def_bool y if MACH_ASM9260 || ARCH_MXS
+	select IRQ_DOMAIN
+	select STMP_DEVICE

drivers/irqchip/Makefile +1 -1

index bb3048f..177f78f 100644
@@ -6,7 +6,7 @@ obj-$(CONFIG_ARCH_EXYNOS)		+= exynos-combiner.o
 obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
 obj-$(CONFIG_ARCH_MMP)			+= irq-mmp.o
 obj-$(CONFIG_ARCH_MVEBU)		+= irq-armada-370-xp.o
-obj-$(CONFIG_ARCH_MXS)			+= irq-mxs.o
+obj-$(CONFIG_IRQ_MXS)			+= irq-mxs.o
 obj-$(CONFIG_ARCH_TEGRA)		+= irq-tegra.o
 obj-$(CONFIG_ARCH_S3C24XX)		+= irq-s3c24xx.o
 obj-$(CONFIG_DW_APB_ICTL)		+= irq-dw-apb-ictl.o

drivers/irqchip/alphascale_asm9260-icoll.h new file +109 -0

109 lines not on this page · raw message

drivers/irqchip/irq-mxs.c +90 -4

156 lines not on this page · raw message

Signature · 1 line

2.5.0

Oleksij Rempel

author

2015-09-18 09:48 UTC #36 lore raw

PATCH v2 2/2 ARM: irqchip: mxs: add Alpascale ASM9260 support

In reply to Oleksij Rempel

From: Oleksij Rempel <[email protected]>

Signed-off-by Oleksij Rempel <[email protected]>

---
 drivers/irqchip/Kconfig                    |   5 ++
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 +++++++++++++++++++++++++++++
 drivers/irqchip/irq-mxs.c                  | 106 +++++++++++++++++++++++++++-
 4 files changed, 220 insertions(+), 2 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

drivers/irqchip/Kconfig +5 -0

index 27b52c8..0dfd840 100644
@@ -187,3 +187,8 @@ config IMX_GPCV2
 	select IRQ_DOMAIN
 	help
 	  Enables the wakeup IRQs for IMX platforms with GPCv2 block
+
+config IRQ_MXS
+	def_bool y if MACH_ASM9260 || ARCH_MXS
+	select IRQ_DOMAIN
+	select STMP_DEVICE

drivers/irqchip/Makefile +1 -1

index bb3048f..177f78f 100644
@@ -6,7 +6,7 @@ obj-$(CONFIG_ARCH_EXYNOS)		+= exynos-combiner.o
 obj-$(CONFIG_ARCH_HIP04)		+= irq-hip04.o
 obj-$(CONFIG_ARCH_MMP)			+= irq-mmp.o
 obj-$(CONFIG_ARCH_MVEBU)		+= irq-armada-370-xp.o
-obj-$(CONFIG_ARCH_MXS)			+= irq-mxs.o
+obj-$(CONFIG_IRQ_MXS)			+= irq-mxs.o
 obj-$(CONFIG_ARCH_TEGRA)		+= irq-tegra.o
 obj-$(CONFIG_ARCH_S3C24XX)		+= irq-s3c24xx.o
 obj-$(CONFIG_DW_APB_ICTL)		+= irq-dw-apb-ictl.o

drivers/irqchip/alphascale_asm9260-icoll.h new file +109 -0

109 lines not on this page · raw message

drivers/irqchip/irq-mxs.c +105 -1

160 lines not on this page · raw message

Signature · 1 line

2.5.0

Marc Zyngier

2015-09-18 10:16 UTC #37 lore raw

In reply to Oleksij Rempel

On Fri, 18 Sep 2015 11:18:40 +0200
Oleksij Rempel <[email protected]> wrote:

Hi Oleksij,

Quoted 8 lines

v2:
- move some parts from patch 2 to 1
- make separate irq_chip for ICOLL and ASM9260
Oleksij Rempel (2):
ARM: irqchip: mxs: prepare driver for HW with different offsets
ARM: irqchip: mxs: add Alpascale ASM9260 support

Quoted 7 lines

 drivers/irqchip/Kconfig                    |   5 +
 drivers/irqchip/Makefile                   |   2 +-
 drivers/irqchip/alphascale_asm9260-icoll.h | 109 +++++++++++++++++
 drivers/irqchip/irq-mxs.c                  | 188 ++++++++++++++++++++++++++---
 4 files changed, 286 insertions(+), 18 deletions(-)
 create mode 100644 drivers/irqchip/alphascale_asm9260-icoll.h

Next time, please add a sensible title to your cover letter. Something like "Add support for ASM9260 interrupt controller". "changelog" is hardly a useful subject when you're dealing with a few dozen of patches at the same time.

Thanks,

M.

Signature · 1 line

Jazz is not dead. It just smells funny.

Thomas Gleixner

2015-09-18 10:19 UTC #38 lore raw

In reply to Oleksij Rempel

On Fri, 18 Sep 2015, Oleksij Rempel wrote:

'changelog' is not really a descriptive subject line for a cover letter.

Thanks,

tglx