[PATCH] arm: GIC: Do not try to register more then NR_IRQS interrupts

[PATCH] arm: GIC: Do not try to register more then NR_IRQS interrupts

[Pawel Moll]

This change limits number of GIC-originating interrupts to the
platform maximum (defined by NR_IRQS) while still initialising
all distributor registers.

Signed-off-by: Pawel Moll <pawel.moll@arm.com>
---
 arch/arm/common/gic.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
index 772f95f..20e1412 100644
--- a/arch/arm/common/gic.c
+++ b/arch/arm/common/gic.c
@@ -231,14 +231,6 @@ void __init gic_dist_init(unsigned int gic_nr, void __iomem *base,
 	max_irq = (max_irq + 1) * 32;
 
 	/*
-	 * The GIC only supports up to 1020 interrupt sources.
-	 * Limit this to either the architected maximum, or the
-	 * platform maximum.
-	 */
-	if (max_irq > max(1020, NR_IRQS))
-		max_irq = max(1020, NR_IRQS);
-
-	/*
 	 * Set all global interrupts to be level triggered, active low.
 	 */
 	for (i = 32; i < max_irq; i += 16)
@@ -264,6 +256,12 @@ void __init gic_dist_init(unsigned int gic_nr, void __iomem *base,
 		writel(0xffffffff, base + GIC_DIST_ENABLE_CLEAR + i * 4 / 32);
 
 	/*
+	 * Limit number of interrupts registered to the platform maximum
+	 */
+	WARN_ON(max_irq > NR_IRQS);
+	max_irq = min(max_irq, NR_IRQS);
+
+	/*
 	 * Setup the Linux IRQ subsystem.
 	 */
 	for (i = irq_start; i < gic_data[gic_nr].irq_offset + max_irq; i++) {
-- 
1.6.3.3

[PATCH] arm: GIC: Do not try to register more then NR_IRQS interrupts

[Catalin Marinas]

On 22 November 2010 13:08, Pawel Moll <pawel.moll@arm.com> wrote:
> This change limits number of GIC-originating interrupts to the
> platform maximum (defined by NR_IRQS) while still initialising
> all distributor registers.
>
> Signed-off-by: Pawel Moll <pawel.moll@arm.com>

This looks better to me:

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

[PATCH] arm: GIC: Do not try to register more then NR_IRQS interrupts

[Uwe Kleine-König]

On Mon, Nov 22, 2010 at 01:08:15PM +0000, Pawel Moll wrote:
> This change limits number of GIC-originating interrupts to the
> platform maximum (defined by NR_IRQS) while still initialising
> all distributor registers.
> 
> Signed-off-by: Pawel Moll <pawel.moll@arm.com>
> ---
>  arch/arm/common/gic.c |   14 ++++++--------
>  1 files changed, 6 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/arm/common/gic.c b/arch/arm/common/gic.c
> index 772f95f..20e1412 100644
> --- a/arch/arm/common/gic.c
> +++ b/arch/arm/common/gic.c
> @@ -231,14 +231,6 @@ void __init gic_dist_init(unsigned int gic_nr, void __iomem *base,
>  	max_irq = (max_irq + 1) * 32;
>  
>  	/*
> -	 * The GIC only supports up to 1020 interrupt sources.
> -	 * Limit this to either the architected maximum, or the
> -	 * platform maximum.
> -	 */
> -	if (max_irq > max(1020, NR_IRQS))
> -		max_irq = max(1020, NR_IRQS);
> -
> -	/*
>  	 * Set all global interrupts to be level triggered, active low.
>  	 */
>  	for (i = 32; i < max_irq; i += 16)
> @@ -264,6 +256,12 @@ void __init gic_dist_init(unsigned int gic_nr, void __iomem *base,
>  		writel(0xffffffff, base + GIC_DIST_ENABLE_CLEAR + i * 4 / 32);
>  
>  	/*
> +	 * Limit number of interrupts registered to the platform maximum
> +	 */
> +	WARN_ON(max_irq > NR_IRQS);
> +	max_irq = min(max_irq, NR_IRQS);
> +
> +	/*
Maybe make this

	if (WARN_ON(max_irq > NR_IRQS))
		max_irq = NR_IRQS;

That said I wonder if this is correct, at least it looks wrong to me.

First (judging from the name) max_irq is used as irq number, while
NR_IRQS must not be used as such (i.e. NR_IRQS - 1 is the last valid
number).  But maybe the variable is just (IMHO) bad.

>  	 * Setup the Linux IRQ subsystem.
>  	 */
>  	for (i = irq_start; i < gic_data[gic_nr].irq_offset + max_irq; i++) {
And further shouldn't gic_data[gic_nr].irq_offset be taken into account
when limiting max_irq?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] arm: GIC: Do not try to register more then NR_IRQS interrupts

[Pawel Moll]

> > +   WARN_ON(max_irq > NR_IRQS);
> > +   max_irq = min(max_irq, NR_IRQS);
> Maybe make this
>       if (WARN_ON(max_irq > NR_IRQS))
>               max_irq = NR_IRQS;

Fine by me :-)

> First (judging from the name) max_irq is used as irq number, while
> NR_IRQS must not be used as such (i.e. NR_IRQS - 1 is the last valid
> number).  But maybe the variable is just (IMHO) bad.

Yeah, the "max_irq" is actually "gic_irqs" rather than anything else.

> >      * Setup the Linux IRQ subsystem.
> >      */
> >     for (i = irq_start; i < gic_data[gic_nr].irq_offset + max_irq;
> i++) {
> And further shouldn't gic_data[gic_nr].irq_offset be taken into account
> when limiting max_irq?

That's what happens if you try to make as little changes as possible ;-) I should have Done It Right (TM) immediately. A patch follows.

Cheers!

Pawe?

-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.