[PATCH] clk: rockchip: rk3588: don't disable unused I2S MCLK output gates

[PATCH] clk: rockchip: rk3588: don't disable unused I2S MCLK output gates

[Daniele Briguglio]

No in-tree board references these gates yet. Boards drive the codec
MCLK through the parent I2S*_8CH_MCLKOUT, and now that the gates are
managed clocks, clk_disable_unused() turns them off at boot. On a board
that relied on firmware leaving the output enabled, that cuts the MCLK
and analog audio stops working.

Mark the four gates CLK_IGNORE_UNUSED so an unreferenced gate keeps the
state firmware left. A board that wants the kernel to own the gate can
reference I2S*_8CH_MCLKOUT_TO_IO from DT instead.

Fixes: 02b9b0bb6269 ("clk: rockchip: rk3588: add GATE_GRF clocks for I2S MCLK output to IO")
Reported-by: Diederik de Haas <diederik@cknow-tech.com>
Closes: https://lore.kernel.org/linux-rockchip/DJGDSS875DDO.22TYPVYK5X8KZ@cknow-tech.com/
Tested-by: Diederik de Haas <diederik@cknow-tech.com>
Signed-off-by: Daniele Briguglio <hello@superkali.me>
---
 drivers/clk/rockchip/clk-rk3588.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/rockchip/clk-rk3588.c b/drivers/clk/rockchip/clk-rk3588.c
index 2ba9976654c..86953f9ffee 100644
--- a/drivers/clk/rockchip/clk-rk3588.c
+++ b/drivers/clk/rockchip/clk-rk3588.c
@@ -895,7 +895,7 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
 	MUX(I2S2_2CH_MCLKOUT, "i2s2_2ch_mclkout", i2s2_2ch_mclkout_p, CLK_SET_RATE_PARENT,
 			RK3588_CLKSEL_CON(30), 2, 1, MFLAGS),
 	GATE_GRF(I2S2_2CH_MCLKOUT_TO_IO, "i2s2_2ch_mclkout_to_io", "i2s2_2ch_mclkout",
-			0, RK3588_SYSGRF_SOC_CON6, 2, GFLAGS, grf_type_sys),
+			CLK_IGNORE_UNUSED, RK3588_SYSGRF_SOC_CON6, 2, GFLAGS, grf_type_sys),
 
 	COMPOSITE(CLK_I2S3_2CH_SRC, "clk_i2s3_2ch_src", gpll_aupll_p, 0,
 			RK3588_CLKSEL_CON(30), 8, 1, MFLAGS, 3, 5, DFLAGS,
@@ -912,7 +912,7 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
 	MUX(I2S3_2CH_MCLKOUT, "i2s3_2ch_mclkout", i2s3_2ch_mclkout_p, CLK_SET_RATE_PARENT,
 			RK3588_CLKSEL_CON(32), 2, 1, MFLAGS),
 	GATE_GRF(I2S3_2CH_MCLKOUT_TO_IO, "i2s3_2ch_mclkout_to_io", "i2s3_2ch_mclkout",
-			0, RK3588_SYSGRF_SOC_CON6, 7, GFLAGS, grf_type_sys),
+			CLK_IGNORE_UNUSED, RK3588_SYSGRF_SOC_CON6, 7, GFLAGS, grf_type_sys),
 	GATE(PCLK_ACDCDIG, "pclk_acdcdig", "pclk_audio_root", 0,
 			RK3588_CLKGATE_CON(7), 11, GFLAGS),
 	GATE(HCLK_I2S0_8CH, "hclk_i2s0_8ch", "hclk_audio_root", 0,
@@ -942,7 +942,7 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
 	MUX(I2S0_8CH_MCLKOUT, "i2s0_8ch_mclkout", i2s0_8ch_mclkout_p, CLK_SET_RATE_PARENT,
 			RK3588_CLKSEL_CON(28), 2, 2, MFLAGS),
 	GATE_GRF(I2S0_8CH_MCLKOUT_TO_IO, "i2s0_8ch_mclkout_to_io", "i2s0_8ch_mclkout",
-			0, RK3588_SYSGRF_SOC_CON6, 0, GFLAGS, grf_type_sys),
+			CLK_IGNORE_UNUSED, RK3588_SYSGRF_SOC_CON6, 0, GFLAGS, grf_type_sys),
 
 	GATE(HCLK_PDM1, "hclk_pdm1", "hclk_audio_root", 0,
 			RK3588_CLKGATE_CON(9), 6, GFLAGS),
