On Wed, 25 May 2022 at 01:03, Marijn Suijten <marijn.suijten@xxxxxxxxxxxxxx> wrote: > > On 2022-05-24 02:43:01, Dmitry Baryshkov wrote: > > Hi, > > > > On Tue, 24 May 2022 at 00:38, Marijn Suijten > > <marijn.suijten@xxxxxxxxxxxxxx> 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
