Re: [PATCH] Revert "of/irq: Mark initialised interrupt controllers as populated"

[Philipp Zabel <p.zabel@pengutronix.de>]

[Added Shawn to Cc]

Am Montag, den 08.08.2016, 12:02 -0500 schrieb Rob Herring:
> On Mon, Aug 8, 2016 at 11:47 AM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > This reverts commit 15cc2ed6dcf91a8658e084be4e140147161819d7 ("of/irq:
> > Mark initialised interrupt controllers as populated"), which breaks the
> > i.MX6 GPC (General Power Controller). The GPC is both an interrupt
> > controller and a power domain controller. If the interrupt code sets the
> > OF_POPULATED bit early, the power domain driver is silently skipped, as
> > are the GPU and VPU drivers that are missing their power domain.
> 
> What makes the GPC need to be handled early as an interrupt
> controller? Looks like the timer is not dependent on it, so maybe it
> can be converted to a regular driver?

I'm not sure if this should be possible. The GPT timers are all parented
to the GPC (their parent soc DT node has the interrupt-parent property)
and they are initialised with CLOCKSOURCE_OF_DECLARE.

I get a bunch of "no irq domain found" warnings right before the gpc
driver is probed if I drop the IRQCHIP_DECLARE and move GPC irq
initialisation into a core_initcall platform device probe. This the
backtrace with a WARN_ONCE inserted just before the pr_warn in
irq_create_fwspec_mapping:

[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at /ptx/work/dude/WORK_A/pza/backup/linux/kernel/irq/irqdomain.c:584 irq_create_fwspec_mapping+0x330/0x344
[    0.000000] no irq domain found
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.8.0-rc1-20160808-1+ #301
[    0.000000] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[    0.000000] Backtrace: 
[    0.000000] [<8010d824>] (dump_backtrace) from [<8010da70>] (show_stack+0x20/0x24)
[    0.000000]  r7:80d480a0 r6:80d480a0 r5:600000d3 r4:00000000
[    0.000000] [<8010da50>] (show_stack) from [<8045b4e8>] (dump_stack+0xb0/0xdc)
[    0.000000] [<8045b438>] (dump_stack) from [<80122bfc>] (__warn+0xf8/0x110)
[    0.000000]  r9:80191a54 r8:00000248 r7:00000009 r6:80aa9c50 r5:00000000 r4:80d01e30
[    0.000000] [<80122b04>] (__warn) from [<80122c5c>] (warn_slowpath_fmt+0x48/0x50)
[    0.000000]  r9:80c52a48 r8:00000002 r7:00000000 r6:815724e8 r5:80d01e90 r4:80aa9e54
[    0.000000] [<80122c18>] (warn_slowpath_fmt) from [<80191a54>] (irq_create_fwspec_mapping+0x330/0x344)
[    0.000000]  r3:80d8ac1d r2:80aa9e54
[    0.000000]  r4:00000000
[    0.000000] [<80191724>] (irq_create_fwspec_mapping) from [<80191acc>] (irq_create_of_mapping+0x64/0x6c)
[    0.000000]  r7:00000000 r6:815724e8 r5:af75317c r4:af002c00
[    0.000000] [<80191a68>] (irq_create_of_mapping) from [<806b5130>] (irq_of_parse_and_map+0x34/0x3c)
[    0.000000] [<806b50fc>] (irq_of_parse_and_map) from [<80c36634>] (mxc_timer_init_dt+0x74/0x100)
[    0.000000] [<80c365c0>] (mxc_timer_init_dt) from [<80c36738>] (imx31_timer_init_dt+0x30/0x34)
[    0.000000]  r9:80c52a48 r8:80d90000 r7:ffffffff r6:80d024c0 r5:00000001 r4:af75317c
[    0.000000] [<80c36708>] (imx31_timer_init_dt) from [<80c36224>] (clocksource_probe+0x50/0xac)
[    0.000000]  r5:00000001 r4:af75317c
[    0.000000] [<80c361d4>] (clocksource_probe) from [<80c04a0c>] (time_init+0x34/0x38)
[    0.000000]  r5:80d90000 r4:00000001
[    0.000000] [<80c049d8>] (time_init) from [<80c00bf4>] (start_kernel+0x260/0x3a8)
[    0.000000] [<80c00994>] (start_kernel) from [<1000807c>] (0x1000807c)
[    0.000000]  r10:00000000 r9:412fc09a r8:1000406a r7:80d07aac r6:80c52a44 r5:80d02540
[    0.000000]  r4:80d90294
[    0.000000] ---[ end trace 0000000000000000 ]---
[    0.000000] irq: no irq domain found for /soc/aips-bus@02000000/gpc@020dc000 !

To me it looks like we have to keep using IRQCHIP_DECLARE to set up the
GPC irq domain early.

> A simple alternative here would be to set the OF_POPULATED before the
> callback function is called, so your GPC callback can clear it.

I'll send a new patch to try that.

regards
Philipp