diff mbox

clk: imx7d: do not set the parent of IMX7D_ENET_AXI_ROOT_SRC

Message ID 1468590782-8436-1-git-send-email-fabio.estevam@nxp.com (mailing list archive)
State New, archived
Headers show

Commit Message

Fabio Estevam July 15, 2016, 1:53 p.m. UTC
Booting the kernel on a imx7s-warp leads to several warnings like these:

[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:3536 lock_release+0x2f8/0x330
[    0.000000] releasing a pinned lock

[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:2722 trace_hardirqs_on_caller+0x1ac/0x1f4
[    0.000000] DEBUG_LOCKS_WARN_ON(unlikely(early_boot_irqs_disabled))

[    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.7.0-rc7-next-20160715 #404

[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: CPU: 0 PID: 0 at kernel/time/sched_clock.c:179 sched_clock_register+0x44/0x1f8
[    0.000000] Modules linked in:

[    0.000591] ------------[ cut here ]------------
[    0.000610] WARNING: CPU: 0 PID: 0 at kernel/time/sched_clock.c:179 sched_clock_register+0x44/0x1f8

[    0.002084] ------------[ cut here ]------------
[    0.002104] WARNING: CPU: 0 PID: 0 at init/main.c:576 start_kernel+0x258/0x3b0
[    0.002114] Interrupts were enabled early


This fix is along the same lines as 5e33ebff7edd ("clk: imx7d: do not
set parent of ethernet time/ref clocks") and the explanation from that
commit is:

"The reason for the warning is that setting the parent enables the ENET
 PLL since we are using CLK_OPS_PARENT_ENABLE. Enabling the ENET PLL can
 cause clk_pllv3_wait_lock to sleep. See also:
 commit fc8726a2c021 ("clk: core: support clocks which requires parents
 enable (part 2)")."

imx7s-warp does not even use the FEC interface, so we should not really
configure the parent of IMX7D_ENET_AXI_ROOT_SRC in the common MX7 clock
driver code.

The dts file should use the assigned-clocks/assigned-clock-parents method,
so simply remove the configuration of IMX7D_ENET_AXI_ROOT_SRC parent.

Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
 drivers/clk/imx/clk-imx7d.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Fabio Estevam Aug. 10, 2016, 11:37 a.m. UTC | #1
Hi Mike,

On Fri, Jul 15, 2016 at 10:53 AM, Fabio Estevam <fabio.estevam@nxp.com> wrote:
> Booting the kernel on a imx7s-warp leads to several warnings like these:
>
> [    0.000000] ------------[ cut here ]------------
> [    0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:3536 lock_release+0x2f8/0x330
> [    0.000000] releasing a pinned lock
>
> [    0.000000] ------------[ cut here ]------------
> [    0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:2722 trace_hardirqs_on_caller+0x1ac/0x1f4
> [    0.000000] DEBUG_LOCKS_WARN_ON(unlikely(early_boot_irqs_disabled))
>
> [    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.7.0-rc7-next-20160715 #404
>
> [    0.000000] ------------[ cut here ]------------
> [    0.000000] WARNING: CPU: 0 PID: 0 at kernel/time/sched_clock.c:179 sched_clock_register+0x44/0x1f8
> [    0.000000] Modules linked in:
>
> [    0.000591] ------------[ cut here ]------------
> [    0.000610] WARNING: CPU: 0 PID: 0 at kernel/time/sched_clock.c:179 sched_clock_register+0x44/0x1f8
>
> [    0.002084] ------------[ cut here ]------------
> [    0.002104] WARNING: CPU: 0 PID: 0 at init/main.c:576 start_kernel+0x258/0x3b0
> [    0.002114] Interrupts were enabled early
>
>
> This fix is along the same lines as 5e33ebff7edd ("clk: imx7d: do not
> set parent of ethernet time/ref clocks") and the explanation from that
> commit is:
>
> "The reason for the warning is that setting the parent enables the ENET
>  PLL since we are using CLK_OPS_PARENT_ENABLE. Enabling the ENET PLL can
>  cause clk_pllv3_wait_lock to sleep. See also:
>  commit fc8726a2c021 ("clk: core: support clocks which requires parents
>  enable (part 2)")."
>
> imx7s-warp does not even use the FEC interface, so we should not really
> configure the parent of IMX7D_ENET_AXI_ROOT_SRC in the common MX7 clock
> driver code.
>
> The dts file should use the assigned-clocks/assigned-clock-parents method,
> so simply remove the configuration of IMX7D_ENET_AXI_ROOT_SRC parent.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>

Do you think this one could be applied to 4.8-rc?

Thanks
Stephen Boyd Aug. 11, 2016, 10:28 p.m. UTC | #2
On 08/10, Fabio Estevam wrote:
> Hi Mike,
> 
> On Fri, Jul 15, 2016 at 10:53 AM, Fabio Estevam <fabio.estevam@nxp.com> wrote:
> > Booting the kernel on a imx7s-warp leads to several warnings like these:
> >
> > [    0.000000] ------------[ cut here ]------------
> > [    0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:3536 lock_release+0x2f8/0x330
> > [    0.000000] releasing a pinned lock
> >
> > [    0.000000] ------------[ cut here ]------------
> > [    0.000000] WARNING: CPU: 0 PID: 0 at kernel/locking/lockdep.c:2722 trace_hardirqs_on_caller+0x1ac/0x1f4
> > [    0.000000] DEBUG_LOCKS_WARN_ON(unlikely(early_boot_irqs_disabled))
> >
> > [    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.7.0-rc7-next-20160715 #404
> >
> > [    0.000000] ------------[ cut here ]------------
> > [    0.000000] WARNING: CPU: 0 PID: 0 at kernel/time/sched_clock.c:179 sched_clock_register+0x44/0x1f8
> > [    0.000000] Modules linked in:
> >
> > [    0.000591] ------------[ cut here ]------------
> > [    0.000610] WARNING: CPU: 0 PID: 0 at kernel/time/sched_clock.c:179 sched_clock_register+0x44/0x1f8
> >
> > [    0.002084] ------------[ cut here ]------------
> > [    0.002104] WARNING: CPU: 0 PID: 0 at init/main.c:576 start_kernel+0x258/0x3b0
> > [    0.002114] Interrupts were enabled early
> >
> >
> > This fix is along the same lines as 5e33ebff7edd ("clk: imx7d: do not
> > set parent of ethernet time/ref clocks") and the explanation from that
> > commit is:
> >
> > "The reason for the warning is that setting the parent enables the ENET
> >  PLL since we are using CLK_OPS_PARENT_ENABLE. Enabling the ENET PLL can
> >  cause clk_pllv3_wait_lock to sleep. See also:
> >  commit fc8726a2c021 ("clk: core: support clocks which requires parents
> >  enable (part 2)")."
> >
> > imx7s-warp does not even use the FEC interface, so we should not really
> > configure the parent of IMX7D_ENET_AXI_ROOT_SRC in the common MX7 clock
> > driver code.
> >
> > The dts file should use the assigned-clocks/assigned-clock-parents method,
> > so simply remove the configuration of IMX7D_ENET_AXI_ROOT_SRC parent.
> >
> > Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> 
> Do you think this one could be applied to 4.8-rc?
>

Is there a "Fixes:" tag that can be applied here? Does this
depend on some sort of DTS changes to add assigned clocks and
parents?
Fabio Estevam Aug. 11, 2016, 11:53 p.m. UTC | #3
Hi Stephen,

On Thu, Aug 11, 2016 at 7:28 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:

> Is there a "Fixes:" tag that can be applied here? Does this

This parent assignment exists since the mx7d clk driver was introduced
in commit 8f6d8094b21 ("ARM: imx: add imx7d clk tree support").

> depend on some sort of DTS changes to add assigned clocks and
> parents?

All the mx7 dts users assign the ENET clocks in their dts files.

As imx7s-warp is the only affected board and it will be only available
in 4.9, I think it is OK to apply this one into clk-next if you are
happt with it.

Thanks,

Fabio Estevam
Stefan Agner Aug. 11, 2016, 11:57 p.m. UTC | #4
Hi,

On 2016-08-11 16:53, Fabio Estevam wrote:
> Hi Stephen,
> 
> On Thu, Aug 11, 2016 at 7:28 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> 
>> Is there a "Fixes:" tag that can be applied here? Does this
> 
> This parent assignment exists since the mx7d clk driver was introduced
> in commit 8f6d8094b21 ("ARM: imx: add imx7d clk tree support").
> 
>> depend on some sort of DTS changes to add assigned clocks and
>> parents?
> 
> All the mx7 dts users assign the ENET clocks in their dts files.

I don't think so, at least not that root clock.

Otherwise grep -e IMX7D_PLL_ENET_MAIN_250M_CLK arch/arm/boot/dts/imx7*
should return something...

I guess in practice it is not a problem because the boot loader
configures the clock selection already. Still, I think we should also
fix our device trees and select the root clock there...

--
Stefan
Fabio Estevam Aug. 12, 2016, 1:35 a.m. UTC | #5
Hi Stefan,

On Thu, Aug 11, 2016 at 8:57 PM, Stefan Agner <stefan@agner.ch> wrote:

> I don't think so, at least not that root clock.
>
> Otherwise grep -e IMX7D_PLL_ENET_MAIN_250M_CLK arch/arm/boot/dts/imx7*
> should return something...

Yes, you are right. The enet root clock parent assignment is missing in dts.

Other enet clocks are assigned.

> I guess in practice it is not a problem because the boot loader
> configures the clock selection already. Still, I think we should also
> fix our device trees and select the root clock there...

Yes, I didn't see any error on mx7d-sdb when I tested this patch, but
to be on the safe side I will update all the mx7 fec users to assign
enet root clk in the dts.

Thanks
Stephen Boyd Aug. 12, 2016, 1:45 a.m. UTC | #6
On 08/11, Fabio Estevam wrote:
> Hi Stefan,
> 
> On Thu, Aug 11, 2016 at 8:57 PM, Stefan Agner <stefan@agner.ch> wrote:
> 
> > I don't think so, at least not that root clock.
> >
> > Otherwise grep -e IMX7D_PLL_ENET_MAIN_250M_CLK arch/arm/boot/dts/imx7*
> > should return something...
> 
> Yes, you are right. The enet root clock parent assignment is missing in dts.
> 
> Other enet clocks are assigned.
> 
> > I guess in practice it is not a problem because the boot loader
> > configures the clock selection already. Still, I think we should also
> > fix our device trees and select the root clock there...
> 
> Yes, I didn't see any error on mx7d-sdb when I tested this patch, but
> to be on the safe side I will update all the mx7 fec users to assign
> enet root clk in the dts.
> 

Ok it sounds like this isn't urgent for fixes. I'll go ahead and
queue it up for v4.9
Fabio Estevam Aug. 12, 2016, 1:47 a.m. UTC | #7
On Thu, Aug 11, 2016 at 10:45 PM, Stephen Boyd <sboyd@codeaurora.org> wrote:

> Ok it sounds like this isn't urgent for fixes. I'll go ahead and
> queue it up for v4.9

Thanks, Stephen.
Stephen Boyd Aug. 13, 2016, 12:57 a.m. UTC | #8
On 07/15, Fabio Estevam wrote:
> Booting the kernel on a imx7s-warp leads to several warnings like these:
> 
> [    0.000000] ------------[ cut here ]------------

Applied to clk-next
diff mbox

Patch

diff --git a/drivers/clk/imx/clk-imx7d.c b/drivers/clk/imx/clk-imx7d.c
index 6ed4f8f..9257972 100644
--- a/drivers/clk/imx/clk-imx7d.c
+++ b/drivers/clk/imx/clk-imx7d.c
@@ -860,8 +860,6 @@  static void __init imx7d_clocks_init(struct device_node *ccm_node)
 	/* 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]);
 
-	clk_set_parent(clks[IMX7D_ENET_AXI_ROOT_SRC], clks[IMX7D_PLL_ENET_MAIN_250M_CLK]);
-
 	/* set uart module clock's parent clock source that must be great then 80MHz */
 	clk_set_parent(clks[IMX7D_UART1_ROOT_SRC], clks[IMX7D_OSC_24M_CLK]);