[PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Stefan Agner]

If a clock gets enabled early during boot time, it can lead to a PLL
startup. The wait_lock function makes sure that the PLL is really
stareted up before it gets used. However, the function sleeps which
leads to scheduling and an error:
bad: scheduling from the idle thread!
...

Use udelay in case IRQ's are still disabled.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/clk/imx/clk-pllv3.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index c05c43d..b5ff561 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c
@@ -63,7 +63,10 @@ static int clk_pllv3_wait_lock(struct clk_pllv3 *pll)
 			break;
 		if (time_after(jiffies, timeout))
 			break;
-		usleep_range(50, 500);
+		if (unlikely(irqs_disabled()))
+			udelay(50);
+		else
+			usleep_range(50, 500);
 	} while (1);
 
 	return readl_relaxed(pll->base) & BM_PLL_LOCK ? 0 : -ETIMEDOUT;
-- 
2.7.0

[PATCH 2/2] clk: imx: return correct frequency for Ethernet PLL

[Stefan Agner]

The i.MX 7 designs Ethernet PLL provides a 1000MHz reference clock.
Store the reference clock in the clk_pllv3 structure according to
the PLL type.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/clk/imx/clk-pllv3.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
index b5ff561..c344ae6 100644
--- a/drivers/clk/imx/clk-pllv3.c
+++ b/drivers/clk/imx/clk-pllv3.c
@@ -44,6 +44,7 @@ struct clk_pllv3 {
 	u32		powerdown;
 	u32		div_mask;
 	u32		div_shift;
+	unsigned long	ref_clock;
 };
 
 #define to_clk_pllv3(_hw) container_of(_hw, struct clk_pllv3, hw)
@@ -289,7 +290,9 @@ static const struct clk_ops clk_pllv3_av_ops = {
 static unsigned long clk_pllv3_enet_recalc_rate(struct clk_hw *hw,
 						unsigned long parent_rate)
 {
-	return 500000000;
+	struct clk_pllv3 *pll = to_clk_pllv3(hw);
+
+	return pll->ref_clock;
 }
 
 static const struct clk_ops clk_pllv3_enet_ops = {
@@ -329,7 +332,11 @@ struct clk *imx_clk_pllv3(enum imx_pllv3_type type, const char *name,
 		break;
 	case IMX_PLLV3_ENET_IMX7:
 		pll->powerdown = IMX7_ENET_PLL_POWER;
+		pll->ref_clock = 1000000000;
+		ops = &clk_pllv3_enet_ops;
+		break;
 	case IMX_PLLV3_ENET:
+		pll->ref_clock = 500000000;
 		ops = &clk_pllv3_enet_ops;
 		break;
 	default:
-- 
2.7.0

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Joshua Clayton]

On Fri, 29 Jan 2016 14:49:23 -0800
Stefan Agner <stefan@agner.ch> wrote:

> If a clock gets enabled early during boot time, it can lead to a PLL
> startup. The wait_lock function makes sure that the PLL is really
> stareted up before it gets used. However, the function sleeps which
s/stareted/started/

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Stephen Boyd]

On 01/29, Stefan Agner wrote:
> If a clock gets enabled early during boot time, it can lead to a PLL
> startup. The wait_lock function makes sure that the PLL is really
> stareted up before it gets used. However, the function sleeps which
> leads to scheduling and an error:
> bad: scheduling from the idle thread!
> ...

Can you please share the full splat? I have no clue what's going
on.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Stefan Agner]

On 2016-01-29 17:16, Stephen Boyd wrote:
> On 01/29, Stefan Agner wrote:
>> If a clock gets enabled early during boot time, it can lead to a PLL
>> startup. The wait_lock function makes sure that the PLL is really
>> stareted up before it gets used. However, the function sleeps which
>> leads to scheduling and an error:
>> bad: scheduling from the idle thread!
>> ...
> 
> Can you please share the full splat? I have no clue what's going
> on.

Finally, full splat:

...
[    0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=2,
Nodes=1
[    0.000000] Running RCU self tests
[    0.000000] Hierarchical RCU implementation.
[    0.000000]  RCU lockdep checking is enabled.
[    0.000000]  Build-time adjustment of leaf fanout to 32.
[    0.000000]  RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=2.
[    0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=32,
nr_cpu_ids=2
[    0.000000] NR_IRQS:16 nr_irqs:16 16
[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:3407
lock_release+0x398/0x3a0()
[    0.000000] releasing a pinned lock
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
4.5.0-rc1-00013-gdb45d67 #40
[    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.000000] Backtrace:
[    0.000000] [<c0014230>] (dump_backtrace) from [<c001442c>]
(show_stack+0x18/0x1c)
[    0.000000]  r7:c006f8f8 r6:00000d4f r5:00000000 r4:c0ad541c
[    0.000000] [<c0014414>] (show_stack) from [<c02de8fc>]
(dump_stack+0x80/0x90)
[    0.000000] [<c02de87c>] (dump_stack) from [<c00263c0>]
(warn_slowpath_common+0x88/0xb8)
[    0.000000]  r5:00000009 r4:c0abdc58
[    0.000000] [<c0026338>] (warn_slowpath_common) from [<c0026428>]
(warn_slowpath_fmt+0x38/0x40)
[    0.000000]  r8:00000002 r7:00000001 r6:cedc3fd0 r5:c0ac24e0
r4:c0984048
[    0.000000] [<c00263f4>] (warn_slowpath_fmt) from [<c006f8f8>]
(lock_release+0x398/0x3a0)
[    0.000000]  r3:0000fff0 r2:c0984048
[    0.000000]  r4:c0ac2500
[    0.000000] [<c006f560>] (lock_release) from [<c07cc298>]
(_raw_spin_unlock_irq+0x20/0x34)
[    0.000000]  r10:00000000 r9:c0ac2384 r8:cedc3fd0 r7:c0abe6d0
r6:00000001 r5:cedc3fc0
[    0.000000]  r4:cedc3fc0
[    0.000000] [<c07cc278>] (_raw_spin_unlock_irq) from [<c0055298>]
(dequeue_task_idle+0x14/0x30)
[    0.000000]  r5:cedc3fc0 r4:cedc3fc0
[    0.000000] [<c0055284>] (dequeue_task_idle) from [<c004d764>]
(deactivate_task+0x64/0x68)
[    0.000000]  r5:cedc3fc0 r4:c0ac2040
[    0.000000] [<c004d700>] (deactivate_task) from [<c07c6fd0>]
(__schedule+0x29c/0x67c)
[    0.000000]  r7:c0abe6d0 r6:c0abafc0 r5:c0ac2040 r4:cedc3fc0
[    0.000000] [<c07c6d34>] (__schedule) from [<c07c73f8>]
(schedule+0x48/0xa0)
[    0.000000]  r10:c0b0d91c r9:00000036 r8:c1331c3c r7:00000000
r6:0006ddd0 r5:c0abde08
[    0.000000]  r4:c0abc000
[    0.000000] [<c07c73b0>] (schedule) from [<c07cbd38>]
(schedule_hrtimeout_range_clock+0xbc/0x130)
[    0.000000]  r5:c0abde08 r4:00000001
[    0.000000] [<c07cbc7c>] (schedule_hrtimeout_range_clock) from
[<c07cbdc0>] (schedule_hrtimeout_range+0x14/0x18)
[    0.000000]  r7:00000003 r6:ffff8ad1 r5:ce804fc0 r4:c0abe100
[    0.000000] [<c07cbdac>] (schedule_hrtimeout_range) from [<c07cb854>]
(usleep_range+0x64/0x6c)
[    0.000000] [<c07cb7f0>] (usleep_range) from [<c0578468>]
(clk_pllv3_wait_lock+0x80/0xbc)
[    0.000000] [<c05783e8>] (clk_pllv3_wait_lock) from [<c05784d0>]
(clk_pllv3_prepare+0x2c/0x30)
[    0.000000]  r7:00000003 r6:d0864490 r5:c1331c3c r4:ce807680
[    0.000000] [<c05784a4>] (clk_pllv3_prepare) from [<c0571d28>]
(clk_core_prepare+0xa0/0xc4)
[    0.000000] [<c0571c88>] (clk_core_prepare) from [<c0572230>]
(clk_prepare+0x20/0x38)
[    0.000000]  r5:c1331c3c r4:ce80c040
[    0.000000] [<c0572210>] (clk_prepare) from [<c0a8e980>]
(imx7d_clocks_init+0x5ee0/0x5f6c)
[    0.000000]  r5:c1331c3c r4:ce80c040
[    0.000000] [<c0a88aa0>] (imx7d_clocks_init) from [<c0a88618>]
(of_clk_init+0x148/0x1d8)
[    0.000000]  r10:cede50b4 r9:00000003 r8:00000001 r7:c0abdf60
r6:c0abdf68 r5:00000000
[    0.000000]  r4:ce8043c0
[    0.000000] [<c0a884d0>] (of_clk_init) from [<c0a63918>]
(time_init+0x30/0x38)
[    0.000000]  r10:cefffb80 r9:c0aa6a48 r8:c0b24000 r7:ffffffff
r6:c0abe4c0 r5:c0b24000
[    0.000000]  r4:00000000
[    0.000000] [<c0a638e8>] (time_init) from [<c0a5fc38>]
(start_kernel+0x2b4/0x3ec)
[    0.000000] [<c0a5f984>] (start_kernel) from [<8000807c>]
(0x8000807c)
[    0.000000]  r10:00000000 r9:410fc075 r8:8000406a r7:c0ac39b4
r6:c0aa6a44 r5:c0abe540
[    0.000000]  r4:c0b24294
[    0.000000] ---[ end trace cb88537fdc8fa200 ]---
[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:2601
trace_hardirqs_on_caller+0x1e8/0x1fc()
[    0.000000] DEBUG_LOCKS_WARN_ON(unlikely(early_boot_irqs_disabled))
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W      
4.5.0-rc1-00013-gdb45d67 #40
[    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)   
[    0.000000] Backtrace:
[    0.000000] [<c0014230>] (dump_backtrace) from [<c001442c>]
(show_stack+0x18/0x1c)
[    0.000000]  r7:c006cb24 r6:00000a29 r5:00000000 r4:c0ad541c
[    0.000000] [<c0014414>] (show_stack) from [<c02de8fc>]
(dump_stack+0x80/0x90)
[    0.000000] [<c02de87c>] (dump_stack) from [<c00263c0>]
(warn_slowpath_common+0x88/0xb8)
[    0.000000]  r5:00000009 r4:c0abdc88
[    0.000000] [<c0026338>] (warn_slowpath_common) from [<c0026428>]
(warn_slowpath_fmt+0x38/0x40)
[    0.000000]  r8:cedc3fd0 r7:c0abe6d0 r6:00000001 r5:cedc3fc0
r4:c098066c
[    0.000000] [<c00263f4>] (warn_slowpath_fmt) from [<c006cb24>]
(trace_hardirqs_on_caller+0x1e8/0x1fc)
[    0.000000]  r3:c0983e1c r2:c098066c
[    0.000000]  r4:c07cc2a4
[    0.000000] [<c006c93c>] (trace_hardirqs_on_caller) from [<c006cb4c>]
(trace_hardirqs_on+0x14/0x18)
[    0.000000]  r7:c0abe6d0 r6:00000001 r5:cedc3fc0 r4:cedc3fc0
[    0.000000] [<c006cb38>] (trace_hardirqs_on) from [<c07cc2a4>]
(_raw_spin_unlock_irq+0x2c/0x34)
[    0.000000] [<c07cc278>] (_raw_spin_unlock_irq) from [<c0055298>]
(dequeue_task_idle+0x14/0x30)
[    0.000000]  r5:cedc3fc0 r4:cedc3fc0
[    0.000000] [<c0055284>] (dequeue_task_idle) from [<c004d764>]
(deactivate_task+0x64/0x68)
[    0.000000]  r5:cedc3fc0 r4:c0ac2040
[    0.000000] [<c004d700>] (deactivate_task) from [<c07c6fd0>]
(__schedule+0x29c/0x67c)
[    0.000000]  r7:c0abe6d0 r6:c0abafc0 r5:c0ac2040 r4:cedc3fc0
[    0.000000] [<c07c6d34>] (__schedule) from [<c07c73f8>]
(schedule+0x48/0xa0)
[    0.000000]  r10:c0b0d91c r9:00000036 r8:c1331c3c r7:00000000
r6:0006ddd0 r5:c0abde08
[    0.000000]  r4:c0abc000
[    0.000000] [<c07c73b0>] (schedule) from [<c07cbd38>]
(schedule_hrtimeout_range_clock+0xbc/0x130)
[    0.000000]  r5:c0abde08 r4:00000001
[    0.000000] [<c07c73b0>] (schedule) from [<c07cbd38>]
(schedule_hrtimeout_range_clock+0xbc/0x130)
[    0.000000]  r5:c0abde08 r4:00000001
[    0.000000] [<c07cbc7c>] (schedule_hrtimeout_range_clock) from
[<c07cbdc0>] (schedule_hrtimeout_range+0x14/0x18)
[    0.000000]  r7:00000003 r6:ffff8ad1 r5:ce804fc0 r4:c0abe100
[    0.000000] [<c07cbdac>] (schedule_hrtimeout_range) from [<c07cb854>]
(usleep_range+0x64/0x6c)
[    0.000000] [<c07cb7f0>] (usleep_range) from [<c0578468>]
(clk_pllv3_wait_lock+0x80/0xbc)
[    0.000000] [<c05783e8>] (clk_pllv3_wait_lock) from [<c05784d0>]
(clk_pllv3_prepare+0x2c/0x30)
[    0.000000]  r7:00000003 r6:d0864490 r5:c1331c3c r4:ce807680
[    0.000000] [<c05784a4>] (clk_pllv3_prepare) from [<c0571d28>]
(clk_core_prepare+0xa0/0xc4)
[    0.000000] [<c0571c88>] (clk_core_prepare) from [<c0572230>]
(clk_prepare+0x20/0x38)
[    0.000000]  r5:c1331c3c r4:ce80c040
[    0.000000] [<c0572210>] (clk_prepare) from [<c0a8e980>]
(imx7d_clocks_init+0x5ee0/0x5f6c)
[    0.000000]  r5:c1331c3c r4:ce80c040
[    0.000000] [<c0a88aa0>] (imx7d_clocks_init) from [<c0a88618>]
(of_clk_init+0x148/0x1d8)
[    0.000000]  r10:cede50b4 r9:00000003 r8:00000001 r7:c0abdf60
r6:c0abdf68 r5:00000000
[    0.000000]  r4:ce8043c0
[    0.000000] [<c0a884d0>] (of_clk_init) from [<c0a63918>]
(time_init+0x30/0x38)
[    0.000000]  r10:cefffb80 r9:c0aa6a48 r8:c0b24000 r7:ffffffff
r6:c0abe4c0 r5:c0b24000
[    0.000000]  r4:00000000
[    0.000000] [<c0a638e8>] (time_init) from [<c0a5fc38>]
(start_kernel+0x2b4/0x3ec)
[    0.000000] [<c0a5f984>] (start_kernel) from [<8000807c>]
(0x8000807c)
[    0.000000]  r10:00000000 r9:410fc075 r8:8000406a r7:c0ac39b4
r6:c0aa6a44 r5:c0abe540
[    0.000000]  r4:c0b24294
[    0.000000] ---[ end trace cb88537fdc8fa201 ]---
[    0.000000] bad: scheduling from the idle thread!                    
                                                                        
                                                                  
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W      
4.5.0-rc1-00013-gdb45d67 #40
[    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.000000] Backtrace:
[    0.000000] [<c0014230>] (dump_backtrace) from [<c001442c>]
(show_stack+0x18/0x1c)
...

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Stephen Boyd]

On 01/29, Stefan Agner wrote:
> If a clock gets enabled early during boot time, it can lead to a PLL
> startup. The wait_lock function makes sure that the PLL is really
> stareted up before it gets used. However, the function sleeps which
> leads to scheduling and an error:
> bad: scheduling from the idle thread!
> ...
> 
> Use udelay in case IRQ's are still disabled.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>

This is really old. Shawn, are you picking these up? I'm removing
these from my queue for now.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Shawn Guo]

On Fri, Apr 15, 2016 at 06:00:53PM -0700, Stephen Boyd wrote:
> On 01/29, Stefan Agner wrote:
> > If a clock gets enabled early during boot time, it can lead to a PLL
> > startup. The wait_lock function makes sure that the PLL is really
> > stareted up before it gets used. However, the function sleeps which
> > leads to scheduling and an error:
> > bad: scheduling from the idle thread!
> > ...
> > 
> > Use udelay in case IRQ's are still disabled.
> > 
> > Signed-off-by: Stefan Agner <stefan@agner.ch>
> 
> This is really old. Shawn, are you picking these up? I'm removing
> these from my queue for now.

Yes, I'm picking them up.

Shawn

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Dong Aisheng]

On Fri, Jan 29, 2016 at 02:49:23PM -0800, Stefan Agner wrote:
> If a clock gets enabled early during boot time, it can lead to a PLL
> startup. The wait_lock function makes sure that the PLL is really
> stareted up before it gets used. However, the function sleeps which
> leads to scheduling and an error:
> bad: scheduling from the idle thread!
> ...
> 
> Use udelay in case IRQ's are still disabled.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/clk/imx/clk-pllv3.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
> index c05c43d..b5ff561 100644
> --- a/drivers/clk/imx/clk-pllv3.c
> +++ b/drivers/clk/imx/clk-pllv3.c
> @@ -63,7 +63,10 @@ static int clk_pllv3_wait_lock(struct clk_pllv3 *pll)
>  			break;
>  		if (time_after(jiffies, timeout))
>  			break;
> -		usleep_range(50, 500);
> +		if (unlikely(irqs_disabled()))

This causes a bit confusion that clk_pllv3_prepare is sleepable.
But this line indicates it's possible to be called in irq context.
Although it's only happened during kernel boot phase where irq is
still not enabled.
It seems schedule_debug() somehow did not catch it during early boot
phase. Maybe schedule guys can help explain.

My question is if it's really worthy to introduce this confusion
to fix the issue since the delay is minor?

Furthermore, shouldn't it be udelay(500)?

Shawn,
What's your idea?

Regards
Dong Aisheng

> +			udelay(50);
> +		else
> +			usleep_range(50, 500);
>  	} while (1);
>  
>  	return readl_relaxed(pll->base) & BM_PLL_LOCK ? 0 : -ETIMEDOUT;
> -- 
> 2.7.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Shawn Guo]

On Thu, Apr 21, 2016 at 11:45:20AM +0800, Dong Aisheng wrote:
> On Fri, Jan 29, 2016 at 02:49:23PM -0800, Stefan Agner wrote:
> > If a clock gets enabled early during boot time, it can lead to a PLL
> > startup. The wait_lock function makes sure that the PLL is really
> > stareted up before it gets used. However, the function sleeps which
> > leads to scheduling and an error:
> > bad: scheduling from the idle thread!
> > ...
> > 
> > Use udelay in case IRQ's are still disabled.
> > 
> > Signed-off-by: Stefan Agner <stefan@agner.ch>
> > ---
> >  drivers/clk/imx/clk-pllv3.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
> > index c05c43d..b5ff561 100644
> > --- a/drivers/clk/imx/clk-pllv3.c
> > +++ b/drivers/clk/imx/clk-pllv3.c
> > @@ -63,7 +63,10 @@ static int clk_pllv3_wait_lock(struct clk_pllv3 *pll)
> >  			break;
> >  		if (time_after(jiffies, timeout))
> >  			break;
> > -		usleep_range(50, 500);
> > +		if (unlikely(irqs_disabled()))
> 
> This causes a bit confusion that clk_pllv3_prepare is sleepable.
> But this line indicates it's possible to be called in irq context.
> Although it's only happened during kernel boot phase where irq is
> still not enabled.
> It seems schedule_debug() somehow did not catch it during early boot
> phase. Maybe schedule guys can help explain.
> 
> My question is if it's really worthy to introduce this confusion
> to fix the issue since the delay is minor?

