Re: [GIT pull] irq updates for 4.13

[Thomas Gleixner <tglx@linutronix.de>]

On Tue, 11 Jul 2017, Thomas Gleixner wrote:
> On Mon, 10 Jul 2017, Linus Torvalds wrote:
> > On Mon, Jul 10, 2017 at 6:35 AM, Sebastian Reichel
> > <sebastian.reichel@collabora.co.uk> wrote:
> > >
> > > This patch apparently breaks OMAP platform:
> > >
> > > 46e48e257360f0845fe17089713cbad4db611e70 is the first bad commit
> > > commit 46e48e257360f0845fe17089713cbad4db611e70
> > > Author: Thomas Gleixner <tglx@linutronix.de>
> > > Date:   Thu Jun 29 23:33:38 2017 +0200
> > >
> > >     genirq: Move irq resource handling out of spinlocked region
> > >
> > > Boot failure log from Droid 4:
> > > [ ... snip snip ..]
> > >
> > > Droid 4 boots current master again after applying the patch below
> > > (which is git revet of above patch, but I provide the patch, since
> > > it did not revet cleanly).
> > 
> > Hmm. Do you actually need the full revert?
> > 
> > I think it's only the __setup_irq() part that looks like it may be garbage.
> > 
> > For example, I think it releases the resources twice if the
> > __irq_set_trigger() call fails.
> 
> Yes, I missed that. Sorry.
> 
> > But it looks questionably in other ways too - notably, the change to
> > make the request call be in the same context as the freeing is done is
> > apparently done entirely for symmetry reasons, not for any actual
> > *reason* reasons.
> 
> There is a reasons reason. The whole purpose was to move out the
> request/free resources call from the spinlocked and irq disabled reason.
> I noticed the free ordering issue, when I was working on that.
> 
> The fact that the patch breaks the OMAP boot, points to something else.
> 
> The only user of the irq_request_resources() callback at the moment is the
> GPIO subsystem and some pinctrl drivers, which are not involved in the OMAP
> case. In case of OMAP it uses the gpiolib generic implementation which
> does:
> 
>       try_module_get(chip->gpiodev->owner);
>       gpiochip_lock_as_irq(chip, d->hwirq);

So Tony actually provided the part of dmesg which shows the initial
failure, which subsequently leads to the splat Sebastian reported.

Unhandled fault: external abort on non-linefetch (0x1028) at 0xfb050034
pgd = c0004000 [fb050034] *pgd=49011452(bad)
Internal error: : 1028 [#1] SMP ARM
Workqueue: events deferred_probe_work_func
task: ce1d41c0 task.stack: ce1fc000
PC is at omap_gpio_get_direction+0x2c/0x44
LR is at _raw_spin_lock_irqsave+0x40/0x4c
pc : [<c0509258>]    lr : [<c08263c4>]    psr: 60000093
sp : ce1fdb78  ip : c0dce42c  fp : ce22d810
r10: ce22d800  r9 : 00000000  r8 : ce22d900
r7 : 00000016  r6 : ce223864  r5 : fb050034  r4 : 00000020
r3 : ce1d41c0  r2 : 00000000  r1 : a0000013  r0 : a0000013
Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 10c5387d  Table: 80004019  DAC: 00000051
Process kworker/0:1 (pid: 14, stack limit = 0xce1fc218)

The callstack is:

omap_gpio_get_direction
gpiochip_lock_as_irq
gpiochip_irq_reqres
__setup_irq
request_threaded_irq
smc_probe
smc_drv_probe
platform_drv_probe
....

So the SMC91X network driver request an IRQ, which ends up calling into the
GPIO interrupt setup and that fails. I have no idea why that would not fail
with the patch reverted. Dusting off a Beaglebone board....

Thanks,

	tglx