diff mbox

irqdomain breaks ap4 boot

Message ID 20120810123804.GK1614@linux-sh.org (mailing list archive)
State Superseded
Headers show

Commit Message

Paul Mundt Aug. 10, 2012, 12:38 p.m. UTC 
On Thu, Aug 09, 2012 at 11:10:43PM -0700, Kuninori Morimoto wrote:
> 
> Hi Paul
> 
> > >     sh: intc: Handle domain association for sparseirq pre-allocated vectors.
> > >     
> > >     Presently it's assumed that the irqdomain code handles the irq_desc
> > >     allocation for us, but this isn't necessarily the case when we've
> > >     pre-allocated IRQs via sparseirq. Previously we had a -EEXIST check in
> > >     the code that attempted to trap these cases and simply update them
> > >     in-place, but this behaviour was inadvertently lost in the transition to
> > >     irqdomains.
> > >     
> > >     This simply restores the previous behaviour, first attempting to let the
> > >     irqdomain core fetch the allocation for us, and falling back to an
> > >     in-place domain association in the extant IRQ case. Fixes up regressions
> > >     on platforms that pre-allocate legacy IRQs (specifically ARM-based
> > >     SH-Mobile platforms, as SH stopped pre-allocating vectors some time ago).
> > >     
> > >     Reported-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > >     Signed-off-by: Paul Mundt <lethal@linux-sh.org>
> > 
> > This patch solved mackerel board crush bug on linus/master
> 
> ecovec/armadillo/marzen board were OK on this patch,
> but kzm9g board still has problem.
> I'm using "linus/master + paul/sh-latest"
> 
> ================= kernel log ============================-
> ....
> Preemptible hierarchical RCU implementation.                                    
>         RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=1.                    
> NR_IRQS:16 nr_irqs:16 16                                                        
> intc: Registered controller 'sh73a0-intcs' with 77 IRQs                         
> intc: Registered controller 'sh73a0-intca-irq-pins' with 32 IRQs                
> ------------[ cut here ]------------                                            
> WARNING: at /opt/usr/src/WORK/morimoto/gitlinux/linux-2.6/kernel/irq/irqdomain.)
> error: irq_desc already associated; irq=552 hwirq=0x228                         

I screwed up the multi-evt case, it should be trying to associate irq2,
not irq. Try this:

---


--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Tetsuyuki Kobayashi Aug. 17, 2012, 5:54 a.m. UTC | #1 
Hello, Paul

I tried kzm9g board on v3.6-rc2 and got the same error as Morimoto's.

(2012/08/10 21:38), Paul Mundt wrote:
> On Thu, Aug 09, 2012 at 11:10:43PM -0700, Kuninori Morimoto wrote:

