[05/10] ARM: s3c64xx: enable sparse IRQ support

Commit Message

Arnd Bergmann March 8, 2015, 9:42 p.m. UTC

On Friday 06 March 2015 17:43:16 Charles Keepax wrote:
> On Mon, Mar 02, 2015 at 01:35:58PM +0100, Arnd Bergmann wrote:
> > This is another prerequisite for enabling multiplatform
> > support, and it is the part I am least certain about.
> > 
> > I assume it will cause the extra boot message "Cannot
> > allocate irq_descs @ IRQ%d, assuming pre-allocated" to
> > be printed, but otherwise work ok. This definitely needs
> > to be tested on real hardware to see if it works.
> > 
> > Signed-off-by: Arnd Bergmann <[e-mail]>
> > ---
> 
> This one appears to cause some problems with the IRQs on
> Cragganmore, I need to look into it a bit more but it looks like
> one of the PMICs can't allocate its IRQs:
> 
> [    0.961455] wm831x 1-0034: WM8311 revision C
> [    0.965066] wm831x 1-0034: Failed to allocate IRQs: -17
> 
> And the CODEC can't request its IRQ:
> 
> [    4.252735] arizona spi0.1: WM5102 revision C
> [    4.269763] arizona spi0.1: Failed to request primary IRQ 263: -22
> 
> Hopefully I can look into this a little more next week.
> 

My interpretation is that I mistakenly set the .nr_irqs value for
craggamore to include all irqs that the board has, while the
wm831x tries to allocate the irq descriptors itself and fails if
they are already allocated. If that is the only problem here, it
would get fixed by this change:


The samsung-gpio driver does not allocate irq descriptors for itself
though, otherwise we could make the S3C64XX_NR_IRQS number smaller.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Mark Brown March 18, 2015, 12:02 a.m. UTC | #1

On Sun, Mar 08, 2015 at 10:42:59PM +0100, Arnd Bergmann wrote:
> On Friday 06 March 2015 17:43:16 Charles Keepax wrote:

> > This one appears to cause some problems with the IRQs on
> > Cragganmore, I need to look into it a bit more but it looks like
> > one of the PMICs can't allocate its IRQs:

> > [    0.961455] wm831x 1-0034: WM8311 revision C
> > [    0.965066] wm831x 1-0034: Failed to allocate IRQs: -17

> > And the CODEC can't request its IRQ:

> > [    4.252735] arizona spi0.1: WM5102 revision C
> > [    4.269763] arizona spi0.1: Failed to request primary IRQ 263: -22

> > Hopefully I can look into this a little more next week.

> My interpretation is that I mistakenly set the .nr_irqs value for
> craggamore to include all irqs that the board has, while the
> wm831x tries to allocate the irq descriptors itself and fails if
> they are already allocated. If that is the only problem here, it
> would get fixed by this change:

> diff --git a/arch/arm/mach-s3c64xx/mach-crag6410.c b/arch/arm/mach-s3c64xx/mach-crag6410.c
> index f395a5617142..cbe353a5450e 100644
> --- a/arch/arm/mach-s3c64xx/mach-crag6410.c
> +++ b/arch/arm/mach-s3c64xx/mach-crag6410.c
> @@ -855,7 +855,7 @@ static void __init crag6410_machine_init(void)
>  MACHINE_START(WLF_CRAGG_6410, "Wolfson Cragganmore 6410")
>  	/* Maintainer: Mark Brown <[e-mail]> */
>  	.atag_offset	= 0x100,
> -	.nr_irqs	= S3C64XX_NR_IRQS + 160,
> +	.nr_irqs	= S3C64XX_NR_IRQS,
>  	.init_irq	= s3c6410_init_irq,
>  	.map_io		= crag6410_map_io,
>  	.init_machine	= crag6410_machine_init,

> The samsung-gpio driver does not allocate irq descriptors for itself
> though, otherwise we could make the S3C64XX_NR_IRQS number smaller.

That's not the only thing, I'm still seeing an issue even with the
change above.  The VICs are also complaining about preallocated
descriptors, but they just assume the descriptors were preallocated and
carry on happily.  I'd also expect this to be affecting both wm831xs but
it's only affecting the WM8311 on the base board, not the primary
WM8312, so this seems to be a red herring.

I rather suspect that the issue is at least partly that the interrupt
numbering is off - the CODEC has 24 interrupts allocated to it in
crag6410.h which is nowhere near enough and the PMICs have 32 each
rather than the 58 they need.  This is broken for the existing code too
so we should get a fix to make that right in before anything else.  A
trivial change to fix that doesn't seem to have helped though but I
probably just miscounted or something.

Arnd Bergmann March 22, 2015, 2:54 a.m. UTC | #2

On Wednesday 18 March 2015, Mark Brown wrote:
> On Sun, Mar 08, 2015 at 10:42:59PM +0100, Arnd Bergmann wrote:

> > diff --git a/arch/arm/mach-s3c64xx/mach-crag6410.c b/arch/arm/mach-s3c64xx/mach-crag6410.c
> > index f395a5617142..cbe353a5450e 100644
> > --- a/arch/arm/mach-s3c64xx/mach-crag6410.c
> > +++ b/arch/arm/mach-s3c64xx/mach-crag6410.c
> > @@ -855,7 +855,7 @@ static void __init crag6410_machine_init(void)
> >  MACHINE_START(WLF_CRAGG_6410, "Wolfson Cragganmore 6410")
> >       /* Maintainer: Mark Brown <[e-mail]> */
> >       .atag_offset    = 0x100,
> > -     .nr_irqs        = S3C64XX_NR_IRQS + 160,
> > +     .nr_irqs        = S3C64XX_NR_IRQS,
> >       .init_irq       = s3c6410_init_irq,
> >       .map_io         = crag6410_map_io,
> >       .init_machine   = crag6410_machine_init,
> 
> > The samsung-gpio driver does not allocate irq descriptors for itself
> > though, otherwise we could make the S3C64XX_NR_IRQS number smaller.
> 
> That's not the only thing, I'm still seeing an issue even with the
> change above.  The VICs are also complaining about preallocated
> descriptors, but they just assume the descriptors were preallocated and
> carry on happily.

Yes, that is the behavior I expected to see. The VIC driver would be
fine in theory with allocating the descriptors dynamically, but
the secondary irqchip (s3c_irq_eint) would need significant changes.

IIRC, another problem is the way that the first NR_IRQS_LEGACY interrupts
are always preallocated, but would not need to, once we fix the eint.
	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/arm/mach-s3c64xx/mach-crag6410.c b/arch/arm/mach-s3c64xx/mach-crag6410.c
index f395a5617142..cbe353a5450e 100644
--- a/arch/arm/mach-s3c64xx/mach-crag6410.c
+++ b/arch/arm/mach-s3c64xx/mach-crag6410.c
@@ -855,7 +855,7 @@  static void __init crag6410_machine_init(void)
 MACHINE_START(WLF_CRAGG_6410, "Wolfson Cragganmore 6410")
 	/* Maintainer: Mark Brown <[e-mail]> */
 	.atag_offset	= 0x100,
-	.nr_irqs	= S3C64XX_NR_IRQS + 160,
+	.nr_irqs	= S3C64XX_NR_IRQS,
 	.init_irq	= s3c6410_init_irq,
 	.map_io		= crag6410_map_io,
 	.init_machine	= crag6410_machine_init,