I do not understand why it's confusing.  The code already obviously
indicates this is a special handling for cases where irq is still not
enabled, rather than for irq context.

The patch is to fix the "bad: scheduling from the idle thread!" warning
rather than minimize the delay.  Do you have an opinion on how to fix
the warning?

> Furthermore, shouldn't it be udelay(500)?

500 is for the worst case of sleep, and 50 is good enough for delay.

Shawn

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Dong Aisheng]

Hi Shawn,

On Tue, Apr 26, 2016 at 9:23 AM, Shawn Guo <shawnguo@kernel.org> wrote:
> On Thu, Apr 21, 2016 at 11:45:20AM +0800, Dong Aisheng wrote:
>> On Fri, Jan 29, 2016 at 02:49:23PM -0800, Stefan Agner wrote:
>> > If a clock gets enabled early during boot time, it can lead to a PLL
>> > startup. The wait_lock function makes sure that the PLL is really
>> > stareted up before it gets used. However, the function sleeps which
>> > leads to scheduling and an error:
>> > bad: scheduling from the idle thread!
>> > ...
>> >
>> > Use udelay in case IRQ's are still disabled.
>> >
>> > Signed-off-by: Stefan Agner <stefan@agner.ch>
>> > ---
>> >  drivers/clk/imx/clk-pllv3.c | 5 ++++-
>> >  1 file changed, 4 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
>> > index c05c43d..b5ff561 100644
>> > --- a/drivers/clk/imx/clk-pllv3.c
>> > +++ b/drivers/clk/imx/clk-pllv3.c
>> > @@ -63,7 +63,10 @@ static int clk_pllv3_wait_lock(struct clk_pllv3 *pll)
>> >                     break;
>> >             if (time_after(jiffies, timeout))
>> >                     break;
>> > -           usleep_range(50, 500);
>> > +           if (unlikely(irqs_disabled()))
>>
>> This causes a bit confusion that clk_pllv3_prepare is sleepable.
>> But this line indicates it's possible to be called in irq context.
>> Although it's only happened during kernel boot phase where irq is
>> still not enabled.
>> It seems schedule_debug() somehow did not catch it during early boot
>> phase. Maybe schedule guys can help explain.
>>
>> My question is if it's really worthy to introduce this confusion
>> to fix the issue since the delay is minor?
>
> I do not understand why it's confusing.  The code already obviously
> indicates this is a special handling for cases where irq is still not
> enabled, rather than for irq context.
>

The code itself has nothing telling it's a special handling for the
case where irq is
still not enabled.
Even it tells, it may still cause confusing by adding complexity in
clk_pllv3_prepare()
which actually should be called in non-atomic context as it could sleep.

> The patch is to fix the "bad: scheduling from the idle thread!" warning
> rather than minimize the delay.  Do you have an opinion on how to fix
> the warning?
>

I just wonder maybe we could simply just using udelay(50) instead of
usleep_range(50, 500) to eliminate the confusing since it's minor cast.
What do you think of it?

>> Furthermore, shouldn't it be udelay(500)?
>
> 500 is for the worst case of sleep, and 50 is good enough for delay.
>

Yes, you''re right.
We have a loop, so 50ns one time should be good.

> Shawn

Regards
Dong Aisheng

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Shawn Guo]

On Tue, Apr 26, 2016 at 01:51:13PM +0800, Dong Aisheng wrote:
> Hi Shawn,
> 
> On Tue, Apr 26, 2016 at 9:23 AM, Shawn Guo <shawnguo@kernel.org> wrote:
> > On Thu, Apr 21, 2016 at 11:45:20AM +0800, Dong Aisheng wrote:
> >> On Fri, Jan 29, 2016 at 02:49:23PM -0800, Stefan Agner wrote:
> >> > If a clock gets enabled early during boot time, it can lead to a PLL
> >> > startup. The wait_lock function makes sure that the PLL is really
> >> > stareted up before it gets used. However, the function sleeps which
> >> > leads to scheduling and an error:
> >> > bad: scheduling from the idle thread!
> >> > ...
> >> >
> >> > Use udelay in case IRQ's are still disabled.
> >> >
> >> > Signed-off-by: Stefan Agner <stefan@agner.ch>
> >> > ---
> >> >  drivers/clk/imx/clk-pllv3.c | 5 ++++-
> >> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
> >> > index c05c43d..b5ff561 100644
> >> > --- a/drivers/clk/imx/clk-pllv3.c
> >> > +++ b/drivers/clk/imx/clk-pllv3.c
> >> > @@ -63,7 +63,10 @@ static int clk_pllv3_wait_lock(struct clk_pllv3 *pll)
> >> >                     break;
> >> >             if (time_after(jiffies, timeout))
> >> >                     break;
> >> > -           usleep_range(50, 500);
> >> > +           if (unlikely(irqs_disabled()))
> >>
> >> This causes a bit confusion that clk_pllv3_prepare is sleepable.
> >> But this line indicates it's possible to be called in irq context.
> >> Although it's only happened during kernel boot phase where irq is
> >> still not enabled.
> >> It seems schedule_debug() somehow did not catch it during early boot
> >> phase. Maybe schedule guys can help explain.
> >>
> >> My question is if it's really worthy to introduce this confusion
> >> to fix the issue since the delay is minor?
> >
> > I do not understand why it's confusing.  The code already obviously
> > indicates this is a special handling for cases where irq is still not
> > enabled, rather than for irq context.
> >
> 
> The code itself has nothing telling it's a special handling for the
> case where irq is
> still not enabled.

I think the following if-clause is telling that.

	if (unlikely(irqs_disabled()))

> Even it tells, it may still cause confusing by adding complexity in
> clk_pllv3_prepare()
> which actually should be called in non-atomic context as it could sleep.

I agree with you on that.

> > The patch is to fix the "bad: scheduling from the idle thread!" warning
> > rather than minimize the delay.  Do you have an opinion on how to fix
> > the warning?
> >
> 
> I just wonder maybe we could simply just using udelay(50) instead of
> usleep_range(50, 500) to eliminate the confusing since it's minor cast.
> What do you think of it?

I'm fine with it.  Since I haven't sent the patch to clk maintainers, I
could replace Stefan's patch with yours, if you can send me a patch
quickly.

Shawn

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Lucas Stach]

Am Dienstag, den 26.04.2016, 13:51 +0800 schrieb Dong Aisheng:
> Hi Shawn,
> 
> On Tue, Apr 26, 2016 at 9:23 AM, Shawn Guo <shawnguo@kernel.org> wrote:
> > On Thu, Apr 21, 2016 at 11:45:20AM +0800, Dong Aisheng wrote:
> >> On Fri, Jan 29, 2016 at 02:49:23PM -0800, Stefan Agner wrote:
> >> > If a clock gets enabled early during boot time, it can lead to a PLL
> >> > startup. The wait_lock function makes sure that the PLL is really
> >> > stareted up before it gets used. However, the function sleeps which
> >> > leads to scheduling and an error:
> >> > bad: scheduling from the idle thread!
> >> > ...
> >> >
> >> > Use udelay in case IRQ's are still disabled.
> >> >
> >> > Signed-off-by: Stefan Agner <stefan@agner.ch>
> >> > ---
> >> >  drivers/clk/imx/clk-pllv3.c | 5 ++++-
> >> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
> >> > index c05c43d..b5ff561 100644
> >> > --- a/drivers/clk/imx/clk-pllv3.c
> >> > +++ b/drivers/clk/imx/clk-pllv3.c
> >> > @@ -63,7 +63,10 @@ static int clk_pllv3_wait_lock(struct clk_pllv3 *pll)
> >> >                     break;
> >> >             if (time_after(jiffies, timeout))
> >> >                     break;
> >> > -           usleep_range(50, 500);
> >> > +           if (unlikely(irqs_disabled()))
> >>
> >> This causes a bit confusion that clk_pllv3_prepare is sleepable.
> >> But this line indicates it's possible to be called in irq context.
> >> Although it's only happened during kernel boot phase where irq is
> >> still not enabled.
> >> It seems schedule_debug() somehow did not catch it during early boot
> >> phase. Maybe schedule guys can help explain.
> >>
> >> My question is if it's really worthy to introduce this confusion
> >> to fix the issue since the delay is minor?
> >
> > I do not understand why it's confusing.  The code already obviously
> > indicates this is a special handling for cases where irq is still not
> > enabled, rather than for irq context.
> >
> 
> The code itself has nothing telling it's a special handling for the
> case where irq is
> still not enabled.
> Even it tells, it may still cause confusing by adding complexity in
> clk_pllv3_prepare()
> which actually should be called in non-atomic context as it could sleep.
> 
> > The patch is to fix the "bad: scheduling from the idle thread!" warning
> > rather than minimize the delay.  Do you have an opinion on how to fix
> > the warning?
> >
> 
> I just wonder maybe we could simply just using udelay(50) instead of
> usleep_range(50, 500) to eliminate the confusing since it's minor cast.
> What do you think of it?

Why should we needless burn CPU time in the likely case of
clk_pllv3_prepare() being called in sleepable context?

Using udelay() in a sleepable context is even more confusing than having
the special case for clk_prepare() being called in atomic context IMHO.

Regards,
Lucas

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Dong Aisheng]

On Tue, Apr 26, 2016 at 5:31 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Dienstag, den 26.04.2016, 13:51 +0800 schrieb Dong Aisheng:
>> Hi Shawn,
>>
>> On Tue, Apr 26, 2016 at 9:23 AM, Shawn Guo <shawnguo@kernel.org> wrote:
>> > On Thu, Apr 21, 2016 at 11:45:20AM +0800, Dong Aisheng wrote:
>> >> On Fri, Jan 29, 2016 at 02:49:23PM -0800, Stefan Agner wrote:
>> >> > If a clock gets enabled early during boot time, it can lead to a PLL
>> >> > startup. The wait_lock function makes sure that the PLL is really
>> >> > stareted up before it gets used. However, the function sleeps which
>> >> > leads to scheduling and an error:
>> >> > bad: scheduling from the idle thread!
>> >> > ...
>> >> >
>> >> > Use udelay in case IRQ's are still disabled.
>> >> >
>> >> > Signed-off-by: Stefan Agner <stefan@agner.ch>
>> >> > ---
>> >> >  drivers/clk/imx/clk-pllv3.c | 5 ++++-
>> >> >  1 file changed, 4 insertions(+), 1 deletion(-)
>> >> >
>> >> > diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
>> >> > index c05c43d..b5ff561 100644
>> >> > --- a/drivers/clk/imx/clk-pllv3.c
>> >> > +++ b/drivers/clk/imx/clk-pllv3.c
>> >> > @@ -63,7 +63,10 @@ static int clk_pllv3_wait_lock(struct clk_pllv3 *pll)
>> >> >                     break;
>> >> >             if (time_after(jiffies, timeout))
>> >> >                     break;
>> >> > -           usleep_range(50, 500);
>> >> > +           if (unlikely(irqs_disabled()))
>> >>
>> >> This causes a bit confusion that clk_pllv3_prepare is sleepable.
>> >> But this line indicates it's possible to be called in irq context.
>> >> Although it's only happened during kernel boot phase where irq is
>> >> still not enabled.
>> >> It seems schedule_debug() somehow did not catch it during early boot
>> >> phase. Maybe schedule guys can help explain.
>> >>
>> >> My question is if it's really worthy to introduce this confusion
>> >> to fix the issue since the delay is minor?
>> >
>> > I do not understand why it's confusing.  The code already obviously
>> > indicates this is a special handling for cases where irq is still not
>> > enabled, rather than for irq context.
>> >
>>
>> The code itself has nothing telling it's a special handling for the
>> case where irq is
>> still not enabled.
>> Even it tells, it may still cause confusing by adding complexity in
>> clk_pllv3_prepare()
>> which actually should be called in non-atomic context as it could sleep.
>>
>> > The patch is to fix the "bad: scheduling from the idle thread!" warning
>> > rather than minimize the delay.  Do you have an opinion on how to fix
>> > the warning?
>> >
>>
>> I just wonder maybe we could simply just using udelay(50) instead of
>> usleep_range(50, 500) to eliminate the confusing since it's minor cast.
>> What do you think of it?
>
> Why should we needless burn CPU time in the likely case of
> clk_pllv3_prepare() being called in sleepable context?
>

I think because the delay time is not big.
And pll clks are system basic clocks and less likely to be frequently
enabled/disabled.

> Using udelay() in a sleepable context is even more confusing than having
> the special case for clk_prepare() being called in atomic context IMHO.
>

I can't agree having special case by checking
unlikely(irqs_disabled()) is better
which is obviously more confusing IMHO.
I'd more like to hide it from users.

I see other platforms like samsung&tegra also implements pll using udelay.
But difference is that they implement it in .enable(I) clalback not prepare.

How about simply remove usleep_range to poll instead of using udelay?
Cause most PLL enable may be more faster than 50ns.

And according to kernel doc, for delay time less than 10ns,
udelay or polling is recommended.

> Regards,
> Lucas
>

Regards
Dong Aisheng

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Dong Aisheng]

On Tue, Apr 26, 2016 at 7:16 PM, Dong Aisheng <dongas86@gmail.com> wrote:
> On Tue, Apr 26, 2016 at 5:31 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
>> Am Dienstag, den 26.04.2016, 13:51 +0800 schrieb Dong Aisheng:
>>> Hi Shawn,
>>>
>>> On Tue, Apr 26, 2016 at 9:23 AM, Shawn Guo <shawnguo@kernel.org> wrote:
>>> > On Thu, Apr 21, 2016 at 11:45:20AM +0800, Dong Aisheng wrote:
>>> >> On Fri, Jan 29, 2016 at 02:49:23PM -0800, Stefan Agner wrote:
>>> >> > If a clock gets enabled early during boot time, it can lead to a PLL
>>> >> > startup. The wait_lock function makes sure that the PLL is really
>>> >> > stareted up before it gets used. However, the function sleeps which
>>> >> > leads to scheduling and an error:
>>> >> > bad: scheduling from the idle thread!
>>> >> > ...
>>> >> >
>>> >> > Use udelay in case IRQ's are still disabled.
>>> >> >
>>> >> > Signed-off-by: Stefan Agner <stefan@agner.ch>
>>> >> > ---
>>> >> >  drivers/clk/imx/clk-pllv3.c | 5 ++++-
>>> >> >  1 file changed, 4 insertions(+), 1 deletion(-)
>>> >> >
>>> >> > diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c
>>> >> > index c05c43d..b5ff561 100644
>>> >> > --- a/drivers/clk/imx/clk-pllv3.c
>>> >> > +++ b/drivers/clk/imx/clk-pllv3.c
>>> >> > @@ -63,7 +63,10 @@ static int clk_pllv3_wait_lock(struct clk_pllv3 *pll)
>>> >> >                     break;
>>> >> >             if (time_after(jiffies, timeout))
>>> >> >                     break;
>>> >> > -           usleep_range(50, 500);
>>> >> > +           if (unlikely(irqs_disabled()))
>>> >>
>>> >> This causes a bit confusion that clk_pllv3_prepare is sleepable.
>>> >> But this line indicates it's possible to be called in irq context.
>>> >> Although it's only happened during kernel boot phase where irq is
>>> >> still not enabled.
>>> >> It seems schedule_debug() somehow did not catch it during early boot
>>> >> phase. Maybe schedule guys can help explain.
>>> >>
>>> >> My question is if it's really worthy to introduce this confusion
>>> >> to fix the issue since the delay is minor?
>>> >
>>> > I do not understand why it's confusing.  The code already obviously
>>> > indicates this is a special handling for cases where irq is still not
>>> > enabled, rather than for irq context.
>>> >
>>>
>>> The code itself has nothing telling it's a special handling for the
>>> case where irq is
>>> still not enabled.
>>> Even it tells, it may still cause confusing by adding complexity in
>>> clk_pllv3_prepare()
>>> which actually should be called in non-atomic context as it could sleep.
>>>
>>> > The patch is to fix the "bad: scheduling from the idle thread!" warning
>>> > rather than minimize the delay.  Do you have an opinion on how to fix
>>> > the warning?
>>> >
>>>
>>> I just wonder maybe we could simply just using udelay(50) instead of
>>> usleep_range(50, 500) to eliminate the confusing since it's minor cast.
>>> What do you think of it?
>>
>> Why should we needless burn CPU time in the likely case of
>> clk_pllv3_prepare() being called in sleepable context?
>>
>
> I think because the delay time is not big.
> And pll clks are system basic clocks and less likely to be frequently
> enabled/disabled.
>
>> Using udelay() in a sleepable context is even more confusing than having
>> the special case for clk_prepare() being called in atomic context IMHO.
>>
>
> I can't agree having special case by checking
> unlikely(irqs_disabled()) is better
> which is obviously more confusing IMHO.
> I'd more like to hide it from users.
>
> I see other platforms like samsung&tegra also implements pll using udelay.
> But difference is that they implement it in .enable(I) clalback not prepare.
>
> How about simply remove usleep_range to poll instead of using udelay?
> Cause most PLL enable may be more faster than 50ns.
>
> And according to kernel doc, for delay time less than 10ns,
> udelay or polling is recommended.
>

Shawn,
What's your suggestion?

Regards
Dong Aisheng

>> Regards,
>> Lucas
>>
>
> Regards
> Dong Aisheng

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Shawn Guo]

On Tue, Apr 26, 2016 at 07:27:03PM +0800, Dong Aisheng wrote:
> Shawn,
> What's your suggestion?

I think this needs more discussion, and I just dropped Stefan's patch
from my tree.

We need to firstly understand why this is happening.  The .prepare hook
is defined to be non-atomic context, and so that we call sleep function
in there.  We did everything right.  Why are we getting the warning?  If
I'm correct, this warning only happens on i.MX7D.  Why is that?

Shawn

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Dong Aisheng]

On Wed, Apr 27, 2016 at 9:58 AM, Shawn Guo <shawnguo@kernel.org> wrote:
> On Tue, Apr 26, 2016 at 07:27:03PM +0800, Dong Aisheng wrote:
>> Shawn,
>> What's your suggestion?
>
> I think this needs more discussion, and I just dropped Stefan's patch
> from my tree.
>
> We need to firstly understand why this is happening.  The .prepare hook
> is defined to be non-atomic context, and so that we call sleep function
> in there.  We did everything right.  Why are we getting the warning?  If
> I'm correct, this warning only happens on i.MX7D.  Why is that?
>

This is mainly caused by during kernel early booting, there's only one init idle
task running.
See:
void __init sched_init(void)
{
.....
        /*
         * Make us the idle thread. Technically, schedule() should not be
         * called from this thread, however somewhere below it might be,
         * but because we are the idle thread, we just pick up running again
         * when this runqueue becomes "idle".
         */
        init_idle(current, smp_processor_id());
...
}

And the idle sched class indicates it's not valid to schedule for idle task.
const struct sched_class idle_sched_class = {
        /* .next is NULL */
        /* no enqueue/yield_task for idle tasks */

        /* dequeue is not valid, we print a debug message there: */
        .dequeue_task           = dequeue_task_idle,
...........

}

/*
 * It is not legal to sleep in the idle task - print a warning
 * message if some code attempts to do it:
 */
static void
dequeue_task_idle(struct rq *rq, struct task_struct *p, int flags)
{
        raw_spin_unlock_irq(&rq->lock);
        printk(KERN_ERR "bad: scheduling from the idle thread!\n");
        dump_stack();
        raw_spin_lock_irq(&rq->lock);
}


Below is the full log of imx7d booting FYI.

You can ignore the first two warning (releasing a pinned lock) which
is a side affection
of idle task schedule during booting.

[    0.000000] Booting Linux on physical CPU 0x0
[    0.000000] Linux version 4.6.0-rc1-00011-gee55b3d17805-dirty
(b29396@shlinux2) (gcc version 4.9.1 (GCC) ) #779 SMP Tue Apr 26
18:17:26 CST 2016
[    0.000000] CPU: ARMv7 Processor [410fc075] revision 5 (ARMv7), cr=10c5387d
[    0.000000] CPU: div instructions available: patching division code
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing
instruction cache
[    0.000000] Machine model: Freescale i.MX7 SabreSD Board
[    0.000000] cma: Reserved 16 MiB at 0xbf000000
[    0.000000] Memory policy: Data cache writealloc
[    0.000000] On node 0 totalpages: 262144
[    0.000000] free_area_init_node: node 0, pgdat c0d72900,
node_mem_map ef7f9000
[    0.000000]   Normal zone: 1536 pages used for memmap
[    0.000000]   Normal zone: 0 pages reserved
[    0.000000]   Normal zone: 196608 pages, LIFO batch:31
[    0.000000]   HighMem zone: 65536 pages, LIFO batch:15
[    0.000000] percpu: Embedded 13 pages/cpu @ef7bd000 s20544 r8192
d24512 u53248
[    0.000000] pcpu-alloc: s20544 r8192 d24512 u53248 alloc=13*4096
[    0.000000] pcpu-alloc: [0] 0 [0] 1
[    0.000000] Built 1 zonelists in Zone order, mobility grouping on.
Total pages: 260608
[    0.000000] Kernel command line: console=ttymxc0,115200
root=/dev/nfs ip=dhcp
nfsroot=10.192.224.44:/data/rootfs_home/b29396/rootfs-yocto-L4.1.X-RC5,v3,tcp
log_buf_len=2M
[    0.000000] log_buf_len: 2097152 bytes
[    0.000000] early log buf free: 260724(99%)
[    0.000000] PID hash table entries: 4096 (order: 2, 16384 bytes)
[    0.000000] Dentry cache hash table entries: 131072 (order: 7, 524288 bytes)
[    0.000000] Inode-cache hash table entries: 65536 (order: 6, 262144 bytes)
[    0.000000] Memory: 999884K/1048576K available (8196K kernel code,
464K rwdata, 2828K rodata, 1024K init, 8223K bss, 32308K reserved,
16384K cma-reserved, 245760K highmem)
[    0.000000] Virtual kernel memory layout:
[    0.000000]     vector  : 0xffff0000 - 0xffff1000   (   4 kB)
[    0.000000]     fixmap  : 0xffc00000 - 0xfff00000   (3072 kB)
[    0.000000]     vmalloc : 0xf0800000 - 0xff800000   ( 240 MB)
[    0.000000]     lowmem  : 0xc0000000 - 0xf0000000   ( 768 MB)
[    0.000000]     pkmap   : 0xbfe00000 - 0xc0000000   (   2 MB)
[    0.000000]     modules : 0xbf000000 - 0xbfe00000   (  14 MB)
[    0.000000]       .text : 0xc0008000 - 0xc0bc406c   (12017 kB)
[    0.000000]       .init : 0xc0c00000 - 0xc0d00000   (1024 kB)
[    0.000000]       .data : 0xc0d00000 - 0xc0d74100   ( 465 kB)
[    0.000000]        .bss : 0xc0d76000 - 0xc157dfa0   (8224 kB)
[    0.000000] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=2, Nodes=1
[    0.000000] Running RCU self tests
[    0.000000] Hierarchical RCU implementation.
[    0.000000]  RCU lockdep checking is enabled.
[    0.000000]  Build-time adjustment of leaf fanout to 32.
[    0.000000]  RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=2.
[    0.000000] RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
[    0.000000] NR_IRQS:16 nr_irqs:16 16
[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:3422
lock_release+0x340/0x378
[    0.000000] releasing a pinned lock
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
4.6.0-rc1-00011-gee55b3d17805-dirty #779
[    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.000000] Backtrace:
[    0.000000] [<c010c42c>] (dump_backtrace) from [<c010c624>]
(show_stack+0x18/0x1c)
[    0.000000]  r6:600000d3 r5:00000000 r4:c0d21f9c r3:00000000
[    0.000000] [<c010c60c>] (show_stack) from [<c03dfec8>]
(dump_stack+0xb4/0xe8)
[    0.000000] [<c03dfe14>] (dump_stack) from [<c0124ce8>] (__warn+0xd8/0x104)
[    0.000000]  r10:c0d069c0 r9:c016e7ec r8:00000d5e r7:00000009
r6:c0ad6a04 r5:00000000
[    0.000000]  r4:c0d01ad8 r3:00000001
[    0.000000] [<c0124c10>] (__warn) from [<c0124d54>]
(warn_slowpath_fmt+0x40/0x48)
[    0.000000]  r9:00000001 r8:c154243c r7:00000002 r6:00000020
r5:c0d06500 r4:c0ad7ac4
[    0.000000] [<c0124d18>] (warn_slowpath_fmt) from [<c016e7ec>]
(lock_release+0x340/0x378)
[    0.000000]  r3:00000001 r2:c0ad7ac4
[    0.000000]  r4:00000001
[    0.000000] [<c016e4ac>] (lock_release) from [<c08f9cf4>]
(_raw_spin_unlock_irq+0x20/0x34)
[    0.000000]  r10:c08f4fdc r9:c0d02b0c r8:c0d06844 r7:ef7c14d0
r6:00000001 r5:ef7c14c0
[    0.000000]  r4:ef7c14c0
[    0.000000] [<c08f9cd4>] (_raw_spin_unlock_irq) from [<c015437c>]
(dequeue_task_idle+0x14/0x30)
[    0.000000]  r4:ef7c14c0 r3:c0154368
[    0.000000] [<c0154368>] (dequeue_task_idle) from [<c014c96c>]
(deactivate_task+0x64/0x68)
[    0.000000]  r4:c0d06500 r3:c0154368
[    0.000000] [<c014c908>] (deactivate_task) from [<c08f4b44>]
(__schedule+0x2e8/0x738)
[    0.000000]  r6:c0d06500 r5:ef7c14c0 r4:c0c754c0 r3:00000002
[    0.000000] [<c08f485c>] (__schedule) from [<c08f4fdc>] (schedule+0x48/0xa0)
[    0.000000]  r10:00000018 r9:0000001c r8:00000003 r7:00000000
r6:0007a120 r5:00000000
[    0.000000]  r4:c0d00000
[    0.000000] [<c08f4f94>] (schedule) from [<c08f9750>]
(schedule_hrtimeout_range_clock+0xac/0x12c)
[    0.000000]  r4:0006ddd0 r3:c0d06500
[    0.000000] [<c08f96a4>] (schedule_hrtimeout_range_clock) from
[<c08f97f4>] (schedule_hrtimeout_range+0x24/0x2c)
[    0.000000]  r7:00000001 r6:c0d02100 r5:ffff8ad1 r4:ef004f80
[    0.000000] [<c08f97d0>] (schedule_hrtimeout_range) from
[<c08f922c>] (usleep_range+0x54/0x5c)
[    0.000000] [<c08f91d8>] (usleep_range) from [<c0698148>]
(clk_pllv3_wait_lock+0x7c/0xb4)
[    0.000000] [<c06980cc>] (clk_pllv3_wait_lock) from [<c06981b0>]
(clk_pllv3_prepare+0x30/0x34)
[    0.000000]  r6:ef007080 r5:ef00ef00 r4:ef007600 r3:000017c0
[    0.000000] [<c0698180>] (clk_pllv3_prepare) from [<c069203c>]
(clk_core_prepare+0x98/0xc4)
[    0.000000] [<c0691fa4>] (clk_core_prepare) from [<c069201c>]
(clk_core_prepare+0x78/0xc4)
[    0.000000]  r5:ef00ef00 r4:ef007900
[    0.000000] [<c0691fa4>] (clk_core_prepare) from [<c069201c>]
(clk_core_prepare+0x78/0xc4)
[    0.000000]  r5:ef00ef00 r4:ef00e800
[    0.000000] [<c0691fa4>] (clk_core_prepare) from [<c069201c>]
(clk_core_prepare+0x78/0xc4)
[    0.000000]  r5:ef00ef00 r4:ef00eb80
[    0.000000] [<c0691fa4>] (clk_core_prepare) from [<c069201c>]
(clk_core_prepare+0x78/0xc4)
[    0.000000]  r5:ef00ef00 r4:ef00ef00
[    0.000000] [<c0691fa4>] (clk_core_prepare) from [<c0692be8>]
(clk_core_prepare_enable+0x1c/0x5c)
[    0.000000]  r5:ef00ef00 r4:ef011f80
[    0.000000] [<c0692bcc>] (clk_core_prepare_enable) from
[<c0692eb4>] (__clk_set_parent_before+0x38/0x7c)
[    0.000000]  r5:ef00ef00 r4:ef011f80
[    0.000000] [<c0692e7c>] (__clk_set_parent_before) from
[<c0693004>] (clk_core_set_parent+0x10c/0x170)
[    0.000000]  r7:00000001 r6:ef00ef00 r5:00000000 r4:ef011f80
[    0.000000] [<c0692ef8>] (clk_core_set_parent) from [<c069308c>]
(clk_set_parent+0x24/0x28)
[    0.000000]  r8:00000003 r7:c0d5d434 r6:00000000 r5:ef02b440
r4:00000009 r3:c15603bc
[    0.000000] [<c0693068>] (clk_set_parent) from [<c0c4ade8>]
(imx7d_clocks_init+0x5f4c/0x5f8c)
[    0.000000] [<c0c44e9c>] (imx7d_clocks_init) from [<c0c30e00>]
(of_clk_init+0x10c/0x1d4)
[    0.000000]  r10:00000001 r9:c0d01f68 r8:00000000 r7:c0d01f60
r6:ef7e3d60 r5:ef004340
[    0.000000]  r4:00000002
[    0.000000] [<c0c30cf4>] (of_clk_init) from [<c0c048d4>]
(time_init+0x2c/0x38)
[    0.000000]  r10:efffcac0 r9:c0c5da48 r8:c0d76000 r7:c0d028c0
r6:ffffffff r5:c0d76000
[    0.000000]  r4:00000000
[    0.000000] [<c0c048a8>] (time_init) from [<c0c00b7c>]
(start_kernel+0x218/0x398)
[    0.000000] [<c0c00964>] (start_kernel) from [<8000807c>] (0x8000807c)
[    0.000000]  r10:00000000 r9:410fc075 r8:8000406a r7:c0d07e8c
r6:c0c5da44 r5:c0d0296c
[    0.000000]  r4:c0d76294
[    0.000000] ---[ end trace cb88537fdc8fa200 ]---
[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:2617
trace_hardirqs_on_caller+0x1ac/0x1f0
[    0.000000] DEBUG_LOCKS_WARN_ON(unlikely(early_boot_irqs_disabled))
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W
4.6.0-rc1-00011-gee55b3d17805-dirty #779
[    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.000000] Backtrace:
[    0.000000] [<c010c42c>] (dump_backtrace) from [<c010c624>]
(show_stack+0x18/0x1c)
[    0.000000]  r6:600000d3 r5:00000000 r4:c0d21f9c r3:00000000
[    0.000000] [<c010c60c>] (show_stack) from [<c03dfec8>]
(dump_stack+0xb4/0xe8)
[    0.000000] [<c03dfe14>] (dump_stack) from [<c0124ce8>] (__warn+0xd8/0x104)
[    0.000000]  r10:c08f4fdc r9:c016b918 r8:00000a39 r7:00000009
r6:c0ad6a04 r5:00000000
[    0.000000]  r4:c0d01af8 r3:00000000
[    0.000000] [<c0124c10>] (__warn) from [<c0124d54>]
(warn_slowpath_fmt+0x40/0x48)
[    0.000000]  r9:c0d02b0c r8:c0d06844 r7:ef7c14d0 r6:00000001
r5:ef7c14c0 r4:c0ad3ea0
[    0.000000] [<c0124d18>] (warn_slowpath_fmt) from [<c016b918>]
(trace_hardirqs_on_caller+0x1ac/0x1f0)
[    0.000000]  r3:c0ad7890 r2:c0ad3ea0
[    0.000000]  r4:c08f9d00
[    0.000000] [<c016b76c>] (trace_hardirqs_on_caller) from
[<c016b970>] (trace_hardirqs_on+0x14/0x18)
[    0.000000]  r6:00000001 r5:ef7c14c0 r4:ef7c14c0 r3:600000d3
[    0.000000] [<c016b95c>] (trace_hardirqs_on) from [<c08f9d00>]
(_raw_spin_unlock_irq+0x2c/0x34)
[    0.000000] [<c08f9cd4>] (_raw_spin_unlock_irq) from [<c015437c>]
(dequeue_task_idle+0x14/0x30)
[    0.000000]  r4:ef7c14c0 r3:c0154368
[    0.000000] [<c0154368>] (dequeue_task_idle) from [<c014c96c>]
(deactivate_task+0x64/0x68)
[    0.000000]  r4:c0d06500 r3:c0154368
[    0.000000] [<c014c908>] (deactivate_task) from [<c08f4b44>]
(__schedule+0x2e8/0x738)
[    0.000000]  r6:c0d06500 r5:ef7c14c0 r4:c0c754c0 r3:00000002
[    0.000000] [<c08f485c>] (__schedule) from [<c08f4fdc>] (schedule+0x48/0xa0)
[    0.000000]  r10:00000018 r9:0000001c r8:00000003 r7:00000000
r6:0007a120 r5:00000000
[    0.000000]  r4:c0d00000
[    0.000000] [<c08f4f94>] (schedule) from [<c08f9750>]
(schedule_hrtimeout_range_clock+0xac/0x12c)
[    0.000000]  r4:0006ddd0 r3:c0d06500
[    0.000000] [<c08f96a4>] (schedule_hrtimeout_range_clock) from
[<c08f97f4>] (schedule_hrtimeout_range+0x24/0x2c)
[    0.000000]  r7:00000001 r6:c0d02100 r5:ffff8ad1 r4:ef004f80
[    0.000000] [<c08f97d0>] (schedule_hrtimeout_range) from
[<c08f922c>] (usleep_range+0x54/0x5c)
[    0.000000] [<c08f91d8>] (usleep_range) from [<c0698148>]
(clk_pllv3_wait_lock+0x7c/0xb4)
[    0.000000] [<c06980cc>] (clk_pllv3_wait_lock) from [<c06981b0>]
(clk_pllv3_prepare+0x30/0x34)
[    0.000000]  r6:ef007080 r5:ef00ef00 r4:ef007600 r3:000017c0
[    0.000000] [<c0698180>] (clk_pllv3_prepare) from [<c069203c>]
(clk_core_prepare+0x98/0xc4)
[    0.000000] [<c0691fa4>] (clk_core_prepare) from [<c069201c>]
(clk_core_prepare+0x78/0xc4)
[    0.000000]  r5:ef00ef00 r4:ef007900
[    0.000000] [<c0691fa4>] (clk_core_prepare) from [<c069201c>]
(clk_core_prepare+0x78/0xc4)
[    0.000000]  r5:ef00ef00 r4:ef00e800
[    0.000000] [<c0691fa4>] (clk_core_prepare) from [<c069201c>]
(clk_core_prepare+0x78/0xc4)
[    0.000000]  r5:ef00ef00 r4:ef00eb80
[    0.000000] [<c0691fa4>] (clk_core_prepare) from [<c069201c>]
(clk_core_prepare+0x78/0xc4)
[    0.000000]  r5:ef00ef00 r4:ef00ef00
[    0.000000] [<c0691fa4>] (clk_core_prepare) from [<c0692be8>]
(clk_core_prepare_enable+0x1c/0x5c)
[    0.000000]  r5:ef00ef00 r4:ef011f80
[    0.000000] [<c0692bcc>] (clk_core_prepare_enable) from
[<c0692eb4>] (__clk_set_parent_before+0x38/0x7c)
[    0.000000]  r5:ef00ef00 r4:ef011f80
[    0.000000] [<c0692e7c>] (__clk_set_parent_before) from
[<c0693004>] (clk_core_set_parent+0x10c/0x170)
[    0.000000]  r7:00000001 r6:ef00ef00 r5:00000000 r4:ef011f80
[    0.000000] [<c0692ef8>] (clk_core_set_parent) from [<c069308c>]
(clk_set_parent+0x24/0x28)
[    0.000000]  r8:00000003 r7:c0d5d434 r6:00000000 r5:ef02b440
r4:00000009 r3:c15603bc
[    0.000000] [<c0693068>] (clk_set_parent) from [<c0c4ade8>]
(imx7d_clocks_init+0x5f4c/0x5f8c)
[    0.000000] [<c0c44e9c>] (imx7d_clocks_init) from [<c0c30e00>]
(of_clk_init+0x10c/0x1d4)
[    0.000000]  r10:00000001 r9:c0d01f68 r8:00000000 r7:c0d01f60
r6:ef7e3d60 r5:ef004340
[    0.000000]  r4:00000002
[    0.000000] [<c0c30cf4>] (of_clk_init) from [<c0c048d4>]
(time_init+0x2c/0x38)
[    0.000000]  r10:efffcac0 r9:c0c5da48 r8:c0d76000 r7:c0d028c0
r6:ffffffff r5:c0d76000
[    0.000000]  r4:00000000
[    0.000000] [<c0c048a8>] (time_init) from [<c0c00b7c>]
(start_kernel+0x218/0x398)
[    0.000000] [<c0c00964>] (start_kernel) from [<8000807c>] (0x8000807c)
[    0.000000]  r10:00000000 r9:410fc075 r8:8000406a r7:c0d07e8c
r6:c0c5da44 r5:c0d0296c
[    0.000000]  r4:c0d76294
[    0.000000] ---[ end trace cb88537fdc8fa201 ]---
[    0.000000] bad: scheduling from the idle thread!
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W
4.6.0-rc1-00011-gee55b3d17805-dirty #779
[    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.000000] Backtrace:
[    0.000000] [<c010c42c>] (dump_backtrace) from [<c010c624>]
(show_stack+0x18/0x1c)
[    0.000000]  r6:60000053 r5:00000000 r4:c0d21f9c r3:00000000
[    0.000000] [<c010c60c>] (show_stack) from [<c03dfec8>]
(dump_stack+0xb4/0xe8)
[    0.000000] [<c03dfe14>] (dump_stack) from [<c0154388>]
(dequeue_task_idle+0x20/0x30)
[    0.000000]  r10:c08f4fdc r9:c0d02b0c r8:c0d06844 r7:ef7c14d0
r6:00000001 r5:ef7c14c0
[    0.000000]  r4:ef7c14c0 r3:00000000
[    0.000000] [<c0154368>] (dequeue_task_idle) from [<c014c96c>]
(deactivate_task+0x64/0x68)
[    0.000000]  r4:c0d06500 r3:c0154368
[    0.000000] [<c014c908>] (deactivate_task) from [<c08f4b44>]
(__schedule+0x2e8/0x738)
[    0.000000]  r6:c0d06500 r5:ef7c14c0 r4:c0c754c0 r3:00000002
[    0.000000] [<c08f485c>] (__schedule) from [<c08f4fdc>] (schedule+0x48/0xa0)
[    0.000000]  r10:00000018 r9:0000001c r8:00000003 r7:00000000
r6:0007a120 r5:00000000
[    0.000000]  r4:c0d00000
[    0.000000] [<c08f4f94>] (schedule) from [<c08f9750>]
(schedule_hrtimeout_range_clock+0xac/0x12c)
[    0.000000]  r4:0006ddd0 r3:c0d06500
[    0.000000] [<c08f96a4>] (schedule_hrtimeout_range_clock) from
[<c08f97f4>] (schedule_hrtimeout_range+0x24/0x2c)
[    0.000000]  r7:00000001 r6:c0d02100 r5:ffff8ad1 r4:ef004f80
[    0.000000] [<c08f97d0>] (schedule_hrtimeout_range) from
[<c08f922c>] (usleep_range+0x54/0x5c)
[    0.000000] [<c08f91d8>] (usleep_range) from [<c0698148>]
(clk_pllv3_wait_lock+0x7c/0xb4)
[    0.000000] [<c06980cc>] (clk_pllv3_wait_lock) from [<c06981b0>]
(clk_pllv3_prepare+0x30/0x34)
[    0.000000]  r6:ef007080 r5:ef00ef00 r4:ef007600 r3:000017c0
[    0.000000] [<c0698180>] (clk_pllv3_prepare) from [<c069203c>]
(clk_core_prepare+0x98/0xc4)
[    0.000000] [<c0691fa4>] (clk_core_prepare) from [<c069201c>]
(clk_core_prepare+0x78/0xc4)
[    0.000000]  r5:ef00ef00 r4:ef007900
[    0.000000] [<c0691fa4>] (clk_core_prepare) from [<c069201c>]
(clk_core_prepare+0x78/0xc4)
[    0.000000]  r5:ef00ef00 r4:ef00e800
[    0.000000] [<c0691fa4>] (clk_core_prepare) from [<c069201c>]
(clk_core_prepare+0x78/0xc4)
[    0.000000]  r5:ef00ef00 r4:ef00eb80
[    0.000000] [<c0691fa4>] (clk_core_prepare) from [<c069201c>]
(clk_core_prepare+0x78/0xc4)
[    0.000000]  r5:ef00ef00 r4:ef00ef00
[    0.000000] [<c0691fa4>] (clk_core_prepare) from [<c0692be8>]
(clk_core_prepare_enable+0x1c/0x5c)
[    0.000000]  r5:ef00ef00 r4:ef011f80
[    0.000000] [<c0692bcc>] (clk_core_prepare_enable) from
[<c0692eb4>] (__clk_set_parent_before+0x38/0x7c)
[    0.000000]  r5:ef00ef00 r4:ef011f80
[    0.000000] [<c0692e7c>] (__clk_set_parent_before) from
[<c0693004>] (clk_core_set_parent+0x10c/0x170)
[    0.000000]  r7:00000001 r6:ef00ef00 r5:00000000 r4:ef011f80
[    0.000000] [<c0692ef8>] (clk_core_set_parent) from [<c069308c>]
(clk_set_parent+0x24/0x28)
[    0.000000]  r8:00000003 r7:c0d5d434 r6:00000000 r5:ef02b440
r4:00000009 r3:c15603bc
[    0.000000] [<c0693068>] (clk_set_parent) from [<c0c4ade8>]
(imx7d_clocks_init+0x5f4c/0x5f8c)
[    0.000000] [<c0c44e9c>] (imx7d_clocks_init) from [<c0c30e00>]
(of_clk_init+0x10c/0x1d4)
[    0.000000]  r10:00000001 r9:c0d01f68 r8:00000000 r7:c0d01f60
r6:ef7e3d60 r5:ef004340
[    0.000000]  r4:00000002
[    0.000000] [<c0c30cf4>] (of_clk_init) from [<c0c048d4>]
(time_init+0x2c/0x38)
[    0.000000]  r10:efffcac0 r9:c0c5da48 r8:c0d76000 r7:c0d028c0
r6:ffffffff r5:c0d76000
[    0.000000]  r4:00000000
[    0.000000] [<c0c048a8>] (time_init) from [<c0c00b7c>]
(start_kernel+0x218/0x398)
[    0.000000] [<c0c00964>] (start_kernel) from [<8000807c>] (0x8000807c)
[    0.000000]  r10:00000000 r9:410fc075 r8:8000406a r7:c0d07e8c
r6:c0c5da44 r5:c0d0296c
[    0.000000]  r4:c0d76294
[    0.000000] Architected cp15 timer(s) running at 8.00MHz (virt).
[    0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff
max_cycles: 0x1d854df40, max_idle_ns: 440795202120 ns
[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at kernel/time/sched_clock.c:179
sched_clock_register+0x44/0x21c
[    0.000000] Modules linked in:
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W
4.6.0-rc1-00011-gee55b3d17805-dirty #779
[    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.000000] Backtrace:
[    0.000000] [<c010c42c>] (dump_backtrace) from [<c010c624>]
(show_stack+0x18/0x1c)
[    0.000000]  r6:60000053 r5:00000000 r4:c0d21f9c r3:00000000
[    0.000000] [<c010c60c>] (show_stack) from [<c03dfec8>]
(dump_stack+0xb4/0xe8)
[    0.000000] [<c03dfe14>] (dump_stack) from [<c0124ce8>] (__warn+0xd8/0x104)
[    0.000000]  r10:c0b55504 r9:c0c13a04 r8:000000b3 r7:00000009
r6:c0adb860 r5:00000000
[    0.000000]  r4:00000000 r3:00000000
[    0.000000] [<c0124c10>] (__warn) from [<c0124dc8>]
(warn_slowpath_null+0x28/0x30)
[    0.000000]  r9:6fbdeeb3 r8:07c96afc r7:c067a55c r6:00000038
r5:007a1200 r4:c0d164c0
[    0.000000] [<c0124da0>] (warn_slowpath_null) from [<c0c13a04>]
(sched_clock_register+0x44/0x21c)
[    0.000000] [<c0c139c0>] (sched_clock_register) from [<c0c2e148>]
(arch_timer_common_init+0x1c8/0x22c)
[    0.000000]  r9:6fbdeeb3 r8:07c96afc r7:c155ec88 r6:c0ae143c
r5:c0b5551c r4:c0d5bf00
[    0.000000] [<c0c2df80>] (arch_timer_common_init) from [<c0c2e438>]
(arch_timer_of_init+0x28c/0x2d8)
[    0.000000]  r10:efffcac0 r9:c0c5da48 r8:c0d76000 r7:c0d5bf00
r6:00000000 r5:c155ec88
[    0.000000]  r4:00000012
[    0.000000] [<c0c2e1ac>] (arch_timer_of_init) from [<c0c2de38>]
(clocksource_probe+0x50/0x90)
[    0.000000]  r8:c0d76000 r7:c0d028c0 r6:ffffffff r5:00000001
r4:ef7dae9c r3:c0c2e1ac
[    0.000000] [<c0c2dde8>] (clocksource_probe) from [<c0c048d8>]
(time_init+0x30/0x38)
[    0.000000]  r5:c0d76000 r4:00000000
[    0.000000] [<c0c048a8>] (time_init) from [<c0c00b7c>]
(start_kernel+0x218/0x398)
[    0.000000] [<c0c00964>] (start_kernel) from [<8000807c>] (0x8000807c)
[    0.000000]  r10:00000000 r9:410fc075 r8:8000406a r7:c0d07e8c
r6:c0c5da44 r5:c0d0296c
[    0.000000]  r4:c0d76294
[    0.000000] ---[ end trace cb88537fdc8fa202 ]---
[    0.000007] sched_clock: 56 bits at 8MHz, resolution 125ns, wraps
every 2199023255500ns
[    0.000022] Switching to timer-based delay loop, resolution 125ns
[    0.000575] Switching to timer-based delay loop, resolution 41ns
[    0.000587] ------------[ cut here ]------------
[    0.000604] WARNING: CPU: 0 PID: 0 at kernel/time/sched_clock.c:179
sched_clock_register+0x44/0x21c
[    0.000613] Modules linked in:
[    0.000629] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W
4.6.0-rc1-00011-gee55b3d17805-dirty #779
[    0.000638] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.000646] Backtrace:
[    0.000667] [<c010c42c>] (dump_backtrace) from [<c010c624>]
(show_stack+0x18/0x1c)
[    0.000676]  r6:60000053 r5:00000000 r4:c0d21f9c r3:00000000
[    0.000712] [<c010c60c>] (show_stack) from [<c03dfec8>]
(dump_stack+0xb4/0xe8)
[    0.000730] [<c03dfe14>] (dump_stack) from [<c0124ce8>] (__warn+0xd8/0x104)
[    0.000739]  r10:efffcac0 r9:c0c13a04 r8:000000b3 r7:00000009
r6:c0adb860 r5:00000000
[    0.000772]  r4:00000000 r3:00000000
[    0.000795] [<c0124c10>] (__warn) from [<c0124dc8>]
(warn_slowpath_null+0x28/0x30)
[    0.000805]  r9:c0c5da48 r8:00000003 r7:c067b098 r6:00000020
r5:016e3600 r4:c0d164c0
[    0.000846] [<c0124da0>] (warn_slowpath_null) from [<c0c13a04>]
(sched_clock_register+0x44/0x21c)
[    0.000866] [<c0c139c0>] (sched_clock_register) from [<c0c2eb5c>]
(_mxc_timer_init+0x144/0x240)
[    0.000875]  r9:c0c5da48 r8:00000003 r7:016e3600 r6:c155ece8
r5:f0880024 r4:ef002400
[    0.000919] [<c0c2ea18>] (_mxc_timer_init) from [<c0c2ed14>]
(mxc_timer_init_dt+0xbc/0xe8)
[    0.000928]  r7:00000000 r6:ef7e084c r5:ef002400 r4:c155ece8
[    0.000965] [<c0c2ec58>] (mxc_timer_init_dt) from [<c0c2ed84>]
(imx6dl_timer_init_dt+0x14/0x18)
[    0.000975]  r8:c0d76000 r7:c0d028c0 r6:ffffffff r5:00000002
r4:ef7e084c r3:c0c2ed70
[    0.001018] [<c0c2ed70>] (imx6dl_timer_init_dt) from [<c0c2de38>]
(clocksource_probe+0x50/0x90)
[    0.001035] [<c0c2dde8>] (clocksource_probe) from [<c0c048d8>]
(time_init+0x30/0x38)
[    0.001045]  r5:c0d76000 r4:00000000
[    0.001069] [<c0c048a8>] (time_init) from [<c0c00b7c>]
(start_kernel+0x218/0x398)
[    0.001083] [<c0c00964>] (start_kernel) from [<8000807c>] (0x8000807c)
[    0.001092]  r10:00000000 r9:410fc075 r8:8000406a r7:c0d07e8c
r6:c0c5da44 r5:c0d0296c
[    0.001125]  r4:c0d76294
[    0.001137] ---[ end trace cb88537fdc8fa203 ]---
[    0.001156] sched_clock: 32 bits at 24MHz, resolution 41ns, wraps
every 89478484971ns
[    0.001174] clocksource: mxc_timer1: mask: 0xffffffff max_cycles:
0xffffffff, max_idle_ns: 79635851949 ns
[    0.002055] ------------[ cut here ]------------
[    0.002073] WARNING: CPU: 0 PID: 0 at init/main.c:575
start_kernel+0x240/0x398
[    0.002082] Interrupts were enabled early
[    0.002090] Modules linked in:
[    0.002106] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W
4.6.0-rc1-00011-gee55b3d17805-dirty #779
[    0.002116] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.002124] Backtrace:
[    0.002145] [<c010c42c>] (dump_backtrace) from [<c010c624>]
(show_stack+0x18/0x1c)
[    0.002155]  r6:60000053 r5:00000000 r4:c0d21f9c r3:00000000
[    0.002191] [<c010c60c>] (show_stack) from [<c03dfec8>]
(dump_stack+0xb4/0xe8)
[    0.002209] [<c03dfe14>] (dump_stack) from [<c0124ce8>] (__warn+0xd8/0x104)
[    0.002219]  r10:efffcac0 r9:c0c00ba4 r8:0000023f r7:00000009
r6:c0acc620 r5:00000000
[    0.002252]  r4:c0d01f88 r3:00000000
[    0.002276] [<c0124c10>] (__warn) from [<c0124d54>]
(warn_slowpath_fmt+0x40/0x48)
[    0.002285]  r9:c0c5da48 r8:c0d76000 r7:c0d028c0 r6:ffffffff
r5:c0d76000 r4:c0acc660
[    0.002326] [<c0124d18>] (warn_slowpath_fmt) from [<c0c00ba4>]
(start_kernel+0x240/0x398)
[    0.002335]  r3:60000053 r2:c0acc660
[    0.002352]  r4:00000000
[    0.002368] [<c0c00964>] (start_kernel) from [<8000807c>] (0x8000807c)
[    0.002378]  r10:00000000 r9:410fc075 r8:8000406a r7:c0d07e8c
r6:c0c5da44 r5:c0d0296c
[    0.002411]  r4:c0d76294
[    0.002423] ---[ end trace cb88537fdc8fa204 ]---
[    0.002536] Console: colour dummy device 80x30
[    0.002550] Lock dependency validator: Copyright (c) 2006 Red Hat,
Inc., Ingo Molnar
[    0.002559] ... MAX_LOCKDEP_SUBCLASSES:  8
[    0.002568] ... MAX_LOCK_DEPTH:          48
[    0.002576] ... MAX_LOCKDEP_KEYS:        8191
[    0.002585] ... CLASSHASH_SIZE:          4096
[    0.002594] ... MAX_LOCKDEP_ENTRIES:     32768
[    0.002602] ... MAX_LOCKDEP_CHAINS:      65536
[    0.002611] ... CHAINHASH_SIZE:          32768
[    0.002620]  memory used by lock dependency info: 5167 kB
[    0.002629]  per task-struct memory footprint: 1536 bytes
[    0.002652] Calibrating delay loop (skipped), value calculated
using timer frequency.. 48.00 BogoMIPS (lpj=240000)
[    0.002669] pid_max: default: 32768 minimum: 301
[    0.002826] Mount-cache hash table entries: 2048 (order: 1, 8192 bytes)
[    0.002842] Mountpoint-cache hash table entries: 2048 (order: 1, 8192 bytes)
.....................

Regards
Dong Aisheng

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Fabio Estevam]

On Tue, Apr 26, 2016 at 11:45 PM, Dong Aisheng <dongas86@gmail.com> wrote:

>> We need to firstly understand why this is happening.  The .prepare hook
>> is defined to be non-atomic context, and so that we call sleep function
>> in there.  We did everything right.  Why are we getting the warning?  If
>> I'm correct, this warning only happens on i.MX7D.  Why is that?
>>
>
> This is mainly caused by during kernel early booting, there's only one init idle
> task running.
> See:
> void __init sched_init(void)
> {
> .....
>         /*
>          * Make us the idle thread. Technically, schedule() should not be
>          * called from this thread, however somewhere below it might be,
>          * but because we are the idle thread, we just pick up running again
>          * when this runqueue becomes "idle".
>          */
>         init_idle(current, smp_processor_id());
> ...
> }
>
> And the idle sched class indicates it's not valid to schedule for idle task.
> const struct sched_class idle_sched_class = {
>         /* .next is NULL */
>         /* no enqueue/yield_task for idle tasks */
>
>         /* dequeue is not valid, we print a debug message there: */
>         .dequeue_task           = dequeue_task_idle,
> ...........
>
> }
>
> /*
>  * It is not legal to sleep in the idle task - print a warning
>  * message if some code attempts to do it:
>  */
> static void
> dequeue_task_idle(struct rq *rq, struct task_struct *p, int flags)
> {
>         raw_spin_unlock_irq(&rq->lock);
>         printk(KERN_ERR "bad: scheduling from the idle thread!\n");
>         dump_stack();
>         raw_spin_lock_irq(&rq->lock);
> }
>
>
> Below is the full log of imx7d booting FYI.

This does not answer Shawn's question: why do we see this only on mx7d?

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Stefan Agner]

