Message-ID: <CAA8EJpofvyxH22qWs5HLqG-EkKkecbFySXd36YDmK8cdeNaGUg@mail.gmail.com>
Date:   Wed, 25 May 2022 11:01:33 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Marijn Suijten <marijn.suijten@...ainline.org>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        phone-devel@...r.kernel.org, Stephen Boyd <sboyd@...nel.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>,
        Michael Turquette <mturquette@...libre.com>,
        Rob Clark <robdclark@...il.com>,
        Abhinav Kumar <quic_abhinavk@...cinc.com>,
        Sean Paul <sean@...rly.run>, David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Rajeev Nandan <quic_rajeevny@...cinc.com>,
        Vladimir Lypak <vladimir.lypak@...il.com>,
        Arnd Bergmann <arnd@...db.de>,
        Jonathan Marek <jonathan@...ek.ca>, linux-clk@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-msm@...r.kernel.org,
        dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org
Subject: Re: [PATCH 0/9] drm/msm/dsi_phy: Replace parent names with clk_hw pointers

On Wed, 25 May 2022 at 01:03, Marijn Suijten
<marijn.suijten@...ainline.org> wrote:
>
> On 2022-05-24 02:43:01, Dmitry Baryshkov wrote:
> > Hi,
> >
> > On Tue, 24 May 2022 at 00:38, Marijn Suijten
> > <marijn.suijten@...ainline.org> wrote:
> > >
> > > As stated in [1] I promised to tackle and send this series.
> > >
> > > parent_hw pointers are easier to manage and cheaper to use than
> > > repeatedly formatting the parent name and subsequently leaving the clk
> > > framework to perform lookups based on that name.
> > >
> > > This series starts out by adding extra constructors for divider, mux and
> > > fixed-factor clocks that have parent_hw(s) pointer argument(s) instead
> > > of some DT index or name.  Followed by individual patches performing the
> > > conversion, one DSI PHY at a time.
> > >
> > > dsi_phy_28nm_8960 includes an extra fixup to replace "eternal"
> > > devm_kzalloc allocations (for the lifetime of the device) with
> > > stack-local char arrays, like all the other DSI PHY drivers.
> > >
> > > I couldn't help but notice that clock names are wildly varying:
> > >
> > > - Some use underscores in the _clk suffix where others have nothing;
> > > - Some have an _ after the %d, others have not;
> > > - Some use a _pll suffix after dsi%d or even _phy_pll suffix.
> > >
> > > Are there any thoughts or feelings towards unifying these?
> > > Theoretically no clock names are used anywhere in the kernel, and
> > > everything is based on a phandle + index in DT (I have yet to validate
> > > this).  Obviously no .name/.fw_name will be updated to not break DT.
> >
> > I'd say, leave them as is. Even if they are historical, we don't have
> > a strong pressure to change them.
>
> Leave them as it is, or - as suggested below - clean them up?

Let's leave the names as is for now, convert all clock drivers to
fetch clocks from DT and decide how to continue with clock names
afterwards.

> > Significant number of older platforms still use names to identify the
> > clock. And moreover apq8096/msm8960 uses dsi1/dsi2 instead of
> > dsi0/dsi1.
> >
> > Probably we should call the next cycle "The Cycle of clocks cleaning".
> > I can volunteer to take care of 8960/8064/8016/8996, as at least I can
> > test them. But if you wish, you (or anybody else of course) can take
> > any of these platforms too, just ping me, so that I won't spend time
> > duplicating somebody's efforts.
>
> We can at least clean up the names of clocks that are not "exported" by
> the drivers.  However, we should also convert all other clk drivers to
> utilize DT to define clk dependencies instead of depending on global
> names, and already got quite some platforms tackled.  At that point we
> can just convert all names (give or take the often discussed "backwards
> compatbility" between the kernel and some ancient DT someone may still
> be running on their device).
>
> I don't own any device for the SoCs you mentioned, all good from my
> side if you take them.  We should probably note down all clock drivers
> that still need conversion and split them across devs with physical
> access, then I can check what I still have lying around here as well.

Can you please make a google spreadsheet? Then anybody can take a look
and volunteer (or check that the platform is being taken care of).
I have 8064 (and thus I can cover 8960 too), 8016, 8096 on my desk and
qcs404 and 8998 in the remote lab (but I can leave them to somebody
else).

> > > Which, by the way, is there a particular reason for:
> > >
> > >   #define DSI_BYTE_PLL_CLK              0
> > >   #define DSI_PIXEL_PLL_CLK             1
> > >
> > > To not be in the dt-bindings and used in the DT?
> >
> > Before my restructure of the DSI PHY subsys, each driver defined them
> > separately. And the idea of moving them to a dt-bindings header didn't
> > come to my mind. Feel free to do so, it looks like a good idea.
> > Just as a note, DP PHY also uses 0 for the link clock and 1 for the
> > pixel clock. What do you think about having a single header for these
> > names?
>
> No worries, it's already much better to have them defined once :), now
> we can just go one step further and move it to dt-bindings.  Great to
> clean up the "magic constant indices" for the DP PHY as well
> (phy-qcom-qmp.c is the only one defining these clocks, right?)

No, phy-qcom-edp.c also uses these magic numbers.

> and I
> think we're fine having them in one header, pending someone suggesting a
> name as I have no idea what to call it nor where to put it.  Under
> dt-bindings/clock most likely, but what common name would we choose?
> Something including qcom and mdss?

dt-bindings/phy/phy-qcom-dsi.h and dt-bindings/phy/phy-qcom-dp.h?


-- 
With best wishes
Dmitry

Powered by blists - more mailing lists