Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v4 0/5] clk: rockchip: rk3588: add I2S MCLK output gate clocks
       [not found] <20260419-rk3588-mclk-gate-grf-v4-0-513a42dd1dcc@superkali.me>
@ 2026-04-27 12:23 ` Heiko Stuebner
       [not found] ` <20260419-rk3588-mclk-gate-grf-v4-5-513a42dd1dcc@superkali.me>
  1 sibling, 0 replies; 6+ messages in thread
From: Heiko Stuebner @ 2026-04-27 12:23 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Daniele Briguglio
  Cc: Heiko Stuebner, Nicolas Frattaroli, linux-clk, devicetree,
	linux-arm-kernel, linux-rockchip, linux-kernel,
	Krzysztof Kozlowski, Ricardo Pardini


On Sun, 19 Apr 2026 13:43:05 +0200, Daniele Briguglio wrote:
> On RK3588, the four I2S master clock (MCLK) outputs to external IO
> pins are gated by bits in SYS_GRF SOC_CON6 (offset 0x0318). These
> are set-to-disable gates with hiword mask semantics.
> 
> The TRM documents the reset value of these bits as 0 (gate open),
> but in practice the Rockchip firmware (BL31) may set them during
> early boot, preventing the MCLK signal from reaching external audio
> codecs. The kernel should manage these gates explicitly so that
> audio functionality does not depend on bootloader register state.
> 
> [...]

Applied, thanks!

[1/5] dt-bindings: clock: rockchip,rk3588-cru: add I2S MCLK output to IO clock IDs
      commit: 56c2ca0ae7cb9254c4c2b82baa0afe29feaa274e
[2/5] clk: rockchip: allow grf_type_sys lookup in aux_grf_table
      commit: 28820fc7983b9c8e160c0095067a570bdfcae1f0
[3/5] clk: rockchip: add helper to register auxiliary GRFs
      commit: 32d1d88c4165d0da31d3bfda912e80e8110d6fc1
[4/5] soc: rockchip: rk3588: add SYS_GRF SOC_CON6 register offset
      commit: 06c990bffdbea7cf655e728f4423ecd13fb030f6
