[PATCH] clk: rockchip: rk3588: make refclko25m_ethX critical

[PATCH] clk: rockchip: rk3588: make refclko25m_ethX critical

[Alexander Shiyan]

Ethernet phys normally need a 25MHz refclk input. On a lot of boards
this is done with a dedicated 25MHz crystal. But the rk3588 CRU also
provides a means for that via the refclko25m_ethX clock outputs that
can be used for that function.

The mdio bus normally probes devices on the bus at runtime, by reading
specific phy registers. This requires the phy to be running and thus
also being supplied by its reference clock.

While there exist the possibility and dt-binding to declare these
input clocks for each phy in the phy-dt-node, this is only relevant
_after_ the phy has been detected and during the drivers probe-run.

This results in a chicken-and-egg-problem. The refclks in the CRU are
running on boot of course, but phy-probing can very well happen after
clk_disable_unused has run.

In the past I tried to make clock-handling part of the mdio bus code [0]
but that wasn't very well received, due to it being specific to OF and
clocks with the consensus being that resources needed for detection
need to be enabled before.

So to make probing ethernet phys using the internal refclks possible,
make those 2 clocks critical.

[0] https://lore.kernel.org/netdev/13590315.F0gNSz5aLb@diego/T/

Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com>
---
 drivers/clk/rockchip/clk-rk3588.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3588.c b/drivers/clk/rockchip/clk-rk3588.c