@@ -2229,7 +2229,7 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
 	MUX(I2S1_8CH_MCLKOUT, "i2s1_8ch_mclkout", i2s1_8ch_mclkout_p, CLK_SET_RATE_PARENT,
 			RK3588_PMU_CLKSEL_CON(9), 2, 2, MFLAGS),
 	GATE_GRF(I2S1_8CH_MCLKOUT_TO_IO, "i2s1_8ch_mclkout_to_io", "i2s1_8ch_mclkout",
-			0, RK3588_SYSGRF_SOC_CON6, 1, GFLAGS, grf_type_sys),
+			CLK_IGNORE_UNUSED, RK3588_SYSGRF_SOC_CON6, 1, GFLAGS, grf_type_sys),
 	GATE(PCLK_PMU1, "pclk_pmu1", "pclk_pmu0_root", CLK_IS_CRITICAL,
 			RK3588_PMU_CLKGATE_CON(1), 0, GFLAGS),
 	GATE(CLK_DDR_FAIL_SAFE, "clk_ddr_fail_safe", "clk_pmu0", CLK_IGNORE_UNUSED,

base-commit: 7edfb7fb58ee058298e18fde76a6077ef17d19d8
-- 
2.47.3

Re: [PATCH] clk: rockchip: rk3588: don't disable unused I2S MCLK output gates

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 3905 bytes --]

Hi,

On Wed, Jun 24, 2026 at 02:39:14PM +0200, Daniele Briguglio wrote:
> No in-tree board references these gates yet. Boards drive the codec
> MCLK through the parent I2S*_8CH_MCLKOUT, and now that the gates are
> managed clocks, clk_disable_unused() turns them off at boot. On a board
> that relied on firmware leaving the output enabled, that cuts the MCLK
> and analog audio stops working.
> 
> Mark the four gates CLK_IGNORE_UNUSED so an unreferenced gate keeps the
> state firmware left. A board that wants the kernel to own the gate can
> reference I2S*_8CH_MCLKOUT_TO_IO from DT instead.
> 
> Fixes: 02b9b0bb6269 ("clk: rockchip: rk3588: add GATE_GRF clocks for I2S MCLK output to IO")
> Reported-by: Diederik de Haas <diederik@cknow-tech.com>
> Closes: https://lore.kernel.org/linux-rockchip/DJGDSS875DDO.22TYPVYK5X8KZ@cknow-tech.com/
> Tested-by: Diederik de Haas <diederik@cknow-tech.com>
> Signed-off-by: Daniele Briguglio <hello@superkali.me>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Greetings,

-- Sebastian

