On 3/7/2025 2:17 PM, Dmitry Baryshkov wrote: > On Thu, Mar 06, 2025 at 02:25:35PM +0530, Jagadeesh Kona wrote: >> Add support for runtime power management, PLL configuration and enabling >> critical clocks in qcom_cc_really_probe() to commonize the clock >> controller probe. > > Please split this into two commits: one for the runtime PM and another > one for clock configuration, because ... > Sure, will split this in the next series. >> >> The runtime power management is not required for all clock controllers, >> hence handle the rpm based on use_rpm flag in clock controller descriptor. >> Also the power domains need to be kept enabled during pll configuration, >> hence attach all required power domains prior to calling get_sync() on the >> device. >> >> Signed-off-by: Jagadeesh Kona <quic_jkona@xxxxxxxxxxx> >> --- >> drivers/clk/qcom/common.c | 45 ++++++++++++++++++++++++++++++++++++--------- >> drivers/clk/qcom/common.h | 16 ++++++++++++++++ >> 2 files changed, 52 insertions(+), 9 deletions(-) > > [...] > >> + >> + for (i = 0; i < desc->num_plls; i++) >> + qcom_cc_clk_pll_configure(desc->plls[i], regmap); >> + >> + for (i = 0 ; i < desc->num_clks_cfg; i++) >> + regmap_update_bits(regmap, clks_cfg[i].offset, >> + clks_cfg[i].mask, clks_cfg[i].mask); >> + > > ... just calling regmap_update_bits() looks like a step backwards. In > the past several years we got several sensible wrappers and helpers. I > suggest having a callback instead of a fixed 'update bits' table. > Sure, will check and add a callback to handle all these clock config settings. Thanks, Jagadeesh >> reset = &cc->reset; >> reset->rcdev.of_node = dev->of_node; >> reset->rcdev.ops = &qcom_reset_ops; > > The RPM part is fine with me. >