[5/5] clk: rockchip: rk3588: add GATE_GRF clocks for I2S MCLK output to IO
      commit: 02b9b0bb626989b947d82bbe4e050f0254e2046d

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 5/5] clk: rockchip: rk3588: add GATE_GRF clocks for I2S MCLK output to IO
       [not found] ` <20260419-rk3588-mclk-gate-grf-v4-5-513a42dd1dcc@superkali.me>
@ 2026-06-23 11:10   ` Diederik de Haas
  2026-06-23 12:05     ` Heiko Stübner
  0 siblings, 1 reply; 6+ messages in thread
From: Diederik de Haas @ 2026-06-23 11:10 UTC (permalink / raw)
  To: Daniele Briguglio, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Heiko Stuebner
  Cc: Nicolas Frattaroli, linux-clk, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel, Ricardo Pardini

[Resending it against v4 which wasn't present in my INBOX, but was the
version accepted and used in my kernel]

On Tue Jun 23, 2026 at 1:08 PM CEST, Daniele Briguglio wrote:
> The I2S MCLK outputs on RK3588 are gated by bits in the SYS_GRF
> register SOC_CON6 (offset 0x318). These gates control whether the
> internal CRU MCLK signals reach the external IO pins connected to
> audio codecs.
>
> The kernel should explicitly manage these gates so that audio
> functionality does not depend on bootloader register state. This is
> analogous to what was done for RK3576 SAI MCLK outputs [1].
>
> Register the SYS_GRF as an auxiliary GRF with grf_type_sys using
> rockchip_clk_add_grf(), and add GATE_GRF entries for all four I2S
> MCLK output gates:
>
>   - I2S0_8CH_MCLKOUT_TO_IO (bit 0)
>   - I2S1_8CH_MCLKOUT_TO_IO (bit 1)
>   - I2S2_2CH_MCLKOUT_TO_IO (bit 2)
>   - I2S3_2CH_MCLKOUT_TO_IO (bit 7)
>
> Board DTS files that need MCLK on an IO pin can reference these
> clocks, e.g.:
>
>     clocks = <&cru I2S0_8CH_MCLKOUT_TO_IO>;
>
> Tested on the Youyeetoo YY3588 (RK3588) with an ES8388 codec on I2S0.

Doesn't this break audio on a lot of RK3588 based boards?
I have a kernel with this patch set and since then analog audio on my NanoPC-T6
LTS and my WIP NanoPC-T6 Plus stopped working.
Until I did s/I2S0_8CH_MCLKOUT/I2S0_8CH_MCLKOUT_TO_IO/ in my dts[i] files.

And I wouldn't be surprised if the same thing applies to other RK3588 based
boards? The same dtb file with a 7.1 kernel, without this patch set, works.

Cheers,
  Diederik

> [1] https://lore.kernel.org/r/20250305-rk3576-sai-v1-2-64e6cf863e9a@collabora.com/
>
> Tested-by: Ricardo Pardini <ricardo@pardini.net>
> Signed-off-by: Daniele Briguglio <hello@superkali.me>
> ---
>  drivers/clk/rockchip/clk-rk3588.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/clk/rockchip/clk-rk3588.c b/drivers/clk/rockchip/clk-rk3588.c
> index 1694223f4f84..2ba9976654cf 100644
> --- a/drivers/clk/rockchip/clk-rk3588.c
> +++ b/drivers/clk/rockchip/clk-rk3588.c
> @@ -5,11 +5,13 @@
>   */
>  
>  #include <linux/clk-provider.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/of.h>
>  #include <linux/of_address.h>
>  #include <linux/platform_device.h>
>  #include <linux/syscore_ops.h>
>  #include <dt-bindings/clock/rockchip,rk3588-cru.h>
> +#include <soc/rockchip/rk3588_grf.h>
>  #include "clk.h"
>  
>  #define RK3588_GRF_SOC_STATUS0		0x600
> @@ -892,6 +894,8 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
>  			RK3588_CLKGATE_CON(8), 0, GFLAGS),
>  	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),
>  
>  	COMPOSITE(CLK_I2S3_2CH_SRC, "clk_i2s3_2ch_src", gpll_aupll_p, 0,
>  			RK3588_CLKSEL_CON(30), 8, 1, MFLAGS, 3, 5, DFLAGS,
> @@ -907,6 +911,8 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
>  			RK3588_CLKGATE_CON(8), 4, GFLAGS),
>  	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),
>  	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,
> @@ -935,6 +941,8 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
>  			RK3588_CLKGATE_CON(7), 10, GFLAGS),
>  	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),
>  
>  	GATE(HCLK_PDM1, "hclk_pdm1", "hclk_audio_root", 0,
>  			RK3588_CLKGATE_CON(9), 6, GFLAGS),
> @@ -2220,6 +2228,8 @@ static struct rockchip_clk_branch rk3588_early_clk_branches[] __initdata = {
>  			RK3588_PMU_CLKGATE_CON(2), 13, GFLAGS),
>  	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),
>  	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,
> @@ -2439,6 +2449,7 @@ static struct rockchip_clk_branch rk3588_clk_branches[] = {
>  static void __init rk3588_clk_early_init(struct device_node *np)
>  {
>  	struct rockchip_clk_provider *ctx;
> +	struct regmap *sys_grf;
>  	unsigned long clk_nr_clks, max_clk_id1, max_clk_id2;
>  	void __iomem *reg_base;
>  
> @@ -2479,6 +2490,11 @@ static void __init rk3588_clk_early_init(struct device_node *np)
>  			&rk3588_cpub1clk_data, rk3588_cpub1clk_rates,
>  			ARRAY_SIZE(rk3588_cpub1clk_rates));
>  
> +	/* Register SYS_GRF for I2S MCLK output to IO gate clocks */
> +	sys_grf = syscon_regmap_lookup_by_compatible("rockchip,rk3588-sys-grf");
> +	if (!IS_ERR(sys_grf))
> +		rockchip_clk_add_grf(ctx, sys_grf, grf_type_sys);
> +
>  	rockchip_clk_register_branches(ctx, rk3588_early_clk_branches,
>  				       ARRAY_SIZE(rk3588_early_clk_branches));
>  



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 5/5] clk: rockchip: rk3588: add GATE_GRF clocks for I2S MCLK output to IO
  2026-06-23 11:10   ` [PATCH v4 5/5] clk: rockchip: rk3588: add GATE_GRF clocks for I2S MCLK output to IO Diederik de Haas
@ 2026-06-23 12:05     ` Heiko Stübner
  2026-06-23 12:33       ` Daniele Briguglio
  0 siblings, 1 reply; 6+ messages in thread
From: Heiko Stübner @ 2026-06-23 12:05 UTC (permalink / raw)
  To: Daniele Briguglio, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Diederik de Haas
  Cc: Nicolas Frattaroli, linux-clk, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel, Ricardo Pardini

Hi Diederick,

