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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YS8+hnrf6FZVif0D@Marijn-Arch-PC.localdomain>
Date:   Wed, 1 Sep 2021 10:49:10 +0200
From:   Marijn Suijten <marijn.suijten@...ainline.org>
To:     Stephen Boyd <sboyd@...nel.org>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>
Cc:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        linux-arm-msm@...r.kernel.org, phone-devel@...r.kernel.org,
        ~postmarketos/upstreaming@...ts.sr.ht,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...ainline.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Martin Botka <martin.botka@...ainline.org>,
        Jami Kettunen <jami.kettunen@...ainline.org>,
        Pavel Dubrova <pashadubrova@...il.com>,
        Andy Gross <agross@...nel.org>,
        Michael Turquette <mturquette@...libre.com>,
        Rob Clark <robdclark@...il.com>, Sean Paul <sean@...rly.run>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        Abhinav Kumar <abhinavk@...eaurora.org>,
        Jonathan Marek <jonathan@...ek.ca>,
        Matthias Kaehlcke <mka@...omium.org>,
        Douglas Anderson <dianders@...omium.org>,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org,
        Stephen Boyd <swboyd@...omium.org>
Subject: Re: [PATCH v2 1/2] drm/msm/dsi: Use "ref" fw clock instead of global
 name for VCO parent

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