On 2016-04-26 19:56, Fabio Estevam wrote:
> On Tue, Apr 26, 2016 at 11:45 PM, Dong Aisheng <dongas86@gmail.com> wrote:
> 
>>> We need to firstly understand why this is happening.  The .prepare hook
>>> is defined to be non-atomic context, and so that we call sleep function
>>> in there.  We did everything right.  Why are we getting the warning?  If
>>> I'm correct, this warning only happens on i.MX7D.  Why is that?
>>>
>>
>> This is mainly caused by during kernel early booting, there's only one init idle
>> task running.
>> See:
>> void __init sched_init(void)
>> {
>> .....
>>         /*
>>          * Make us the idle thread. Technically, schedule() should not be
>>          * called from this thread, however somewhere below it might be,
>>          * but because we are the idle thread, we just pick up running again
>>          * when this runqueue becomes "idle".
>>          */
>>         init_idle(current, smp_processor_id());
>> ...
>> }
>>
>> And the idle sched class indicates it's not valid to schedule for idle task.
>> const struct sched_class idle_sched_class = {
>>         /* .next is NULL */
>>         /* no enqueue/yield_task for idle tasks */
>>
>>         /* dequeue is not valid, we print a debug message there: */
>>         .dequeue_task           = dequeue_task_idle,
>> ...........
>>
>> }
>>
>> /*
>>  * It is not legal to sleep in the idle task - print a warning
>>  * message if some code attempts to do it:
>>  */
>> static void
>> dequeue_task_idle(struct rq *rq, struct task_struct *p, int flags)
>> {
>>         raw_spin_unlock_irq(&rq->lock);
>>         printk(KERN_ERR "bad: scheduling from the idle thread!\n");
>>         dump_stack();
>>         raw_spin_lock_irq(&rq->lock);
>> }
>>
>>
>> Below is the full log of imx7d booting FYI.
> 
> This does not answer Shawn's question: why do we see this only on mx7d?

I was wondering that too.... My theory is that either on i.MX 6 the
clocks enable almost instantly leading to no sleep, or they are just
bootloader/firmware on...?

--
Stefan

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Dong Aisheng]

On Wed, Apr 27, 2016 at 3:28 PM, Stefan Agner <stefan@agner.ch> wrote:
> On 2016-04-26 19:56, Fabio Estevam wrote:
>> On Tue, Apr 26, 2016 at 11:45 PM, Dong Aisheng <dongas86@gmail.com> wrote:
>>
>>>> We need to firstly understand why this is happening.  The .prepare hook
>>>> is defined to be non-atomic context, and so that we call sleep function
>>>> in there.  We did everything right.  Why are we getting the warning?  If
>>>> I'm correct, this warning only happens on i.MX7D.  Why is that?
>>>>
>>>
>>> This is mainly caused by during kernel early booting, there's only one init idle
>>> task running.
>>> See:
>>> void __init sched_init(void)
>>> {
>>> .....
>>>         /*
>>>          * Make us the idle thread. Technically, schedule() should not be
>>>          * called from this thread, however somewhere below it might be,
>>>          * but because we are the idle thread, we just pick up running again
>>>          * when this runqueue becomes "idle".
>>>          */
>>>         init_idle(current, smp_processor_id());
>>> ...
>>> }
>>>
>>> And the idle sched class indicates it's not valid to schedule for idle task.
>>> const struct sched_class idle_sched_class = {
>>>         /* .next is NULL */
>>>         /* no enqueue/yield_task for idle tasks */
>>>
>>>         /* dequeue is not valid, we print a debug message there: */
>>>         .dequeue_task           = dequeue_task_idle,
>>> ...........
>>>
>>> }
>>>
>>> /*
>>>  * It is not legal to sleep in the idle task - print a warning
>>>  * message if some code attempts to do it:
>>>  */
>>> static void
>>> dequeue_task_idle(struct rq *rq, struct task_struct *p, int flags)
>>> {
>>>         raw_spin_unlock_irq(&rq->lock);
>>>         printk(KERN_ERR "bad: scheduling from the idle thread!\n");
>>>         dump_stack();
>>>         raw_spin_lock_irq(&rq->lock);
>>> }
>>>
>>>
>>> Below is the full log of imx7d booting FYI.
>>
>> This does not answer Shawn's question: why do we see this only on mx7d?
>
> I was wondering that too.... My theory is that either on i.MX 6 the
> clocks enable almost instantly leading to no sleep, or they are just
> bootloader/firmware on...?
>

Yes, for core plls, they're already enabled in bootloader.
We observed this issue on MX7D is because we rudely enable all clocks
for it and MX7D AV PLLs which will lead to sleep reveals this issue.

Regards
Dong Aisheng

> --
> Stefan

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Dong Aisheng]

On Wed, Apr 27, 2016 at 9:58 AM, Shawn Guo <shawnguo@kernel.org> wrote:
> On Tue, Apr 26, 2016 at 07:27:03PM +0800, Dong Aisheng wrote:
>> Shawn,
>> What's your suggestion?
>
> I think this needs more discussion, and I just dropped Stefan's patch
> from my tree.
>
> We need to firstly understand why this is happening.  The .prepare hook
> is defined to be non-atomic context, and so that we call sleep function
> in there.  We did everything right.  Why are we getting the warning?  If
> I'm correct, this warning only happens on i.MX7D.  Why is that?
>

Why Stefan's patch works (checking irqs_disabled()) is because during kernel
time init, the irq is still not enabled. It fixes the issue indirectly.
See:
asmlinkage __visible void __init start_kernel(void)
{
        /*
         * Set up the scheduler prior starting any interrupts (such as the
         * timer interrupt). Full topology setup happens at smp_init()
         * time - but meanwhile we still have a functioning scheduler.
         */
        sched_init();
        .............
        time_init();
        ..............
        WARN(!irqs_disabled(), "Interrupts were enabled early\n");
        early_boot_irqs_disabled = false;
        local_irq_enable();
}

The issue can only happen when PLL enable causes a schedule during
imx_clock_init().
Not all PLL has this issue.
The issue happens on MX7D pll_audio_main_clk/pll_video_main_clk
which requires more delay time and cause usleep.
Because clk framework does not support MX7D clock types (operation requires
parents on), we simply enable all clocks in imx7d_clocks_init().

If apply my this patch series:
https://lkml.org/lkml/2016/4/20/199
The issue can also be gone.

Regards
Dong Aisheng

> Shawn

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Shawn Guo]

On Wed, Apr 27, 2016 at 10:57:21AM +0800, Dong Aisheng wrote:
> On Wed, Apr 27, 2016 at 9:58 AM, Shawn Guo <shawnguo@kernel.org> wrote:
> > On Tue, Apr 26, 2016 at 07:27:03PM +0800, Dong Aisheng wrote:
> >> Shawn,
> >> What's your suggestion?
> >
> > I think this needs more discussion, and I just dropped Stefan's patch
> > from my tree.
> >
> > We need to firstly understand why this is happening.  The .prepare hook
> > is defined to be non-atomic context, and so that we call sleep function
> > in there.  We did everything right.  Why are we getting the warning?  If
> > I'm correct, this warning only happens on i.MX7D.  Why is that?
> >
> 
> Why Stefan's patch works (checking irqs_disabled()) is because during kernel
> time init, the irq is still not enabled. It fixes the issue indirectly.
> See:
> asmlinkage __visible void __init start_kernel(void)
> {
>         /*
>          * Set up the scheduler prior starting any interrupts (such as the
>          * timer interrupt). Full topology setup happens at smp_init()
>          * time - but meanwhile we still have a functioning scheduler.
>          */
>         sched_init();
>         .............
>         time_init();
>         ..............
>         WARN(!irqs_disabled(), "Interrupts were enabled early\n");
>         early_boot_irqs_disabled = false;
>         local_irq_enable();
> }
> 
> The issue can only happen when PLL enable causes a schedule during
> imx_clock_init().
> Not all PLL has this issue.
> The issue happens on MX7D pll_audio_main_clk/pll_video_main_clk
> which requires more delay time and cause usleep.
> Because clk framework does not support MX7D clock types (operation requires
> parents on), we simply enable all clocks in imx7d_clocks_init().
> 
> If apply my this patch series:
> https://lkml.org/lkml/2016/4/20/199
> The issue can also be gone.

Thanks for the info.  It sounds like that we are fixing the problem in
the wrong place, i.e. clk_pllv3_prepare().  The function does nothing
wrong, since the .prepare hook is defined to be one that can sleep.  If
we see sleep warning in a context calling clk_prepare(), that probably
means we shouldn't make the function call from that context.

Shawn

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Stefan Agner]

On 2016-04-27 00:24, Shawn Guo wrote:
> On Wed, Apr 27, 2016 at 10:57:21AM +0800, Dong Aisheng wrote:
>> On Wed, Apr 27, 2016 at 9:58 AM, Shawn Guo <shawnguo@kernel.org> wrote:
>> > On Tue, Apr 26, 2016 at 07:27:03PM +0800, Dong Aisheng wrote:
>> >> Shawn,
>> >> What's your suggestion?
>> >
>> > I think this needs more discussion, and I just dropped Stefan's patch
>> > from my tree.
>> >
>> > We need to firstly understand why this is happening.  The .prepare hook
>> > is defined to be non-atomic context, and so that we call sleep function
>> > in there.  We did everything right.  Why are we getting the warning?  If
>> > I'm correct, this warning only happens on i.MX7D.  Why is that?
>> >
>>
>> Why Stefan's patch works (checking irqs_disabled()) is because during kernel
>> time init, the irq is still not enabled. It fixes the issue indirectly.
>> See:
>> asmlinkage __visible void __init start_kernel(void)
>> {
>>         /*
>>          * Set up the scheduler prior starting any interrupts (such as the
>>          * timer interrupt). Full topology setup happens at smp_init()
>>          * time - but meanwhile we still have a functioning scheduler.
>>          */
>>         sched_init();
>>         .............
>>         time_init();
>>         ..............
>>         WARN(!irqs_disabled(), "Interrupts were enabled early\n");
>>         early_boot_irqs_disabled = false;
>>         local_irq_enable();
>> }
>>
>> The issue can only happen when PLL enable causes a schedule during
>> imx_clock_init().
>> Not all PLL has this issue.
>> The issue happens on MX7D pll_audio_main_clk/pll_video_main_clk
>> which requires more delay time and cause usleep.
>> Because clk framework does not support MX7D clock types (operation requires
>> parents on), we simply enable all clocks in imx7d_clocks_init().
>>
>> If apply my this patch series:
>> https://lkml.org/lkml/2016/4/20/199
>> The issue can also be gone.
> 
> Thanks for the info.  It sounds like that we are fixing the problem in
> the wrong place, i.e. clk_pllv3_prepare().  The function does nothing
> wrong, since the .prepare hook is defined to be one that can sleep.  If
> we see sleep warning in a context calling clk_prepare(), that probably
> means we shouldn't make the function call from that context.
> 

According to the stack trace in the answer to Stephens question the call
comes from imx7d_clocks_init. I doubt we can avoid that those clocks get
enabled there...

--
Stefan

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Dong Aisheng]

On Wed, Apr 27, 2016 at 3:24 PM, Shawn Guo <shawnguo@kernel.org> wrote:
> On Wed, Apr 27, 2016 at 10:57:21AM +0800, Dong Aisheng wrote:
>> On Wed, Apr 27, 2016 at 9:58 AM, Shawn Guo <shawnguo@kernel.org> wrote:
>> > On Tue, Apr 26, 2016 at 07:27:03PM +0800, Dong Aisheng wrote:
>> >> Shawn,
>> >> What's your suggestion?
>> >
>> > I think this needs more discussion, and I just dropped Stefan's patch
>> > from my tree.
>> >
>> > We need to firstly understand why this is happening.  The .prepare hook
>> > is defined to be non-atomic context, and so that we call sleep function
>> > in there.  We did everything right.  Why are we getting the warning?  If
>> > I'm correct, this warning only happens on i.MX7D.  Why is that?
>> >
>>
>> Why Stefan's patch works (checking irqs_disabled()) is because during kernel
>> time init, the irq is still not enabled. It fixes the issue indirectly.
>> See:
>> asmlinkage __visible void __init start_kernel(void)
>> {
>>         /*
>>          * Set up the scheduler prior starting any interrupts (such as the
>>          * timer interrupt). Full topology setup happens at smp_init()
>>          * time - but meanwhile we still have a functioning scheduler.
>>          */
>>         sched_init();
>>         .............
>>         time_init();
>>         ..............
>>         WARN(!irqs_disabled(), "Interrupts were enabled early\n");
>>         early_boot_irqs_disabled = false;
>>         local_irq_enable();
>> }
>>
>> The issue can only happen when PLL enable causes a schedule during
>> imx_clock_init().
>> Not all PLL has this issue.
>> The issue happens on MX7D pll_audio_main_clk/pll_video_main_clk
>> which requires more delay time and cause usleep.
>> Because clk framework does not support MX7D clock types (operation requires
>> parents on), we simply enable all clocks in imx7d_clocks_init().
>>
>> If apply my this patch series:
>> https://lkml.org/lkml/2016/4/20/199
>> The issue can also be gone.
>
> Thanks for the info.  It sounds like that we are fixing the problem in
> the wrong place, i.e. clk_pllv3_prepare().  The function does nothing
> wrong, since the .prepare hook is defined to be one that can sleep.  If
> we see sleep warning in a context calling clk_prepare(), that probably
> means we shouldn't make the function call from that context.
>

Yes, i agree.
I'm trying to get rid of these calls.

Simply remove them or delay to arch_init will cause kernel fail to boot.
Still checking the root cause.

> Shawn

Regards
Dong Aisheng

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Stefan Agner]

On 2016-04-26 19:57, Dong Aisheng wrote:
> On Wed, Apr 27, 2016 at 9:58 AM, Shawn Guo <shawnguo@kernel.org> wrote:
>> On Tue, Apr 26, 2016 at 07:27:03PM +0800, Dong Aisheng wrote:
>>> Shawn,
>>> What's your suggestion?
>>
>> I think this needs more discussion, and I just dropped Stefan's patch
>> from my tree.
>>
>> We need to firstly understand why this is happening.  The .prepare hook
>> is defined to be non-atomic context, and so that we call sleep function
>> in there.  We did everything right.  Why are we getting the warning?  If
>> I'm correct, this warning only happens on i.MX7D.  Why is that?
>>
> 
> Why Stefan's patch works (checking irqs_disabled()) is because during kernel
> time init, the irq is still not enabled. It fixes the issue indirectly.
> See:
> asmlinkage __visible void __init start_kernel(void)
> {
>         /*
>          * Set up the scheduler prior starting any interrupts (such as the
>          * timer interrupt). Full topology setup happens at smp_init()
>          * time - but meanwhile we still have a functioning scheduler.
>          */
>         sched_init();
>         .............
>         time_init();
>         ..............
>         WARN(!irqs_disabled(), "Interrupts were enabled early\n");
>         early_boot_irqs_disabled = false;
>         local_irq_enable();
> }
> 
> The issue can only happen when PLL enable causes a schedule during
> imx_clock_init().
> Not all PLL has this issue.
> The issue happens on MX7D pll_audio_main_clk/pll_video_main_clk
> which requires more delay time and cause usleep.
> Because clk framework does not support MX7D clock types (operation requires
> parents on), we simply enable all clocks in imx7d_clocks_init().
> 
> If apply my this patch series:
> https://lkml.org/lkml/2016/4/20/199
> The issue can also be gone.

Oh ok, it does make sense to enable as few clocks as possible.

However, even if we do not enable lots of clocks at that time, and this
helps to avoid the problem for now, it could still be that
someone/something requests a clock early during boot, leading to a PLL
enable... Shouldn't we make sure that those base clocks can be enabled
even during early boot..?

--
Stefan

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Dong Aisheng]

On Wed, Apr 27, 2016 at 3:34 PM, Stefan Agner <stefan@agner.ch> wrote:
> On 2016-04-26 19:57, Dong Aisheng wrote:
>> On Wed, Apr 27, 2016 at 9:58 AM, Shawn Guo <shawnguo@kernel.org> wrote:
>>> On Tue, Apr 26, 2016 at 07:27:03PM +0800, Dong Aisheng wrote:
>>>> Shawn,
>>>> What's your suggestion?
>>>
>>> I think this needs more discussion, and I just dropped Stefan's patch
>>> from my tree.
>>>
>>> We need to firstly understand why this is happening.  The .prepare hook
>>> is defined to be non-atomic context, and so that we call sleep function
>>> in there.  We did everything right.  Why are we getting the warning?  If
>>> I'm correct, this warning only happens on i.MX7D.  Why is that?
>>>
>>
>> Why Stefan's patch works (checking irqs_disabled()) is because during kernel
>> time init, the irq is still not enabled. It fixes the issue indirectly.
>> See:
>> asmlinkage __visible void __init start_kernel(void)
>> {
>>         /*
>>          * Set up the scheduler prior starting any interrupts (such as the
>>          * timer interrupt). Full topology setup happens at smp_init()
>>          * time - but meanwhile we still have a functioning scheduler.
>>          */
>>         sched_init();
>>         .............
>>         time_init();
>>         ..............
>>         WARN(!irqs_disabled(), "Interrupts were enabled early\n");
>>         early_boot_irqs_disabled = false;
>>         local_irq_enable();
>> }
>>
>> The issue can only happen when PLL enable causes a schedule during
>> imx_clock_init().
>> Not all PLL has this issue.
>> The issue happens on MX7D pll_audio_main_clk/pll_video_main_clk
>> which requires more delay time and cause usleep.
>> Because clk framework does not support MX7D clock types (operation requires
>> parents on), we simply enable all clocks in imx7d_clocks_init().
>>
>> If apply my this patch series:
>> https://lkml.org/lkml/2016/4/20/199
>> The issue can also be gone.
>
> Oh ok, it does make sense to enable as few clocks as possible.
>
> However, even if we do not enable lots of clocks at that time, and this
> helps to avoid the problem for now, it could still be that
> someone/something requests a clock early during boot, leading to a PLL
> enable... Shouldn't we make sure that those base clocks can be enabled
> even during early boot..?
>

The point as Shawn pointed is that we even shouldn't call clk_prepare_enable
at that time.
So we may first try to eliminate those callings in imx7d_clocks_init.

> --
> Stefan

Regards
Dong Aisheng

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Thomas Gleixner]

On Wed, 27 Apr 2016, Dong Aisheng wrote:
> On Wed, Apr 27, 2016 at 9:58 AM, Shawn Guo <shawnguo@kernel.org> wrote:
> > On Tue, Apr 26, 2016 at 07:27:03PM +0800, Dong Aisheng wrote:
> >> Shawn,
> >> What's your suggestion?
> >
> > I think this needs more discussion, and I just dropped Stefan's patch
> > from my tree.
> >
> > We need to firstly understand why this is happening.  The .prepare hook
> > is defined to be non-atomic context, and so that we call sleep function
> > in there.  We did everything right.  Why are we getting the warning?  If
> > I'm correct, this warning only happens on i.MX7D.  Why is that?
> >
> 
> Why Stefan's patch works (checking irqs_disabled()) is because during kernel
> time init, the irq is still not enabled. It fixes the issue indirectly.
> See:
> asmlinkage __visible void __init start_kernel(void)
> {
>         /*
>          * Set up the scheduler prior starting any interrupts (such as the
>          * timer interrupt). Full topology setup happens at smp_init()
>          * time - but meanwhile we still have a functioning scheduler.
>          */
>         sched_init();
>         .............
>         time_init();
>         ..............
>         WARN(!irqs_disabled(), "Interrupts were enabled early\n");
>         early_boot_irqs_disabled = false;
>         local_irq_enable();
> }
> 
> The issue can only happen when PLL enable causes a schedule during
> imx_clock_init().

Calling a function which might sleep _BEFORE_ kernel_init() is wrong. Don't
try to work around such an issue by doing magic irq_disabled() checks and busy
loops. Fix the call site and be done with it.

Thanks,

	tglx

[RFC PATCH 1/1] clk: imx7d: move clk setting out of imx7d_clocks_init

[Dong Aisheng]

During kernel early booting(e.g. in time_init()), there's only one
init idle task running, and the idle sched class indicates that it's
not valid to schedule for idle task. If it happens the kernel
will complain with a error message as follows:
[    0.000000] bad: scheduling from the idle thread!

We can observe this warning on an i.MX7D SDB board. See full log below.
It is caused by imx7d_clocks_init function called in time_init
invokes a lot clk_prepare_enable to enable many clocks and it happens
that the Audio/Video PLLs need large delay causes a sleep.

Since we should not sleep during time_init, this patch fundamentally
moves all clk_prepare_enable and clk_set_parent out of imx7d_clocks_init
and use a postcore init function imx7d_clocks_setup to do it later instead.
Then we simply reply on the bootloader settings to do early boot.

