[PATCH 07/27] clk: rockchip: Add RK3572 clock and reset support
sashiko-bot at kernel.org
sashiko-bot at kernel.org
Thu Oct 22 09:13:03 PDT 2026
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] clk: rockchip: missing CLK_AUDIO_FRAC_0..2 in rk3572_clk_branches
- [Medium] clk: rockchip: internal FACTOR(0) branches overwrite PLL_PPLL in rk3572_clk_init()
--
commit 9602441daaa896927eb56cc9b1d0e4d99173c907
Author: Cunhao Lu <1579567540 at qq.com>
clk: rockchip: Add RK3572 clock and reset support
This commit adds clock and soft-reset controller driver support for the
Rockchip rk3572 SoC.
> diff --git a/drivers/clk/rockchip/clk-rk3572.c b/drivers/clk/rockchip/clk-rk3572.c
> new file mode 100644
> index 0000000000000..78fd35f5f27d1
> --- /dev/null
> +++ b/drivers/clk/rockchip/clk-rk3572.c
[ ... ]
> +PNAME(mux_24m_frac_int_mclk4_p) = {
> + "xin24m", "clk_audio_frac_0", "clk_audio_frac_1", "clk_audio_frac_2",
> + "clk_audio_frac_3", "clk_audio_int_0", "clk_audio_int_1", "clk_audio_int_2",
> + "sai0_mclk_in", "sai1_mclk_in", "sai2_mclk_in", "sai3_mclk_in",
> +};
[ ... ]
> +PNAME(mux_24m_frac_int_mclki_p) = {
> + "xin24m", "clk_audio_int_0", "clk_audio_int_1", "clk_audio_int_2", "clk_audio_frac_0",
> + "clk_audio_frac_1", "clk_audio_frac_2", "clk_audio_frac_3", "sai0_mclk_in",
> + "sai1_mclk_in", "sai2_mclk_in", "sai3_mclk_in", "sai4_mclk_in",
> +};
> +
> +PNAME(mux_24m_frac_int_mclki_sclki_p) = {
> + "xin24m", "clk_audio_int_0", "clk_audio_int_1", "clk_audio_int_2", "clk_audio_frac_0",
> + "clk_audio_frac_1", "clk_audio_frac_2", "clk_audio_frac_3", "sai0_mclk_in",
> + "sai1_mclk_in", "sai2_mclk_in", "sai3_mclk_in", "sai4_mclk_in", "sai0_sclk_in",
> + "sai1_sclk_in", "sai2_sclk_in", "sai3_sclk_in", "sai4_sclk_in",
> +};
[ ... ]
> +static struct rockchip_pll_clock rk3572_pll_clks[] __initdata = {
> + /* Keep BPLL and LPLL enabled for firmware consumers. */
> + [bpll] = PLL(pll_rk3588_core, PLL_BPLL, "bpll", mux_pll_p,
> + CLK_IGNORE_UNUSED, RK3572_PLL_CON(0),
> + RK3572_MODE_CON1, 0, 15, 0, rk3572_pll_rates),
> + [lpll] = PLL(pll_rk3588_core, PLL_LPLL, "lpll", mux_pll_p,
> + CLK_IGNORE_UNUSED, RK3572_LPLL_CON(16),
> + RK3572_LPLL_MODE_CON0, 0, 15, 0, rk3572_pll_rates),
> + [vpll] = PLL(pll_rk3588, PLL_VPLL, "vpll", mux_pll_p,
> + 0, RK3572_PLL_CON(88),
> + RK3572_MODE_CON0, 4, 15, 0, rk3572_pll_rates),
> + [aupll] = PLL(pll_rk3588, PLL_AUPLL, "aupll", mux_aupll_p,
> + 0, RK3572_PLL_CON(96),
> + RK3572_MODE_CON0, 6, 15, 0, rk3572_aupll_rates),
> + [cpll] = PLL(pll_rk3588, PLL_CPLL, "cpll", mux_pll_p,
> + 0, RK3572_PLL_CON(104),
> + RK3572_MODE_CON0, 8, 15, 0, rk3572_pll_rates),
> + [gpll] = PLL(pll_rk3588, PLL_GPLL, "gpll", mux_pll_p,
> + CLK_IGNORE_UNUSED, RK3572_PLL_CON(112),
> + RK3572_MODE_CON0, 2, 15, 0, rk3572_pll_rates),
> + [ppll] = PLL(pll_rk3588_ddr, PLL_PPLL, "ppll", mux_pll_p,
> + 0, RK3572_PPLL_CON(128),
> + RK3572_PPLL_CON(134), 15, 15, 0, rk3572_ppll_rates),
> +};
> +
> +static struct rockchip_clk_branch rk3572_clk_branches[] __initdata = {
> + /* fixed */
> + FACTOR(0, "xin12m", "xin24m", 0, 1, 2),
> +
> + COMPOSITE_FRACMUX(CLK_32K, "clk_32k", "xin24m", CLK_IS_CRITICAL,
> + RK3572_PMU_CLKSEL_CON(21), 0,
> + RK3572_PMU_CLKGATE_CON(7), 14, GFLAGS, NULL),
> +
> + FACTOR(0, "clk_spll_div12", "spll", 0, 1, 12),
> + FACTOR(0, "clk_spll_div6", "spll", 0, 1, 6),
> + FACTOR(0, "clk_spll_div4", "spll", 0, 1, 4),
> + FACTOR(0, "clk_spll_div2", "spll", 0, 1, 2),
[Severity: Medium]
Will registering these fixed-factor branches with an id of 0 overwrite the
ppll entry in ctx->clk_data.clks[0]?
In include/dt-bindings/clock/rockchip,rk3572-cru.h, the binding id for ppll
is 0:
#define PLL_PPLL 0
During rk3572_clk_init() in drivers/clk/rockchip/clk-rk3572.c,
rockchip_clk_register_plls() runs before rockchip_clk_register_branches():
rockchip_clk_register_plls(ctx, rk3572_pll_clks,
ARRAY_SIZE(rk3572_pll_clks),
RK3572_GRF_SOC_STATUS0);
...
rockchip_clk_register_branches(ctx, rk3572_clk_branches,
ARRAY_SIZE(rk3572_clk_branches));
When rockchip_clk_register_branches() iterates over rk3572_clk_branches[],
it unconditionally calls rockchip_clk_set_lookup(ctx, clk, list->id) in
drivers/clk/rockchip/clk.h:
static inline void rockchip_clk_set_lookup(struct rockchip_clk_provider *ctx,
struct clk *clk, unsigned int id)
{
ctx->clk_data.clks[id] = clk;
}
Because xin12m (which also has no consumers in this file) and
clk_spll_div12, clk_spll_div6, clk_spll_div4, and clk_spll_div2 all pass 0
as their id, rockchip_clk_register_branches() overwrites
ctx->clk_data.clks[0] with clk_spll_div2. Any device tree consumer
requesting ppll will receive clk_spll_div2 instead.
[ ... ]
> + MUX(CLK_AUDIO_FRAC_0_SRC, "clk_audio_frac_0_src", gpll_cpll_aupll_24m_p, 0,
> + RK3572_CLKSEL_CON(13), 0, 2, MFLAGS),
> + MUX(CLK_AUDIO_FRAC_1_SRC, "clk_audio_frac_1_src", gpll_cpll_aupll_24m_p, 0,
> + RK3572_CLKSEL_CON(15), 0, 2, MFLAGS),
> + MUX(CLK_AUDIO_FRAC_2_SRC, "clk_audio_frac_2_src", gpll_cpll_aupll_24m_p, 0,
> + RK3572_CLKSEL_CON(17), 0, 2, MFLAGS),
> + MUX(CLK_AUDIO_FRAC_3_SRC, "clk_audio_frac_3_src", gpll_cpll_aupll_24m_p, 0,
> + RK3572_CLKSEL_CON(19), 0, 2, MFLAGS),
> + COMPOSITE_FRAC(CLK_AUDIO_FRAC_3, "clk_audio_frac_3", "clk_audio_frac_3_src", 0,
> + RK3572_CLKSEL_CON(18), 0,
> + RK3572_CLKGATE_CON(1), 14, GFLAGS),
[Severity: Medium]
Are the fractional divider definitions for clk_audio_frac_0,
clk_audio_frac_1, and clk_audio_frac_2 missing from rk3572_clk_branches[]
here?
In include/dt-bindings/clock/rockchip,rk3572-cru.h, all four fractional
audio clocks and their parent muxes are defined:
#define CLK_AUDIO_FRAC_0 25
#define CLK_AUDIO_FRAC_1 26
#define CLK_AUDIO_FRAC_2 27
#define CLK_AUDIO_FRAC_3 28
#define CLK_AUDIO_FRAC_0_SRC 29
#define CLK_AUDIO_FRAC_1_SRC 30
#define CLK_AUDIO_FRAC_2_SRC 31
#define CLK_AUDIO_FRAC_3_SRC 32
However, rk3572_clk_branches[] in drivers/clk/rockchip/clk-rk3572.c only
registers the four source muxes (clk_audio_frac_0_src through
clk_audio_frac_3_src) and clk_audio_frac_3, omitting clk_audio_frac_0,
clk_audio_frac_1, and clk_audio_frac_2.
Because mux_24m_frac_int_mclk4_p, mux_24m_frac_int_mclki_p, and
mux_24m_frac_int_mclki_sclki_p list clk_audio_frac_0, clk_audio_frac_1, and
clk_audio_frac_2 as parents, 13 downstream audio clocks (clk_pdm0_src_top,
mclk_asrc0_src_0, mclk_asrc1_src_1, mclk_pdm0_src_top, mclk_sai0 through
mclk_sai5, and mclk_spdif_tx0 through mclk_spdif_tx2) will fail to resolve
those parents when rk3572_clk_init() -> rockchip_clk_register_branches()
runs, and device tree lookups for those three clock ids will fail to find a
registered clock.
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-add-board-ok3572-c-v1-0-0b0fd85a31ee@qq.com?part=7
More information about the linux-phy
mailing list