clk: imx7d: do not set the parent of IMX7D_ENET_AXI_ROOT_SRC

PATCH New Fabio Estevam 4 participants 9 messages Jul 15, 2016 – Aug 13, 2016

Subsystems (3)

Arm i.MX and Layerscape Common clk NXP i.MX clock drivers

lore

Share & tools

lore

Fabio Estevam

author

2016-07-15 13:53 UTC #0 lore raw

PATCH 1/1 clk: imx7d: do not set the parent of IMX7D_ENET_AXI_ROOT_SRC

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 <[email protected]>

---
 drivers/clk/imx/clk-imx7d.c | 2 --
 1 file changed, 2 deletions(-)

drivers/clk/imx/clk-imx7d.c +0 -2

index 6ed4f8f..9257972 100644
@@ -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]);

Signature · 1 line

1.9.1

Fabio Estevam

reviewer

2016-08-10 11:37 UTC #1 lore raw

PATCH 1/1 clk: imx7d: do not set the parent of IMX7D_ENET_AXI_ROOT_SRC

Hi Mike,

Quoted 36 lines · Fabio Estevam

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)")."

Quoted 8 lines

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 <[email protected]>

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

Thanks

Stephen Boyd

2016-08-11 22:28 UTC #2 lore raw

PATCH 1/1 clk: imx7d: do not set the parent of IMX7D_ENET_AXI_ROOT_SRC

Quoted 43 lines · Fabio Estevam

Hi Mike,
Quoted 40 lines · Fabio Estevam
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.

Quoted 7 lines

Quoted 4 lines
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 <[email protected]>
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?

Signature · 2 lines

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

Fabio Estevam

reviewer

2016-08-11 23:53 UTC #3 lore raw

PATCH 1/1 clk: imx7d: do not set the parent of IMX7D_ENET_AXI_ROOT_SRC

Hi Stephen,

Quoted 1 line · Stephen Boyd

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").

Quoted 2 lines

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

2016-08-11 23:57 UTC #4 lore raw

PATCH 1/1 clk: imx7d: do not set the parent of IMX7D_ENET_AXI_ROOT_SRC

Hi,

Quoted 6 lines · Fabio Estevam

Hi Stephen,
Quoted 1 line · Stephen Boyd
Is there a "Fixes:" tag that can be applied here? Does this

Quoted 7 lines

This parent assignment exists since the mx7d clk driver was introduced in commit 8f6d8094b21 ("ARM: imx: add imx7d clk tree support").
Quoted 2 lines
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

reviewer

2016-08-12 01:35 UTC #5 lore raw

PATCH 1/1 clk: imx7d: do not set the parent of IMX7D_ENET_AXI_ROOT_SRC

Hi Stefan,

Quoted 4 lines · Stefan Agner

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.

Quoted 3 lines

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

2016-08-12 01:45 UTC #6 lore raw

PATCH 1/1 clk: imx7d: do not set the parent of IMX7D_ENET_AXI_ROOT_SRC

Quoted 13 lines · Fabio Estevam

Hi Stefan,
Quoted 4 lines · Stefan Agner
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.

Quoted 8 lines

Quoted 3 lines
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

Signature · 2 lines

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

Fabio Estevam

reviewer

2016-08-12 01:47 UTC #7 lore raw

PATCH 1/1 clk: imx7d: do not set the parent of IMX7D_ENET_AXI_ROOT_SRC

Quoted 2 lines · Stephen Boyd

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

Applied

2016-08-13 00:57 UTC #8 lore raw

PATCH 1/1 clk: imx7d: do not set the parent of IMX7D_ENET_AXI_ROOT_SRC

In reply to Fabio Estevam

Quoted 3 lines · Fabio Estevam

Booting the kernel on a imx7s-warp leads to several warnings like these:
[    0.000000] ------------[ cut here ]------------

Applied to clk-next

Signature · 2 lines

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