[PATCH] clk: imx8mq: prevent sys1_pll_266m gating

Subsystems: common clk framework, nxp i.mx clock drivers, the rest

STALE1940d

4 messages, 3 authors, 2021-05-17 · open the first message on its own page

[PATCH] clk: imx8mq: prevent sys1_pll_266m gating

From: Lucas Stach <l.stach@pengutronix.de>
Date: 2021-05-07 17:10:36 

Gating sys1_pll_266m while the usdhc_nand_bus clock is still active (due
to being enabled in to bootloader) leads to spurious failures of the
uSDHC module.

b04383b6a558 ("clk: imx8mq: Define gates for pll1/2 fixed dividers")
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
We probably need some solution to keep parent clocks active on the i.MX8M clock
architecture, as long as any consumers are active, as the reference manual
states that disabling a parent clock may lead to undefined behavior. This needs
more work in the clock framework and/or driver. This patch fixes the obvious
regression until we have such a solution.
---
 drivers/clk/imx/clk-imx8mq.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index 4dd4ae9d022b..fce983add1fc 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -372,7 +372,6 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
 	hws[IMX8MQ_SYS1_PLL_133M_CG] = imx_clk_hw_gate("sys1_pll_133m_cg", "sys1_pll_out", base + 0x30, 15);
 	hws[IMX8MQ_SYS1_PLL_160M_CG] = imx_clk_hw_gate("sys1_pll_160m_cg", "sys1_pll_out", base + 0x30, 17);
 	hws[IMX8MQ_SYS1_PLL_200M_CG] = imx_clk_hw_gate("sys1_pll_200m_cg", "sys1_pll_out", base + 0x30, 19);
-	hws[IMX8MQ_SYS1_PLL_266M_CG] = imx_clk_hw_gate("sys1_pll_266m_cg", "sys1_pll_out", base + 0x30, 21);
 	hws[IMX8MQ_SYS1_PLL_400M_CG] = imx_clk_hw_gate("sys1_pll_400m_cg", "sys1_pll_out", base + 0x30, 23);
 	hws[IMX8MQ_SYS1_PLL_800M_CG] = imx_clk_hw_gate("sys1_pll_800m_cg", "sys1_pll_out", base + 0x30, 25);
 
@@ -382,7 +381,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
 	hws[IMX8MQ_SYS1_PLL_133M] = imx_clk_hw_fixed_factor("sys1_pll_133m", "sys1_pll_133m_cg", 1, 6);
 	hws[IMX8MQ_SYS1_PLL_160M] = imx_clk_hw_fixed_factor("sys1_pll_160m", "sys1_pll_160m_cg", 1, 5);
 	hws[IMX8MQ_SYS1_PLL_200M] = imx_clk_hw_fixed_factor("sys1_pll_200m", "sys1_pll_200m_cg", 1, 4);
-	hws[IMX8MQ_SYS1_PLL_266M] = imx_clk_hw_fixed_factor("sys1_pll_266m", "sys1_pll_266m_cg", 1, 3);
+	hws[IMX8MQ_SYS1_PLL_266M] = imx_clk_hw_fixed_factor("sys1_pll_266m", "sys1_pll_out", 1, 3);
 	hws[IMX8MQ_SYS1_PLL_400M] = imx_clk_hw_fixed_factor("sys1_pll_400m", "sys1_pll_400m_cg", 1, 2);
 	hws[IMX8MQ_SYS1_PLL_800M] = imx_clk_hw_fixed_factor("sys1_pll_800m", "sys1_pll_800m_cg", 1, 1);
 
-- 
2.29.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

RE: [PATCH] clk: imx8mq: prevent sys1_pll_266m gating

From: Jacky Bai <ping.bai@nxp.com>
Date: 2021-05-08 00:45:42 

Subject: [PATCH] clk: imx8mq: prevent sys1_pll_266m gating

Gating sys1_pll_266m while the usdhc_nand_bus clock is still active (due to
being enabled in to bootloader) leads to spurious failures of the uSDHC
module.

b04383b6a558 ("clk: imx8mq: Define gates for pll1/2 fixed dividers")
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
We probably need some solution to keep parent clocks active on the i.MX8M
clock architecture, as long as any consumers are active, as the reference
manual states that disabling a parent clock may lead to undefined behavior.
No such limitation, should be Doc issue.

This needs more work in the clock framework and/or driver. This patch fixes
the obvious regression until we have such a solution.
Personally, I prefer to reverted the whole patch b04383b6a558 ("clk: imx8mq: Define gates for pll1/2 fixed dividers").
As instancing those gates increasing the complexity of AMP system clock management, no other obvious benefit.

BR
Jacky Bai
quoted hunk
---
 drivers/clk/imx/clk-imx8mq.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index 4dd4ae9d022b..fce983add1fc 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -372,7 +372,6 @@ static int imx8mq_clocks_probe(struct
platform_device *pdev)
 	hws[IMX8MQ_SYS1_PLL_133M_CG] =
