Quoting Dmitry Baryshkov (2023-11-06 18:00:04) > On Tue, 7 Nov 2023 at 03:36, Stephen Boyd <sboyd@xxxxxxxxxx> wrote: > > > > Quoting Stephen Boyd (2023-11-03 18:24:47) > > > > I looked at this more today. It seems that I need to both read the > > config register at init and also move over the rcg to the safe source at > > init (i.e. park the clk at init). That's doable with a call to > > clk_rcg2_shared_disable() for the clk_ops::init callback. Otherwise I > > get a stuck clk warning. > > > > Either > > > > disp_cc_mdss_mdp_clk status stuck at 'off' > > > > or > > > > disp_cc_mdss_rot_clk status stuck at 'on' > > > > When I don't park the rcg, the disp_cc_mdss_rot_clk gets stuck during > > disabling of unused clks. I think I understand that problem. What > > happens is disp_cc_mdss_mdp_clk_src and disp_cc_mdss_rot_clk_src are > > both sourcing from disp_cc_pll0 at boot. Fixing the parent mapping makes > > it so that enabling and then disabling disp_cc_mdss_ahb_clk causes > > disp_cc_pll0 to be turned off when disp_cc_mdss_rot_clk_src is sourced > > from it (and the branch disp_cc_mdss_rot_clk is enabled). If we park > > both the rcgs at clk registration time we avoid this problem because the > > PLL is disabled but nothing is actually a child clk. The act of reading > > the config register and stashing that in the 'parked_cfg' only helps > > avoid duplicate calls to change the rate, and doesn't help when we try > > to repoint the clk at XO when the parent PLL is off. > > > > The part I still don't understand is why reading the config register at > > init and stashing that in 'parked_cfg' fixes the disp_cc_mdss_mdp_clk > > stuck at off problem. I see that the branch clk is turned off and on > > many times during boot and there aren't any warnings regardless of > > stashing the config register. That means we should be moving the RCG to > > XO source, between forcibly enabling and disabling the RCG, which > > presumably would complain about being unable to update the config > > register, but it doesn't. Only after late init does the clk fail to > > enable, and the source is still XO at that time. Something else must be > > happening that wedges the branch to the point that it can't be > > recovered. But simply reporting the proper parent is enough for > > disp_cc_mdss_mdp_clk. > > I suppose that the issue is caused by mdss_gdsc or mmcx also being > shut down at the late_init. And if I remember correctly, properly > parking disp_cc_mdss_mdp_clk to the XO solves this issue. This is > where is_enabled comes to play. Adding it changes the > clk_disable_unused behaviour. The thing is that disp_cc_mdss_mdp_clk_src has been parked to XO by the time late_init runs. The branch clk (disp_cc_mdss_mdp_clk) has been enabled and disabled repeatedly during boot as well, and all those times nothing has signaled a failure. That means the RCG has supposedly switched away from the disp_cc_pll0 to XO (parked) and the branch isn't stuck on or off. So how does turning the mdss_gdsc or mmcx off during late_init cause the branch to get stuck off if the parent of the RCG is XO? Is something changing the parent back to the display PLL?