ARM: OMAP2+: irq: Increase no of supported interrupts to 128

PATCH New Vaibhav Hiremath 3 participants 6 messages May 8, 2012 – May 11, 2012

Subsystems (3)

Arm (32-bit) Arm sub-architectures OMAP2+ support

lore

Share & tools

lore

Vaibhav Hiremath

author

2012-05-08 14:17 UTC #0 lore raw

PATCH 1/1 ARM: OMAP2+: irq: Increase no of supported interrupts to 128

With addition to TI81XX, AM33XX family of devices, the number of interrupts supported has increased to 128, compared to 96. The current implementation for irq handling is hardcoded to use 96 interrupts (with 3 register-sets to handle), this patch cleanups the code, to increase maximum number of interrupts support to 128, with dynamic detection of no of registers required for handling all interrupts.

Signed-off-by Vaibhav Hiremath <[email protected]>

Signed-off-by Afzal Mohammed <[email protected]>

Cc Tony Lindgren <[email protected]>

Cc Kevin Hilman <[email protected]>

Cc Paul Walmsley <[email protected]>

---
Ideally, we should use dynamic allocation to allocate memory for registers/arrays, may be too much cleanup for this patch, so as of now restricting to minimal changes to fit devices like, am33xx/ti81xx.

 arch/arm/mach-omap2/irq.c              |   47 +++++++++++++++----------------
 arch/arm/plat-omap/include/plat/irqs.h |    7 +++-
 2 files changed, 28 insertions(+), 26 deletions(-)

arch/arm/mach-omap2/irq.c +23 ~~-24~~

index 80f3ced..ea5aed9 100644
@@ -58,10 +58,12 @@
 static struct omap_irq_bank {
 	void __iomem *base_reg;
 	unsigned int nr_irqs;
+	unsigned int nr_regs_req;
 } __attribute__ ((aligned(4))) irq_banks[] = {
 	{
 		/* MPU INTC */
 		.nr_irqs	= 96,
+		.nr_regs_req	= 3,
 	},
 };

@@ -73,8 +75,8 @@ struct omap3_intc_regs {
 	u32 protection;
 	u32 idle;
 	u32 threshold;
-	u32 ilr[INTCPS_NR_IRQS];
-	u32 mir[INTCPS_NR_MIR_REGS];
+	u32 ilr[INTCPS_MAX_NR_IRQS];
+	u32 mir[INTCPS_MAX_NR_REGS_REQ];
 };
 
 /* INTC bank register get/set */

