Quoting Stephen Boyd (2019-08-21 11:10:08) > Quoting Stephen Boyd (2019-07-29 15:46:51) > > Quoting Bjorn Andersson (2019-07-22 22:14:46) > > > As clocks are registered their parents are resolved and the parent_map > > > is updated to cache the clk_core objects of each existing parent. > > > But in the event of a clock being unregistered this cache will carry > > > dangling pointers if not invalidated, so do this for all children of the > > > clock being unregistered. > > > > > > Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> > > > --- > > > > > > This resolves the issue seen where the DSI PLL (and it's provided clocks) is > > > being registered and unregistered multiple times due to probe deferral. > > > > > > Marking it RFC because I don't fully understand the life of the clock yet. > > > > The concept sounds sane but the implementation is going to be not much > > fun. The problem is that a clk can be in many different parent caches, > > even ones for clks that aren't currently parented to it. We would need > > to walk the entire tree(s) and find anywhere that we've cached the > > clk_core pointer and invalidate it. Maybe we can speed that up a little > > bit by keeping a reference to the entry of each parent cache that is for > > the parent we're removing, essentially holding an inverse cache, but I'm > > not sure it will provide any benefit besides wasting space for this one > > operation that we shouldn't be doing very often if at all. > > > > It certainly sounds easier to iterate through the whole tree and just > > invalidate entries in all the caches under the prepare lock. We can > > optimize it later. > > Here's an attempt at the simple approach. There's another problem where > the cached 'hw' member of the parent data is held around when we don't > know when the caller has destroyed it. Not much else we can do for that > though. > > ---8<--- > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index c0990703ce54..f42a803fb11a 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -3737,6 +3737,37 @@ static const struct clk_ops clk_nodrv_ops = { > .set_parent = clk_nodrv_set_parent, > }; > > +static void clk_core_evict_parent_cache_subtree(struct clk_core *root, > + struct clk_core *target) > +{ > + int i; > + struct clk_core *child; > + > + if (!root) > + return; I don't think we need this part. Child is always a valid pointer.