On Tue, Aug 19, 2025 at 12:41:00PM +0200, Stephan Gerhold wrote: > On Tue, Aug 19, 2025 at 04:19:26AM +0300, Dmitry Baryshkov wrote: > > On Mon, Aug 18, 2025 at 11:41:16AM +0200, Stephan Gerhold wrote: > > > On Sat, Aug 16, 2025 at 04:55:00PM +0300, Dmitry Baryshkov wrote: > > > > On Thu, Aug 14, 2025 at 02:38:45PM +0200, Stephan Gerhold wrote: > > > > > On Thu, Aug 14, 2025 at 02:55:44PM +0300, Dmitry Baryshkov wrote: > > > > > > On Thu, Aug 14, 2025 at 11:18:05AM +0200, Stephan Gerhold wrote: > > > > > With my changes in this series the clock state is always consistent > > > > > with > > > > > the state returned by the clk APIs. We just delay the call to > > > > > clk_set_parent() until we know that it can succeed. > > > > > > > > I know. But what happens when we power down the PHY? The clock is > > > > assumed to have the PHY clock as a parent, but it's supposedly not > > > > clocking. > > > > > > > > > > I don't think this is a big problem in practice, given that these clocks > > > are only consumed by a single driver that manages both PHY and clocks > > > anyway. The clock should always get disabled before the PHY is powered > > > down. > > > > > > > Another option would be to introduce a safe config for the PHYs and make > > > > sure that the PHY is brought up every time we need it to be up (e.g. via > > > > pm_runtime). > > > > > > I considered that as well, but what exactly would I use as "safe" > > > configuration? There are lots of PHY configuration registers that are > > > set based on the rate or other parameters of the panel/display > > > connected. > > > > > > Implementing something like clk_rcg2_shared_ops could presumably work, > > > with the limitation that it will only work if the clock is really off > > > during boot and not already running from XO. Otherwise, I think the > > > simple approach of delaying the clk_set_parent() implemented in this > > > series is still the most straightforward way to solve this issue. > > > > I know that it works, but it feels a bit clumsy to me. > > > > I realize that adding a field to the platform_driver struct feels a bit > weird, but I think in general requiring more control about when exactly > assigned-clock-parents/rates are applied is a valid use case. The reason > we haven't seen more of these issues is likely mainly because people > just avoid using assigned-clock-parents/rates in these use cases, even > if it would be the right way to describe the hardware. > > I'm happy to try implementing the workaround in the Qualcomm clock > drivers, but hearing more opinions about the more general approach of > this patch series would also be good.
I completely agree here. -- With best wishes Dmitry