Am Dienstag, 23. Juni 2026, 13:10:49 Mitteleuropäische Sommerzeit schrieb Diederik de Haas:
> [Resending it against v4 which wasn't present in my INBOX, but was the
> version accepted and used in my kernel]
> 
> On Tue Jun 23, 2026 at 1:08 PM CEST, Daniele Briguglio wrote:
> > The I2S MCLK outputs on RK3588 are gated by bits in the SYS_GRF
> > register SOC_CON6 (offset 0x318). These gates control whether the
> > internal CRU MCLK signals reach the external IO pins connected to
> > audio codecs.
> >
> > The kernel should explicitly manage these gates so that audio
> > functionality does not depend on bootloader register state. This is
> > analogous to what was done for RK3576 SAI MCLK outputs [1].
> >
> > Register the SYS_GRF as an auxiliary GRF with grf_type_sys using
> > rockchip_clk_add_grf(), and add GATE_GRF entries for all four I2S
> > MCLK output gates:
> >
> >   - I2S0_8CH_MCLKOUT_TO_IO (bit 0)
> >   - I2S1_8CH_MCLKOUT_TO_IO (bit 1)
> >   - I2S2_2CH_MCLKOUT_TO_IO (bit 2)
> >   - I2S3_2CH_MCLKOUT_TO_IO (bit 7)
> >
> > Board DTS files that need MCLK on an IO pin can reference these
> > clocks, e.g.:
> >
> >     clocks = <&cru I2S0_8CH_MCLKOUT_TO_IO>;
> >
> > Tested on the Youyeetoo YY3588 (RK3588) with an ES8388 codec on I2S0.
> 
> Doesn't this break audio on a lot of RK3588 based boards?
> I have a kernel with this patch set and since then analog audio on my NanoPC-T6
> LTS and my WIP NanoPC-T6 Plus stopped working.
> Until I did s/I2S0_8CH_MCLKOUT/I2S0_8CH_MCLKOUT_TO_IO/ in my dts[i] files.
> 
> And I wouldn't be surprised if the same thing applies to other RK3588 based
> boards? The same dtb file with a 7.1 kernel, without this patch set, works.

can you check if adding CLK_IGNORE_UNUSED [0] changes the situation for you?

What I assume is happening is that when the clocks were not declared they were
just left running, while now the kernel turns of unused (but defined) clocks.


[0] example in
https://elixir.bootlin.com/linux/v7.1.1/source/drivers/clk/rockchip/clk-rk3588.c#L865





^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 5/5] clk: rockchip: rk3588: add GATE_GRF clocks for I2S MCLK output to IO
  2026-06-23 12:05     ` Heiko Stübner
@ 2026-06-23 12:33       ` Daniele Briguglio
  2026-06-23 13:05         ` Diederik de Haas
  0 siblings, 1 reply; 6+ messages in thread
From: Daniele Briguglio @ 2026-06-23 12:33 UTC (permalink / raw)
  To: Heiko Stuebner, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Diederik de Haas
  Cc: Nicolas Frattaroli, linux-clk, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel, Ricardo Pardini

Hi Heiko,

> can you check if adding CLK_IGNORE_UNUSED changes the situation for you?
> What I assume is happening is that when the clocks were not declared they were
> just left running, while now the kernel turns off unused (but defined) clocks.

That lines up with what I see. The gates are set-to-disable and reset to
open, so before the series the bit just kept whatever the boot firmware
left it at.

Diederik, the cleanest way to confirm is to read SOC_CON6 before Linux
touches it, e.g. md.l 0xfd58c318 at the U-Boot prompt (bit 0 is I2S0). If
it comes up clear there, the gate is open, and if audio then breaks once
the kernel is up, that points at clk_disable_unused turning it off because
nothing references it.

If that turns out to be the case, CLK_IGNORE_UNUSED on the gates is a
reasonable way to stop the kernel from closing a gate the firmware already
left open, for boards that would rather not switch their DTS to _TO_IO.
Where a board does reference _TO_IO the consumer holds it open anyway, so
that path is unaffected either way.

Best regards,
Daniele


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 5/5] clk: rockchip: rk3588: add GATE_GRF clocks for I2S MCLK output to IO
  2026-06-23 12:33       ` Daniele Briguglio
@ 2026-06-23 13:05         ` Diederik de Haas
  2026-06-23 13:08           ` Diederik de Haas
  0 siblings, 1 reply; 6+ messages in thread
From: Diederik de Haas @ 2026-06-23 13:05 UTC (permalink / raw)
  To: Daniele Briguglio, Heiko Stuebner, Michael Turquette,
	Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Diederik de Haas
  Cc: Nicolas Frattaroli, linux-clk, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel, Ricardo Pardini