[    0.000000] bad: scheduling from the idle thread!
[    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W       4.6.0-rc3-00064-gded8bc08fb45-dirty #836
[    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
[    0.000000] Backtrace:
[    0.000000] [<c010c42c>] (dump_backtrace) from [<c010c624>] (show_stack+0x18/0x1c)
[    0.000000]  r6:60000053 r5:00000000 r4:c0d21f9c r3:00000000
[    0.000000] [<c010c60c>] (show_stack) from [<c03e0228>] (dump_stack+0xb4/0xe8)
[    0.000000] [<c03e0174>] (dump_stack) from [<c0154388>] (dequeue_task_idle+0x20/0x30)
[    0.000000]  r10:c08f5554 r9:c0d02b0c r8:c0d06844 r7:ef7c14d0 r6:00000001 r5:ef7c14c0
[    0.000000]  r4:ef7c14c0 r3:00000000
[    0.000000] [<c0154368>] (dequeue_task_idle) from [<c014c968>] (deactivate_task+0x64/0x68)
[    0.000000]  r4:c0d06500 r3:c0154368
[    0.000000] [<c014c904>] (deactivate_task) from [<c08f50bc>] (__schedule+0x2e8/0x738)
[    0.000000]  r6:c0d06500 r5:ef7c14c0 r4:c0c744c0 r3:00000002
[    0.000000] [<c08f4dd4>] (__schedule) from [<c08f5554>] (schedule+0x48/0xa0)
[    0.000000]  r10:000001b6 r9:00000003 r8:00000036 r7:00000000 r6:0007a120 r5:00000000
[    0.000000]  r4:c0d00000
[    0.000000] [<c08f550c>] (schedule) from [<c08f9cc8>] (schedule_hrtimeout_range_clock+0xac/0x12c)
[    0.000000]  r4:0006ddd0 r3:c0d06500
[    0.000000] [<c08f9c1c>] (schedule_hrtimeout_range_clock) from [<c08f9d6c>] (schedule_hrtimeout_range+0x24/0x2c)
[    0.000000]  r7:c0d5d434 r6:c0d02100 r5:ffff8ad1 r4:ef00c040
[    0.000000] [<c08f9d48>] (schedule_hrtimeout_range) from [<c08f97a4>] (usleep_range+0x54/0x5c)
[    0.000000] [<c08f9750>] (usleep_range) from [<c06984c8>] (clk_pllv3_wait_lock+0x7c/0xb4)
[    0.000000] [<c069844c>] (clk_pllv3_wait_lock) from [<c069852c>] (clk_pllv3_prepare+0x2c/0x30)
[    0.000000]  r6:00000000 r5:c15603f4 r4:ef007680 r3:0000201b
[    0.000000] [<c0698500>] (clk_pllv3_prepare) from [<c0691eb8>] (clk_core_prepare+0x98/0xc8)
[    0.000000] [<c0691e20>] (clk_core_prepare) from [<c06923d4>] (clk_prepare+0x20/0x38)
[    0.000000]  r5:c15603f4 r4:ef00c100
[    0.000000] [<c06923b4>] (clk_prepare) from [<c0c4a55c>] (imx7d_clocks_init+0x6108/0x6188)
[    0.000000]  r4:ef00c100 r3:00000001
[    0.000000] [<c0c44454>] (imx7d_clocks_init) from [<c0c30e38>] (of_clk_init+0x10c/0x1d4)
[    0.000000]  r10:00000001 r9:c0d01f68 r8:00000000 r7:c0d01f60 r6:ef7e3d60 r5:ef004340
[    0.000000]  r4:00000002
[    0.000000] [<c0c30d2c>] (of_clk_init) from [<c0c048b0>] (time_init+0x2c/0x38)
[    0.000000]  r10:efffcac0 r9:c0c5ca48 r8:c0d76000 r7:c0d028c0 r6:ffffffff r5:c0d76000
[    0.000000]  r4:00000000
[    0.000000] [<c0c04884>] (time_init) from [<c0c00b7c>] (start_kernel+0x218/0x398)
[    0.000000] [<c0c00964>] (start_kernel) from [<8000807c>] (0x8000807c)
[    0.000000]  r10:00000000 r9:410fc075 r8:8000406a r7:c0d07e8c r6:c0c5ca44 r5:c0d0296c
[    0.000000]  r4:c0d76294
[    0.000000] Architected cp15 timer(s) running at 8.00MHz (virt).
[    0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff max_cycles: 0x1d854df40, max_idle_ns: 440795202120 ns

Cc: Michael Turquette <mturquette@baylibre.com>
Cc: Stephen Boyd <sboyd@codeaurora.org>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Stefan Agner <stefan@agner.ch>
Cc: Lucas Stach <l.stach@pengutronix.de>
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
---
 drivers/clk/imx/clk-imx7d.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
index 7912be83c4af..3be2e9371491 100644
--- a/drivers/clk/imx/clk-imx7d.c
+++ b/drivers/clk/imx/clk-imx7d.c
@@ -378,7 +378,6 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
 {
 	struct device_node *np;
 	void __iomem *base;
-	int i;
 
 	clks[IMX7D_CLK_DUMMY] = imx_clk_fixed("dummy", 0);
 	clks[IMX7D_OSC_24M_CLK] = of_clk_get_by_name(ccm_node, "osc");
@@ -409,13 +408,6 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
 	clks[IMX7D_PLL_AUDIO_MAIN_BYPASS] = imx_clk_mux_flags("pll_audio_main_bypass", base + 0xf0, 16, 1, pll_audio_bypass_sel, ARRAY_SIZE(pll_audio_bypass_sel), CLK_SET_RATE_PARENT);
 	clks[IMX7D_PLL_VIDEO_MAIN_BYPASS] = imx_clk_mux_flags("pll_video_main_bypass", base + 0x130, 16, 1, pll_video_bypass_sel, ARRAY_SIZE(pll_video_bypass_sel), CLK_SET_RATE_PARENT);
 
-	clk_set_parent(clks[IMX7D_PLL_ARM_MAIN_BYPASS], clks[IMX7D_PLL_ARM_MAIN]);
-	clk_set_parent(clks[IMX7D_PLL_DRAM_MAIN_BYPASS], clks[IMX7D_PLL_DRAM_MAIN]);
-	clk_set_parent(clks[IMX7D_PLL_SYS_MAIN_BYPASS], clks[IMX7D_PLL_SYS_MAIN]);
-	clk_set_parent(clks[IMX7D_PLL_ENET_MAIN_BYPASS], clks[IMX7D_PLL_ENET_MAIN]);
-	clk_set_parent(clks[IMX7D_PLL_AUDIO_MAIN_BYPASS], clks[IMX7D_PLL_AUDIO_MAIN]);
-	clk_set_parent(clks[IMX7D_PLL_VIDEO_MAIN_BYPASS], clks[IMX7D_PLL_VIDEO_MAIN]);
-
 	clks[IMX7D_PLL_ARM_MAIN_CLK] = imx_clk_gate("pll_arm_main_clk", "pll_arm_main_bypass", base + 0x60, 13);
 	clks[IMX7D_PLL_DRAM_MAIN_CLK] = imx_clk_gate("pll_dram_main_clk", "pll_dram_main_bypass", base + 0x70, 13);
 	clks[IMX7D_PLL_SYS_MAIN_CLK] = imx_clk_gate("pll_sys_main_clk", "pll_sys_main_bypass", base + 0xb0, 13);
@@ -846,6 +838,13 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
 	clk_data.clk_num = ARRAY_SIZE(clks);
 	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
 
+}
+CLK_OF_DECLARE(imx7d, "fsl,imx7d-ccm", imx7d_clocks_init);
+
+static int __init imx7d_clocks_setup(void)
+{
+	int i;
+
 	/* TO BE FIXED LATER
 	 * Enable all clock to bring up imx7, otherwise system will be halt and block
 	 * the other part upstream Because imx7d clock design changed, clock framework
@@ -855,6 +854,13 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
 	for (i = 0; i < IMX7D_CLK_END; i++)
 		clk_prepare_enable(clks[i]);
 
+	clk_set_parent(clks[IMX7D_PLL_ARM_MAIN_BYPASS], clks[IMX7D_PLL_ARM_MAIN]);
+	clk_set_parent(clks[IMX7D_PLL_DRAM_MAIN_BYPASS], clks[IMX7D_PLL_DRAM_MAIN]);
+	clk_set_parent(clks[IMX7D_PLL_SYS_MAIN_BYPASS], clks[IMX7D_PLL_SYS_MAIN]);
+	clk_set_parent(clks[IMX7D_PLL_ENET_MAIN_BYPASS], clks[IMX7D_PLL_ENET_MAIN]);
+	clk_set_parent(clks[IMX7D_PLL_AUDIO_MAIN_BYPASS], clks[IMX7D_PLL_AUDIO_MAIN]);
+	clk_set_parent(clks[IMX7D_PLL_VIDEO_MAIN_BYPASS], clks[IMX7D_PLL_VIDEO_MAIN]);
+
 	/* use old gpt clk setting, gpt1 root clk must be twice as gpt counter freq */
 	clk_set_parent(clks[IMX7D_GPT1_ROOT_SRC], clks[IMX7D_OSC_24M_CLK]);
 
@@ -874,5 +880,6 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
 
 	imx_register_uart_clocks(uart_clks);
 
+	return 0;
 }
-CLK_OF_DECLARE(imx7d, "fsl,imx7d-ccm", imx7d_clocks_init);
+postcore_initcall(imx7d_clocks_setup);
-- 
1.9.1

Re: [RFC PATCH 1/1] clk: imx7d: move clk setting out of imx7d_clocks_init

[Dong Aisheng]

On Fri, Apr 29, 2016 at 05:45:15PM +0800, Dong Aisheng wrote:
> During kernel early booting(e.g. in time_init()), there's only one
> init idle task running, and the idle sched class indicates that it's
> not valid to schedule for idle task. If it happens the kernel
> will complain with a error message as follows:
> [    0.000000] bad: scheduling from the idle thread!
> 
> We can observe this warning on an i.MX7D SDB board. See full log below.
> It is caused by imx7d_clocks_init function called in time_init
> invokes a lot clk_prepare_enable to enable many clocks and it happens
> that the Audio/Video PLLs need large delay causes a sleep.
> 
> Since we should not sleep during time_init, this patch fundamentally
> moves all clk_prepare_enable and clk_set_parent out of imx7d_clocks_init
> and use a postcore init function imx7d_clocks_setup to do it later instead.
> Then we simply reply on the bootloader settings to do early boot.
> 

Hi Shawn,

This is a draft RFC patch. Pls help check if you can accept
this way. If yes, i can also convert other platforms.

BTW, one know issue is we may need check if need extra fix
for imx_register_uart_clocks(uart_clks) separately in case
the uart clocks are not enabled in bootloader.

Regards
Dong Aisheng

> [    0.000000] bad: scheduling from the idle thread!
> [    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W       4.6.0-rc3-00064-gded8bc08fb45-dirty #836
> [    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
> [    0.000000] Backtrace:
> [    0.000000] [<c010c42c>] (dump_backtrace) from [<c010c624>] (show_stack+0x18/0x1c)
> [    0.000000]  r6:60000053 r5:00000000 r4:c0d21f9c r3:00000000
> [    0.000000] [<c010c60c>] (show_stack) from [<c03e0228>] (dump_stack+0xb4/0xe8)
> [    0.000000] [<c03e0174>] (dump_stack) from [<c0154388>] (dequeue_task_idle+0x20/0x30)
> [    0.000000]  r10:c08f5554 r9:c0d02b0c r8:c0d06844 r7:ef7c14d0 r6:00000001 r5:ef7c14c0
> [    0.000000]  r4:ef7c14c0 r3:00000000
> [    0.000000] [<c0154368>] (dequeue_task_idle) from [<c014c968>] (deactivate_task+0x64/0x68)
> [    0.000000]  r4:c0d06500 r3:c0154368
> [    0.000000] [<c014c904>] (deactivate_task) from [<c08f50bc>] (__schedule+0x2e8/0x738)
> [    0.000000]  r6:c0d06500 r5:ef7c14c0 r4:c0c744c0 r3:00000002
> [    0.000000] [<c08f4dd4>] (__schedule) from [<c08f5554>] (schedule+0x48/0xa0)
> [    0.000000]  r10:000001b6 r9:00000003 r8:00000036 r7:00000000 r6:0007a120 r5:00000000
> [    0.000000]  r4:c0d00000
> [    0.000000] [<c08f550c>] (schedule) from [<c08f9cc8>] (schedule_hrtimeout_range_clock+0xac/0x12c)
> [    0.000000]  r4:0006ddd0 r3:c0d06500
> [    0.000000] [<c08f9c1c>] (schedule_hrtimeout_range_clock) from [<c08f9d6c>] (schedule_hrtimeout_range+0x24/0x2c)
> [    0.000000]  r7:c0d5d434 r6:c0d02100 r5:ffff8ad1 r4:ef00c040
> [    0.000000] [<c08f9d48>] (schedule_hrtimeout_range) from [<c08f97a4>] (usleep_range+0x54/0x5c)
> [    0.000000] [<c08f9750>] (usleep_range) from [<c06984c8>] (clk_pllv3_wait_lock+0x7c/0xb4)
> [    0.000000] [<c069844c>] (clk_pllv3_wait_lock) from [<c069852c>] (clk_pllv3_prepare+0x2c/0x30)
> [    0.000000]  r6:00000000 r5:c15603f4 r4:ef007680 r3:0000201b
> [    0.000000] [<c0698500>] (clk_pllv3_prepare) from [<c0691eb8>] (clk_core_prepare+0x98/0xc8)
> [    0.000000] [<c0691e20>] (clk_core_prepare) from [<c06923d4>] (clk_prepare+0x20/0x38)
> [    0.000000]  r5:c15603f4 r4:ef00c100
> [    0.000000] [<c06923b4>] (clk_prepare) from [<c0c4a55c>] (imx7d_clocks_init+0x6108/0x6188)
> [    0.000000]  r4:ef00c100 r3:00000001
> [    0.000000] [<c0c44454>] (imx7d_clocks_init) from [<c0c30e38>] (of_clk_init+0x10c/0x1d4)
> [    0.000000]  r10:00000001 r9:c0d01f68 r8:00000000 r7:c0d01f60 r6:ef7e3d60 r5:ef004340
> [    0.000000]  r4:00000002
> [    0.000000] [<c0c30d2c>] (of_clk_init) from [<c0c048b0>] (time_init+0x2c/0x38)
> [    0.000000]  r10:efffcac0 r9:c0c5ca48 r8:c0d76000 r7:c0d028c0 r6:ffffffff r5:c0d76000
> [    0.000000]  r4:00000000
> [    0.000000] [<c0c04884>] (time_init) from [<c0c00b7c>] (start_kernel+0x218/0x398)
> [    0.000000] [<c0c00964>] (start_kernel) from [<8000807c>] (0x8000807c)
> [    0.000000]  r10:00000000 r9:410fc075 r8:8000406a r7:c0d07e8c r6:c0c5ca44 r5:c0d0296c
> [    0.000000]  r4:c0d76294
> [    0.000000] Architected cp15 timer(s) running at 8.00MHz (virt).
> [    0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff max_cycles: 0x1d854df40, max_idle_ns: 440795202120 ns
> 
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
>  drivers/clk/imx/clk-imx7d.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
> index 7912be83c4af..3be2e9371491 100644
> --- a/drivers/clk/imx/clk-imx7d.c
> +++ b/drivers/clk/imx/clk-imx7d.c
> @@ -378,7 +378,6 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
>  {
>  	struct device_node *np;
>  	void __iomem *base;
> -	int i;
>  
>  	clks[IMX7D_CLK_DUMMY] = imx_clk_fixed("dummy", 0);
>  	clks[IMX7D_OSC_24M_CLK] = of_clk_get_by_name(ccm_node, "osc");
> @@ -409,13 +408,6 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
>  	clks[IMX7D_PLL_AUDIO_MAIN_BYPASS] = imx_clk_mux_flags("pll_audio_main_bypass", base + 0xf0, 16, 1, pll_audio_bypass_sel, ARRAY_SIZE(pll_audio_bypass_sel), CLK_SET_RATE_PARENT);
>  	clks[IMX7D_PLL_VIDEO_MAIN_BYPASS] = imx_clk_mux_flags("pll_video_main_bypass", base + 0x130, 16, 1, pll_video_bypass_sel, ARRAY_SIZE(pll_video_bypass_sel), CLK_SET_RATE_PARENT);
>  
> -	clk_set_parent(clks[IMX7D_PLL_ARM_MAIN_BYPASS], clks[IMX7D_PLL_ARM_MAIN]);
> -	clk_set_parent(clks[IMX7D_PLL_DRAM_MAIN_BYPASS], clks[IMX7D_PLL_DRAM_MAIN]);
> -	clk_set_parent(clks[IMX7D_PLL_SYS_MAIN_BYPASS], clks[IMX7D_PLL_SYS_MAIN]);
> -	clk_set_parent(clks[IMX7D_PLL_ENET_MAIN_BYPASS], clks[IMX7D_PLL_ENET_MAIN]);
> -	clk_set_parent(clks[IMX7D_PLL_AUDIO_MAIN_BYPASS], clks[IMX7D_PLL_AUDIO_MAIN]);
> -	clk_set_parent(clks[IMX7D_PLL_VIDEO_MAIN_BYPASS], clks[IMX7D_PLL_VIDEO_MAIN]);
> -
>  	clks[IMX7D_PLL_ARM_MAIN_CLK] = imx_clk_gate("pll_arm_main_clk", "pll_arm_main_bypass", base + 0x60, 13);
>  	clks[IMX7D_PLL_DRAM_MAIN_CLK] = imx_clk_gate("pll_dram_main_clk", "pll_dram_main_bypass", base + 0x70, 13);
>  	clks[IMX7D_PLL_SYS_MAIN_CLK] = imx_clk_gate("pll_sys_main_clk", "pll_sys_main_bypass", base + 0xb0, 13);
> @@ -846,6 +838,13 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
>  	clk_data.clk_num = ARRAY_SIZE(clks);
>  	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
>  
> +}
> +CLK_OF_DECLARE(imx7d, "fsl,imx7d-ccm", imx7d_clocks_init);
> +
> +static int __init imx7d_clocks_setup(void)
> +{
> +	int i;
> +
>  	/* TO BE FIXED LATER
>  	 * Enable all clock to bring up imx7, otherwise system will be halt and block
>  	 * the other part upstream Because imx7d clock design changed, clock framework
> @@ -855,6 +854,13 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
>  	for (i = 0; i < IMX7D_CLK_END; i++)
>  		clk_prepare_enable(clks[i]);
>  
> +	clk_set_parent(clks[IMX7D_PLL_ARM_MAIN_BYPASS], clks[IMX7D_PLL_ARM_MAIN]);
> +	clk_set_parent(clks[IMX7D_PLL_DRAM_MAIN_BYPASS], clks[IMX7D_PLL_DRAM_MAIN]);
> +	clk_set_parent(clks[IMX7D_PLL_SYS_MAIN_BYPASS], clks[IMX7D_PLL_SYS_MAIN]);
> +	clk_set_parent(clks[IMX7D_PLL_ENET_MAIN_BYPASS], clks[IMX7D_PLL_ENET_MAIN]);
> +	clk_set_parent(clks[IMX7D_PLL_AUDIO_MAIN_BYPASS], clks[IMX7D_PLL_AUDIO_MAIN]);
> +	clk_set_parent(clks[IMX7D_PLL_VIDEO_MAIN_BYPASS], clks[IMX7D_PLL_VIDEO_MAIN]);
> +
>  	/* use old gpt clk setting, gpt1 root clk must be twice as gpt counter freq */
>  	clk_set_parent(clks[IMX7D_GPT1_ROOT_SRC], clks[IMX7D_OSC_24M_CLK]);
>  
> @@ -874,5 +880,6 @@ static void __init imx7d_clocks_init(struct device_node *ccm_node)
>  
>  	imx_register_uart_clocks(uart_clks);
>  
> +	return 0;
>  }
> -CLK_OF_DECLARE(imx7d, "fsl,imx7d-ccm", imx7d_clocks_init);
> +postcore_initcall(imx7d_clocks_setup);
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH 1/1] clk: imx7d: move clk setting out of imx7d_clocks_init

[Lucas Stach]

Am Freitag, den 29.04.2016, 17:55 +0800 schrieb Dong Aisheng:
> On Fri, Apr 29, 2016 at 05:45:15PM +0800, Dong Aisheng wrote:
> > During kernel early booting(e.g. in time_init()), there's only one
> > init idle task running, and the idle sched class indicates that it's
> > not valid to schedule for idle task. If it happens the kernel
> > will complain with a error message as follows:
> > [    0.000000] bad: scheduling from the idle thread!
> > 
> > We can observe this warning on an i.MX7D SDB board. See full log below.
> > It is caused by imx7d_clocks_init function called in time_init
> > invokes a lot clk_prepare_enable to enable many clocks and it happens
> > that the Audio/Video PLLs need large delay causes a sleep.
> > 
> > Since we should not sleep during time_init, this patch fundamentally
> > moves all clk_prepare_enable and clk_set_parent out of imx7d_clocks_init
> > and use a postcore init function imx7d_clocks_setup to do it later instead.
> > Then we simply reply on the bootloader settings to do early boot.
> > 
> 
> Hi Shawn,
> 
> This is a draft RFC patch. Pls help check if you can accept
> this way. If yes, i can also convert other platforms.
> 
> BTW, one know issue is we may need check if need extra fix
> for imx_register_uart_clocks(uart_clks) separately in case
> the uart clocks are not enabled in bootloader.

The sole purpose of imx_register_uart_clocks() is to keep earlycon
clocks enabled. Earlycon already operates under the assumption that the
relevant clocks are enabled by the bootloader as earlycon must work
before the clock driver is initialized. So there is no problem here to
fix here.

Regards,
Lucas

Re: [RFC PATCH 1/1] clk: imx7d: move clk setting out of imx7d_clocks_init

[Stefan Agner]

On 2016-04-29 02:45, Dong Aisheng wrote:
> During kernel early booting(e.g. in time_init()), there's only one
> init idle task running, and the idle sched class indicates that it's
> not valid to schedule for idle task. If it happens the kernel
> will complain with a error message as follows:
> [    0.000000] bad: scheduling from the idle thread!
> 
> We can observe this warning on an i.MX7D SDB board. See full log below.
> It is caused by imx7d_clocks_init function called in time_init
> invokes a lot clk_prepare_enable to enable many clocks and it happens
> that the Audio/Video PLLs need large delay causes a sleep.
> 
> Since we should not sleep during time_init, this patch fundamentally
> moves all clk_prepare_enable and clk_set_parent out of imx7d_clocks_init
> and use a postcore init function imx7d_clocks_setup to do it later instead.
> Then we simply reply on the bootloader settings to do early boot.

Is this really a good idea? What happens if something requests a clock
before postcore initcalls get called? E.g. clock source is initialized
before that, which might request a clock...

Ok, we can just say that all those clocks need to be bootloader on, but
how do we know? Do we catch if the bootloader does not adhere to that? 

--
Stefan


> 
> [    0.000000] bad: scheduling from the idle thread!
> [    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W      
> 4.6.0-rc3-00064-gded8bc08fb45-dirty #836
> [    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
> [    0.000000] Backtrace:
> [    0.000000] [<c010c42c>] (dump_backtrace) from [<c010c624>]
> (show_stack+0x18/0x1c)
> [    0.000000]  r6:60000053 r5:00000000 r4:c0d21f9c r3:00000000
> [    0.000000] [<c010c60c>] (show_stack) from [<c03e0228>]
> (dump_stack+0xb4/0xe8)
> [    0.000000] [<c03e0174>] (dump_stack) from [<c0154388>]
> (dequeue_task_idle+0x20/0x30)
> [    0.000000]  r10:c08f5554 r9:c0d02b0c r8:c0d06844 r7:ef7c14d0
> r6:00000001 r5:ef7c14c0
> [    0.000000]  r4:ef7c14c0 r3:00000000
> [    0.000000] [<c0154368>] (dequeue_task_idle) from [<c014c968>]
> (deactivate_task+0x64/0x68)
> [    0.000000]  r4:c0d06500 r3:c0154368
> [    0.000000] [<c014c904>] (deactivate_task) from [<c08f50bc>]
> (__schedule+0x2e8/0x738)
> [    0.000000]  r6:c0d06500 r5:ef7c14c0 r4:c0c744c0 r3:00000002
> [    0.000000] [<c08f4dd4>] (__schedule) from [<c08f5554>] (schedule+0x48/0xa0)
> [    0.000000]  r10:000001b6 r9:00000003 r8:00000036 r7:00000000
> r6:0007a120 r5:00000000
> [    0.000000]  r4:c0d00000
> [    0.000000] [<c08f550c>] (schedule) from [<c08f9cc8>]
> (schedule_hrtimeout_range_clock+0xac/0x12c)
> [    0.000000]  r4:0006ddd0 r3:c0d06500
> [    0.000000] [<c08f9c1c>] (schedule_hrtimeout_range_clock) from
> [<c08f9d6c>] (schedule_hrtimeout_range+0x24/0x2c)
> [    0.000000]  r7:c0d5d434 r6:c0d02100 r5:ffff8ad1 r4:ef00c040
> [    0.000000] [<c08f9d48>] (schedule_hrtimeout_range) from
> [<c08f97a4>] (usleep_range+0x54/0x5c)
> [    0.000000] [<c08f9750>] (usleep_range) from [<c06984c8>]
> (clk_pllv3_wait_lock+0x7c/0xb4)
> [    0.000000] [<c069844c>] (clk_pllv3_wait_lock) from [<c069852c>]
> (clk_pllv3_prepare+0x2c/0x30)
> [    0.000000]  r6:00000000 r5:c15603f4 r4:ef007680 r3:0000201b
> [    0.000000] [<c0698500>] (clk_pllv3_prepare) from [<c0691eb8>]
> (clk_core_prepare+0x98/0xc8)
> [    0.000000] [<c0691e20>] (clk_core_prepare) from [<c06923d4>]
> (clk_prepare+0x20/0x38)
> [    0.000000]  r5:c15603f4 r4:ef00c100
> [    0.000000] [<c06923b4>] (clk_prepare) from [<c0c4a55c>]
> (imx7d_clocks_init+0x6108/0x6188)
> [    0.000000]  r4:ef00c100 r3:00000001
> [    0.000000] [<c0c44454>] (imx7d_clocks_init) from [<c0c30e38>]
> (of_clk_init+0x10c/0x1d4)
> [    0.000000]  r10:00000001 r9:c0d01f68 r8:00000000 r7:c0d01f60
> r6:ef7e3d60 r5:ef004340
> [    0.000000]  r4:00000002
> [    0.000000] [<c0c30d2c>] (of_clk_init) from [<c0c048b0>]
> (time_init+0x2c/0x38)
> [    0.000000]  r10:efffcac0 r9:c0c5ca48 r8:c0d76000 r7:c0d028c0
> r6:ffffffff r5:c0d76000
> [    0.000000]  r4:00000000
> [    0.000000] [<c0c04884>] (time_init) from [<c0c00b7c>]
> (start_kernel+0x218/0x398)
> [    0.000000] [<c0c00964>] (start_kernel) from [<8000807c>] (0x8000807c)
> [    0.000000]  r10:00000000 r9:410fc075 r8:8000406a r7:c0d07e8c
> r6:c0c5ca44 r5:c0d0296c
> [    0.000000]  r4:c0d76294
> [    0.000000] Architected cp15 timer(s) running at 8.00MHz (virt).
> [    0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff
> max_cycles: 0x1d854df40, max_idle_ns: 440795202120 ns
> 
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Stephen Boyd <sboyd@codeaurora.org>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> ---
>  drivers/clk/imx/clk-imx7d.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
> index 7912be83c4af..3be2e9371491 100644
> --- a/drivers/clk/imx/clk-imx7d.c
> +++ b/drivers/clk/imx/clk-imx7d.c
> @@ -378,7 +378,6 @@ static void __init imx7d_clocks_init(struct
> device_node *ccm_node)
>  {
>  	struct device_node *np;
>  	void __iomem *base;
> -	int i;
>  
>  	clks[IMX7D_CLK_DUMMY] = imx_clk_fixed("dummy", 0);
>  	clks[IMX7D_OSC_24M_CLK] = of_clk_get_by_name(ccm_node, "osc");
> @@ -409,13 +408,6 @@ static void __init imx7d_clocks_init(struct
> device_node *ccm_node)
>  	clks[IMX7D_PLL_AUDIO_MAIN_BYPASS] =
> imx_clk_mux_flags("pll_audio_main_bypass", base + 0xf0, 16, 1,
> pll_audio_bypass_sel, ARRAY_SIZE(pll_audio_bypass_sel),
> CLK_SET_RATE_PARENT);
>  	clks[IMX7D_PLL_VIDEO_MAIN_BYPASS] =
> imx_clk_mux_flags("pll_video_main_bypass", base + 0x130, 16, 1,
> pll_video_bypass_sel, ARRAY_SIZE(pll_video_bypass_sel),
> CLK_SET_RATE_PARENT);
>  
> -	clk_set_parent(clks[IMX7D_PLL_ARM_MAIN_BYPASS], clks[IMX7D_PLL_ARM_MAIN]);
> -	clk_set_parent(clks[IMX7D_PLL_DRAM_MAIN_BYPASS], clks[IMX7D_PLL_DRAM_MAIN]);
> -	clk_set_parent(clks[IMX7D_PLL_SYS_MAIN_BYPASS], clks[IMX7D_PLL_SYS_MAIN]);
> -	clk_set_parent(clks[IMX7D_PLL_ENET_MAIN_BYPASS], clks[IMX7D_PLL_ENET_MAIN]);
> -	clk_set_parent(clks[IMX7D_PLL_AUDIO_MAIN_BYPASS], clks[IMX7D_PLL_AUDIO_MAIN]);
> -	clk_set_parent(clks[IMX7D_PLL_VIDEO_MAIN_BYPASS], clks[IMX7D_PLL_VIDEO_MAIN]);
> -
>  	clks[IMX7D_PLL_ARM_MAIN_CLK] = imx_clk_gate("pll_arm_main_clk",
> "pll_arm_main_bypass", base + 0x60, 13);
>  	clks[IMX7D_PLL_DRAM_MAIN_CLK] = imx_clk_gate("pll_dram_main_clk",
> "pll_dram_main_bypass", base + 0x70, 13);
>  	clks[IMX7D_PLL_SYS_MAIN_CLK] = imx_clk_gate("pll_sys_main_clk",
> "pll_sys_main_bypass", base + 0xb0, 13);
> @@ -846,6 +838,13 @@ static void __init imx7d_clocks_init(struct
> device_node *ccm_node)
>  	clk_data.clk_num = ARRAY_SIZE(clks);
>  	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
>  
> +}
> +CLK_OF_DECLARE(imx7d, "fsl,imx7d-ccm", imx7d_clocks_init);
> +
> +static int __init imx7d_clocks_setup(void)
> +{
> +	int i;
> +
>  	/* TO BE FIXED LATER
>  	 * Enable all clock to bring up imx7, otherwise system will be halt and block
>  	 * the other part upstream Because imx7d clock design changed, clock framework
> @@ -855,6 +854,13 @@ static void __init imx7d_clocks_init(struct
> device_node *ccm_node)
>  	for (i = 0; i < IMX7D_CLK_END; i++)
>  		clk_prepare_enable(clks[i]);
>  
> +	clk_set_parent(clks[IMX7D_PLL_ARM_MAIN_BYPASS], clks[IMX7D_PLL_ARM_MAIN]);
> +	clk_set_parent(clks[IMX7D_PLL_DRAM_MAIN_BYPASS], clks[IMX7D_PLL_DRAM_MAIN]);
> +	clk_set_parent(clks[IMX7D_PLL_SYS_MAIN_BYPASS], clks[IMX7D_PLL_SYS_MAIN]);
> +	clk_set_parent(clks[IMX7D_PLL_ENET_MAIN_BYPASS], clks[IMX7D_PLL_ENET_MAIN]);
> +	clk_set_parent(clks[IMX7D_PLL_AUDIO_MAIN_BYPASS], clks[IMX7D_PLL_AUDIO_MAIN]);
> +	clk_set_parent(clks[IMX7D_PLL_VIDEO_MAIN_BYPASS], clks[IMX7D_PLL_VIDEO_MAIN]);
> +
>  	/* use old gpt clk setting, gpt1 root clk must be twice as gpt counter freq */
>  	clk_set_parent(clks[IMX7D_GPT1_ROOT_SRC], clks[IMX7D_OSC_24M_CLK]);
>  
> @@ -874,5 +880,6 @@ static void __init imx7d_clocks_init(struct
> device_node *ccm_node)
>  
>  	imx_register_uart_clocks(uart_clks);
>  
> +	return 0;
>  }
> -CLK_OF_DECLARE(imx7d, "fsl,imx7d-ccm", imx7d_clocks_init);
> +postcore_initcall(imx7d_clocks_setup);

Re: [RFC PATCH 1/1] clk: imx7d: move clk setting out of imx7d_clocks_init

[Dong Aisheng]

On Fri, Apr 29, 2016 at 07:04:33PM -0700, Stefan Agner wrote:
> On 2016-04-29 02:45, Dong Aisheng wrote:
> > During kernel early booting(e.g. in time_init()), there's only one
> > init idle task running, and the idle sched class indicates that it's
> > not valid to schedule for idle task. If it happens the kernel
> > will complain with a error message as follows:
> > [    0.000000] bad: scheduling from the idle thread!
> > 
> > We can observe this warning on an i.MX7D SDB board. See full log below.
> > It is caused by imx7d_clocks_init function called in time_init
> > invokes a lot clk_prepare_enable to enable many clocks and it happens
> > that the Audio/Video PLLs need large delay causes a sleep.
> > 
> > Since we should not sleep during time_init, this patch fundamentally
> > moves all clk_prepare_enable and clk_set_parent out of imx7d_clocks_init
> > and use a postcore init function imx7d_clocks_setup to do it later instead.
> > Then we simply reply on the bootloader settings to do early boot.
> 
> Is this really a good idea? What happens if something requests a clock
> before postcore initcalls get called? E.g. clock source is initialized
> before that, which might request a clock...
> 

I think clock source driver usually will do clock configuration by itself
and it does not depends on imx7d_clocks_init.
e.g. smp_twd.c / timer-imx-gpt.c

However, one exception is that if the clock source needs change clock
parent, it may have to be done in its driver too.
Up till now we still have no such requirement and no board do like that.

> Ok, we can just say that all those clocks need to be bootloader on, but
> how do we know? Do we catch if the bootloader does not adhere to that? 
> 

clk source driver may need to handle that work and it seems we already
do that work in timer-imx-gpt.c.

But as i replied in another earlier email, clk source driver itself
may also result in a possible sleep with clock operation which is
a potential break too.
We may need discuss to find a suitable generic solution.

Regards
Dong Aisheng

> --
> Stefan
> 
> 
> > 
> > [    0.000000] bad: scheduling from the idle thread!
> > [    0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W      
> > 4.6.0-rc3-00064-gded8bc08fb45-dirty #836
> > [    0.000000] Hardware name: Freescale i.MX7 Dual (Device Tree)
> > [    0.000000] Backtrace:
> > [    0.000000] [<c010c42c>] (dump_backtrace) from [<c010c624>]
> > (show_stack+0x18/0x1c)
> > [    0.000000]  r6:60000053 r5:00000000 r4:c0d21f9c r3:00000000
> > [    0.000000] [<c010c60c>] (show_stack) from [<c03e0228>]
> > (dump_stack+0xb4/0xe8)
> > [    0.000000] [<c03e0174>] (dump_stack) from [<c0154388>]
> > (dequeue_task_idle+0x20/0x30)
> > [    0.000000]  r10:c08f5554 r9:c0d02b0c r8:c0d06844 r7:ef7c14d0
> > r6:00000001 r5:ef7c14c0
> > [    0.000000]  r4:ef7c14c0 r3:00000000
> > [    0.000000] [<c0154368>] (dequeue_task_idle) from [<c014c968>]
> > (deactivate_task+0x64/0x68)
> > [    0.000000]  r4:c0d06500 r3:c0154368
> > [    0.000000] [<c014c904>] (deactivate_task) from [<c08f50bc>]
> > (__schedule+0x2e8/0x738)
> > [    0.000000]  r6:c0d06500 r5:ef7c14c0 r4:c0c744c0 r3:00000002
> > [    0.000000] [<c08f4dd4>] (__schedule) from [<c08f5554>] (schedule+0x48/0xa0)
> > [    0.000000]  r10:000001b6 r9:00000003 r8:00000036 r7:00000000
> > r6:0007a120 r5:00000000
> > [    0.000000]  r4:c0d00000
> > [    0.000000] [<c08f550c>] (schedule) from [<c08f9cc8>]
> > (schedule_hrtimeout_range_clock+0xac/0x12c)
> > [    0.000000]  r4:0006ddd0 r3:c0d06500
> > [    0.000000] [<c08f9c1c>] (schedule_hrtimeout_range_clock) from
> > [<c08f9d6c>] (schedule_hrtimeout_range+0x24/0x2c)
> > [    0.000000]  r7:c0d5d434 r6:c0d02100 r5:ffff8ad1 r4:ef00c040
> > [    0.000000] [<c08f9d48>] (schedule_hrtimeout_range) from
> > [<c08f97a4>] (usleep_range+0x54/0x5c)
> > [    0.000000] [<c08f9750>] (usleep_range) from [<c06984c8>]
> > (clk_pllv3_wait_lock+0x7c/0xb4)
> > [    0.000000] [<c069844c>] (clk_pllv3_wait_lock) from [<c069852c>]
> > (clk_pllv3_prepare+0x2c/0x30)
> > [    0.000000]  r6:00000000 r5:c15603f4 r4:ef007680 r3:0000201b
> > [    0.000000] [<c0698500>] (clk_pllv3_prepare) from [<c0691eb8>]
> > (clk_core_prepare+0x98/0xc8)
> > [    0.000000] [<c0691e20>] (clk_core_prepare) from [<c06923d4>]
> > (clk_prepare+0x20/0x38)
> > [    0.000000]  r5:c15603f4 r4:ef00c100
> > [    0.000000] [<c06923b4>] (clk_prepare) from [<c0c4a55c>]
> > (imx7d_clocks_init+0x6108/0x6188)
> > [    0.000000]  r4:ef00c100 r3:00000001
> > [    0.000000] [<c0c44454>] (imx7d_clocks_init) from [<c0c30e38>]
> > (of_clk_init+0x10c/0x1d4)
> > [    0.000000]  r10:00000001 r9:c0d01f68 r8:00000000 r7:c0d01f60
> > r6:ef7e3d60 r5:ef004340
> > [    0.000000]  r4:00000002
> > [    0.000000] [<c0c30d2c>] (of_clk_init) from [<c0c048b0>]
> > (time_init+0x2c/0x38)
> > [    0.000000]  r10:efffcac0 r9:c0c5ca48 r8:c0d76000 r7:c0d028c0
> > r6:ffffffff r5:c0d76000
> > [    0.000000]  r4:00000000
> > [    0.000000] [<c0c04884>] (time_init) from [<c0c00b7c>]
> > (start_kernel+0x218/0x398)
> > [    0.000000] [<c0c00964>] (start_kernel) from [<8000807c>] (0x8000807c)
> > [    0.000000]  r10:00000000 r9:410fc075 r8:8000406a r7:c0d07e8c
> > r6:c0c5ca44 r5:c0d0296c
> > [    0.000000]  r4:c0d76294
> > [    0.000000] Architected cp15 timer(s) running at 8.00MHz (virt).
> > [    0.000000] clocksource: arch_sys_counter: mask: 0xffffffffffffff
> > max_cycles: 0x1d854df40, max_idle_ns: 440795202120 ns
> > 
> > Cc: Michael Turquette <mturquette@baylibre.com>
> > Cc: Stephen Boyd <sboyd@codeaurora.org>
> > Cc: Shawn Guo <shawnguo@kernel.org>
> > Cc: Stefan Agner <stefan@agner.ch>
> > Cc: Lucas Stach <l.stach@pengutronix.de>
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > ---
> >  drivers/clk/imx/clk-imx7d.c | 25 ++++++++++++++++---------
> >  1 file changed, 16 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
> > index 7912be83c4af..3be2e9371491 100644
> > --- a/drivers/clk/imx/clk-imx7d.c
> > +++ b/drivers/clk/imx/clk-imx7d.c
> > @@ -378,7 +378,6 @@ static void __init imx7d_clocks_init(struct
> > device_node *ccm_node)
> >  {
> >  	struct device_node *np;
> >  	void __iomem *base;
> > -	int i;
> >  
> >  	clks[IMX7D_CLK_DUMMY] = imx_clk_fixed("dummy", 0);
> >  	clks[IMX7D_OSC_24M_CLK] = of_clk_get_by_name(ccm_node, "osc");
> > @@ -409,13 +408,6 @@ static void __init imx7d_clocks_init(struct
> > device_node *ccm_node)
> >  	clks[IMX7D_PLL_AUDIO_MAIN_BYPASS] =
> > imx_clk_mux_flags("pll_audio_main_bypass", base + 0xf0, 16, 1,
> > pll_audio_bypass_sel, ARRAY_SIZE(pll_audio_bypass_sel),
> > CLK_SET_RATE_PARENT);
> >  	clks[IMX7D_PLL_VIDEO_MAIN_BYPASS] =
> > imx_clk_mux_flags("pll_video_main_bypass", base + 0x130, 16, 1,
> > pll_video_bypass_sel, ARRAY_SIZE(pll_video_bypass_sel),
> > CLK_SET_RATE_PARENT);
> >  
> > -	clk_set_parent(clks[IMX7D_PLL_ARM_MAIN_BYPASS], clks[IMX7D_PLL_ARM_MAIN]);
> > -	clk_set_parent(clks[IMX7D_PLL_DRAM_MAIN_BYPASS], clks[IMX7D_PLL_DRAM_MAIN]);
> > -	clk_set_parent(clks[IMX7D_PLL_SYS_MAIN_BYPASS], clks[IMX7D_PLL_SYS_MAIN]);
> > -	clk_set_parent(clks[IMX7D_PLL_ENET_MAIN_BYPASS], clks[IMX7D_PLL_ENET_MAIN]);
> > -	clk_set_parent(clks[IMX7D_PLL_AUDIO_MAIN_BYPASS], clks[IMX7D_PLL_AUDIO_MAIN]);
> > -	clk_set_parent(clks[IMX7D_PLL_VIDEO_MAIN_BYPASS], clks[IMX7D_PLL_VIDEO_MAIN]);
> > -
> >  	clks[IMX7D_PLL_ARM_MAIN_CLK] = imx_clk_gate("pll_arm_main_clk",
> > "pll_arm_main_bypass", base + 0x60, 13);
> >  	clks[IMX7D_PLL_DRAM_MAIN_CLK] = imx_clk_gate("pll_dram_main_clk",
> > "pll_dram_main_bypass", base + 0x70, 13);
> >  	clks[IMX7D_PLL_SYS_MAIN_CLK] = imx_clk_gate("pll_sys_main_clk",
> > "pll_sys_main_bypass", base + 0xb0, 13);
> > @@ -846,6 +838,13 @@ static void __init imx7d_clocks_init(struct
> > device_node *ccm_node)
> >  	clk_data.clk_num = ARRAY_SIZE(clks);
> >  	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_data);
> >  
> > +}
> > +CLK_OF_DECLARE(imx7d, "fsl,imx7d-ccm", imx7d_clocks_init);
> > +
> > +static int __init imx7d_clocks_setup(void)
> > +{
> > +	int i;
> > +
> >  	/* TO BE FIXED LATER
> >  	 * Enable all clock to bring up imx7, otherwise system will be halt and block
> >  	 * the other part upstream Because imx7d clock design changed, clock framework
> > @@ -855,6 +854,13 @@ static void __init imx7d_clocks_init(struct
> > device_node *ccm_node)
> >  	for (i = 0; i < IMX7D_CLK_END; i++)
> >  		clk_prepare_enable(clks[i]);
> >  
> > +	clk_set_parent(clks[IMX7D_PLL_ARM_MAIN_BYPASS], clks[IMX7D_PLL_ARM_MAIN]);
> > +	clk_set_parent(clks[IMX7D_PLL_DRAM_MAIN_BYPASS], clks[IMX7D_PLL_DRAM_MAIN]);
> > +	clk_set_parent(clks[IMX7D_PLL_SYS_MAIN_BYPASS], clks[IMX7D_PLL_SYS_MAIN]);
> > +	clk_set_parent(clks[IMX7D_PLL_ENET_MAIN_BYPASS], clks[IMX7D_PLL_ENET_MAIN]);
> > +	clk_set_parent(clks[IMX7D_PLL_AUDIO_MAIN_BYPASS], clks[IMX7D_PLL_AUDIO_MAIN]);
> > +	clk_set_parent(clks[IMX7D_PLL_VIDEO_MAIN_BYPASS], clks[IMX7D_PLL_VIDEO_MAIN]);
> > +
> >  	/* use old gpt clk setting, gpt1 root clk must be twice as gpt counter freq */
> >  	clk_set_parent(clks[IMX7D_GPT1_ROOT_SRC], clks[IMX7D_OSC_24M_CLK]);
> >  
> > @@ -874,5 +880,6 @@ static void __init imx7d_clocks_init(struct
> > device_node *ccm_node)
> >  
> >  	imx_register_uart_clocks(uart_clks);
> >  
> > +	return 0;
> >  }
> > -CLK_OF_DECLARE(imx7d, "fsl,imx7d-ccm", imx7d_clocks_init);
> > +postcore_initcall(imx7d_clocks_setup);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-clk" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Stefan Agner]

On 2016-04-27 03:15, Thomas Gleixner wrote:
> On Wed, 27 Apr 2016, Dong Aisheng wrote:
>> Why Stefan's patch works (checking irqs_disabled()) is because during kernel
>> time init, the irq is still not enabled. It fixes the issue indirectly.
>> See:
>> asmlinkage __visible void __init start_kernel(void)
>> {
>>         /*
>>          * Set up the scheduler prior starting any interrupts (such as the
>>          * timer interrupt). Full topology setup happens at smp_init()
>>          * time - but meanwhile we still have a functioning scheduler.
>>          */
>>         sched_init();
>>         .............
>>         time_init();
>>         ..............
>>         WARN(!irqs_disabled(), "Interrupts were enabled early\n");
>>         early_boot_irqs_disabled = false;
>>         local_irq_enable();
>> }
>>
>> The issue can only happen when PLL enable causes a schedule during
>> imx_clock_init().
> 
> Calling a function which might sleep _BEFORE_ kernel_init() is wrong. Don't
> try to work around such an issue by doing magic irq_disabled() checks and busy
> loops. Fix the call site and be done with it.

What do you mean exactly by fix the call site? The patch I proposed
(https://lkml.org/lkml/2016/1/29/695) fixes the call site...

But I see Dong's argument here, irqs_disabled is the wrong way to figure
out whether we are in kernel_init. What is the right approach to
distinguish whether we are allowed to sleep?

--
Stefan

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Dong Aisheng]

Hi Thomas,

On Wed, Apr 27, 2016 at 12:15:00PM +0200, Thomas Gleixner wrote:
> On Wed, 27 Apr 2016, Dong Aisheng wrote:
> > On Wed, Apr 27, 2016 at 9:58 AM, Shawn Guo <shawnguo@kernel.org> wrote:
> > > On Tue, Apr 26, 2016 at 07:27:03PM +0800, Dong Aisheng wrote:
> > >> Shawn,
> > >> What's your suggestion?
> > >
> > > I think this needs more discussion, and I just dropped Stefan's patch
> > > from my tree.
> > >
> > > We need to firstly understand why this is happening.  The .prepare hook
> > > is defined to be non-atomic context, and so that we call sleep function
> > > in there.  We did everything right.  Why are we getting the warning?  If
> > > I'm correct, this warning only happens on i.MX7D.  Why is that?
> > >
> > 
> > Why Stefan's patch works (checking irqs_disabled()) is because during kernel
> > time init, the irq is still not enabled. It fixes the issue indirectly.
> > See:
> > asmlinkage __visible void __init start_kernel(void)
> > {
> >         /*
> >          * Set up the scheduler prior starting any interrupts (such as the
> >          * timer interrupt). Full topology setup happens at smp_init()
> >          * time - but meanwhile we still have a functioning scheduler.
> >          */
> >         sched_init();
> >         .............
> >         time_init();
> >         ..............
> >         WARN(!irqs_disabled(), "Interrupts were enabled early\n");
> >         early_boot_irqs_disabled = false;
> >         local_irq_enable();
> > }
> > 
> > The issue can only happen when PLL enable causes a schedule during
> > imx_clock_init().
> 
> Calling a function which might sleep _BEFORE_ kernel_init() is wrong. Don't
> try to work around such an issue by doing magic irq_disabled() checks and busy
> loops. Fix the call site and be done with it.
> 

IRQ chip and clocksource are also initialised before kernel_init()
which may call clk APIs like clk_prepare_enable().
We may not be able to guarantee those clocks are unsleepable.

e.g.
For IRQ chip, the arm,gic documents the optional clocks property in
Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt.
Although there seems no user currently, not sure there might be
a exception in the future.

For clocksource driver, it's quite common to call clk APIs to
enable and get clock rate which may also cause sleep.
e.g. ARM twd clock in arch/arm/kernel/smp_twd.c.

If we have to avoid the possible sleep caused by these clocks,
we may need to manually check it and change the related clocks
(e.g PLLs) from sleepable to busy loops.
But that is not a good solution and may also not stable when
clock varies.

It looks like not easy to find a generic solution for them.
What's your suggestion?

And Stephen,

Besides above possible sleep, the clk framework itself may also
cause unexpected sleep before kernel_init().
1) clk framework now supports CLK_IS_CRITICAL flag which allows to
enable the clock during register in __clk_core_init(), it usually
can happen in time_init()->of_clk_init();

2) clk framework also support set default configuration like set_parent
and set_rate during register clk providers which also happens in
time_init()->of_clk_init().

Any ideas?
Do you think if we should move them onto a bit later stage after
kernel_init()?

Regards
Dong Aisheng

> Thanks,
> 
> 	tglx
> 
> 

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Thomas Gleixner]

On Thu, 2 Jun 2016, Dong Aisheng wrote:
> On Wed, Apr 27, 2016 at 12:15:00PM +0200, Thomas Gleixner wrote:
> > Calling a function which might sleep _BEFORE_ kernel_init() is wrong. Don't
> > try to work around such an issue by doing magic irq_disabled() checks and busy
> > loops. Fix the call site and be done with it.
> > 
> 
> IRQ chip and clocksource are also initialised before kernel_init()
> which may call clk APIs like clk_prepare_enable().
> We may not be able to guarantee those clocks are unsleepable.
> 
> e.g.
> For IRQ chip, the arm,gic documents the optional clocks property in
> Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt.
> Although there seems no user currently, not sure there might be
> a exception in the future.
> 
> For clocksource driver, it's quite common to call clk APIs to
> enable and get clock rate which may also cause sleep.
> e.g. ARM twd clock in arch/arm/kernel/smp_twd.c.
> 
> If we have to avoid the possible sleep caused by these clocks,
> we may need to manually check it and change the related clocks
> (e.g PLLs) from sleepable to busy loops.
> But that is not a good solution and may also not stable when
> clock varies.
> 
> It looks like not easy to find a generic solution for them.
> What's your suggestion?

I think we should split the prepare callbacks up and move the handling logic
into the core.

clk_ops.prepare()	Legacy callback
clk_ops.prepare_hw()	Callback which writes to the hardware
clk_ops.prepare_done()	Callback which checks whether the prepare is completed

So now the core can do:

clk_prepare()
{
	/* Legacy code should go away */
	if (ops->prepare) {
	      ops->prepare();
	      return;
	}

	if (ops->prepare_hw)
	      ops->prepare_hw();

   	if (!ops->prepare_done())
		return;

	if (early_boot_or_suspend_resume()) {
		/*
		 * Busy loop when we can't schedule in early boot,
		 * suspend and resume.
		 */
		while (!ops->prepare_done())
		      ;
	} else {
		while (!ops->prepare_done())
		      usleep(clk->prepare_delay);
	}
}

As a side effect that allows us to remove the gazillion of

     	    while (!hw_ready)
	    	  usleep();

copies all over the place and we have a single point of control where we allow
the clocks to busy loop.

Toughts?

Thanks,

	tglx

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Dong Aisheng]

On Mon, Jun 06, 2016 at 03:20:03PM +0200, Thomas Gleixner wrote:
> On Thu, 2 Jun 2016, Dong Aisheng wrote:
> > On Wed, Apr 27, 2016 at 12:15:00PM +0200, Thomas Gleixner wrote:
> > > Calling a function which might sleep _BEFORE_ kernel_init() is wrong. Don't
> > > try to work around such an issue by doing magic irq_disabled() checks and busy
> > > loops. Fix the call site and be done with it.
> > > 
> > 
> > IRQ chip and clocksource are also initialised before kernel_init()
> > which may call clk APIs like clk_prepare_enable().
> > We may not be able to guarantee those clocks are unsleepable.
> > 
> > e.g.
> > For IRQ chip, the arm,gic documents the optional clocks property in
> > Documentation/devicetree/bindings/interrupt-controller/arm,gic.txt.
> > Although there seems no user currently, not sure there might be
> > a exception in the future.
> > 
> > For clocksource driver, it's quite common to call clk APIs to
> > enable and get clock rate which may also cause sleep.
> > e.g. ARM twd clock in arch/arm/kernel/smp_twd.c.
> > 
> > If we have to avoid the possible sleep caused by these clocks,
> > we may need to manually check it and change the related clocks
> > (e.g PLLs) from sleepable to busy loops.
> > But that is not a good solution and may also not stable when
> > clock varies.
> > 
> > It looks like not easy to find a generic solution for them.
> > What's your suggestion?
> 
> I think we should split the prepare callbacks up and move the handling logic
> into the core.
> 
> clk_ops.prepare()	Legacy callback
> clk_ops.prepare_hw()	Callback which writes to the hardware
> clk_ops.prepare_done()	Callback which checks whether the prepare is completed
> 
> So now the core can do:
> 
> clk_prepare()
> {
> 	/* Legacy code should go away */
> 	if (ops->prepare) {
> 	      ops->prepare();
> 	      return;
> 	}
> 
> 	if (ops->prepare_hw)
> 	      ops->prepare_hw();
> 
>    	if (!ops->prepare_done())
> 		return;
> 
> 	if (early_boot_or_suspend_resume()) {
> 		/*
> 		 * Busy loop when we can't schedule in early boot,
> 		 * suspend and resume.
> 		 */
> 		while (!ops->prepare_done())
> 		      ;
> 	} else {
> 		while (!ops->prepare_done())
> 		      usleep(clk->prepare_delay);
> 	}
> }
> 
> As a side effect that allows us to remove the gazillion of
> 
>      	    while (!hw_ready)
> 	    	  usleep();
> 
> copies all over the place and we have a single point of control where we allow
> the clocks to busy loop.
> 
> Toughts?
> 

Great, that looks like a possible solution.
If we doing this way there's one more question that
should we do it for other clk APIs hold by prepare_lock which
all may sleep too in theory?
e.g. clk_{set|get}_rate/clk_{set|get}_parent. (possible more)
(clk_recalc_rate/clk_round_rate may also need be considered due to
it may be called within above function.)

And it seems many exist platforms doing that work in
CLK_OF_DECLARE init function in time_init().
e.g.
drivers/clk/imx/clk-imx6ul.c
drivers/clk/rockchip/clk-rk3188.c
drivers/clk/ti/clk-44xx.c
...

Then it may need introduce a lot changes and increase many new core APIs.
Is that a problem?

> Thanks,
> 
> 	tglx

Regards
Dong Aisheng

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Thomas Gleixner]

On Tue, 7 Jun 2016, Dong Aisheng wrote:
> Then it may need introduce a lot changes and increase many new core APIs.
> Is that a problem?

No. That's all better than each driver having broken workarounds. It's a
common problem so it wants to be addressed at the core level. There you have a
central point to do this and you can still catch abusers which call stuff from
the wrong context. The hacks in the drivers don't allow that because they look
at the context, i.e. irq disabled, instead of checking the system state.

Thanks,

	tglx

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Stefan Agner]

On 2016-06-09 13:08, Thomas Gleixner wrote:
> On Tue, 7 Jun 2016, Dong Aisheng wrote:
>> Then it may need introduce a lot changes and increase many new core APIs.
>> Is that a problem?
> 
> No. That's all better than each driver having broken workarounds. It's a
> common problem so it wants to be addressed at the core level. There you have a
> central point to do this and you can still catch abusers which call stuff from
> the wrong context. The hacks in the drivers don't allow that because they look
> at the context, i.e. irq disabled, instead of checking the system state.

IMHO, the hacky part of my patch was how I detected whether to use sleep
or delay. That said I am ok with API extension too, I guess it is fairly
common use case... I found at least 6 clock prepare functions with sleep
in it (and some udelays, all between 1-100).

Your proposed solution uses "early_boot_or_suspend_resume" which I did
not found as a convenient function in the wild :-)

How would you implement that?

--
Stefan

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Thomas Gleixner]