@@ -182,6 +184,7 @@ static void __init omap_init_irq(u32 base, int nr_irqs,
 		struct omap_irq_bank *bank = irq_banks + i;
 
 		bank->nr_irqs = nr_irqs;
+		bank->nr_regs_req = 0;
 
 		/* Static mapping, never released */
 		bank->base_reg = ioremap(base, SZ_4K);

@@ -192,8 +195,10 @@ static void __init omap_init_irq(u32 base, int nr_irqs,
 
 		omap_irq_bank_init_one(bank);
 
-		for (j = 0; j < bank->nr_irqs; j += 32)
+		for (j = 0; j < bank->nr_irqs; j += 32) {
 			omap_alloc_gc(bank->base_reg + j, j + irq_base, 32);
+			bank->nr_regs_req++;
+		}
 
 		nr_of_irqs += bank->nr_irqs;
 		nr_banks++;

@@ -218,25 +223,19 @@ void __init ti81xx_init_irq(void)
 	omap_init_irq(OMAP34XX_IC_BASE, 128, NULL);
 }
 
-static inline void omap_intc_handle_irq(void __iomem *base_addr, struct pt_regs *regs)
+static inline void omap_intc_handle_irq(void __iomem *base_addr,
+		unsigned int no_regs_req, struct pt_regs *regs)
 {
-	u32 irqnr;
+	u32 irqnr = 0;
 
 	do {
-		irqnr = readl_relaxed(base_addr + 0x98);
-		if (irqnr)
-			goto out;
-
-		irqnr = readl_relaxed(base_addr + 0xb8);
-		if (irqnr)
-			goto out;
+		int i = 0;
 
-		irqnr = readl_relaxed(base_addr + 0xd8);
-#ifdef CONFIG_SOC_OMAPTI816X
-		if (irqnr)
-			goto out;
-		irqnr = readl_relaxed(base_addr + 0xf8);
-#endif
+		for (i = 0; i < no_regs_req; i++) {
+			irqnr = readl_relaxed(base_addr + 0x98 + (0x20 * i));
+			if (irqnr)
+				goto out;
+		}
 
 out:
 		if (!irqnr)

@@ -255,7 +254,7 @@ out:
 asmlinkage void __exception_irq_entry omap2_intc_handle_irq(struct pt_regs *regs)
 {
 	void __iomem *base_addr = OMAP2_IRQ_BASE;
-	omap_intc_handle_irq(base_addr, regs);
+	omap_intc_handle_irq(base_addr, irq_banks[0].nr_regs_req, regs);
 }
 
 int __init omap_intc_of_init(struct device_node *node,

@@ -296,10 +295,10 @@ void omap_intc_save_context(void)
 			intc_bank_read_reg(bank, INTC_IDLE);
 		intc_context[ind].threshold =
 			intc_bank_read_reg(bank, INTC_THRESHOLD);
-		for (i = 0; i < INTCPS_NR_IRQS; i++)
+		for (i = 0; i < bank->nr_irqs; i++)
 			intc_context[ind].ilr[i] =
 				intc_bank_read_reg(bank, (0x100 + 0x4*i));
-		for (i = 0; i < INTCPS_NR_MIR_REGS; i++)
+		for (i = 0; i < bank->nr_regs_req; i++)
 			intc_context[ind].mir[i] =
 				intc_bank_read_reg(&irq_banks[0], INTC_MIR0 +
 				(0x20 * i));

@@ -322,10 +321,10 @@ void omap_intc_restore_context(void)
 					bank, INTC_IDLE);
 		intc_bank_write_reg(intc_context[ind].threshold,
 					bank, INTC_THRESHOLD);
-		for (i = 0; i < INTCPS_NR_IRQS; i++)
+		for (i = 0; i < bank->nr_irqs; i++)
 			intc_bank_write_reg(intc_context[ind].ilr[i],
 				bank, (0x100 + 0x4*i));
-		for (i = 0; i < INTCPS_NR_MIR_REGS; i++)
+		for (i = 0; i < bank->nr_regs_req; i++)
 			intc_bank_write_reg(intc_context[ind].mir[i],
 				 &irq_banks[0], INTC_MIR0 + (0x20 * i));
 	}

@@ -356,6 +355,6 @@ void omap3_intc_resume_idle(void)
 asmlinkage void __exception_irq_entry omap3_intc_handle_irq(struct pt_regs *regs)
 {
 	void __iomem *base_addr = OMAP3_IRQ_BASE;
-	omap_intc_handle_irq(base_addr, regs);
+	omap_intc_handle_irq(base_addr, irq_banks[0].nr_regs_req, regs);
 }
 #endif /* CONFIG_ARCH_OMAP3 */

arch/arm/plat-omap/include/plat/irqs.h +5 -2

index 37bbbbb..8e1b6ae 100644
@@ -441,8 +441,11 @@
 
 #define OMAP_IRQ_BIT(irq)	(1 << ((irq) % 32))
 
-#define INTCPS_NR_MIR_REGS	3
-#define INTCPS_NR_IRQS		96
+/*
+ * Max from AM33XX device
+ */
+#define INTCPS_MAX_NR_REGS_REQ	4
+#define INTCPS_MAX_NR_IRQS	128
 
 #include <mach/hardware.h>

--
1.7.0.4

Kevin Hilman

2012-05-08 18:39 UTC #1 lore raw

PATCH 1/1 ARM: OMAP2+: irq: Increase no of supported interrupts to 128

Quoted 9 lines · Vaibhav Hiremath

With addition to TI81XX, AM33XX family of devices, the number of interrupts supported has increased to 128, compared to 96. The current implementation for irq handling is hardcoded to use 96 interrupts (with 3 register-sets to handle), this patch cleanups the code, to increase maximum number of interrupts support to 128, with dynamic detection of no of registers required for handling all interrupts.

Quoted 8 lines

Signed-off-by Vaibhav Hiremath <[email protected]>
Signed-off-by Afzal Mohammed <[email protected]>
Cc Tony Lindgren <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Paul Walmsley <[email protected]>
---
Ideally, we should use dynamic allocation to allocate memory for registers/arrays,

Yes.

Quoted 1 line

may be too much cleanup for this patch,

There is no such thing as too much cleanup. ;)
And the INTC is in need of it, IMO.

Quoted 2 lines

so as of now restricting to minimal changes to fit devices like, am33xx/ti81xx.

Then someone else will have to do the cleanup later. It would be greatly appreciated if you could do the necessary cleanup in order to cleanly add support for more SoCs. Yes, we probably should've insisted when support for TI81xx was added, but that one slipped in under the radar.

For starters, the notion of a banks this code is a rather messed up and needs a cleanup. A bank is supposed to be a group of 32 interrupts,
and the INTC is made up of 3 (or 4) banks. However, the current
code creates a single "bank" of 96 (or 128) interrupts.

It also confuses what registers are part of the bank and what are global to the INTC. This confusion is both in init and in context save/restore.

IMO, to clean this up, first the notion of banks needs to be fixed in that code there is a distinction between what acts on banks and what works on the whole INTC.

Then, the init/alloc should be done dynamically based on the number of interrupts passed to omap_init_irq()

Kevin

Hiremath, Vaibhav

author

2012-05-09 09:58 UTC #2 lore raw

PATCH 1/1 ARM: OMAP2+: irq: Increase no of supported interrupts to 128

Quoted 20 lines · Kevin

Quoted 9 lines · Vaibhav Hiremath
With addition to TI81XX, AM33XX family of devices, the number of interrupts supported has increased to 128, compared to 96. The current implementation for irq handling is hardcoded to use 96 interrupts (with 3 register-sets to handle), this patch cleanups the code, to increase maximum number of interrupts support to 128, with dynamic detection of no of registers required for handling all interrupts.
Quoted 8 lines
Signed-off-by Vaibhav Hiremath <[email protected]>
Signed-off-by Afzal Mohammed <[email protected]>
Cc Tony Lindgren <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Paul Walmsley <[email protected]>
---
Ideally, we should use dynamic allocation to allocate memory for registers/arrays,

Quoted 2 lines

Yes.

Thanks Kevin, I will put this activity in my TODO list.

Quoted 5 lines

Quoted 1 line
may be too much cleanup for this patch,
There is no such thing as too much cleanup. ;)
And the INTC is in need of it, IMO.