>> ecovec/armadillo/marzen board were OK on this patch,
>> but kzm9g board still has problem.
>> I'm using "linus/master + paul/sh-latest"
>>
>> ================= kernel log ============================-
>> ....
>> Preemptible hierarchical RCU implementation.
>>          RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=1.
>> NR_IRQS:16 nr_irqs:16 16
>> intc: Registered controller 'sh73a0-intcs' with 77 IRQs
>> intc: Registered controller 'sh73a0-intca-irq-pins' with 32 IRQs
>> ------------[ cut here ]------------
>> WARNING: at /opt/usr/src/WORK/morimoto/gitlinux/linux-2.6/kernel/irq/irqdomain.)
>> error: irq_desc already associated; irq=552 hwirq=0x228
>
> I screwed up the multi-evt case, it should be trying to associate irq2,
> not irq. Try this:
>
> ---
>
> diff --git a/drivers/sh/intc/core.c b/drivers/sh/intc/core.c
> index 32c26d7..8f32a13 100644
> --- a/drivers/sh/intc/core.c
> +++ b/drivers/sh/intc/core.c
> @@ -355,7 +355,7 @@ int __init register_intc_controller(struct intc_desc *desc)
>   			if (unlikely(res)) {
>   				if (res == -EEXIST) {
>   					res = irq_domain_associate(d->domain,
> -								   irq, irq);
> +								   irq2, irq2);
>   					if (unlikely(res)) {
>   						pr_err("domain association "
>   						       "failure\n");

I tried this patch but there is still the same error.
I checked quickly by debugger, and I found the failing 
irq_domain_associate() is not called here (line 357) but the one at line 
328.





--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kuninori Morimoto Aug. 20, 2012, 1:14 a.m. UTC | #2 
Hi Paul

Thank you for your patch

> > diff --git a/drivers/sh/intc/core.c b/drivers/sh/intc/core.c
> > index 32c26d7..8f32a13 100644
> > --- a/drivers/sh/intc/core.c
> > +++ b/drivers/sh/intc/core.c
> > @@ -355,7 +355,7 @@ int __init register_intc_controller(struct intc_desc *desc)
> >   			if (unlikely(res)) {
> >   				if (res == -EEXIST) {
> >   					res = irq_domain_associate(d->domain,
> > -								   irq, irq);
> > +								   irq2, irq2);
> >   					if (unlikely(res)) {
> >   						pr_err("domain association "
> >   						       "failure\n");
> 
> I tried this patch but there is still the same error.
> I checked quickly by debugger, and I found the failing 
> irq_domain_associate() is not called here (line 357) but the one at line 
> 328.

But, I got same result.

This WARNING is indicating
> intc: domain association failure    
      
So, as kobayashi-san said, 
it has happend on previous irq_domain_associate(), not here.

Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tetsuyuki Kobayashi Aug. 31, 2012, 6:55 a.m. UTC | #3 
Simon-san,

This patch may be just a quick fix but could you apply this patch to 
kzm9g branch for convience ?


(2012/08/10 21:38), Paul Mundt wrote:

> I screwed up the multi-evt case, it should be trying to associate irq2,
> not irq. Try this:
>
> ---
>
> diff --git a/drivers/sh/intc/core.c b/drivers/sh/intc/core.c
> index 32c26d7..8f32a13 100644
> --- a/drivers/sh/intc/core.c
> +++ b/drivers/sh/intc/core.c
> @@ -355,7 +355,7 @@ int __init register_intc_controller(struct intc_desc *desc)
>   			if (unlikely(res)) {
>   				if (res == -EEXIST) {
>   					res = irq_domain_associate(d->domain,
> -								   irq, irq);
> +								   irq2, irq2);
>   					if (unlikely(res)) {
>   						pr_err("domain association "
>   						       "failure\n");
>

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Simon Horman Aug. 31, 2012, 7:17 a.m. UTC | #4 
Hi Kobayashi-san,

I'd rather only apply patches that are ready for upstream.
Is this patch suitable for merging upstream?

On Fri, Aug 31, 2012 at 03:55:09PM +0900, Tetsuyuki Kobayashi wrote:
> Simon-san,
> 
> This patch may be just a quick fix but could you apply this patch to
> kzm9g branch for convience ?
> 
> 
> (2012/08/10 21:38), Paul Mundt wrote:
> 
> >I screwed up the multi-evt case, it should be trying to associate irq2,
> >not irq. Try this:
> >
> >---
> >
> >diff --git a/drivers/sh/intc/core.c b/drivers/sh/intc/core.c
> >index 32c26d7..8f32a13 100644
> >--- a/drivers/sh/intc/core.c
> >+++ b/drivers/sh/intc/core.c
> >@@ -355,7 +355,7 @@ int __init register_intc_controller(struct intc_desc *desc)
> >  			if (unlikely(res)) {
> >  				if (res == -EEXIST) {
> >  					res = irq_domain_associate(d->domain,
> >-								   irq, irq);
> >+								   irq2, irq2);
> >  					if (unlikely(res)) {
> >  						pr_err("domain association "
> >  						       "failure\n");
> >
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Mundt Aug. 31, 2012, 10:36 a.m. UTC | #5 
On Fri, Aug 31, 2012 at 04:17:16PM +0900, Simon Horman wrote:
> Hi Kobayashi-san,
> 
> I'd rather only apply patches that are ready for upstream.
> Is this patch suitable for merging upstream?
> 
Yes, it's already in my tree and Linus-bound, I just haven't sent it yet
as I was waiting to see if anything else was broken first. The multi-evt
case is pretty uncommon, so there shouldn't be many platforms that even
care about this (I'd be surprised if anything on the ARM side does for
example).
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tetsuyuki Kobayashi Sept. 18, 2012, 2:15 a.m. UTC | #6 
Hi Paul-san,

(2012/08/31 19:36), Paul Mundt wrote:
> On Fri, Aug 31, 2012 at 04:17:16PM +0900, Simon Horman wrote:
>> Hi Kobayashi-san,
>>
>> I'd rather only apply patches that are ready for upstream.
>> Is this patch suitable for merging upstream?
>>
> Yes, it's already in my tree and Linus-bound, I just haven't sent it yet
> as I was waiting to see if anything else was broken first. The multi-evt
> case is pretty uncommon, so there shouldn't be many platforms that even
> care about this (I'd be surprised if anything on the ARM side does for
> example).
>
Could you post this patch ?

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/sh/intc/core.c b/drivers/sh/intc/core.c
index 32c26d7..8f32a13 100644
--- a/drivers/sh/intc/core.c
+++ b/drivers/sh/intc/core.c
@@ -355,7 +355,7 @@  int __init register_intc_controller(struct intc_desc *desc)
 			if (unlikely(res)) {
 				if (res == -EEXIST) {
 					res = irq_domain_associate(d->domain,
-								   irq, irq);
+								   irq2, irq2);
 					if (unlikely(res)) {
 						pr_err("domain association "
 						       "failure\n");