On Thu, 9 Jun 2016, Stefan Agner wrote:
> On 2016-06-09 13:08, Thomas Gleixner wrote:
> > On Tue, 7 Jun 2016, Dong Aisheng wrote:
> >> Then it may need introduce a lot changes and increase many new core APIs.
> >> Is that a problem?
> > 
> > No. That's all better than each driver having broken workarounds. It's a
> > common problem so it wants to be addressed at the core level. There you have a
> > central point to do this and you can still catch abusers which call stuff from
> > the wrong context. The hacks in the drivers don't allow that because they look
> > at the context, i.e. irq disabled, instead of checking the system state.
> 
> IMHO, the hacky part of my patch was how I detected whether to use sleep
> or delay. That said I am ok with API extension too, I guess it is fairly
> common use case... I found at least 6 clock prepare functions with sleep
> in it (and some udelays, all between 1-100).
> 
> Your proposed solution uses "early_boot_or_suspend_resume" which I did
> not found as a convenient function in the wild :-)
> 
> How would you implement that?

Early boot is simple. Supsend/resume is not that hard either. We have a patch
in RT which does exactly what you need. See below.

Then the state check simply becomes:

     system_state != SYSTEM_RUNNING

Thanks,

	tglx

8<---------------------------

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 350dfb08aee3..5e63b681f58e 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -473,6 +473,7 @@ extern enum system_states {
 	SYSTEM_HALT,
 	SYSTEM_POWER_OFF,
 	SYSTEM_RESTART,
+	SYSTEM_SUSPEND,
 } system_state;
 
 #define TAINT_PROPRIETARY_MODULE	0
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index b7342a24f559..bfd9e0982f15 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -285,6 +285,8 @@ static int create_image(int platform_mode)
 
 	local_irq_disable();
 