Indeed it is in need of cleanup...

Quoted 9 lines

Quoted 2 lines
so as of now restricting to minimal changes to fit devices like, am33xx/ti81xx.
Then someone else will have to do the cleanup later. It would be greatly appreciated if you could do the necessary cleanup in order to cleanly add support for more SoCs. Yes, we probably should've insisted when support for TI81xx was added, but that one slipped in under the radar.

Yeah, I understand. As I said I will put this activity in my TODO list.

Quoted 8 lines

For starters, the notion of a banks this code is a rather messed up and needs a cleanup. A bank is supposed to be a group of 32 interrupts,
and the INTC is made up of 3 (or 4) banks. However, the current
code creates a single "bank" of 96 (or 128) interrupts.
It also confuses what registers are part of the bank and what are global to the INTC. This confusion is both in init and in context save/restore.

Quoted 7 lines

IMO, to clean this up, first the notion of banks needs to be fixed in that code there is a distinction between what acts on banks and what works on the whole INTC.
Then, the init/alloc should be done dynamically based on the number of interrupts passed to omap_init_irq()

Kevin,
Let me finish up with am33xx baseport upstream activity which is currently going on at full swing, then next thing I will pick up is this code cleanup.

I still feel, this is still a valid cleanup patch, and can be merged, as it is required/used when we do major cleanup.

Thanks,
Vaibhav

Kevin Hilman

2012-05-10 21:49 UTC #3 lore raw

PATCH 1/1 ARM: OMAP2+: irq: Increase no of supported interrupts to 128

Quoted 63 lines · Vaibhav

Quoted 20 lines · Kevin
Quoted 9 lines · Vaibhav Hiremath
With addition to TI81XX, AM33XX family of devices, the number of interrupts supported has increased to 128, compared to 96. The current implementation for irq handling is hardcoded to use 96 interrupts (with 3 register-sets to handle), this patch cleanups the code, to increase maximum number of interrupts support to 128, with dynamic detection of no of registers required for handling all interrupts.
Quoted 8 lines
Signed-off-by Vaibhav Hiremath <[email protected]>
Signed-off-by Afzal Mohammed <[email protected]>
Cc Tony Lindgren <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Paul Walmsley <[email protected]>
---
Ideally, we should use dynamic allocation to allocate memory for registers/arrays,
Quoted 2 lines
Yes.
Thanks Kevin, I will put this activity in my TODO list.
Quoted 5 lines
Quoted 1 line
may be too much cleanup for this patch,
There is no such thing as too much cleanup. ;)
And the INTC is in need of it, IMO.
Indeed it is in need of cleanup...
Quoted 9 lines
Quoted 2 lines
so as of now restricting to minimal changes to fit devices like, am33xx/ti81xx.
Then someone else will have to do the cleanup later. It would be greatly appreciated if you could do the necessary cleanup in order to cleanly add support for more SoCs. Yes, we probably should've insisted when support for TI81xx was added, but that one slipped in under the radar.
Yeah, I understand. As I said I will put this activity in my TODO list.
Quoted 8 lines
For starters, the notion of a banks this code is a rather messed up and needs a cleanup. A bank is supposed to be a group of 32 interrupts,
and the INTC is made up of 3 (or 4) banks. However, the current
code creates a single "bank" of 96 (or 128) interrupts.
It also confuses what registers are part of the bank and what are global to the INTC. This confusion is both in init and in context save/restore.
Quoted 7 lines
IMO, to clean this up, first the notion of banks needs to be fixed in that code there is a distinction between what acts on banks and what works on the whole INTC.
Then, the init/alloc should be done dynamically based on the number of interrupts passed to omap_init_irq()

Quoted 6 lines

Kevin,
Let me finish up with am33xx baseport upstream activity which is currently going on at full swing, then next thing I will pick up is this code cleanup.
I still feel, this is still a valid cleanup patch, and can be merged, as it is required/used when we do major cleanup.

Well, Tony can make that decision.

Personally, I think this patch continues to add confusion on top of an existing mess, and to me provides the proverbial straw that broke the camel's back.

That being said, the INTC core is an obviously important and sensitive piece of code so needs to be handled with care.

In case Tony decides to merge this patch and allow a future** cleanup, I'll provide some comments.

Kevin

** since they rarely happen, we tend to not have too much faith in
promises of mythical "future" cleanups. This is not because we don't trust you personally, but simply based on based experience.

Tony Lindgren

maintainer

2012-05-10 22:10 UTC #4 lore raw

PATCH 1/1 ARM: OMAP2+: irq: Increase no of supported interrupts to 128

Quoted 14 lines · Kevin Hilman

Quoted 6 lines · Vaibhav
Let me finish up with am33xx baseport upstream activity which is currently going on at full swing, then next thing I will pick up is this code cleanup.
I still feel, this is still a valid cleanup patch, and can be merged, as it is required/used when we do major cleanup.
Well, Tony can make that decision.
Personally, I think this patch continues to add confusion on top of an existing mess, and to me provides the proverbial straw that broke the camel's back.

