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

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

[Philipp Zabel]

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.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Jon Hunter <jonathanh@nvidia.com>
Cc: Rob Herring <robh@kernel.org>
---
 drivers/of/irq.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 89a71c6..7d3f93f 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -559,8 +559,6 @@ void __init of_irq_init(const struct of_device_id *matches)
 			 * its children can get processed in a subsequent pass.
 			 */
 			list_add_tail(&desc->list, &intc_parent_list);
-
-			of_node_set_flag(desc->dev, OF_POPULATED);
 		}
 
 		/* Get the next pending parent that might have children */
-- 
2.8.1

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

[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?

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

Rob

>
> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Jon Hunter <jonathanh@nvidia.com>
> Cc: Rob Herring <robh@kernel.org>
> ---
>  drivers/of/irq.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 89a71c6..7d3f93f 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -559,8 +559,6 @@ void __init of_irq_init(const struct of_device_id *matches)
>                          * its children can get processed in a subsequent pass.
>                          */
>                         list_add_tail(&desc->list, &intc_parent_list);
> -
> -                       of_node_set_flag(desc->dev, OF_POPULATED);
>                 }
>
>                 /* Get the next pending parent that might have children */
> --
> 2.8.1
>

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

[Philipp Zabel]

[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