+	system_state = SYSTEM_SUSPEND;
+
 	error = syscore_suspend();
 	if (error) {
 		printk(KERN_ERR "PM: Some system devices failed to power down, "
@@ -314,6 +316,7 @@ static int create_image(int platform_mode)
 	syscore_resume();
 
  Enable_irqs:
+	system_state = SYSTEM_RUNNING;
 	local_irq_enable();
 
  Enable_cpus:
@@ -437,6 +440,7 @@ static int resume_target_kernel(bool platform_mode)
 		goto Enable_cpus;
 
 	local_irq_disable();
+	system_state = SYSTEM_SUSPEND;
 
 	error = syscore_suspend();
 	if (error)
@@ -470,6 +474,7 @@ static int resume_target_kernel(bool platform_mode)
 	syscore_resume();
 
  Enable_irqs:
+	system_state = SYSTEM_RUNNING;
 	local_irq_enable();
 
  Enable_cpus:
@@ -555,6 +560,7 @@ int hibernation_platform_enter(void)
 		goto Enable_cpus;
 
 	local_irq_disable();
+	system_state = SYSTEM_SUSPEND;
 	syscore_suspend();
 	if (pm_wakeup_pending()) {
 		error = -EAGAIN;
@@ -567,6 +573,7 @@ int hibernation_platform_enter(void)
 
  Power_up:
 	syscore_resume();
+	system_state = SYSTEM_RUNNING;
 	local_irq_enable();
 
  Enable_cpus:
diff --git a/kernel/power/suspend.c b/kernel/power/suspend.c
index f9fe133c13e2..80ebc0726290 100644
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -359,6 +359,8 @@ static int suspend_enter(suspend_state_t state, bool *wakeup)
 	arch_suspend_disable_irqs();
 	BUG_ON(!irqs_disabled());
 
+	system_state = SYSTEM_SUSPEND;
+
 	error = syscore_suspend();
 	if (!error) {
 		*wakeup = pm_wakeup_pending();
@@ -375,6 +377,8 @@ static int suspend_enter(suspend_state_t state, bool *wakeup)
 		syscore_resume();
 	}
 
+	system_state = SYSTEM_RUNNING;
+
 	arch_suspend_enable_irqs();
 	BUG_ON(irqs_disabled());
 

Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled

[Dong Aisheng]

On Thu, Jun 09, 2016 at 10:08:01PM +0200, Thomas Gleixner wrote:
> On Tue, 7 Jun 2016, Dong Aisheng wrote:
> > Then it may need introduce a lot changes and increase many new core APIs.
> > Is that a problem?
> 
> No. That's all better than each driver having broken workarounds. It's a
> common problem so it wants to be addressed at the core level. There you have a
> central point to do this and you can still catch abusers which call stuff from
> the wrong context. The hacks in the drivers don't allow that because they look
> at the context, i.e. irq disabled, instead of checking the system state.
> 

Okay, thanks.
If you wouldn't mind i could send out a patch based on your
suggestion for further discussion.

Thanks

Regards
Dong Aisheng

> Thanks,
> 
> 	tglx