[PATCH RESEND 0/3] Orion irqchip and Kirkwood SDIO

[PATCH RESEND 0/3] Orion irqchip and Kirkwood SDIO

[Sebastian Hesselbarth]

Guys,

we somehow forgot this patch set, which was aimed for v3.12/v3.13
and first sent and acked in November 2013 [1]. Now this is rebase on
to v3.15-rc1 and should be taken for v3.16.

In contrast to the original patch set, I reordered the individual
patches by subsystem and made some cosmetic but no functional changes.

>From the original patch set:

This patch set tries to deal with a minor irq issue seen on Marvell
Kirkwood SoCs with irqchip/irq-orion and mvsdio drivers. In contrast
to non-DT irq handling, irqchip driver does handle irqs a little bit
different. First of all, it reads irq cause register once and works
through all bits set while non-DT irq handling read irq cause and
handled only one irq. Second, irqchip reverses irq priorities by
using ffs() instead of fls().
This now, seems to trigger a minor ip design issue in sdio peripheral
where sdio irq can occur upstream while sdio interrupts are all
disabled in the ip registers. This extra, unexpected irq does neither
harm correct function of HW nor SW driver but triggers a warning in
mvsdio irq handler.

I have tried to debug this and can say that the sequence of irq
related events is:
(a) sdio irq is set in upstream irq
(b) generic-chip handler masks sdio irq
(c) sdio irq handler deals with it,
    acks and disables all peripheral irq registers
(d) sdio irq cause is cleared
(e) generic-chip handler unmasks sdio irq
(f) sdio irq is set in upstream irq and cleared little later
(g) sdio irq handler is upset about being called with no irq to handle

This patch set is actually three independent patches in one as mvsdio
irq workaround just motivates irqchip handling changes. The third just
silences a noisy mvsdio dev_notice down to dev_dbg. I have chosen to
keep them together anyway. Below is a more detailed description of the
individual patches.

First patch silences a card detect mechanism related dev_notice to
dev_dbg to not bother users with that.

Second patch works around the spurious irq issue in mvsdio by bailing
out of the irq handler early, if peripheral irq registers indicate that
none should have been fired.

Third patch reverses irq handling priority for irqchip driver to what
non-DT irq did before by using fls() instead of ffs(). The "read cause
once, work through all irqs" handling is maintained.

[1] https://lkml.org/lkml/2013/11/15/276

Sebastian Hesselbarth (3):
  mmc: mvsdio: silence card detect notice
  mmc: mvsdio: workaround for spurious irqs
  irqchip: orion: reverse irq handling priority

 drivers/irqchip/irq-orion.c |  4 ++--
 drivers/mmc/host/mvsdio.c   | 20 +++++++++++++++++---
 2 files changed, 19 insertions(+), 5 deletions(-)

---
Cc: Nicolas Pitre <nico@fluxnic.net>
Cc: Chris Ball <chris@printf.net>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-mmc at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
-- 
1.9.1

[PATCH RESEND 1/3] mmc: mvsdio: silence card detect notice

[Sebastian Hesselbarth]

mvsdio reports method of card detection with dev_notice, while for
removable cards it may be sane, for non-removable cards it is not.
Also, as the user cannot do anything about it, silence the message
by reducing it from dev_notice to dev_dbg.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Acked-by: Jason Cooper <jason@lakedaemon.net>
---
Cc: Nicolas Pitre <nico@fluxnic.net>
Cc: Chris Ball <chris@printf.net>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: linux-mmc at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
---
 drivers/mmc/host/mvsdio.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
index 45aa2206741d..41aca7f28c23 100644
--- a/drivers/mmc/host/mvsdio.c
+++ b/drivers/mmc/host/mvsdio.c
@@ -801,10 +801,10 @@ static int mvsd_probe(struct platform_device *pdev)
 		goto out;
 
 	if (!(mmc->caps & MMC_CAP_NEEDS_POLL))
-		dev_notice(&pdev->dev, "using GPIO for card detection\n");
+		dev_dbg(&pdev->dev, "using GPIO for card detection\n");
 	else