imx_clk_hw_gate("sys1_pll_133m_cg", "sys1_pll_out", base + 0x30, 15);
 	hws[IMX8MQ_SYS1_PLL_160M_CG] =
imx_clk_hw_gate("sys1_pll_160m_cg", "sys1_pll_out", base + 0x30, 17);
 	hws[IMX8MQ_SYS1_PLL_200M_CG] =
imx_clk_hw_gate("sys1_pll_200m_cg", "sys1_pll_out", base + 0x30, 19);
-	hws[IMX8MQ_SYS1_PLL_266M_CG] =
imx_clk_hw_gate("sys1_pll_266m_cg", "sys1_pll_out", base + 0x30, 21);
 	hws[IMX8MQ_SYS1_PLL_400M_CG] =
imx_clk_hw_gate("sys1_pll_400m_cg", "sys1_pll_out", base + 0x30, 23);
 	hws[IMX8MQ_SYS1_PLL_800M_CG] =
imx_clk_hw_gate("sys1_pll_800m_cg", "sys1_pll_out", base + 0x30, 25);

@@ -382,7 +381,7 @@ static int imx8mq_clocks_probe(struct
platform_device *pdev)
 	hws[IMX8MQ_SYS1_PLL_133M] =
imx_clk_hw_fixed_factor("sys1_pll_133m", "sys1_pll_133m_cg", 1, 6);
 	hws[IMX8MQ_SYS1_PLL_160M] =
imx_clk_hw_fixed_factor("sys1_pll_160m", "sys1_pll_160m_cg", 1, 5);
 	hws[IMX8MQ_SYS1_PLL_200M] =
imx_clk_hw_fixed_factor("sys1_pll_200m", "sys1_pll_200m_cg", 1, 4);
-	hws[IMX8MQ_SYS1_PLL_266M] =
imx_clk_hw_fixed_factor("sys1_pll_266m", "sys1_pll_266m_cg", 1, 3);
+	hws[IMX8MQ_SYS1_PLL_266M] =
imx_clk_hw_fixed_factor("sys1_pll_266m",
+"sys1_pll_out", 1, 3);
 	hws[IMX8MQ_SYS1_PLL_400M] =
imx_clk_hw_fixed_factor("sys1_pll_400m", "sys1_pll_400m_cg", 1, 2);
 	hws[IMX8MQ_SYS1_PLL_800M] =
imx_clk_hw_fixed_factor("sys1_pll_800m", "sys1_pll_800m_cg", 1, 1);

--
2.29.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] clk: imx8mq: prevent sys1_pll_266m gating

From: Abel Vesa <hidden>
Date: 2021-05-10 08:54:45 

On 21-05-07 19:10:28, Lucas Stach wrote:
Gating sys1_pll_266m while the usdhc_nand_bus clock is still active (due
to being enabled in to bootloader) leads to spurious failures of the
uSDHC module.

b04383b6a558 ("clk: imx8mq: Define gates for pll1/2 fixed dividers")
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
We probably need some solution to keep parent clocks active on the i.MX8M clock
architecture, as long as any consumers are active, as the reference manual
states that disabling a parent clock may lead to undefined behavior. This needs
more work in the clock framework and/or driver. This patch fixes the obvious
regression until we have such a solution.
I believe the solution here should be a core clock flag that forces
the core to read the gate bit value from register. The problem with this approach
is the clock core doesn't check if the child clocks are enabled when
disabling the parent clock, instead it relies on cached enabled count of the parent.
Basically, the CCF, as of now, isn't able to cope with clocks that can be
enabled/disabled from outside. This should be improved at some point.
quoted hunk
---
 drivers/clk/imx/clk-imx8mq.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index 4dd4ae9d022b..fce983add1fc 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -372,7 +372,6 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
 	hws[IMX8MQ_SYS1_PLL_133M_CG] = imx_clk_hw_gate("sys1_pll_133m_cg", "sys1_pll_out", base + 0x30, 15);
 	hws[IMX8MQ_SYS1_PLL_160M_CG] = imx_clk_hw_gate("sys1_pll_160m_cg", "sys1_pll_out", base + 0x30, 17);
 	hws[IMX8MQ_SYS1_PLL_200M_CG] = imx_clk_hw_gate("sys1_pll_200m_cg", "sys1_pll_out", base + 0x30, 19);
-	hws[IMX8MQ_SYS1_PLL_266M_CG] = imx_clk_hw_gate("sys1_pll_266m_cg", "sys1_pll_out", base + 0x30, 21);
 	hws[IMX8MQ_SYS1_PLL_400M_CG] = imx_clk_hw_gate("sys1_pll_400m_cg", "sys1_pll_out", base + 0x30, 23);
 	hws[IMX8MQ_SYS1_PLL_800M_CG] = imx_clk_hw_gate("sys1_pll_800m_cg", "sys1_pll_out", base + 0x30, 25);
 
@@ -382,7 +381,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
 	hws[IMX8MQ_SYS1_PLL_133M] = imx_clk_hw_fixed_factor("sys1_pll_133m", "sys1_pll_133m_cg", 1, 6);
 	hws[IMX8MQ_SYS1_PLL_160M] = imx_clk_hw_fixed_factor("sys1_pll_160m", "sys1_pll_160m_cg", 1, 5);
 	hws[IMX8MQ_SYS1_PLL_200M] = imx_clk_hw_fixed_factor("sys1_pll_200m", "sys1_pll_200m_cg", 1, 4);
-	hws[IMX8MQ_SYS1_PLL_266M] = imx_clk_hw_fixed_factor("sys1_pll_266m", "sys1_pll_266m_cg", 1, 3);
+	hws[IMX8MQ_SYS1_PLL_266M] = imx_clk_hw_fixed_factor("sys1_pll_266m", "sys1_pll_out", 1, 3);
 	hws[IMX8MQ_SYS1_PLL_400M] = imx_clk_hw_fixed_factor("sys1_pll_400m", "sys1_pll_400m_cg", 1, 2);
 	hws[IMX8MQ_SYS1_PLL_800M] = imx_clk_hw_fixed_factor("sys1_pll_800m", "sys1_pll_800m_cg", 1, 1);
 
-- 
2.29.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] clk: imx8mq: prevent sys1_pll_266m gating

From: Abel Vesa <hidden>
Date: 2021-05-17 10:05:56 

On 21-05-07 19:10:28, Lucas Stach wrote:
Gating sys1_pll_266m while the usdhc_nand_bus clock is still active (due
to being enabled in to bootloader) leads to spurious failures of the
uSDHC module.

b04383b6a558 ("clk: imx8mq: Define gates for pll1/2 fixed dividers")
Maybe this should have a Fixes tag then.

Then Stephen will pick it up himself.
quoted hunk
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
We probably need some solution to keep parent clocks active on the i.MX8M clock
architecture, as long as any consumers are active, as the reference manual
states that disabling a parent clock may lead to undefined behavior. This needs
more work in the clock framework and/or driver. This patch fixes the obvious
regression until we have such a solution.
---
 drivers/clk/imx/clk-imx8mq.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mq.c b/drivers/clk/imx/clk-imx8mq.c
index 4dd4ae9d022b..fce983add1fc 100644
--- a/drivers/clk/imx/clk-imx8mq.c
+++ b/drivers/clk/imx/clk-imx8mq.c
@@ -372,7 +372,6 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
 	hws[IMX8MQ_SYS1_PLL_133M_CG] = imx_clk_hw_gate("sys1_pll_133m_cg", "sys1_pll_out", base + 0x30, 15);
 	hws[IMX8MQ_SYS1_PLL_160M_CG] = imx_clk_hw_gate("sys1_pll_160m_cg", "sys1_pll_out", base + 0x30, 17);
 	hws[IMX8MQ_SYS1_PLL_200M_CG] = imx_clk_hw_gate("sys1_pll_200m_cg", "sys1_pll_out", base + 0x30, 19);
-	hws[IMX8MQ_SYS1_PLL_266M_CG] = imx_clk_hw_gate("sys1_pll_266m_cg", "sys1_pll_out", base + 0x30, 21);
 	hws[IMX8MQ_SYS1_PLL_400M_CG] = imx_clk_hw_gate("sys1_pll_400m_cg", "sys1_pll_out", base + 0x30, 23);
 	hws[IMX8MQ_SYS1_PLL_800M_CG] = imx_clk_hw_gate("sys1_pll_800m_cg", "sys1_pll_out", base + 0x30, 25);
 
@@ -382,7 +381,7 @@ static int imx8mq_clocks_probe(struct platform_device *pdev)
 	hws[IMX8MQ_SYS1_PLL_133M] = imx_clk_hw_fixed_factor("sys1_pll_133m", "sys1_pll_133m_cg", 1, 6);
 	hws[IMX8MQ_SYS1_PLL_160M] = imx_clk_hw_fixed_factor("sys1_pll_160m", "sys1_pll_160m_cg", 1, 5);
 	hws[IMX8MQ_SYS1_PLL_200M] = imx_clk_hw_fixed_factor("sys1_pll_200m", "sys1_pll_200m_cg", 1, 4);
-	hws[IMX8MQ_SYS1_PLL_266M] = imx_clk_hw_fixed_factor("sys1_pll_266m", "sys1_pll_266m_cg", 1, 3);
+	hws[IMX8MQ_SYS1_PLL_266M] = imx_clk_hw_fixed_factor("sys1_pll_266m", "sys1_pll_out", 1, 3);
 	hws[IMX8MQ_SYS1_PLL_400M] = imx_clk_hw_fixed_factor("sys1_pll_400m", "sys1_pll_400m_cg", 1, 2);
 	hws[IMX8MQ_SYS1_PLL_800M] = imx_clk_hw_fixed_factor("sys1_pll_800m", "sys1_pll_800m_cg", 1, 1);
 
-- 
2.29.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help