>  drivers/clk/rockchip/clk-rk3588.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/rockchip/clk-rk3588.c b/drivers/clk/rockchip/clk-rk3588.c
> index 2ba9976654c..86953f9ffee 100644
> --- a/drivers/clk/rockchip/clk-rk3588.c
> +++ b/drivers/clk/rockchip/clk-rk3588.c
> @@ -895,7 +895,7 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
>  	MUX(I2S2_2CH_MCLKOUT, "i2s2_2ch_mclkout", i2s2_2ch_mclkout_p, CLK_SET_RATE_PARENT,
>  			RK3588_CLKSEL_CON(30), 2, 1, MFLAGS),
>  	GATE_GRF(I2S2_2CH_MCLKOUT_TO_IO, "i2s2_2ch_mclkout_to_io", "i2s2_2ch_mclkout",
> -			0, RK3588_SYSGRF_SOC_CON6, 2, GFLAGS, grf_type_sys),
> +			CLK_IGNORE_UNUSED, RK3588_SYSGRF_SOC_CON6, 2, GFLAGS, grf_type_sys),
>  
>  	COMPOSITE(CLK_I2S3_2CH_SRC, "clk_i2s3_2ch_src", gpll_aupll_p, 0,
>  			RK3588_CLKSEL_CON(30), 8, 1, MFLAGS, 3, 5, DFLAGS,
> @@ -912,7 +912,7 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
>  	MUX(I2S3_2CH_MCLKOUT, "i2s3_2ch_mclkout", i2s3_2ch_mclkout_p, CLK_SET_RATE_PARENT,
>  			RK3588_CLKSEL_CON(32), 2, 1, MFLAGS),
>  	GATE_GRF(I2S3_2CH_MCLKOUT_TO_IO, "i2s3_2ch_mclkout_to_io", "i2s3_2ch_mclkout",
> -			0, RK3588_SYSGRF_SOC_CON6, 7, GFLAGS, grf_type_sys),
> +			CLK_IGNORE_UNUSED, RK3588_SYSGRF_SOC_CON6, 7, GFLAGS, grf_type_sys),
>  	GATE(PCLK_ACDCDIG, "pclk_acdcdig", "pclk_audio_root", 0,
>  			RK3588_CLKGATE_CON(7), 11, GFLAGS),
>  	GATE(HCLK_I2S0_8CH, "hclk_i2s0_8ch", "hclk_audio_root", 0,
> @@ -942,7 +942,7 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
>  	MUX(I2S0_8CH_MCLKOUT, "i2s0_8ch_mclkout", i2s0_8ch_mclkout_p, CLK_SET_RATE_PARENT,
>  			RK3588_CLKSEL_CON(28), 2, 2, MFLAGS),
>  	GATE_GRF(I2S0_8CH_MCLKOUT_TO_IO, "i2s0_8ch_mclkout_to_io", "i2s0_8ch_mclkout",
> -			0, RK3588_SYSGRF_SOC_CON6, 0, GFLAGS, grf_type_sys),
> +			CLK_IGNORE_UNUSED, RK3588_SYSGRF_SOC_CON6, 0, GFLAGS, grf_type_sys),
>  
>  	GATE(HCLK_PDM1, "hclk_pdm1", "hclk_audio_root", 0,
>  			RK3588_CLKGATE_CON(9), 6, GFLAGS),
> @@ -2229,7 +2229,7 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
>  	MUX(I2S1_8CH_MCLKOUT, "i2s1_8ch_mclkout", i2s1_8ch_mclkout_p, CLK_SET_RATE_PARENT,
>  			RK3588_PMU_CLKSEL_CON(9), 2, 2, MFLAGS),
>  	GATE_GRF(I2S1_8CH_MCLKOUT_TO_IO, "i2s1_8ch_mclkout_to_io", "i2s1_8ch_mclkout",
> -			0, RK3588_SYSGRF_SOC_CON6, 1, GFLAGS, grf_type_sys),
> +			CLK_IGNORE_UNUSED, RK3588_SYSGRF_SOC_CON6, 1, GFLAGS, grf_type_sys),
>  	GATE(PCLK_PMU1, "pclk_pmu1", "pclk_pmu0_root", CLK_IS_CRITICAL,
>  			RK3588_PMU_CLKGATE_CON(1), 0, GFLAGS),
>  	GATE(CLK_DDR_FAIL_SAFE, "clk_ddr_fail_safe", "clk_pmu0", CLK_IGNORE_UNUSED,
> 
> base-commit: 7edfb7fb58ee058298e18fde76a6077ef17d19d8
> -- 
> 2.47.3
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] clk: rockchip: rk3588: don't disable unused I2S MCLK output gates

[Ricardo Pardini]

