Re: [PATCH v3] clk: thead: th1520-ap: Describe mux clocks with clk_mux

[Drew Fustini <fustini@kernel.org>]

On Tue, Jul 22, 2025 at 08:05:36AM +0000, Yao Zi wrote:
> Mux clocks are now described with a customized ccu_mux structure
> consisting of ccu_internal and ccu_common substructures, and registered
> later with devm_clk_hw_register_mux_parent_data_table(). As this helper
> always allocates a new clk_hw structure, it's extremely hard to use mux
> clocks as parents statically by clk_hw pointers, since CCF has no
> knowledge about the clk_hw structure embedded in ccu_mux.
> 
> This scheme already causes issues for clock c910, which takes a mux
> clock, c910-i0, as a possible parent. With mainline U-Boot that
> reparents c910 to c910-i0 at boottime, c910 is considered as an orphan
> by CCF.
> 
> This patch refactors handling of mux clocks, embeds a clk_mux structure
> in ccu_mux directly. Instead of calling devm_clk_hw_register_mux_*(),
> we could register mux clocks on our own without allocating any new
> clk_hw pointer, fixing c910 clock's issue.
> 
> Fixes: ae81b69fd2b1 ("clk: thead: Add support for T-Head TH1520 AP_SUBSYS clocks")
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
> 
> This replaces the second patch in v2 of series "Fix orphan clocks in
> clk-th1520-ap driver".
> 
> Note that the c910's issue cannot be reproduced with vendor U-Boot,
> which always reparents c910 to its second parent, cpu-pll1. Another way
> to confirm the bug is to examine
> /sys/kernel/debug/clk/c910/clk_possible_parents, which obviously doesn't
> match c910's definition. There's another patch[1] explaining and fixing
> the issue that the unknown parent isn't shown as "(missing)" in debugfs.

Reviewed-by: Drew Fustini <dfustini@kernel.org>

Thank you for refactoring the c910_i0 parent fix without using strings
in the parent data.

Before: 

==> /sys/kernel/debug/clk/c910/clk_possible_parents <==
osc_24m cpu-pll1

After:

==> /sys/kernel/debug/clk/c910/clk_possible_parents <==
c910-i0 cpu-pll1

The system still boots okay without clk_ignore_unused and peripherals
like serial, emmc and ethernet are functional.

Thanks,
Drew