Re: [PATCH v3 06/18] clk: qcom: common: Add support to configure clk regs in qcom_cc_really_probe

[Jagadeesh Kona <quic_jkona@quicinc.com>]

On 3/27/2025 6:20 PM, Dmitry Baryshkov wrote:
> On Thu, Mar 27, 2025 at 03:22:26PM +0530, Jagadeesh Kona wrote:
>> Add support to configure PLLS and clk registers in qcom_cc_really_probe().
>> This ensures all required power domains are enabled and kept ON by runtime
>> PM code in qcom_cc_really_probe() before configuring the PLLS or clock
>> registers.
>>
>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
>> ---
>>  drivers/clk/qcom/common.c | 28 ++++++++++++++++++++++++++++
>>  drivers/clk/qcom/common.h | 19 +++++++++++++++++++
>>  2 files changed, 47 insertions(+)
>>
>> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
>> index 9cbf1c5296dad3ee5477a2f5a445488707663b9d..c4d980c6145834969fada14863360ee81c9aa251 100644
>> --- a/drivers/clk/qcom/common.c
>> +++ b/drivers/clk/qcom/common.c
>> @@ -14,6 +14,8 @@
>>  #include <linux/of.h>
>>  
>>  #include "common.h"
>> +#include "clk-alpha-pll.h"
>> +#include "clk-branch.h"
>>  #include "clk-rcg.h"
>>  #include "clk-regmap.h"
>>  #include "reset.h"
>> @@ -285,6 +287,29 @@ static int qcom_cc_icc_register(struct device *dev,
>>  						     desc->num_icc_hws, icd);
>>  }
>>  
>> +static void qcom_cc_clk_pll_configure(const struct qcom_cc_desc *desc,
>> +				      struct regmap *regmap)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < desc->num_alpha_plls; i++)
>> +		qcom_clk_alpha_pll_configure(desc->alpha_plls[i], regmap);
>> +}
>> +
>> +static void qcom_cc_clk_regs_configure(const struct qcom_cc_desc *desc,
>> +				       struct regmap *regmap)
>> +{
>> +	struct qcom_clk_reg_setting *clk_regs = desc->clk_regs;
>> +	int i;
>> +
>> +	for (i = 0; i < desc->num_clk_cbcrs; i++)
>> +		qcom_branch_set_clk_en(regmap, desc->clk_cbcrs[i]);
>> +
>> +	for (i = 0 ; i < desc->num_clk_regs; i++)
>> +		regmap_update_bits(regmap, clk_regs[i].offset,
>> +				   clk_regs[i].mask, clk_regs[i].val);
> 
> I think there are other semantic functions which we don't want to
> convert to offset-mask-val tuples. See drivers/clk/qcom/clk-branch.h.
> I'd suggest to move setup steps to a driver callback. We can improve it
> later on if it is found to make sense, but it won't block this series
> from being merged.
> 

Yes, there are other wrapper functions as well but they are unused in most
clock controllers. We will check more on how we can improve this in a separate
series.

Thanks,
Jagadeesh

>> +}
>> +
>>  int qcom_cc_really_probe(struct device *dev,
>>  			 const struct qcom_cc_desc *desc, struct regmap *regmap)
>>  {
>> @@ -315,6 +340,9 @@ int qcom_cc_really_probe(struct device *dev,
>>  			return ret;
>>  	}
>>  
>> +	qcom_cc_clk_pll_configure(desc, regmap);
>> +	qcom_cc_clk_regs_configure(desc, regmap);
>> +
>>  	reset = &cc->reset;
>>  	reset->rcdev.of_node = dev->of_node;
>>  	reset->rcdev.ops = &qcom_reset_ops;
>> diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
>> index 9c10bc8c197cd7dfa25ccd245763ad6acb081523..01b1ae52f2dc580350409d6244578944cce571f0 100644
>> --- a/drivers/clk/qcom/common.h
>> +++ b/drivers/clk/qcom/common.h
>> @@ -25,6 +25,19 @@ struct qcom_icc_hws_data {
>>  	int clk_id;
>>  };
>>  
>> +/**
>> + * struct qcom_clk_reg_setting - Represents miscellaneous clock register settings
>> + * @offset: address offset for the clock register
>> + * @mask: bit mask indicating the bits to be updated
>> + * @val: Encoded value to be set within the specified bit mask
>> + *       (e.g., if writing 7 to bits 4-7, mask = 0xF0 and val = 0x70)
>> + */
>> +struct qcom_clk_reg_setting {
>> +	u32 offset;
>> +	u32 mask;
>> +	u32 val;
>> +};
>> +
>>  struct qcom_cc_desc {
>>  	const struct regmap_config *config;
>>  	struct clk_regmap **clks;
>> @@ -38,6 +51,12 @@ struct qcom_cc_desc {
>>  	const struct qcom_icc_hws_data *icc_hws;
>>  	size_t num_icc_hws;
>>  	unsigned int icc_first_node_id;
>> +	u32 *clk_cbcrs;
>> +	size_t num_clk_cbcrs;
>> +	struct clk_alpha_pll **alpha_plls;
>> +	size_t num_alpha_plls;
>> +	struct qcom_clk_reg_setting *clk_regs;
>> +	size_t num_clk_regs;
>>  	bool use_rpm;
>>  };
>>  
>>
>> -- 
>> 2.34.1
>>
>