On 24/06/2026 14:39, Daniele Briguglio wrote:
> No in-tree board references these gates yet. Boards drive the codec
> MCLK through the parent I2S*_8CH_MCLKOUT, and now that the gates are
> managed clocks, clk_disable_unused() turns them off at boot. On a board
> that relied on firmware leaving the output enabled, that cuts the MCLK
> and analog audio stops working.
> 
> Mark the four gates CLK_IGNORE_UNUSED so an unreferenced gate keeps the
> state firmware left. A board that wants the kernel to own the gate can
> reference I2S*_8CH_MCLKOUT_TO_IO from DT instead.
> 
> Fixes: 02b9b0bb6269 ("clk: rockchip: rk3588: add GATE_GRF clocks for I2S MCLK output to IO")
> Reported-by: Diederik de Haas <diederik@cknow-tech.com>
> Closes: https://lore.kernel.org/linux-rockchip/DJGDSS875DDO.22TYPVYK5X8KZ@cknow-tech.com/
> Tested-by: Diederik de Haas <diederik@cknow-tech.com>
> Signed-off-by: Daniele Briguglio <hello@superkali.me>

I've reproduced the failure reported by Diederik on the NanoPC-T6-LTS 
(indirect I2S0_8CH_MCLKOUT) without this patch, then with this patch 
(which fixes it), and re-tested the R58X-Pro (out-of-tree explicit 
I2S0_8CH_MCLKOUT_TO_IO consumer); it all works for me now.

Tested-by: Ricardo Pardini <ricardo@pardini.net> # on R58X-Pro+NanoPC-T6

Thanks,

-- Ricardo

