Re: [PATCH] clk: rockchip: rk3588: let MCLK_SPDIF* rate requests reach the divider

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete bug fix: The patch fixes silent rate configuration failures for S/PDIF clocks by adding CLK_SET_RATE_PARENT, but fails to apply the necessary identical fix to the I2S master clocks (MCLK_I2S*).
--

--- Patch [1]: [PATCH] clk: rockchip: rk3588: let MCLK_SPDIF* rate requests reach the divider ---
commit 3e2508be1e8b1ff2b82e2859ac11176c7413639e
Author: Karl Asseily <karl@asseily.com>

clk: rockchip: rk3588: let MCLK_SPDIF* rate requests reach the divider

This commit adds the CLK_SET_RATE_PARENT flag to the MCLK_SPDIF* clock gates.
This ensures clk_set_rate() requests propagate up to the fractional divider,
fixing an issue where S/PDIF sample rates were silently ignored.

> diff --git a/drivers/clk/rockchip/clk-rk3588.c b/drivers/clk/rockchip/clk-rk3588.c
> index 2ba9976654cf1..b3d577dccfcb7 100644
> --- a/drivers/clk/rockchip/clk-rk3588.c
> +++ b/drivers/clk/rockchip/clk-rk3588.c
> @@ -960,7 +960,7 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
>  			RK3588_CLKSEL_CON(33), 0,
>  			RK3588_CLKGATE_CON(9), 0, GFLAGS,
>  			&rk3588_spdif0_fracmux),
> -	GATE(MCLK_SPDIF0, "mclk_spdif0", "clk_spdif0", 0,
> +	GATE(MCLK_SPDIF0, "mclk_spdif0", "clk_spdif0", CLK_SET_RATE_PARENT,
>  			RK3588_CLKGATE_CON(9), 1, GFLAGS),

[Severity: High]
This is a pre-existing issue, but should the same fix be applied to the I2S
master clocks?  Looking at rk3588_early_clk_branches[], the I2S gates also
lack the CLK_SET_RATE_PARENT flag:

	GATE(MCLK_I2S2_2CH, "mclk_i2s2_2ch", "clk_i2s2_2ch", 0,
			RK3588_CLKGATE_CON(8), 0, GFLAGS),

and:

	GATE(MCLK_I2S0_8CH_TX, "mclk_i2s0_8ch_tx", "clk_i2s0_8ch_tx", 0,
			RK3588_CLKGATE_CON(7), 7, GFLAGS),

If the rockchip_i2s_tdm.c driver calls clk_set_rate() on its mclk_tx or
mclk_rx clocks, will the clock core evaluate the gates as having no .set_rate
capability and return 0 without propagating the request up to the fractional
dividers (e.g., clk_i2s0_8ch_tx_frac)?

Could this cause I2S audio streams with non-default sample rates to play back
at incorrect pitches or fail to lock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820075427.67050-1-karl@asseily.com?part=1