[PATCH] irqchip: omap-intc: improve IRQ handler

[PATCH] irqchip: omap-intc: improve IRQ handler

[Felipe Balbi]

as it turns out the current IRQ number will
*always* be available from SIR register which
renders the reads of PENDING registers as plain
unnecessary overhead.

In order to catch any situation where SIR reads
as zero, we're adding a WARN() to turn it into
a very verbose error and users actually report
it.

With this patch average running time of
omap_intc_handle_irq() reduced from about 28.5us
to 19.8us as measured by the kernel function
profiler.

Tested with BeagleBoneBlack Rev A5C.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---

Before applying, it would be very nice to get reports
from other folks on different platforms, specially OMAP2/3
ones which I don't have (easy) access.

 drivers/irqchip/irq-omap-intc.c | 35 +++++------------------------------
 1 file changed, 5 insertions(+), 30 deletions(-)

diff --git a/drivers/irqchip/irq-omap-intc.c b/drivers/irqchip/irq-omap-intc.c
index 28718d3..a2da6d5 100644
--- a/drivers/irqchip/irq-omap-intc.c
+++ b/drivers/irqchip/irq-omap-intc.c
@@ -315,37 +315,12 @@ static int __init omap_init_irq(u32 base, struct device_node *node)
 static asmlinkage void __exception_irq_entry
 omap_intc_handle_irq(struct pt_regs *regs)
 {
-	u32 irqnr = 0;
-	int handled_irq = 0;
-	int i;
-
-	do {
-		for (i = 0; i < omap_nr_pending; i++) {
-			irqnr = intc_readl(INTC_PENDING_IRQ0 + (0x20 * i));
-			if (irqnr)
-				goto out;
-		}
-
-out:
-		if (!irqnr)
-			break;
-
-		irqnr = intc_readl(INTC_SIR);
-		irqnr &= ACTIVEIRQ_MASK;
+	u32 irqnr;
 
-		if (irqnr) {
-			handle_domain_irq(domain, irqnr, regs);
-			handled_irq = 1;
-		}
-	} while (irqnr);
-
-	/*
-	 * If an irq is masked or deasserted while active, we will
-	 * keep ending up here with no irq handled. So remove it from
-	 * the INTC with an ack.
-	 */
-	if (!handled_irq)
-		omap_ack_irq(NULL);
+	irqnr = intc_readl(INTC_SIR);
+	irqnr &= ACTIVEIRQ_MASK;
+	WARN(!irqnr, "Spuriour IRQ ?\n");
+	handle_domain_irq(domain, irqnr, regs);
 }
 
 void __init omap2_init_irq(void)
-- 
2.2.0

[PATCH] irqchip: omap-intc: improve IRQ handler

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [150102 10:50]:
> as it turns out the current IRQ number will
> *always* be available from SIR register which
> renders the reads of PENDING registers as plain
> unnecessary overhead.
> 
> In order to catch any situation where SIR reads
> as zero, we're adding a WARN() to turn it into
> a very verbose error and users actually report
> it.
> 
> With this patch average running time of
> omap_intc_handle_irq() reduced from about 28.5us
> to 19.8us as measured by the kernel function
> profiler.

That's a nice improvment for an interrupt controller :)
 
> Tested with BeagleBoneBlack Rev A5C.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
> 
> Before applying, it would be very nice to get reports
> from other folks on different platforms, specially OMAP2/3
> ones which I don't have (easy) access.

Seems to behave just fine on omap2 and 3 here, gave it
a quick try on n800, n900, omap3-ldp and 37xx-evm:

Tested-by: Tony Lindgren <tony@atomide.com>

Regards,

Tony
 
>  drivers/irqchip/irq-omap-intc.c | 35 +++++------------------------------
>  1 file changed, 5 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-omap-intc.c b/drivers/irqchip/irq-omap-intc.c
> index 28718d3..a2da6d5 100644
> --- a/drivers/irqchip/irq-omap-intc.c
> +++ b/drivers/irqchip/irq-omap-intc.c
> @@ -315,37 +315,12 @@ static int __init omap_init_irq(u32 base, struct device_node *node)
>  static asmlinkage void __exception_irq_entry
>  omap_intc_handle_irq(struct pt_regs *regs)
>  {
> -	u32 irqnr = 0;
> -	int handled_irq = 0;
> -	int i;
> -
> -	do {
> -		for (i = 0; i < omap_nr_pending; i++) {
> -			irqnr = intc_readl(INTC_PENDING_IRQ0 + (0x20 * i));
> -			if (irqnr)
> -				goto out;
> -		}
> -
> -out:
> -		if (!irqnr)
> -			break;
> -
> -		irqnr = intc_readl(INTC_SIR);
> -		irqnr &= ACTIVEIRQ_MASK;
> +	u32 irqnr;
>  
> -		if (irqnr) {
> -			handle_domain_irq(domain, irqnr, regs);
> -			handled_irq = 1;
> -		}
> -	} while (irqnr);
> -
> -	/*
> -	 * If an irq is masked or deasserted while active, we will
> -	 * keep ending up here with no irq handled. So remove it from
> -	 * the INTC with an ack.
> -	 */
> -	if (!handled_irq)
> -		omap_ack_irq(NULL);
> +	irqnr = intc_readl(INTC_SIR);
> +	irqnr &= ACTIVEIRQ_MASK;
> +	WARN(!irqnr, "Spuriour IRQ ?\n");
> +	handle_domain_irq(domain, irqnr, regs);
>  }
>  
>  void __init omap2_init_irq(void)
> -- 
> 2.2.0
> 

[PATCH] irqchip: omap-intc: improve IRQ handler

[Felipe Balbi]

On Fri, Jan 02, 2015 at 01:32:51PM -0800, Tony Lindgren wrote:
> * Felipe Balbi <balbi@ti.com> [150102 10:50]:
> > as it turns out the current IRQ number will
> > *always* be available from SIR register which
> > renders the reads of PENDING registers as plain
> > unnecessary overhead.
> > 
> > In order to catch any situation where SIR reads
> > as zero, we're adding a WARN() to turn it into
> > a very verbose error and users actually report
> > it.
> > 
> > With this patch average running time of
> > omap_intc_handle_irq() reduced from about 28.5us
> > to 19.8us as measured by the kernel function
> > profiler.
> 
> That's a nice improvment for an interrupt controller :)
>  
> > Tested with BeagleBoneBlack Rev A5C.
> > 
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > ---
> > 
> > Before applying, it would be very nice to get reports
> > from other folks on different platforms, specially OMAP2/3
> > ones which I don't have (easy) access.
> 
> Seems to behave just fine on omap2 and 3 here, gave it
> a quick try on n800, n900, omap3-ldp and 37xx-evm:
> 
> Tested-by: Tony Lindgren <tony@atomide.com>

cool, thanks

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150102/7b7a07f0/attachment.sig>

[PATCH v2] irqchip: omap-intc: improve IRQ handler

[Felipe Balbi]

as it turns out the current IRQ number will
*always* be available from SIR register which
renders the reads of PENDING registers as plain
unnecessary overhead.

In order to catch any situation where SIR reads
as zero, we're adding a WARN() to turn it into
a very verbose error and users actually report
it.

With this patch average running time of
omap_intc_handle_irq() reduced from about 28.5us
to 19.8us as measured by the kernel function
profiler.

Tested with BeagleBoneBlack Rev A5C.

Tested-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
---

Changes since v1:
	- s/Spuriour/Spurious

 drivers/irqchip/irq-omap-intc.c | 35 +++++------------------------------
 1 file changed, 5 insertions(+), 30 deletions(-)

diff --git a/drivers/irqchip/irq-omap-intc.c b/drivers/irqchip/irq-omap-intc.c
index 28718d3..3c97025 100644
--- a/drivers/irqchip/irq-omap-intc.c
+++ b/drivers/irqchip/irq-omap-intc.c
@@ -315,37 +315,12 @@ static int __init omap_init_irq(u32 base, struct device_node *node)
 static asmlinkage void __exception_irq_entry
 omap_intc_handle_irq(struct pt_regs *regs)
 {
-	u32 irqnr = 0;
-	int handled_irq = 0;
-	int i;
-
-	do {
-		for (i = 0; i < omap_nr_pending; i++) {
-			irqnr = intc_readl(INTC_PENDING_IRQ0 + (0x20 * i));
-			if (irqnr)
-				goto out;
-		}
-
-out:
-		if (!irqnr)
-			break;
-
-		irqnr = intc_readl(INTC_SIR);
-		irqnr &= ACTIVEIRQ_MASK;
+	u32 irqnr;
 
-		if (irqnr) {
-			handle_domain_irq(domain, irqnr, regs);
-			handled_irq = 1;
-		}
-	} while (irqnr);
-
-	/*
-	 * If an irq is masked or deasserted while active, we will
-	 * keep ending up here with no irq handled. So remove it from
-	 * the INTC with an ack.
-	 */
-	if (!handled_irq)
-		omap_ack_irq(NULL);
+	irqnr = intc_readl(INTC_SIR);
+	irqnr &= ACTIVEIRQ_MASK;
+	WARN(!irqnr, "Spurious IRQ ?\n");
+	handle_domain_irq(domain, irqnr, regs);
 }
 
 void __init omap2_init_irq(void)
-- 
2.2.0

[PATCH] irqchip: omap-intc: improve IRQ handler

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [150102 10:50]:
> as it turns out the current IRQ number will
> *always* be available from SIR register which
> renders the reads of PENDING registers as plain
> unnecessary overhead.
> 
> In order to catch any situation where SIR reads
> as zero, we're adding a WARN() to turn it into
> a very verbose error and users actually report
> it.
> 
> With this patch average running time of
> omap_intc_handle_irq() reduced from about 28.5us
> to 19.8us as measured by the kernel function
> profiler.
> 
> Tested with BeagleBoneBlack Rev A5C.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>

Jason, looks like this is not showing up in Linux next. The
same for the changes I did for dm81xx.

Regards,

Tony

> ---
> 
> Before applying, it would be very nice to get reports
> from other folks on different platforms, specially OMAP2/3
> ones which I don't have (easy) access.
> 
>  drivers/irqchip/irq-omap-intc.c | 35 +++++------------------------------
>  1 file changed, 5 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-omap-intc.c b/drivers/irqchip/irq-omap-intc.c
> index 28718d3..a2da6d5 100644
> --- a/drivers/irqchip/irq-omap-intc.c
> +++ b/drivers/irqchip/irq-omap-intc.c
> @@ -315,37 +315,12 @@ static int __init omap_init_irq(u32 base, struct device_node *node)
>  static asmlinkage void __exception_irq_entry
>  omap_intc_handle_irq(struct pt_regs *regs)
>  {
> -	u32 irqnr = 0;
> -	int handled_irq = 0;
> -	int i;
> -
> -	do {
> -		for (i = 0; i < omap_nr_pending; i++) {
> -			irqnr = intc_readl(INTC_PENDING_IRQ0 + (0x20 * i));
> -			if (irqnr)
> -				goto out;
> -		}
> -
> -out:
> -		if (!irqnr)
> -			break;
> -
> -		irqnr = intc_readl(INTC_SIR);
> -		irqnr &= ACTIVEIRQ_MASK;
> +	u32 irqnr;
>  
> -		if (irqnr) {
> -			handle_domain_irq(domain, irqnr, regs);
> -			handled_irq = 1;
> -		}
> -	} while (irqnr);
> -
> -	/*
> -	 * If an irq is masked or deasserted while active, we will
> -	 * keep ending up here with no irq handled. So remove it from
> -	 * the INTC with an ack.
> -	 */
> -	if (!handled_irq)
> -		omap_ack_irq(NULL);
> +	irqnr = intc_readl(INTC_SIR);
> +	irqnr &= ACTIVEIRQ_MASK;
> +	WARN(!irqnr, "Spuriour IRQ ?\n");
> +	handle_domain_irq(domain, irqnr, regs);
>  }
>  
>  void __init omap2_init_irq(void)
> -- 
> 2.2.0
> 

[PATCH] irqchip: omap-intc: improve IRQ handler

[Tony Lindgren]

Felipe,

* Tony Lindgren <tony@atomide.com> [150119 13:41]:
> * Felipe Balbi <balbi@ti.com> [150102 10:50]:
> > as it turns out the current IRQ number will
> > *always* be available from SIR register which
> > renders the reads of PENDING registers as plain
> > unnecessary overhead.
> > 
> > In order to catch any situation where SIR reads
> > as zero, we're adding a WARN() to turn it into
> > a very verbose error and users actually report
> > it.
> > 
> > With this patch average running time of
> > omap_intc_handle_irq() reduced from about 28.5us
> > to 19.8us as measured by the kernel function
> > profiler.
> > 
> > Tested with BeagleBoneBlack Rev A5C.
> > 
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> 
> Jason, looks like this is not showing up in Linux next. The
> same for the changes I did for dm81xx.

Can you please resend this to Jason? Looks like this
is still not merged.

Regards,

Tony

> > Before applying, it would be very nice to get reports
> > from other folks on different platforms, specially OMAP2/3
> > ones which I don't have (easy) access.
> > 
> >  drivers/irqchip/irq-omap-intc.c | 35 +++++------------------------------
> >  1 file changed, 5 insertions(+), 30 deletions(-)
> > 
> > diff --git a/drivers/irqchip/irq-omap-intc.c b/drivers/irqchip/irq-omap-intc.c
> > index 28718d3..a2da6d5 100644
> > --- a/drivers/irqchip/irq-omap-intc.c
> > +++ b/drivers/irqchip/irq-omap-intc.c
> > @@ -315,37 +315,12 @@ static int __init omap_init_irq(u32 base, struct device_node *node)
> >  static asmlinkage void __exception_irq_entry
> >  omap_intc_handle_irq(struct pt_regs *regs)
> >  {
> > -	u32 irqnr = 0;
> > -	int handled_irq = 0;
> > -	int i;
> > -
> > -	do {
> > -		for (i = 0; i < omap_nr_pending; i++) {
> > -			irqnr = intc_readl(INTC_PENDING_IRQ0 + (0x20 * i));
> > -			if (irqnr)
> > -				goto out;
> > -		}
> > -
> > -out:
> > -		if (!irqnr)
> > -			break;
> > -
> > -		irqnr = intc_readl(INTC_SIR);
> > -		irqnr &= ACTIVEIRQ_MASK;
> > +	u32 irqnr;
> >  
> > -		if (irqnr) {
> > -			handle_domain_irq(domain, irqnr, regs);
> > -			handled_irq = 1;
> > -		}
> > -	} while (irqnr);
> > -
> > -	/*
> > -	 * If an irq is masked or deasserted while active, we will
> > -	 * keep ending up here with no irq handled. So remove it from
> > -	 * the INTC with an ack.
> > -	 */
> > -	if (!handled_irq)
> > -		omap_ack_irq(NULL);
> > +	irqnr = intc_readl(INTC_SIR);
> > +	irqnr &= ACTIVEIRQ_MASK;
> > +	WARN(!irqnr, "Spuriour IRQ ?\n");
> > +	handle_domain_irq(domain, irqnr, regs);
> >  }
> >  
> >  void __init omap2_init_irq(void)
> > -- 
> > 2.2.0
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] irqchip: omap-intc: improve IRQ handler

[Thomas Gleixner]

On Wed, 15 Jul 2015, Tony Lindgren wrote:
> Felipe,
> 
> * Tony Lindgren <tony@atomide.com> [150119 13:41]:
> > * Felipe Balbi <balbi@ti.com> [150102 10:50]:
> > > as it turns out the current IRQ number will
> > > *always* be available from SIR register which
> > > renders the reads of PENDING registers as plain
> > > unnecessary overhead.
> > > 
> > > In order to catch any situation where SIR reads
> > > as zero, we're adding a WARN() to turn it into
> > > a very verbose error and users actually report
> > > it.
> > > 
> > > With this patch average running time of
> > > omap_intc_handle_irq() reduced from about 28.5us
> > > to 19.8us as measured by the kernel function
> > > profiler.
> > > 
> > > Tested with BeagleBoneBlack Rev A5C.
> > > 
> > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > 
> > Jason, looks like this is not showing up in Linux next. The
> > same for the changes I did for dm81xx.
> 
> Can you please resend this to Jason? Looks like this
> is still not merged.

Please send it to me asap and please cc lkml on irqchip patches.

Thanks,

	tglx

[PATCH] irqchip: omap-intc: improve IRQ handler

[Felipe Balbi]

On Wed, Jul 15, 2015 at 02:36:54PM +0200, Thomas Gleixner wrote:
> On Wed, 15 Jul 2015, Tony Lindgren wrote:
> > Felipe,
> > 
> > * Tony Lindgren <tony@atomide.com> [150119 13:41]:
> > > * Felipe Balbi <balbi@ti.com> [150102 10:50]:
> > > > as it turns out the current IRQ number will
> > > > *always* be available from SIR register which
> > > > renders the reads of PENDING registers as plain
> > > > unnecessary overhead.
> > > > 
> > > > In order to catch any situation where SIR reads
> > > > as zero, we're adding a WARN() to turn it into
> > > > a very verbose error and users actually report
> > > > it.
> > > > 
> > > > With this patch average running time of
> > > > omap_intc_handle_irq() reduced from about 28.5us
> > > > to 19.8us as measured by the kernel function
> > > > profiler.
> > > > 
> > > > Tested with BeagleBoneBlack Rev A5C.
> > > > 
> > > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > 
> > > Jason, looks like this is not showing up in Linux next. The
> > > same for the changes I did for dm81xx.
> > 
> > Can you please resend this to Jason? Looks like this
> > is still not merged.
> 
> Please send it to me asap and please cc lkml on irqchip patches.

building and testing again. Will resend shortly

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150720/268ccb9a/attachment.sig>