On Tue Jun 23, 2026 at 2:33 PM CEST, Daniele Briguglio wrote:
>> can you check if adding CLK_IGNORE_UNUSED changes the situation for you?
>> What I assume is happening is that when the clocks were not declared they were
>> just left running, while now the kernel turns off unused (but defined) clocks.
>
> That lines up with what I see. The gates are set-to-disable and reset to
> open, so before the series the bit just kept whatever the boot firmware
> left it at.
>
> Diederik, the cleanest way to confirm is to read SOC_CON6 before Linux
> touches it, e.g. md.l 0xfd58c318 at the U-Boot prompt (bit 0 is I2S0). If
> it comes up clear there, the gate is open, and if audio then breaks once
> the kernel is up, that points at clk_disable_unused turning it off because
> nothing references it.

NanoPC-T6 LTS
U-Boot: 2026.04-00003-g723f0da896bc

The 0003 comes from me adding patches for NanoPC-T6 Plus support, but
otherwise it's plain upstream U-Boot.

=> md.l 0xfd58c318
fd58c318: 00000600 00000a00 00000000 00000000  ................
fd58c328: 00000300 00092820 0fd58c338: 00000000 00000000 00000000 00000000  ................
fd58c348: 00000000 00000000 00000000 00000000  ................c358: 00000000 00000000 00000000 00000000  ................
fd58c368: 00000000 00000000 00000008: 00000000 00000000 00001000 00000240  ............@...
fd58c388: 0000003f 0000fefe 00000000 000000000 00000000 00000000 00000000  ................
fd58c3a8: 00000000 00000000 00000000 00000000 00000000 00000000 00000000  ................
fd58c3c8: 00000000 00000000 00000000 000000000 00000000 00000000 00000000  ................
fd58c3e8: 00000000 00000000 00000000 00000000  .0000000 00000000 00000000  ................
fd58c408: 00000000 00000000 00000000 00000000  ....

I'll let interpreting it up to you.

> If that turns out to be the case, CLK_IGNORE_UNUSED on the gates is a
> reasonable way to stop the kernel from closing a gate the firmware already
> left open, for boards that would rather not switch their DTS to _TO_IO.

I'm not so sure I agree that CLK_IGNORE_UNUSED is reasonable, but I'll leave
judgement up to others. I'll do the test regardless, though.

Cheers,
  Diederik

> Where a board does reference _TO_IO the consumer holds it open anyway, so
> that path is unaffected either way.
>
> Best regards,
> Daniele



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 5/5] clk: rockchip: rk3588: add GATE_GRF clocks for I2S MCLK output to IO
  2026-06-23 13:05         ` Diederik de Haas
@ 2026-06-23 13:08           ` Diederik de Haas
  0 siblings, 0 replies; 6+ messages in thread
From: Diederik de Haas @ 2026-06-23 13:08 UTC (permalink / raw)
  To: Diederik de Haas, Daniele Briguglio, Heiko Stuebner,
	Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: Nicolas Frattaroli, linux-clk, devicetree, linux-arm-kernel,
	linux-rockchip, linux-kernel, Ricardo Pardini

On Tue Jun 23, 2026 at 3:05 PM CEST, Diederik de Haas wrote:
> On Tue Jun 23, 2026 at 2:33 PM CEST, Daniele Briguglio wrote:
>> If that turns out to be the case, CLK_IGNORE_UNUSED on the gates is a
>> reasonable way to stop the kernel from closing a gate the firmware already
>> left open, for boards that would rather not switch their DTS to _TO_IO.
>
> I'm not so sure I agree that CLK_IGNORE_UNUSED is reasonable, but I'll leave
> judgement up to others. I'll do the test regardless, though.

Ah, just learned those should be added to your patch, in which case my
objection goes away.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-23 13:08 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260419-rk3588-mclk-gate-grf-v4-0-513a42dd1dcc@superkali.me>
2026-04-27 12:23 ` [PATCH v4 0/5] clk: rockchip: rk3588: add I2S MCLK output gate clocks Heiko Stuebner
     [not found] ` <20260419-rk3588-mclk-gate-grf-v4-5-513a42dd1dcc@superkali.me>
2026-06-23 11:10   ` [PATCH v4 5/5] clk: rockchip: rk3588: add GATE_GRF clocks for I2S MCLK output to IO Diederik de Haas
2026-06-23 12:05     ` Heiko Stübner
2026-06-23 12:33       ` Daniele Briguglio
2026-06-23 13:05         ` Diederik de Haas
2026-06-23 13:08           ` Diederik de Haas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox