Re: [PATCH 04/12] clk: qcom: ipq-cmn-pll: Add NSS clock support

Jie Luo <[email protected]> Wed, 29 Jul 2026 21:15:42 +0800
Newsgroups org.kernel.vger.linux-clk,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel
Message-ID <[email protected]>

On 7/28/2026 5:29 PM, Konrad Dybcio wrote:
> On 7/24/26 5:00 PM, Luo Jie wrote:
>> The NSS (network subsystem) clock is derived from the CMN PLL output
>> divided by 2 and then further divided by a configurable 6-bit divider
>> with a valid range of 8-63.
>>
>> Signed-off-by: Luo Jie <[email protected]>
>> ---
> 
> This looks very similar to clk-regmap-divider.c (or at least to
> the generic clk-divider.c implementation).
> 

You're right — this can be converted to use clk_regmap_div_ops directly.

The NSS and PPE clocks both compute rate = parent_rate / (2 * div),
where div is read/written straight from a register field with no other
transform. The only thing blocking reuse of clk_regmap_div_ops is that
implicit 2 *, which none of the existing CLK_DIVIDER_* flags express.

I'll add a new flag CLK_DIVIDER_EVEN_INTEGERS_NO_OFFSET, to clk-
divider.c/clk-provider.h to express that relationship, pass a flags
field through struct clk_regmap_div (clk-regmap-divider.c/.h) so qcom
clock drivers can opt into it, and register both NSS and PPE as plain
clk_regmap_div instances with .ops = &clk_regmap_div_ops instead of
hand-rolling recalc_rate/set_rate.

> [...]
> 
>>  	/* Register the fixed rate output clocks. */
>>  	for (i = 0; i < num_clks; i++) {
>> -		struct clk_parent_data pdata = { .hw = cmn_pll_hw };
>> +		if (fixed_clk[i].rate) {
>> +			struct clk_parent_data pdata = { .hw = cmn_pll_hw };
>> +
>> +			hw = devm_clk_hw_register_fixed_rate_parent_data(dev,
>> +									 fixed_clk[i].name,
>> +									 &pdata, 0,
>> +									 fixed_clk[i].rate);
>> +		} else if (!strcmp(fixed_clk[i].name, "nss")) {
>> +			hw = ipq_cmn_pll_nss_register(pdev, cmn_pll->regmap,
>> +						      cmn_pll_hw);
> 
> Huge "no", this must be a compile-time constant and not rely on
> some funky detection.
> 
> Konrad

Thanks. Will replace the rate/enable_bit/strcmp(name, ...) runtime
inference with an explicit enum cmn_pll_clk_type field, set directly by
each entry's construction macro (CLK_PLL_OUTPUT/CLK_PLL_GATE/
CLK_PLL_NSS/CLK_PLL_PPE/CLK_PLL_PON/CLK_PLL_EPHY_RAW), and dispatch via
switch in ipq_cmn_pll_register_clks().