Re: [PATCH v6 3/8] clk: qcom: Add generic clkref_en support

[Konrad Dybcio]

On 6/22/26 7:11 AM, Qiang Yu wrote:
> Before XO refclk is distributed to PCIe/USB/eDP PHYs, it passes through
> a QREF block. QREF is powered by dedicated LDO rails, and the clkref_en
> register controls whether refclk is gated through to the PHY side.
> 
> These clkref controls are different from typical GCC branch clocks:
> - only a single enable bit is present, without branch-style config bits
> - regulators must be voted before enable and unvoted after disable
> 
> Model this as a dedicated clk_ref clock type with custom clk_ops instead
> of reusing struct clk_branch semantics.
> 
> Also provide a common registration/probe API so the same clkref model
> can be reused regardless of where clkref_en registers are placed, e.g.
> TCSR on glymur and TLMM on SM8750.
> 
> Signed-off-by: Qiang Yu <qiang.yu@xxxxxxxxxxxxxxxx>
> ---

[...]

> +	for (clk_idx = 0; clk_idx < num_clk_refs; clk_idx++) {
> +		clk_ref = &clk_refs[clk_idx];
> +		desc = &descs[clk_idx];
> +
> +		if (!desc->name)
> +			continue;

Carrying over from the previous discussion:

> // this allows "holes" in dt-bindings for $reasons
> if (!desc)
> 	continue;
> 
> // this makes sure the programmer did not omit something important
> // while not taking the entire system down
> if (WARN_ON(!desc->name)
> 	continue;
>
The NULL name check is intentional - the descriptor array is indexed by
clock ID, and mahua has fewer clocks than glymur, leaving holes at
certain indices. So this is expected at runtime. WARN_ON would be noise
log here.


->

Your worry is captured by nullchecking `desc` (i.e. descs[clk_idx])

because in the mahua case we've got (ephemeral indices)

tcsr_cc_mahua_clk_descs[] = {
	[0] = { foo },
	// [1] is unassigned - OK
	[2] = { bar },
};

while (!desc->name) checks for:

tcsr_cc_mahua_clk_descs[] = {
	[0] = { .name = "foo", .offset = 0x10 },
	// name is NULL by virtue of partial struct initialization
	[1] = { .offset = 0x20 },
};

however I overlooked that we actually just have a normal array of
structs.. if we turn it into a struct pointer array with assigmnents
like:

[TCSR_EDP_CLKREF_EN] = &(const struct qcom_clk_ref_desc) {
	.name = "foo",
	...
};

we can achieve that

Konrad