Hi Elaine, On 2025-01-25 06:38, Alex Bee wrote: > Hi Elaine, > > Am 25.01.25 um 02:15 schrieb Elaine Zhang: >> This reverts commit 1d34b9757523c1ad547bd6d040381f62d74a3189. >> >> RK3228 Only GPLL and CPLL, GPLL is a common clock, does not allow >> dclk_vop to change its frequency, CPLL is used by GMAC, >> if dclk_vop use CLK_SET_RATE_PARENT and CLK_SET_RATE_NO_REPARENT flags will >> affect the GMAC function. >> > how do you come to this conclusion? > > On the RK3228, hdmiphy is the default parent of dclk_vop, a clock that is > not even generated by the CRU but is the output of an external HDMI PHY. > The CLK_SET_RATE_NO_REPARENT flag ensures that the parent of dclk_vop never > changes to sclk_vop_pre (and thus never uses CPLL or GPLL). With > CLK_SET_RATE_PARENT we only ensure that we can use all rates of [0] since > there is no divider between dclk_vop and hdmiphy. Overall it is pretty much > the same situation as for RK3328, which handles these clocks in the same > way (see dclk_lcdc for RK3328). I came to similar conclusion for v1 of this patch, see [2]. Maybe we should assign clk parent in DT similar to rk3328.dtsi to make it extra clear that hdmiphy should be used as parent? assigned-clocks = <&cru DCLK_VOP>; assigned-clock-parents = <&cru SCLK_HDMI_PHY>; Also for next revert patch you send, please include the patch author in the recipient list :-) [2] https://lore.kernel.org/all/d95d317c-5f6e-42bd-94a9-e1a6c7685e2f@xxxxxxxxx/ Regards, Jonas > > Regards, > Alex > > [0] [1] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/phy/rockchip/phy-rockchip-inno-hdmi.c?h=v6.13#n293 >> If the client application does not use GMAC and CPLL is free, make this >> change on the local branch. >> >> Signed-off-by: Elaine Zhang <zhangqing@xxxxxxxxxxxxxx> >> --- >> drivers/clk/rockchip/clk-rk3228.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/clk/rockchip/clk-rk3228.c b/drivers/clk/rockchip/clk-rk3228.c >> index ed602c27b624..9c0284607766 100644 >> --- a/drivers/clk/rockchip/clk-rk3228.c >> +++ b/drivers/clk/rockchip/clk-rk3228.c >> @@ -409,7 +409,7 @@ static struct rockchip_clk_branch rk3228_clk_branches[] __initdata = { >> RK2928_CLKSEL_CON(29), 0, 3, DFLAGS), >> DIV(0, "sclk_vop_pre", "sclk_vop_src", 0, >> RK2928_CLKSEL_CON(27), 8, 8, DFLAGS), >> - MUX(DCLK_VOP, "dclk_vop", mux_dclk_vop_p, CLK_SET_RATE_PARENT | CLK_SET_RATE_NO_REPARENT, >> + MUX(DCLK_VOP, "dclk_vop", mux_dclk_vop_p, 0, >> RK2928_CLKSEL_CON(27), 1, 1, MFLAGS), >> >> FACTOR(0, "xin12m", "xin24m", 0, 1, 2), > >