-		dev_notice(&pdev->dev,
-			   "lacking card detect (fall back to polling)\n");
+		dev_dbg(&pdev->dev, "lacking card detect (fall back to polling)\n");
+
 	return 0;
 
 out:
-- 
1.9.1

[PATCH RESEND 1/3] mmc: mvsdio: silence card detect notice

[Ulf Hansson]

On 26 April 2014 21:34, Sebastian Hesselbarth
<sebastian.hesselbarth@gmail.com> wrote:
> mvsdio reports method of card detection with dev_notice, while for
> removable cards it may be sane, for non-removable cards it is not.
> Also, as the user cannot do anything about it, silence the message
> by reducing it from dev_notice to dev_dbg.
>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Acked-by: Jason Cooper <jason@lakedaemon.net>
> ---
> Cc: Nicolas Pitre <nico@fluxnic.net>
> Cc: Chris Ball <chris@printf.net>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: linux-mmc at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-kernel at vger.kernel.org

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

> ---
>  drivers/mmc/host/mvsdio.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
> index 45aa2206741d..41aca7f28c23 100644
> --- a/drivers/mmc/host/mvsdio.c
> +++ b/drivers/mmc/host/mvsdio.c
> @@ -801,10 +801,10 @@ static int mvsd_probe(struct platform_device *pdev)
>                 goto out;
>
>         if (!(mmc->caps & MMC_CAP_NEEDS_POLL))
> -               dev_notice(&pdev->dev, "using GPIO for card detection\n");
> +               dev_dbg(&pdev->dev, "using GPIO for card detection\n");
>         else
> -               dev_notice(&pdev->dev,
> -                          "lacking card detect (fall back to polling)\n");
> +               dev_dbg(&pdev->dev, "lacking card detect (fall back to polling)\n");
> +
>         return 0;
>
>  out:
> --
> 1.9.1
>

[PATCH RESEND 2/3] mmc: mvsdio: workaround for spurious irqs

[Sebastian Hesselbarth]

SDIO controllers found on Marvell Kirkwood SoCs seem to cause a late,
spurious irq although all interrupts have been disabled. This irq
doesn't do any harm, neither to HW nor driver. To avoid some
"unexpected irq" warning later, we workaround above issue by bailing
out of irq handler early, if we didn't expect any.

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Acked-by: Jason Cooper <jason@lakedaemon.net>
---
Cc: Nicolas Pitre <nico@fluxnic.net>
Cc: Chris Ball <chris@printf.net>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: linux-mmc at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
---
 drivers/mmc/host/mvsdio.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
index 41aca7f28c23..9377284f8544 100644
--- a/drivers/mmc/host/mvsdio.c
+++ b/drivers/mmc/host/mvsdio.c
@@ -354,6 +354,20 @@ static irqreturn_t mvsd_irq(int irq, void *dev)
 		intr_status, mvsd_read(MVSD_NOR_INTR_EN),
 		mvsd_read(MVSD_HW_STATE));
 
+	/*
+	 * It looks like, SDIO IP can issue one late, spurious irq
+	 * although all irqs should be disabled. To work around this,
+	 * bail out early, if we didn't expect any irqs to occur.
+	 */
+	if (!mvsd_read(MVSD_NOR_INTR_EN) && !mvsd_read(MVSD_ERR_INTR_EN)) {
+		dev_dbg(host->dev, "spurious irq detected intr 0x%04x intr_en 0x%04x erri 0x%04x erri_en 0x%04x\n",
+			mvsd_read(MVSD_NOR_INTR_STATUS),
+			mvsd_read(MVSD_NOR_INTR_EN),
+			mvsd_read(MVSD_ERR_INTR_STATUS),
+			mvsd_read(MVSD_ERR_INTR_EN));
+		return IRQ_HANDLED;
+	}
+
 	spin_lock(&host->lock);
 
 	/* PIO handling, if needed. Messy business... */
-- 
1.9.1

[PATCH RESEND 2/3] mmc: mvsdio: workaround for spurious irqs

[Ulf Hansson]

