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. 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. > 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? > > And with enough future improvements out of the way, let's round out this > patch-series by stating that it has been successfully tested on: > > - Sony Nile Discovery (Xperia XA2 Ultra): 14nm; > - Sony Seine PDX201 (Xperia 10II): 14nm; > - Sony Loire Suzu (Xperia X): 28nm. > > And no diff is observed in debugfs's clk_summary. > > Unfortunately all other devices in my collection with a 7/10nm DSI PHY > have a DSC panel which we have yet to get working. I will test it on RB3 (10nm) and RB5 (7nm) during one of the next few days. > > [1]: https://lore.kernel.org/linux-arm-msm/20220502214235.s5plebunh4ttjhge@xxxxxxxxxxxxxx/ > > Marijn Suijten (9): > clk: divider: Introduce devm_clk_hw_register_divider_parent_hw() > clk: mux: Introduce devm_clk_hw_register_mux_parent_hws() > clk: fixed-factor: Introduce *clk_hw_register_fixed_factor_parent_hw() > drm/msm/dsi_phy_28nm: Replace parent names with clk_hw pointers > drm/msm/dsi_phy_28nm_8960: Replace parent names with clk_hw pointers > drm/msm/dsi_phy_28nm_8960: Use stack memory for temporary clock names > drm/msm/dsi_phy_14nm: Replace parent names with clk_hw pointers > drm/msm/dsi_phy_10nm: Replace parent names with clk_hw pointers > drm/msm/dsi_phy_7nm: Replace parent names with clk_hw pointers > > drivers/clk/clk-fixed-factor.c | 57 ++++++++++-- > drivers/gpu/drm/msm/dsi/phy/dsi_phy_10nm.c | 92 ++++++++----------- > drivers/gpu/drm/msm/dsi/phy/dsi_phy_14nm.c | 36 ++++---- > drivers/gpu/drm/msm/dsi/phy/dsi_phy_28nm.c | 52 +++++------ > .../gpu/drm/msm/dsi/phy/dsi_phy_28nm_8960.c | 26 ++---- > drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 92 +++++++++---------- > include/linux/clk-provider.h | 34 +++++++ > 7 files changed, 209 insertions(+), 180 deletions(-) > > -- > 2.36.1 -- With best wishes Dmitry