> ---
>   drivers/clk/rockchip/clk-rk3588.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/rockchip/clk-rk3588.c b/drivers/clk/rockchip/clk-rk3588.c
> index 2ba9976654c..86953f9ffee 100644
> --- a/drivers/clk/rockchip/clk-rk3588.c
> +++ b/drivers/clk/rockchip/clk-rk3588.c
> @@ -895,7 +895,7 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
>   	MUX(I2S2_2CH_MCLKOUT, "i2s2_2ch_mclkout", i2s2_2ch_mclkout_p, CLK_SET_RATE_PARENT,
>   			RK3588_CLKSEL_CON(30), 2, 1, MFLAGS),
>   	GATE_GRF(I2S2_2CH_MCLKOUT_TO_IO, "i2s2_2ch_mclkout_to_io", "i2s2_2ch_mclkout",
> -			0, RK3588_SYSGRF_SOC_CON6, 2, GFLAGS, grf_type_sys),
> +			CLK_IGNORE_UNUSED, RK3588_SYSGRF_SOC_CON6, 2, GFLAGS, grf_type_sys),
>   
>   	COMPOSITE(CLK_I2S3_2CH_SRC, "clk_i2s3_2ch_src", gpll_aupll_p, 0,
>   			RK3588_CLKSEL_CON(30), 8, 1, MFLAGS, 3, 5, DFLAGS,
> @@ -912,7 +912,7 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
>   	MUX(I2S3_2CH_MCLKOUT, "i2s3_2ch_mclkout", i2s3_2ch_mclkout_p, CLK_SET_RATE_PARENT,
>   			RK3588_CLKSEL_CON(32), 2, 1, MFLAGS),
>   	GATE_GRF(I2S3_2CH_MCLKOUT_TO_IO, "i2s3_2ch_mclkout_to_io", "i2s3_2ch_mclkout",
> -			0, RK3588_SYSGRF_SOC_CON6, 7, GFLAGS, grf_type_sys),
> +			CLK_IGNORE_UNUSED, RK3588_SYSGRF_SOC_CON6, 7, GFLAGS, grf_type_sys),
>   	GATE(PCLK_ACDCDIG, "pclk_acdcdig", "pclk_audio_root", 0,
>   			RK3588_CLKGATE_CON(7), 11, GFLAGS),
>   	GATE(HCLK_I2S0_8CH, "hclk_i2s0_8ch", "hclk_audio_root", 0,
> @@ -942,7 +942,7 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
>   	MUX(I2S0_8CH_MCLKOUT, "i2s0_8ch_mclkout", i2s0_8ch_mclkout_p, CLK_SET_RATE_PARENT,
>   			RK3588_CLKSEL_CON(28), 2, 2, MFLAGS),
>   	GATE_GRF(I2S0_8CH_MCLKOUT_TO_IO, "i2s0_8ch_mclkout_to_io", "i2s0_8ch_mclkout",
> -			0, RK3588_SYSGRF_SOC_CON6, 0, GFLAGS, grf_type_sys),
> +			CLK_IGNORE_UNUSED, RK3588_SYSGRF_SOC_CON6, 0, GFLAGS, grf_type_sys),
>   
>   	GATE(HCLK_PDM1, "hclk_pdm1", "hclk_audio_root", 0,
>   			RK3588_CLKGATE_CON(9), 6, GFLAGS),
> @@ -2229,7 +2229,7 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
>   	MUX(I2S1_8CH_MCLKOUT, "i2s1_8ch_mclkout", i2s1_8ch_mclkout_p, CLK_SET_RATE_PARENT,
>   			RK3588_PMU_CLKSEL_CON(9), 2, 2, MFLAGS),
>   	GATE_GRF(I2S1_8CH_MCLKOUT_TO_IO, "i2s1_8ch_mclkout_to_io", "i2s1_8ch_mclkout",
> -			0, RK3588_SYSGRF_SOC_CON6, 1, GFLAGS, grf_type_sys),
> +			CLK_IGNORE_UNUSED, RK3588_SYSGRF_SOC_CON6, 1, GFLAGS, grf_type_sys),
>   	GATE(PCLK_PMU1, "pclk_pmu1", "pclk_pmu0_root", CLK_IS_CRITICAL,
>   			RK3588_PMU_CLKGATE_CON(1), 0, GFLAGS),
>   	GATE(CLK_DDR_FAIL_SAFE, "clk_ddr_fail_safe", "clk_pmu0", CLK_IGNORE_UNUSED,
> 
> base-commit: 7edfb7fb58ee058298e18fde76a6077ef17d19d8

Re: [PATCH] clk: rockchip: rk3588: don't disable unused I2S MCLK output gates

[Heiko Stuebner]

On Wed, 24 Jun 2026 14:39:14 +0200, Daniele Briguglio wrote:
> No in-tree board references these gates yet. Boards drive the codec
> MCLK through the parent I2S*_8CH_MCLKOUT, and now that the gates are
> managed clocks, clk_disable_unused() turns them off at boot. On a board
> that relied on firmware leaving the output enabled, that cuts the MCLK
> and analog audio stops working.
> 
> Mark the four gates CLK_IGNORE_UNUSED so an unreferenced gate keeps the
> state firmware left. A board that wants the kernel to own the gate can
> reference I2S*_8CH_MCLKOUT_TO_IO from DT instead.
> 
> [...]

Applied, thanks!

[1/1] clk: rockchip: rk3588: don't disable unused I2S MCLK output gates
      commit: 946352b2f88fd2378f0341312e47dff1e8dc2fac

Best regards,
-- 
Heiko Stuebner <heiko@sntech.de>

Re: [PATCH] clk: rockchip: rk3588: don't disable unused I2S MCLK output gates

[Chen-Yu Tsai]

On Mon, Jun 29, 2026 at 3:13 AM Heiko Stuebner <heiko@sntech.de> wrote:
>
>
> On Wed, 24 Jun 2026 14:39:14 +0200, Daniele Briguglio wrote:
> > No in-tree board references these gates yet. Boards drive the codec
> > MCLK through the parent I2S*_8CH_MCLKOUT, and now that the gates are
> > managed clocks, clk_disable_unused() turns them off at boot. On a board
> > that relied on firmware leaving the output enabled, that cuts the MCLK
> > and analog audio stops working.
> >
> > Mark the four gates CLK_IGNORE_UNUSED so an unreferenced gate keeps the
> > state firmware left. A board that wants the kernel to own the gate can
> > reference I2S*_8CH_MCLKOUT_TO_IO from DT instead.
> >
> > [...]
>
> Applied, thanks!
>
> [1/1] clk: rockchip: rk3588: don't disable unused I2S MCLK output gates
>       commit: 946352b2f88fd2378f0341312e47dff1e8dc2fac

In hindsight maybe it would have been a better idea to map the existing
clock ID I2S*_8CH_MCLKOUT to the new gates, and add (or not add) new
clocks for the internal MCLK gates.

Then you wouldn't need to update the DTs, wouldn't need this workaround,
and wouldn't depend on the bootloader to set the registers correctly when
booting an old DT.


ChenYu

Re: [PATCH] clk: rockchip: rk3588: don't disable unused I2S MCLK output gates

[Brian Masney]

Hi Chen-Yu,

On Thu, Aug 06, 2026 at 01:00:02AM +0800, Chen-Yu Tsai wrote:
> On Mon, Jun 29, 2026 at 3:13 AM Heiko Stuebner <heiko@sntech.de> wrote:
> >
> >
> > On Wed, 24 Jun 2026 14:39:14 +0200, Daniele Briguglio wrote:
> > > No in-tree board references these gates yet. Boards drive the codec
> > > MCLK through the parent I2S*_8CH_MCLKOUT, and now that the gates are
> > > managed clocks, clk_disable_unused() turns them off at boot. On a board
> > > that relied on firmware leaving the output enabled, that cuts the MCLK
> > > and analog audio stops working.
> > >
> > > Mark the four gates CLK_IGNORE_UNUSED so an unreferenced gate keeps the
> > > state firmware left. A board that wants the kernel to own the gate can
> > > reference I2S*_8CH_MCLKOUT_TO_IO from DT instead.
> > >
> > > [...]
> >
> > Applied, thanks!
> >
> > [1/1] clk: rockchip: rk3588: don't disable unused I2S MCLK output gates
> >       commit: 946352b2f88fd2378f0341312e47dff1e8dc2fac
> 
> In hindsight maybe it would have been a better idea to map the existing
> clock ID I2S*_8CH_MCLKOUT to the new gates, and add (or not add) new
> clocks for the internal MCLK gates.
> 
> Then you wouldn't need to update the DTs, wouldn't need this workaround,
> and wouldn't depend on the bootloader to set the registers correctly when
> booting an old DT.

Help me understand for the future: If the approach you describe would
have been used, then the clocks in the kernel would have been mislabeled
in the kernel driver compared to what's actually on the SoC, correct?
That would have been more desirable in order to keep compatibility with
the older DTs?

But the older DTs can still reference the mux, correct? From the
kernel's perspective in this scenario, the important thing is for the
mux to select the appropriate parent. The end gate will always be left
on. From a power management perspective, the power will be cut further
up the clock tree as needed.

Brian

Re: [PATCH] clk: rockchip: rk3588: don't disable unused I2S MCLK output gates

[Chen-Yu Tsai]

On Thu, Aug 6, 2026 at 7:09 AM Brian Masney <bmasney@redhat.com> wrote:
>
> Hi Chen-Yu,
>
> On Thu, Aug 06, 2026 at 01:00:02AM +0800, Chen-Yu Tsai wrote:
> > On Mon, Jun 29, 2026 at 3:13 AM Heiko Stuebner <heiko@sntech.de> wrote:
> > >
> > >
> > > On Wed, 24 Jun 2026 14:39:14 +0200, Daniele Briguglio wrote:
> > > > No in-tree board references these gates yet. Boards drive the codec
> > > > MCLK through the parent I2S*_8CH_MCLKOUT, and now that the gates are
> > > > managed clocks, clk_disable_unused() turns them off at boot. On a board
> > > > that relied on firmware leaving the output enabled, that cuts the MCLK
> > > > and analog audio stops working.
> > > >
> > > > Mark the four gates CLK_IGNORE_UNUSED so an unreferenced gate keeps the
> > > > state firmware left. A board that wants the kernel to own the gate can
> > > > reference I2S*_8CH_MCLKOUT_TO_IO from DT instead.
> > > >
> > > > [...]
> > >
> > > Applied, thanks!
> > >
> > > [1/1] clk: rockchip: rk3588: don't disable unused I2S MCLK output gates
> > >       commit: 946352b2f88fd2378f0341312e47dff1e8dc2fac
> >
> > In hindsight maybe it would have been a better idea to map the existing
> > clock ID I2S*_8CH_MCLKOUT to the new gates, and add (or not add) new
> > clocks for the internal MCLK gates.
> >
> > Then you wouldn't need to update the DTs, wouldn't need this workaround,
> > and wouldn't depend on the bootloader to set the registers correctly when
> > booting an old DT.
>
> Help me understand for the future: If the approach you describe would
> have been used, then the clocks in the kernel would have been mislabeled
> in the kernel driver compared to what's actually on the SoC, correct?
> That would have been more desirable in order to keep compatibility with
> the older DTs?

Which part are you referring to about mislabeled? The DT part would be
mislabeled, sure, but you can rename the macro to a more suitable name
without changing the ID, which is the binding part that cannot be changed.

In the driver, we would change I2S*_8CH_MCLKOUT to point to the gate,
while a new I2S*_8CH_MCLKOUT_INTERNAL or something, points to the original
mux.

Depending on the clk driver (library) design, it might not even need
a clk ID if it's only internal.

And IIRC clks don't even have to be registered in order. They can be
registered, be added as orphans, and when their parent appears, get
integrated into the clk tree.

> But the older DTs can still reference the mux, correct? From the
> kernel's perspective in this scenario, the important thing is for the
> mux to select the appropriate parent. The end gate will always be left
> on. From a power management perspective, the power will be cut further
> up the clock tree as needed.

Correct, old DTs still reference the mux, not the gate, which is a
problem if the gate wasn't enabled already.

Even with this "fix", the old DTs still rely on the bootloader having
enabled the gates beforehand. The flag added is "CLK_IGNORE_UNUSED",
not "CLK_TURN_IT_ON_BUT_LEAVE_IT_UNUSED". And BTW, the gate is missing
the CLK_SET_RATE_PARENT flag.


IMO this is better than having to carry workarounds for old DTs
indefinitely. Up to you and Heiko though.


ChenYu

Re: [PATCH] clk: rockchip: rk3588: don't disable unused I2S MCLK output gates

[Igor Paunovic]

Hi,

Independent reproduction and a Tested-by for this fix, on a third board
family: Orange Pi 5 Plus (rk3588, es8388 clocked from I2S0_8CH_MCLKOUT).

On v7.2-rc6 without this patch the analog codec is completely dead in
both directions - playback silent, capture records exact digital zeroes -
because clk_disable_unused() closes the new i2s0_8ch_mclkout_to_io GRF
gate at the end of boot: SYS_GRF SOC_CON6 reads 0x7c7 (bit 0 set) while
the clock tree shows i2s0_8ch_mclkout enabled for the codec and
i2s0_8ch_mclkout_to_io with an enable count of 0. Re-opening the gate by
hand over /dev/mem brings the codec back the same second, and with the
gate held open the board plays and records normally.

Tested-by: Igor Paunovic <royalnet026@gmail.com>

On scale: grepping v7.2-rc6, every rk3588 board DTS that clocks its
codec from an I2S MCLKOUT still references the plain id rather than the
_TO_IO one - I count 21 files (es8316/es8388/es8323/rt5616/nau8822
across the Orange Pi 5 family, ROCK 5A/5B/5B+/5T/5C/ITX, NanoPC-T6,
CM3588, ArmSoM, CoolPi, the EVBs, QuartzPro64, Gameforce Ace and
Firefly) - so all of these ship with broken analog audio if v7.2 goes
final without this fix. Is anything still blocking it from going in
with the next -rc?

Thanks,
Igor