Quoted 5 lines

That being said, the INTC core is an obviously important and sensitive piece of code so needs to be handled with care.
In case Tony decides to merge this patch and allow a future** cleanup, I'll provide some comments.

Heh I agree with Kevin here :)

Tony

Quoted 3 lines

** since they rarely happen, we tend to not have too much faith in
promises of mythical "future" cleanups. This is not because we don't trust you personally, but simply based on based experience.

Hiremath, Vaibhav

author

2012-05-11 06:56 UTC #5 lore raw

PATCH 1/1 ARM: OMAP2+: irq: Increase no of supported interrupts to 128

In reply to Kevin Hilman

Quoted 78 lines · Kevin

Quoted 63 lines · Vaibhav
Quoted 20 lines · Kevin
Quoted 9 lines · Vaibhav Hiremath
With addition to TI81XX, AM33XX family of devices, the number of interrupts supported has increased to 128, compared to 96. The current implementation for irq handling is hardcoded to use 96 interrupts (with 3 register-sets to handle), this patch cleanups the code, to increase maximum number of interrupts support to 128, with dynamic detection of no of registers required for handling all interrupts.
Quoted 8 lines
Signed-off-by Vaibhav Hiremath <[email protected]>
Signed-off-by Afzal Mohammed <[email protected]>
Cc Tony Lindgren <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Paul Walmsley <[email protected]>
---
Ideally, we should use dynamic allocation to allocate memory for registers/arrays,
Quoted 2 lines
Yes.
Thanks Kevin, I will put this activity in my TODO list.
Quoted 5 lines
Quoted 1 line
may be too much cleanup for this patch,
There is no such thing as too much cleanup. ;)
And the INTC is in need of it, IMO.
Indeed it is in need of cleanup...
Quoted 9 lines
Quoted 2 lines
so as of now restricting to minimal changes to fit devices like, am33xx/ti81xx.
Then someone else will have to do the cleanup later. It would be greatly appreciated if you could do the necessary cleanup in order to cleanly add support for more SoCs. Yes, we probably should've insisted when support for TI81xx was added, but that one slipped in under the radar.
Yeah, I understand. As I said I will put this activity in my TODO list.
Quoted 8 lines
For starters, the notion of a banks this code is a rather messed up and needs a cleanup. A bank is supposed to be a group of 32 interrupts,
and the INTC is made up of 3 (or 4) banks. However, the current
code creates a single "bank" of 96 (or 128) interrupts.
It also confuses what registers are part of the bank and what are global to the INTC. This confusion is both in init and in context save/restore.
Quoted 7 lines
IMO, to clean this up, first the notion of banks needs to be fixed in that code there is a distinction between what acts on banks and what works on the whole INTC.
Then, the init/alloc should be done dynamically based on the number of interrupts passed to omap_init_irq()
Quoted 6 lines
Kevin,
Let me finish up with am33xx baseport upstream activity which is currently going on at full swing, then next thing I will pick up is this code cleanup.
I still feel, this is still a valid cleanup patch, and can be merged, as it is required/used when we do major cleanup.
Well, Tony can make that decision.
Personally, I think this patch continues to add confusion on top of an existing mess, and to me provides the proverbial straw that broke the camel's back.

Quoted 6 lines

That being said, the INTC core is an obviously important and sensitive piece of code so needs to be handled with care.
In case Tony decides to merge this patch and allow a future** cleanup, I'll provide some comments.

I am OK, lets drop this patch as of now, and I will take this when I start cleanup activity.

Quoted 6 lines

Kevin
** since they rarely happen, we tend to not have too much faith in
promises of mythical "future" cleanups. This is not because we don't trust you personally, but simply based on based experience.

Very nicely said :) :) :)

I agree with you, sometimes priorities changes and things doesn't happen the way we wanted to be.

As I said earlier, I am now full time working on upstream (no ifs and buts); else I wouldn't have committed for this activity.

Thanks,
Vaibhav