Re: [PATCH] clk: Avoid DT fetch in possible_parent_show if clk_hw is provided

[Drew Fustini <fustini@kernel.org>]

On Sat, Jul 05, 2025 at 09:58:17AM +0000, Yao Zi wrote:
> When showing a parent for which clk_core_get_parent_by_index fails, we
> may try using the parent's global name or the local name. If this fails
> either, the parent clock's clock-output-names is fetched through
> DT-index.
> 
> struct clk_hw pointer takes precedence with DT-index when registering
> clocks, thus most drivers only zero the index member of struct
> clk_parent_data when providing the parent through struct clk_hw pointer.
> If the pointer cannot resovle to a clock, clk_core_get_parent_by_index
> will fail as well, in which case possible_parent_show will fetch the
> parent's clock-output-names property, treat the unintended, zeroed index
> as valid, and yield a misleading name if the clock controller does come
> with a clocks property.
> 
> Let's add an extra check against the struct clk_hw pointer, and only
> perform the DT-index-based fetch if it isn't provided.
> 
> Fixes: 2d156b78ce8f ("clk: Fix debugfs clk_possible_parents for clks without parent string names")
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
> 
> This was found when fixing the wrong parent description of
> clk-th1520-ap.c[1]. Without the patch,
> 
> 	# cat /sys/kernel/debug/clk/c910/clk_possible_parents
> 	osc_24m cpu-pll1
> 
> The first parent should be c910-i0, provided by an unresolvable struct
> clk_hw pointer. osc_24m is the first (and only) parent specified in
> devicetree for the clock controller. With the patch,
> 
> 	# cat /sys/kernel/debug/clk/c910/clk_possible_parents
> 	(missing) cpu-pll1
> 
> [1]: https://lore.kernel.org/linux-riscv/20250705052028.24611-1-ziyao@disroot.org/
> 
>  drivers/clk/clk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 0565c87656cf..280d3a470228 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -3594,7 +3594,7 @@ static void possible_parent_show(struct seq_file *s, struct clk_core *core,
>  	} else if (core->parents[i].fw_name) {
>  		seq_printf(s, "<%s>(fw)", core->parents[i].fw_name);
>  	} else {
> -		if (core->parents[i].index >= 0)
> +		if (!core->parents[i].hw && core->parents[i].index >= 0)
>  			name = of_clk_get_parent_name(core->of_node, core->parents[i].index);
>  		if (!name)
>  			name = "(missing)";
> -- 
> 2.49.0
> 

Tested-by: Drew Fustini <fustini@kernel.org>

I've tested this using next-20250704 on with the TH1520-based LPi 4a.
Without the patch, I also get the misleading output:

 # cat /sys/kernel/debug/clk/c910/clk_possible_parents
 osc_24m cpu-pll1

With this patch applied, the output now reflects the missing parent:

 # cat /sys/kernel/debug/clk/c910/clk_possible_parents
 (missing) cpu-pll1

Thanks,
Drew