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

[Alexander Shiyan <eagle.alexander923@gmail.com>]

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 |