index 736ce4a543..f6d16ed0cf 100644
--- a/drivers/clk/rockchip/clk-rk3588.c
+++ b/drivers/clk/rockchip/clk-rk3588.c
@@ -794,10 +794,10 @@ static struct rockchip_clk_branch rk3588_clk_branches[] __initdata = {
 	COMPOSITE(MCLK_GMAC0_OUT, "mclk_gmac0_out", gpll_cpll_p, 0,
 			RK3588_CLKSEL_CON(15), 7, 1, MFLAGS, 0, 7, DFLAGS,
 			RK3588_CLKGATE_CON(5), 3, GFLAGS),
-	COMPOSITE(REFCLKO25M_ETH0_OUT, "refclko25m_eth0_out", gpll_cpll_p, 0,
+	COMPOSITE(REFCLKO25M_ETH0_OUT, "refclko25m_eth0_out", gpll_cpll_p, CLK_IS_CRITICAL,
 			RK3588_CLKSEL_CON(15), 15, 1, MFLAGS, 8, 7, DFLAGS,
 			RK3588_CLKGATE_CON(5), 4, GFLAGS),
-	COMPOSITE(REFCLKO25M_ETH1_OUT, "refclko25m_eth1_out", gpll_cpll_p, 0,
+	COMPOSITE(REFCLKO25M_ETH1_OUT, "refclko25m_eth1_out", gpll_cpll_p, CLK_IS_CRITICAL,
 			RK3588_CLKSEL_CON(16), 7, 1, MFLAGS, 0, 7, DFLAGS,
 			RK3588_CLKGATE_CON(5), 5, GFLAGS),
 	COMPOSITE(CLK_CIFOUT_OUT, "clk_cifout_out", gpll_cpll_24m_spll_p, 0,
-- 
2.38.2

Re: [PATCH] clk: rockchip: rk3588: make refclko25m_ethX critical

[Sascha Hauer]

On Mon, 31 Mar 2025 15:39:48 +0300, Alexander Shiyan wrote:
> Ethernet phys normally need a 25MHz refclk input. On a lot of boards
> this is done with a dedicated 25MHz crystal. But the rk3588 CRU also
> provides a means for that via the refclko25m_ethX clock outputs that
> can be used for that function.
> 
> The mdio bus normally probes devices on the bus at runtime, by reading
> specific phy registers. This requires the phy to be running and thus
> also being supplied by its reference clock.
> 
> [...]

Applied, thanks!

[1/1] clk: rockchip: rk3588: make refclko25m_ethX critical
      https://git.pengutronix.de/cgit/barebox/commit/?id=46babe1348e0 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

Re: [PATCH] clk: rockchip: rk3588: make refclko25m_ethX critical

[Sascha Hauer]

On Mon, Mar 31, 2025 at 03:39:48PM +0300, Alexander Shiyan wrote:
> Ethernet phys normally need a 25MHz refclk input. On a lot of boards
> this is done with a dedicated 25MHz crystal. But the rk3588 CRU also
> provides a means for that via the refclko25m_ethX clock outputs that
> can be used for that function.
> 
> The mdio bus normally probes devices on the bus at runtime, by reading
> specific phy registers. This requires the phy to be running and thus
> also being supplied by its reference clock.
> 
> While there exist the possibility and dt-binding to declare these
> input clocks for each phy in the phy-dt-node, this is only relevant
> _after_ the phy has been detected and during the drivers probe-run.
> 
> This results in a chicken-and-egg-problem. The refclks in the CRU are
> running on boot of course, but phy-probing can very well happen after
> clk_disable_unused has run.
> 
> In the past I tried to make clock-handling part of the mdio bus code [0]
> but that wasn't very well received, due to it being specific to OF and
> clocks with the consensus being that resources needed for detection
> need to be enabled before.
> 
> So to make probing ethernet phys using the internal refclks possible,
> make those 2 clocks critical.

Quite an unfortunate situation. I must say that I don't really like this
patch as it's only helps to solve this on RK3588 and additionally this
change is easily lost when we update the clock drivers from Linux.

I don't see an easy solution for this issue, so I applied this patch for
now. Nevertheless it would be great to solve that generically in the
long run.

Sascha

> 
> [0] https://lore.kernel.org/netdev/13590315.F0gNSz5aLb@diego/T/
> 
> Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com>
> ---
>  drivers/clk/rockchip/clk-rk3588.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/rockchip/clk-rk3588.c b/drivers/clk/rockchip/clk-rk3588.c
> index 736ce4a543..f6d16ed0cf 100644
> --- a/drivers/clk/rockchip/clk-rk3588.c
> +++ b/drivers/clk/rockchip/clk-rk3588.c
> @@ -794,10 +794,10 @@ static struct rockchip_clk_branch rk3588_clk_branches[] __initdata = {
>  	COMPOSITE(MCLK_GMAC0_OUT, "mclk_gmac0_out", gpll_cpll_p, 0,
>  			RK3588_CLKSEL_CON(15), 7, 1, MFLAGS, 0, 7, DFLAGS,
>  			RK3588_CLKGATE_CON(5), 3, GFLAGS),
> -	COMPOSITE(REFCLKO25M_ETH0_OUT, "refclko25m_eth0_out", gpll_cpll_p, 0,
> +	COMPOSITE(REFCLKO25M_ETH0_OUT, "refclko25m_eth0_out", gpll_cpll_p, CLK_IS_CRITICAL,
>  			RK3588_CLKSEL_CON(15), 15, 1, MFLAGS, 8, 7, DFLAGS,
>  			RK3588_CLKGATE_CON(5), 4, GFLAGS),
> -	COMPOSITE(REFCLKO25M_ETH1_OUT, "refclko25m_eth1_out", gpll_cpll_p, 0,
> +	COMPOSITE(REFCLKO25M_ETH1_OUT, "refclko25m_eth1_out", gpll_cpll_p, CLK_IS_CRITICAL,
>  			RK3588_CLKSEL_CON(16), 7, 1, MFLAGS, 0, 7, DFLAGS,
>  			RK3588_CLKGATE_CON(5), 5, GFLAGS),
>  	COMPOSITE(CLK_CIFOUT_OUT, "clk_cifout_out", gpll_cpll_24m_spll_p, 0,
> -- 
> 2.38.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH] clk: rockchip: rk3588: make refclko25m_ethX critical

[Alexander Shiyan]

Hello.

In fact, at the moment the patch does not fix anything, since there are
no drivers for rk3588 gmac in barebox, even generic ones.
The patch is taken from the kernel repository, so I think the changes
will not disappear anywhere.

Thanks!

пн, 7 апр. 2025 г. в 16:30, Sascha Hauer <s.hauer@pengutronix.de>:
>
> On Mon, Mar 31, 2025 at 03:39:48PM +0300, Alexander Shiyan wrote:
> > Ethernet phys normally need a 25MHz refclk input. On a lot of boards
> > this is done with a dedicated 25MHz crystal. But the rk3588 CRU also
> > provides a means for that via the refclko25m_ethX clock outputs that
> > can be used for that function.
> >
> > The mdio bus normally probes devices on the bus at runtime, by reading
> > specific phy registers. This requires the phy to be running and thus
> > also being supplied by its reference clock.
> >
> > While there exist the possibility and dt-binding to declare these
> > input clocks for each phy in the phy-dt-node, this is only relevant
> > _after_ the phy has been detected and during the drivers probe-run.
> >
> > This results in a chicken-and-egg-problem. The refclks in the CRU are
> > running on boot of course, but phy-probing can very well happen after
> > clk_disable_unused has run.
> >
> > In the past I tried to make clock-handling part of the mdio bus code [0]
> > but that wasn't very well received, due to it being specific to OF and
> > clocks with the consensus being that resources needed for detection
> > need to be enabled before.
> >
> > So to make probing ethernet phys using the internal refclks possible,
> > make those 2 clocks critical.
>
> Quite an unfortunate situation. I must say that I don't really like this
> patch as it's only helps to solve this on RK3588 and additionally this
> change is easily lost when we update the clock drivers from Linux.
>
> I don't see an easy solution for this issue, so I applied this patch for
> now. Nevertheless it would be great to solve that generically in the
> long run.
>
> Sascha
>
> >
> > [0] https://lore.kernel.org/netdev/13590315.F0gNSz5aLb@diego/T/
> >
> > Signed-off-by: Alexander Shiyan <eagle.alexander923@gmail.com>
> > ---
> >  drivers/clk/rockchip/clk-rk3588.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/clk/rockchip/clk-rk3588.c b/drivers/clk/rockchip/clk-rk3588.c
> > index 736ce4a543..f6d16ed0cf 100644
> > --- a/drivers/clk/rockchip/clk-rk3588.c
> > +++ b/drivers/clk/rockchip/clk-rk3588.c
> > @@ -794,10 +794,10 @@ static struct rockchip_clk_branch rk3588_clk_branches[] __initdata = {
> >       COMPOSITE(MCLK_GMAC0_OUT, "mclk_gmac0_out", gpll_cpll_p, 0,
> >                       RK3588_CLKSEL_CON(15), 7, 1, MFLAGS, 0, 7, DFLAGS,
> >                       RK3588_CLKGATE_CON(5), 3, GFLAGS),
> > -     COMPOSITE(REFCLKO25M_ETH0_OUT, "refclko25m_eth0_out", gpll_cpll_p, 0,
> > +     COMPOSITE(REFCLKO25M_ETH0_OUT, "refclko25m_eth0_out", gpll_cpll_p, CLK_IS_CRITICAL,
> >                       RK3588_CLKSEL_CON(15), 15, 1, MFLAGS, 8, 7, DFLAGS,
> >                       RK3588_CLKGATE_CON(5), 4, GFLAGS),
> > -     COMPOSITE(REFCLKO25M_ETH1_OUT, "refclko25m_eth1_out", gpll_cpll_p, 0,
> > +     COMPOSITE(REFCLKO25M_ETH1_OUT, "refclko25m_eth1_out", gpll_cpll_p, CLK_IS_CRITICAL,
> >                       RK3588_CLKSEL_CON(16), 7, 1, MFLAGS, 0, 7, DFLAGS,
> >                       RK3588_CLKGATE_CON(5), 5, GFLAGS),
> >       COMPOSITE(CLK_CIFOUT_OUT, "clk_cifout_out", gpll_cpll_24m_spll_p, 0,
> > --
> > 2.38.2
> >
> >
> >
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |