* [PATCH] clk: sp7021: fix infrastructure clocks clobbering PLL provider entries
@ 2026-08-20 13:02 Andrew Gaylard
2026-09-29 16:39 ` Brian Masney
0 siblings, 1 reply; 3+ messages in thread
From: Andrew Gaylard @ 2026-08-20 13:02 UTC (permalink / raw)
To: linux-clk
Cc: qinjian, mturquette, sboyd, bmasney, linux-arm-kernel,
Andrew Gaylard
The sp_clk_gates[] array previously included 9 infrastructure gate
clocks (CLK_SYSTEM, CLK_IOCTL, etc.) appended after the 64 DT-visible
gate clocks. The registration loop used the array index directly as the
hws[] index, so infrastructure clocks at positions 64-72 overwrote
hws[PLL_A] through hws[PLL_SYS].
As a result, DT lookups for <&clkc PLL_E_25> (index 67), PLL_E_2P5
(66) and PLL_E_112P5 (68) silently resolved to CLK_RBUS, CLK_IOP and
CLK_SDCTRL0 respectively. The L2SW Ethernet driver believed it was
holding the PLLE outputs, but was actually double-referencing already-
critical infrastructure clocks. The real PLLE outputs remained at
enable_count=0 and were powered down by clk_disable_unused(), causing
an Ethernet hang on boards with fixed-link MAC-to-MAC connections.
Fix by splitting infrastructure clocks into a separate sp_clk_infra[]
array and registering them with a dedicated loop that uses a local hw
variable rather than an hws[] slot. Since these clocks have no DT
consumer they do not need to appear in the of_clk provider table.
With the provider table correct, the L2SW driver's devm_clk_get_enabled()
calls now hold the actual PLLE outputs, making CLK_IS_CRITICAL on plle
and its sub-outputs unnecessary. Remove it.
Signed-off-by: Andrew Gaylard <ag@ffroot.co.za>
---
drivers/clk/clk-sp7021.c | 42 +++++++++++++++++++++++++++++-----------
1 file changed, 31 insertions(+), 11 deletions(-)
diff --git a/drivers/clk/clk-sp7021.c b/drivers/clk/clk-sp7021.c
index 1d067b0af8a7..50c667cbb3dc 100644
--- a/drivers/clk/clk-sp7021.c
+++ b/drivers/clk/clk-sp7021.c
@@ -124,9 +124,14 @@ static const struct sp_clk_gate_info sp_clk_gates[] = {
{ 0x97 },
{ 0x98 }, /* CLK_ICM: Input Capture Module */
{ 0x99, 0, true }, /* CLK_AXI_GLOBAL: AXI interconnect, no driver consumer */
- /* Infrastructure clocks: all default to enabled in hardware but
- * have no driver consumer, so marked critical to prevent gating.
- */
+};
+
+/*
+ * Infrastructure clocks have no DT consumer so are not exposed
+ * through the of_clk provider. They are registered separately and
+ * marked CLK_IS_CRITICAL so clk_disable_unused() never gates them.
+ */
+static const struct sp_clk_gate_info sp_clk_infra[] = {
{ 0x00, 0, true }, /* CLK_SYSTEM: SYSTEM CLKEN mo_clken0 bit 0 */
{ 0x03, 0, true }, /* CLK_IOCTL: IOCTL CLKEN mo_clken0 bit 3 */
{ 0x04, 0, true }, /* CLK_IOP: IOP CLKEN mo_clken0 bit 4 */
@@ -644,25 +649,23 @@ static int sp7021_clk_probe(struct platform_device *pdev)
/*
* PLLE and all its sub-outputs are used by the L2SW Ethernet switch
* (50MHz RMII, 25MHz MII/100M, 2.5MHz MII/10M, 112.5MHz MAC fabric).
- * The L2SW driver explicitly claims these via clock-names in DTS, but
- * CLK_IS_CRITICAL is kept as a belt-and-suspenders guard in case the
- * driver consumer reference isn't sufficient on all board configurations.
+ * The L2SW driver explicitly claims these via clock-names in DTS.
*/
hws[PLL_E] = sp_pll_register(dev, "plle", &pd_ext, PLLE_CTL,
- 6, 2, 50000000, 0, 0, CLK_IS_CRITICAL);
+ 6, 2, 50000000, 0, 0, 0);
if (IS_ERR(hws[PLL_E]))
return PTR_ERR(hws[PLL_E]);
pd_e.hw = hws[PLL_E];
hws[PLL_E_2P5] = sp_pll_register(dev, "plle_2p5", &pd_e, PLLE_CTL,
- 13, -1, 2500000, 0, 0, CLK_IS_CRITICAL);
+ 13, -1, 2500000, 0, 0, 0);
if (IS_ERR(hws[PLL_E_2P5]))
return PTR_ERR(hws[PLL_E_2P5]);
hws[PLL_E_25] = sp_pll_register(dev, "plle_25", &pd_e, PLLE_CTL,
- 12, -1, 25000000, 0, 0, CLK_IS_CRITICAL);
+ 12, -1, 25000000, 0, 0, 0);
if (IS_ERR(hws[PLL_E_25]))
return PTR_ERR(hws[PLL_E_25]);
hws[PLL_E_112P5] = sp_pll_register(dev, "plle_112p5", &pd_e, PLLE_CTL,
- 11, -1, 112500000, 0, 0, CLK_IS_CRITICAL);
+ 11, -1, 112500000, 0, 0, 0);
if (IS_ERR(hws[PLL_E_112P5]))
return PTR_ERR(hws[PLL_E_112P5]);
@@ -689,7 +692,7 @@ static int sp7021_clk_probe(struct platform_device *pdev)
return PTR_ERR(hws[PLL_SYS]);
pd_sys.hw = hws[PLL_SYS];
- /* gates */
+ /* gates, directly mapped into hws[] for DT lookup */
for (i = 0; i < ARRAY_SIZE(sp_clk_gates); i++) {
char name[10];
u32 j = sp_clk_gates[i].reg;
@@ -706,6 +709,23 @@ static int sp7021_clk_probe(struct platform_device *pdev)
return PTR_ERR(hws[i]);
}
+ /* infrastructure gates, not in hws[] */
+ for (i = 0; i < ARRAY_SIZE(sp_clk_infra); i++) {
+ char name[14];
+ u32 j = sp_clk_infra[i].reg;
+ struct clk_hw *hw;
+
+ sprintf(name, "infra_0x%02x", j);
+ hw = devm_clk_hw_register_gate_parent_data(dev, name, &pd_sys,
+ CLK_IS_CRITICAL,
+ clk_base + (j >> 4) * 4,
+ j & 0x0f,
+ CLK_GATE_HIWORD_MASK,
+ NULL);
+ if (IS_ERR(hw))
+ return PTR_ERR(hw);
+ }
+
return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] clk: sp7021: fix infrastructure clocks clobbering PLL provider entries
2026-08-20 13:02 [PATCH] clk: sp7021: fix infrastructure clocks clobbering PLL provider entries Andrew Gaylard
@ 2026-09-29 16:39 ` Brian Masney
2026-10-01 7:39 ` Andrew Gaylard
0 siblings, 1 reply; 3+ messages in thread
From: Brian Masney @ 2026-09-29 16:39 UTC (permalink / raw)
To: Andrew Gaylard
Cc: linux-clk, qinjian, mturquette, sboyd, bmasney, linux-arm-kernel
Hi Andrew,
On 2026-08-20 15:02 +0200, Andrew Gaylard wrote:
> The sp_clk_gates[] array previously included 9 infrastructure gate
> clocks (CLK_SYSTEM, CLK_IOCTL, etc.) appended after the 64 DT-visible
> gate clocks. The registration loop used the array index directly as the
> hws[] index, so infrastructure clocks at positions 64-72 overwrote
> hws[PLL_A] through hws[PLL_SYS].
>
> As a result, DT lookups for <&clkc PLL_E_25> (index 67), PLL_E_2P5
> (66) and PLL_E_112P5 (68) silently resolved to CLK_RBUS, CLK_IOP and
> CLK_SDCTRL0 respectively. The L2SW Ethernet driver believed it was
> holding the PLLE outputs, but was actually double-referencing already-
> critical infrastructure clocks. The real PLLE outputs remained at
> enable_count=0 and were powered down by clk_disable_unused(), causing
> an Ethernet hang on boards with fixed-link MAC-to-MAC connections.
>
> Fix by splitting infrastructure clocks into a separate sp_clk_infra[]
> array and registering them with a dedicated loop that uses a local hw
> variable rather than an hws[] slot. Since these clocks have no DT
> consumer they do not need to appear in the of_clk provider table.
>
> With the provider table correct, the L2SW driver's devm_clk_get_enabled()
> calls now hold the actual PLLE outputs, making CLK_IS_CRITICAL on plle
> and its sub-outputs unnecessary. Remove it.
>
> Signed-off-by: Andrew Gaylard <ag@ffroot.co.za>
This patch does not apply against the tree.
Brian
> ---
> drivers/clk/clk-sp7021.c | 42 +++++++++++++++++++++++++++++-----------
> 1 file changed, 31 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/clk/clk-sp7021.c b/drivers/clk/clk-sp7021.c
> index 1d067b0af8a7..50c667cbb3dc 100644
> --- a/drivers/clk/clk-sp7021.c
> +++ b/drivers/clk/clk-sp7021.c
> @@ -124,9 +124,14 @@ static const struct sp_clk_gate_info sp_clk_gates[] = {
> { 0x97 },
> { 0x98 }, /* CLK_ICM: Input Capture Module */
> { 0x99, 0, true }, /* CLK_AXI_GLOBAL: AXI interconnect, no driver consumer */
> - /* Infrastructure clocks: all default to enabled in hardware but
> - * have no driver consumer, so marked critical to prevent gating.
> - */
> +};
> +
> +/*
> + * Infrastructure clocks have no DT consumer so are not exposed
> + * through the of_clk provider. They are registered separately and
> + * marked CLK_IS_CRITICAL so clk_disable_unused() never gates them.
> + */
> +static const struct sp_clk_gate_info sp_clk_infra[] = {
> { 0x00, 0, true }, /* CLK_SYSTEM: SYSTEM CLKEN mo_clken0 bit 0 */
> { 0x03, 0, true }, /* CLK_IOCTL: IOCTL CLKEN mo_clken0 bit 3 */
> { 0x04, 0, true }, /* CLK_IOP: IOP CLKEN mo_clken0 bit 4 */
> @@ -644,25 +649,23 @@ static int sp7021_clk_probe(struct platform_device *pdev)
> /*
> * PLLE and all its sub-outputs are used by the L2SW Ethernet switch
> * (50MHz RMII, 25MHz MII/100M, 2.5MHz MII/10M, 112.5MHz MAC fabric).
> - * The L2SW driver explicitly claims these via clock-names in DTS, but
> - * CLK_IS_CRITICAL is kept as a belt-and-suspenders guard in case the
> - * driver consumer reference isn't sufficient on all board configurations.
> + * The L2SW driver explicitly claims these via clock-names in DTS.
> */
> hws[PLL_E] = sp_pll_register(dev, "plle", &pd_ext, PLLE_CTL,
> - 6, 2, 50000000, 0, 0, CLK_IS_CRITICAL);
> + 6, 2, 50000000, 0, 0, 0);
> if (IS_ERR(hws[PLL_E]))
> return PTR_ERR(hws[PLL_E]);
> pd_e.hw = hws[PLL_E];
> hws[PLL_E_2P5] = sp_pll_register(dev, "plle_2p5", &pd_e, PLLE_CTL,
> - 13, -1, 2500000, 0, 0, CLK_IS_CRITICAL);
> + 13, -1, 2500000, 0, 0, 0);
> if (IS_ERR(hws[PLL_E_2P5]))
> return PTR_ERR(hws[PLL_E_2P5]);
> hws[PLL_E_25] = sp_pll_register(dev, "plle_25", &pd_e, PLLE_CTL,
> - 12, -1, 25000000, 0, 0, CLK_IS_CRITICAL);
> + 12, -1, 25000000, 0, 0, 0);
> if (IS_ERR(hws[PLL_E_25]))
> return PTR_ERR(hws[PLL_E_25]);
> hws[PLL_E_112P5] = sp_pll_register(dev, "plle_112p5", &pd_e, PLLE_CTL,
> - 11, -1, 112500000, 0, 0, CLK_IS_CRITICAL);
> + 11, -1, 112500000, 0, 0, 0);
> if (IS_ERR(hws[PLL_E_112P5]))
> return PTR_ERR(hws[PLL_E_112P5]);
>
> @@ -689,7 +692,7 @@ static int sp7021_clk_probe(struct platform_device *pdev)
> return PTR_ERR(hws[PLL_SYS]);
> pd_sys.hw = hws[PLL_SYS];
>
> - /* gates */
> + /* gates, directly mapped into hws[] for DT lookup */
> for (i = 0; i < ARRAY_SIZE(sp_clk_gates); i++) {
> char name[10];
> u32 j = sp_clk_gates[i].reg;
> @@ -706,6 +709,23 @@ static int sp7021_clk_probe(struct platform_device *pdev)
> return PTR_ERR(hws[i]);
> }
>
> + /* infrastructure gates, not in hws[] */
> + for (i = 0; i < ARRAY_SIZE(sp_clk_infra); i++) {
> + char name[14];
> + u32 j = sp_clk_infra[i].reg;
> + struct clk_hw *hw;
> +
> + sprintf(name, "infra_0x%02x", j);
> + hw = devm_clk_hw_register_gate_parent_data(dev, name, &pd_sys,
> + CLK_IS_CRITICAL,
> + clk_base + (j >> 4) * 4,
> + j & 0x0f,
> + CLK_GATE_HIWORD_MASK,
> + NULL);
> + if (IS_ERR(hw))
> + return PTR_ERR(hw);
> + }
> +
> return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, clk_data);
> }
>
> --
> 2.53.0
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] clk: sp7021: fix infrastructure clocks clobbering PLL provider entries
2026-09-29 16:39 ` Brian Masney
@ 2026-10-01 7:39 ` Andrew Gaylard
0 siblings, 0 replies; 3+ messages in thread
From: Andrew Gaylard @ 2026-10-01 7:39 UTC (permalink / raw)
To: Brian Masney; +Cc: linux-clk, qinjian, mturquette, sboyd, linux-arm-kernel
Brian Masney <bmasney@redhat.com> writes:
> Hi Andrew,
>
> On 2026-08-20 15:02 +0200, Andrew Gaylard wrote:
>> The sp_clk_gates[] array previously included 9 infrastructure gate
>> clocks (CLK_SYSTEM, CLK_IOCTL, etc.) appended after the 64 DT-visible
>> gate clocks. The registration loop used the array index directly as the
>> hws[] index, so infrastructure clocks at positions 64-72 overwrote
>> hws[PLL_A] through hws[PLL_SYS].
[...]
>
> This patch does not apply against the tree.
>
> Brian
Hi Brian,
Thanks for looking at this path. I have a better fix now, but it needs
another series [0] to land first.
[0] https://lore.kernel.org/linux-arm-kernel/20260924164922.117236-1-ag@ffroot.co.za/
--
Andrew
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-01 7:39 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-20 13:02 [PATCH] clk: sp7021: fix infrastructure clocks clobbering PLL provider entries Andrew Gaylard
2026-09-29 16:39 ` Brian Masney
2026-10-01 7:39 ` Andrew Gaylard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox