Re: [PATCH v2 1/2] drm/msm/dsi: Use "ref" fw clock instead of global name for VCO parent

[Marijn Suijten]

Hi Stephen,

On 2021-08-31 22:35:12, Stephen Boyd wrote:
> Quoting Marijn Suijten (2021-08-30 16:10:26)
> > 
> > I'm 95% sure this shouldn't cause any problems given current DTs and
> > their history, but that's probably not enough.  This might also impact
> > DTs that have not yet been upstreamed, but afaik the general stance is
> > to not care and actually serve as a fair hint/warning before new DTs
> > make it to the list.
> > 
> > If there is a protocol in place to deprecate, warn, and eventually
> > remove this reliance on global clock names I'm more than happy to add
> > .name as a temporary fallback, even if likely unneeded.  Otherwise we
> > might never get rid of it.
> 
> I'm not aware of any protocol to deprecate, warn, and remove the
> fallback name. It's a fallback because it can't ever really be removed.

That is unfortunate, I was hoping for a breaking "kernel release" at
some point where we could say "no more, update your DTs first".  But
that may not be possible in every scenario?

> Anyway, if you're not willing to add the .name then that's fine.

I feel like .name has caused more problems for us than it solves, but in
a fallback position it might be fine.  My main gripe is that I don't
want DT to rely on the clock to also be discoverable through the clock
tree, which we've seen on many occasions (not sure if the former was
done upstream, but: "xo" being renamed to "xo_board", and DSI PLL clocks
loosing +1 causing a naming mismatch with what mmcc expects, to name
some examples).  Omitting .name is the only way to enforce that.

> We can
> deal with the problem easily by adding a .name in the future if someone
> complains that things aren't working. Sound like a plan? If so, it's
> probably good to add some sort of note in the commit text so that when
> the bisector lands on this patch they can realize that this
> intentionally broke them.

I'm all for this but lack the industrial knowledge to sign off on the
approach.  Bjorn and Dmitry should ack/agree before going ahead (you may
wonder why I'm worrying about getting clock drivers and DT in sync on
platforms I don't own...):

We have the following situations:
- apq8064 used the wrong clock.  Bjorn acknowledged that landing the DT
  fix in 5.15, and this patch in 5.16 should give enough time for DT to
  be updated (this is nothing we can fix with .name anyway).
- msm8974 doesn't have the clock at all.  Dmitry recommended to add
  .name for this specific case, but I'm wondering if the 5.15 -> 5.16
  window is enough to update DTs too?
- msm8916 and sdm845 had the missing "ref" clock added three years ago.
  Would a 5.16 kernel still work in any meaningful way with a DT that
  old?

Should we decide on a case-by-case basis whether to add .name, ie. only
for (some/all) of the aforementioned SoCs?

Thanks!

- Marijn