On 26 April 2014 21:34, Sebastian Hesselbarth
<sebastian.hesselbarth@gmail.com> wrote:
> SDIO controllers found on Marvell Kirkwood SoCs seem to cause a late,
> spurious irq although all interrupts have been disabled. This irq
> doesn't do any harm, neither to HW nor driver. To avoid some
> "unexpected irq" warning later, we workaround above issue by bailing
> out of irq handler early, if we didn't expect any.
>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Acked-by: Jason Cooper <jason@lakedaemon.net>
> ---
> Cc: Nicolas Pitre <nico@fluxnic.net>
> Cc: Chris Ball <chris@printf.net>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: linux-mmc at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-kernel at vger.kernel.org

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

> ---
>  drivers/mmc/host/mvsdio.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/mmc/host/mvsdio.c b/drivers/mmc/host/mvsdio.c
> index 41aca7f28c23..9377284f8544 100644
> --- a/drivers/mmc/host/mvsdio.c
> +++ b/drivers/mmc/host/mvsdio.c
> @@ -354,6 +354,20 @@ static irqreturn_t mvsd_irq(int irq, void *dev)
>                 intr_status, mvsd_read(MVSD_NOR_INTR_EN),
>                 mvsd_read(MVSD_HW_STATE));
>
> +       /*
> +        * It looks like, SDIO IP can issue one late, spurious irq
> +        * although all irqs should be disabled. To work around this,
> +        * bail out early, if we didn't expect any irqs to occur.
> +        */
> +       if (!mvsd_read(MVSD_NOR_INTR_EN) && !mvsd_read(MVSD_ERR_INTR_EN)) {
> +               dev_dbg(host->dev, "spurious irq detected intr 0x%04x intr_en 0x%04x erri 0x%04x erri_en 0x%04x\n",
> +                       mvsd_read(MVSD_NOR_INTR_STATUS),
> +                       mvsd_read(MVSD_NOR_INTR_EN),
> +                       mvsd_read(MVSD_ERR_INTR_STATUS),
> +                       mvsd_read(MVSD_ERR_INTR_EN));
> +               return IRQ_HANDLED;
> +       }
> +
>         spin_lock(&host->lock);
>
>         /* PIO handling, if needed. Messy business... */
> --
> 1.9.1
>

[PATCH RESEND 3/3] irqchip: orion: reverse irq handling priority

[Sebastian Hesselbarth]

Non-DT irq handlers were working through irq causes from most-significant
to least-significant bit, while DT irqchip driver does it the other way
round. This revealed some more HW issues on Kirkwood peripheral IP, where
spurious sdio irqs can happen although IP's irq enable registers are all
zero. Although, not directly related with the described issue, reverse
irq bit handling back to original order by replacing ffs() with fls().

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Acked-by: Jason Cooper <jason@lakedaemon.net>
---
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
---
 drivers/irqchip/irq-orion.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-orion.c b/drivers/irqchip/irq-orion.c
