Re: [PATCH 1/2] powerpc: ipic: use set_irq_chip to ensure irq_chip defaults are applied

[Thomas Gleixner <tglx@linutronix.de>]

On Mon, 7 Jun 2010, Esben Haabendal wrote:

> On Mon, 2010-06-07 at 01:45 +0200, Thomas Gleixner wrote:
> 
> > This patch has never been tested with spinlock debugging enabled and
> > will break SMP as it causes a deadlock on irq_desc->lock.
> > 
> > Again: See Documentation/Submit*
> 
> Ok, will do so.
> 
> But do you see the problem?  In __setup_irq(), irq_chip_set_defaults()
> is called first, and then __irq_set_trigger(), which calls
> chip->set_type(). When you request an edge irq with ipic, you get the
> defaults applied to ipic_level_irq_chip, and then it sets
> ipic_edge_irq_chip, which now is missing startup(), enable(), and so
> on.
> 
> Would it be better to change the call order in __setup_irq(), and
> call irq_chip_set_defaults after __irq_set_trigger() ?   Or perhaps
> even calling it twice (again after __irq_set_trigger()) ?

Grmpf, set_type() was never meant to change the chip. It's fatal do
so, as the code in setup_irq already has a reference to desc->chip and
calls the wrong functions anyway aside of having not run through the
set defaults code.

That needs more thought, as it requires to reload the reference.

Ben, any thoughts ?

Thanks,

	tglx