index e25f246cd2fb..34d18b48bb78 100644
--- a/drivers/irqchip/irq-orion.c
+++ b/drivers/irqchip/irq-orion.c
@@ -42,7 +42,7 @@ __exception_irq_entry orion_handle_irq(struct pt_regs *regs)
 		u32 stat = readl_relaxed(gc->reg_base + ORION_IRQ_CAUSE) &
 			gc->mask_cache;
 		while (stat) {
-			u32 hwirq = ffs(stat) - 1;
+			u32 hwirq = __fls(stat);
 			u32 irq = irq_find_mapping(orion_irq_domain,
 						   gc->irq_base + hwirq);
 			handle_IRQ(irq, regs);
@@ -117,7 +117,7 @@ static void orion_bridge_irq_handler(unsigned int irq, struct irq_desc *desc)
 		   gc->mask_cache;
 
 	while (stat) {
-		u32 hwirq = ffs(stat) - 1;
+		u32 hwirq = __fls(stat);
 
 		generic_handle_irq(irq_find_mapping(d, gc->irq_base + hwirq));
 		stat &= ~(1 << hwirq);
-- 
1.9.1

[PATCH RESEND 3/3] irqchip: orion: reverse irq handling priority

[Thomas Gleixner]

On Sat, 26 Apr 2014, Sebastian Hesselbarth wrote:

> Non-DT irq handlers were working through irq causes from most-significant
> to least-significant bit, while DT irqchip driver does it the other way
> round. This revealed some more HW issues on Kirkwood peripheral IP, where
> spurious sdio irqs can happen although IP's irq enable registers are all
> zero. Although, not directly related with the described issue, reverse
> irq bit handling back to original order by replacing ffs() with fls().

So why are we reverting to the original order?

The explanation above is just confusing.

Thanks,

	tglx
 
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Acked-by: Jason Cooper <jason@lakedaemon.net>
> ---
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-kernel at vger.kernel.org
> ---
>  drivers/irqchip/irq-orion.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-orion.c b/drivers/irqchip/irq-orion.c
> index e25f246cd2fb..34d18b48bb78 100644
> --- a/drivers/irqchip/irq-orion.c
> +++ b/drivers/irqchip/irq-orion.c
> @@ -42,7 +42,7 @@ __exception_irq_entry orion_handle_irq(struct pt_regs *regs)
>  		u32 stat = readl_relaxed(gc->reg_base + ORION_IRQ_CAUSE) &
>  			gc->mask_cache;
>  		while (stat) {
> -			u32 hwirq = ffs(stat) - 1;
> +			u32 hwirq = __fls(stat);
>  			u32 irq = irq_find_mapping(orion_irq_domain,
>  						   gc->irq_base + hwirq);
>  			handle_IRQ(irq, regs);
> @@ -117,7 +117,7 @@ static void orion_bridge_irq_handler(unsigned int irq, struct irq_desc *desc)
>  		   gc->mask_cache;
>  
>  	while (stat) {
> -		u32 hwirq = ffs(stat) - 1;
> +		u32 hwirq = __fls(stat);
>  
>  		generic_handle_irq(irq_find_mapping(d, gc->irq_base + hwirq));
>  		stat &= ~(1 << hwirq);
> -- 
> 1.9.1
> 
> 

[PATCH RESEND 3/3] irqchip: orion: reverse irq handling priority

[Sebastian Hesselbarth]

On 04/28/2014 09:39 PM, Thomas Gleixner wrote:
> On Sat, 26 Apr 2014, Sebastian Hesselbarth wrote:
> 
>> Non-DT irq handlers were working through irq causes from most-significant
>> to least-significant bit, while DT irqchip driver does it the other way
>> round. This revealed some more HW issues on Kirkwood peripheral IP, where
>> spurious sdio irqs can happen although IP's irq enable registers are all
>> zero. Although, not directly related with the described issue, reverse
>> irq bit handling back to original order by replacing ffs() with fls().
> 
> So why are we reverting to the original order?
> 
> The explanation above is just confusing.

Actually, I first wanted to reply "The original order worked for
years, so get back to it." But then I thought about finding a better
answer and remembered some comment of Russell a while ago.

I disassembled the generated binary and the original order saves two
instructions for each bit count using clz.

With this patch:
  60:   e3a07001        mov     r7, #1
  64:   e16f3f14        clz     r3, r4
  68:   e263301f        rsb     r3, r3, #31
  6c:   e1c44317        bic     r4, r4, r7, lsl r3
  70:   e5951004        ldr     r1, [r5, #4]

Without this patch:
  60:   e3a06001        mov     r6, #1
  64:   e2643000        rsb     r3, r4, #0
  68:   e0033004        and     r3, r3, r4
  6c:   e16f3f13        clz     r3, r3
  70:   e263301f        rsb     r3, r3, #31
  74:   e1c44316        bic     r4, r4, r6, lsl r3
  78:   e5971004        ldr     r1, [r7, #4]

You want me to reword the commit message accordingly?

Sebastian

>> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
>> Acked-by: Jason Cooper <jason@lakedaemon.net>
>> ---
>> Cc: Jason Cooper <jason@lakedaemon.net>
>> Cc: Andrew Lunn <andrew@lunn.ch>
>> Cc: Gregory Clement <gregory.clement@free-electrons.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: linux-arm-kernel at lists.infradead.org
>> Cc: linux-kernel at vger.kernel.org
>> ---
>>  drivers/irqchip/irq-orion.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/irqchip/irq-orion.c b/drivers/irqchip/irq-orion.c
>> index e25f246cd2fb..34d18b48bb78 100644
>> --- a/drivers/irqchip/irq-orion.c
>> +++ b/drivers/irqchip/irq-orion.c
>> @@ -42,7 +42,7 @@ __exception_irq_entry orion_handle_irq(struct pt_regs *regs)
>>  		u32 stat = readl_relaxed(gc->reg_base + ORION_IRQ_CAUSE) &
>>  			gc->mask_cache;
>>  		while (stat) {
>> -			u32 hwirq = ffs(stat) - 1;
>> +			u32 hwirq = __fls(stat);
>>  			u32 irq = irq_find_mapping(orion_irq_domain,
>>  						   gc->irq_base + hwirq);
>>  			handle_IRQ(irq, regs);
>> @@ -117,7 +117,7 @@ static void orion_bridge_irq_handler(unsigned int irq, struct irq_desc *desc)
>>  		   gc->mask_cache;
>>  
>>  	while (stat) {
>> -		u32 hwirq = ffs(stat) - 1;
>> +		u32 hwirq = __fls(stat);
>>  
>>  		generic_handle_irq(irq_find_mapping(d, gc->irq_base + hwirq));
>>  		stat &= ~(1 << hwirq);
>> -- 
>> 1.9.1
>>
>>

[PATCH RESEND 3/3] irqchip: orion: reverse irq handling priority

[Jason Cooper]

On Mon, Apr 28, 2014 at 10:06:25PM +0200, Sebastian Hesselbarth wrote:
> On 04/28/2014 09:39 PM, Thomas Gleixner wrote:
> > On Sat, 26 Apr 2014, Sebastian Hesselbarth wrote:
> > 
> >> Non-DT irq handlers were working through irq causes from most-significant
> >> to least-significant bit, while DT irqchip driver does it the other way
> >> round. This revealed some more HW issues on Kirkwood peripheral IP, where
> >> spurious sdio irqs can happen although IP's irq enable registers are all
> >> zero. Although, not directly related with the described issue, reverse
> >> irq bit handling back to original order by replacing ffs() with fls().
> > 
> > So why are we reverting to the original order?
> > 
> > The explanation above is just confusing.
> 
> Actually, I first wanted to reply "The original order worked for
> years, so get back to it." But then I thought about finding a better
> answer and remembered some comment of Russell a while ago.
> 


> I disassembled the generated binary and the original order saves two
> instructions for each bit count using clz.
> 
> With this patch:
>   60:   e3a07001        mov     r7, #1
>   64:   e16f3f14        clz     r3, r4
>   68:   e263301f        rsb     r3, r3, #31
>   6c:   e1c44317        bic     r4, r4, r7, lsl r3
>   70:   e5951004        ldr     r1, [r5, #4]
> 
> Without this patch:
>   60:   e3a06001        mov     r6, #1
>   64:   e2643000        rsb     r3, r4, #0
>   68:   e0033004        and     r3, r3, r4
>   6c:   e16f3f13        clz     r3, r3
>   70:   e263301f        rsb     r3, r3, #31
>   74:   e1c44316        bic     r4, r4, r6, lsl r3
>   78:   e5971004        ldr     r1, [r7, #4]


> You want me to reword the commit message accordingly?

Please do.  I would even quote the above.

thx,

Jason.

[PATCH v2 3/3] irqchip: orion: reverse irq handling priority

[Sebastian Hesselbarth]

Non-DT irq handlers were working through irq causes from most-significant
to least-significant bit, while DT irqchip driver does it the other way
round. This revealed some more HW issues on Kirkwood peripheral IP, where
spurious sdio irqs can happen although irqs are masked.

Also, the generated binaries show that original non-DT order compared
to DT order save two instructions for each bit count check:

irqchip DT order with ffs():
  60:   e3a06001        mov     r6, #1
  64:   e2643000        rsb     r3, r4, #0
  68:   e0033004        and     r3, r3, r4
  6c:   e16f3f13        clz     r3, r3
  70:   e263301f        rsb     r3, r3, #31
  74:   e1c44316        bic     r4, r4, r6, lsl r3
  78:   e5971004        ldr     r1, [r7, #4]

Original non-DT order with fls():
  60:   e3a07001        mov     r7, #1
  64:   e16f3f14        clz     r3, r4
  68:   e263301f        rsb     r3, r3, #31
  6c:   e1c44317        bic     r4, r4, r7, lsl r3
  70:   e5951004        ldr     r1, [r5, #4]

Therefore, reverse irq bit handling back to original order by replacing
ffs() with fls().

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Acked-by: Jason Cooper <jason@lakedaemon.net>
---
Changelog:
v1->v2:
- reword commit msg to state less number of instructions

Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Gregory Clement <gregory.clement@free-electrons.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
---
 drivers/irqchip/irq-orion.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-orion.c b/drivers/irqchip/irq-orion.c
index e25f246cd2fb..34d18b48bb78 100644
--- a/drivers/irqchip/irq-orion.c
+++ b/drivers/irqchip/irq-orion.c
@@ -42,7 +42,7 @@ __exception_irq_entry orion_handle_irq(struct pt_regs *regs)
 		u32 stat = readl_relaxed(gc->reg_base + ORION_IRQ_CAUSE) &
 			gc->mask_cache;
 		while (stat) {
-			u32 hwirq = ffs(stat) - 1;
+			u32 hwirq = __fls(stat);
 			u32 irq = irq_find_mapping(orion_irq_domain,
 						   gc->irq_base + hwirq);
 			handle_IRQ(irq, regs);
@@ -117,7 +117,7 @@ static void orion_bridge_irq_handler(unsigned int irq, struct irq_desc *desc)
 		   gc->mask_cache;
 
 	while (stat) {
-		u32 hwirq = ffs(stat) - 1;
+		u32 hwirq = __fls(stat);
 
 		generic_handle_irq(irq_find_mapping(d, gc->irq_base + hwirq));
 		stat &= ~(1 << hwirq);
-- 
1.9.1

[PATCH v2 3/3] irqchip: orion: reverse irq handling priority

[Jason Cooper]

On Mon, Apr 28, 2014 at 11:12:08PM +0200, Sebastian Hesselbarth wrote:
> Non-DT irq handlers were working through irq causes from most-significant
> to least-significant bit, while DT irqchip driver does it the other way
> round. This revealed some more HW issues on Kirkwood peripheral IP, where
> spurious sdio irqs can happen although irqs are masked.
> 
> Also, the generated binaries show that original non-DT order compared
> to DT order save two instructions for each bit count check:
> 
> irqchip DT order with ffs():
>   60:   e3a06001        mov     r6, #1
>   64:   e2643000        rsb     r3, r4, #0
>   68:   e0033004        and     r3, r3, r4
>   6c:   e16f3f13        clz     r3, r3
>   70:   e263301f        rsb     r3, r3, #31
>   74:   e1c44316        bic     r4, r4, r6, lsl r3
>   78:   e5971004        ldr     r1, [r7, #4]
> 
> Original non-DT order with fls():
>   60:   e3a07001        mov     r7, #1
>   64:   e16f3f14        clz     r3, r4
>   68:   e263301f        rsb     r3, r3, #31
>   6c:   e1c44317        bic     r4, r4, r7, lsl r3
>   70:   e5951004        ldr     r1, [r5, #4]
> 
> Therefore, reverse irq bit handling back to original order by replacing
> ffs() with fls().
> 
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Acked-by: Jason Cooper <jason@lakedaemon.net>
> ---
> Changelog:
> v1->v2:
> - reword commit msg to state less number of instructions
> 
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Gregory Clement <gregory.clement@free-electrons.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-kernel at vger.kernel.org
> ---
>  drivers/irqchip/irq-orion.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied to mvebu/irqchip for routing through the tip tree.  Tweaked
capitalization of subject line to match the other commits in that
directory.

thx,

Jason.