Re: [REGRESSION] Re: [PATCH v1] ASoC: rockchip: i2s_tdm: Re-add the set_sysclk callback

[Detlev Casanova <detlev.casanova@collabora.com>]

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

Hi Lucas, sorry for breaking support on your board.
I'm also surprised that you were not added in CC of the original patch.

Can you try to keep the patch and apply the attached one instead ?

Detlev.

On 2/18/26 07:19, Luca Ceresoli wrote:
> Hello Detlev,
>
> On Tue Feb 10, 2026 at 10:54 AM CET, Luca Ceresoli wrote:
>> Hello Detlev,
>>
>> On Fri Jan 17, 2025 at 5:31 PM CET, Detlev Casanova wrote:
>>> In commit
>>> 9e2ab4b18ebd ("ASoC: rockchip: i2s-tdm: Fix inaccurate sampling rates"),
>>> the set_sysclk callback was removed as considered unused as the mclk rate
>>> can be set in the hw_params callback.
>>> The difference between hw_params and set_sysclk is that the former is
>>> called with the audio sampling rate set in the params (e.g.: 48000 Hz)
>>> while the latter is called with a clock rate already computed with
>>>    sampling_rate * mclk-fs (e.g.: 48000 * 256)
>>>
>>> For HDMI audio using the Rockchip I2S TDM driver, the mclk-fs value must
>>> be set to 128 instead of the default 256, and that value is set in the
>>> device tree at the machine driver level (like a simple-audio-card
>>> compatible node).
>>> Therefore, the i2s_tdm driver has no idea that another mclk-fs value can
>>> be configured and simply computes the mclk rate in the hw_params callback
>>> with DEFAULT_MCLK_FS * params_rate(params), which is wrong for HDMI
>>> audio.
>>>
>>> Re-add the set_sysclk callback so that the mclk rate is computed by the
>>> machine driver which has the correct mclk-fs value set in its device tree
>>> node.
>> I'm afraid I just found this commit breaks audio capture on the RK3308.
>>
>> Using 'arecord -Vmono -d 2 -c 8 -f S16_LE -r 96000 /dev/null' I get:
>>
>>    rockchip-i2s-tdm ff320000.i2s: ASoC error (-22): at snd_soc_dai_hw_params() on ff320000.i2s
>>     ff320000.i2s-rk3308-hifi: ASoC error (-22): at __soc_pcm_hw_params() on ff320000.i2s-rk3308-hifi
>>
>> Tested on:
>>
>>   * Radxa Rock Pi S
>>   * Upstream kernel
>>   * arm64 defconfig
>>
>> Tested kernel versions:
>>
>>   * v6.12 works
>>   * 5323186e2e8d (this commit) fails
>>   * 5323186e2e8d^ works
>>   * 21cfbeae7d7c (same patch on stable/linux-6.12.y) fails
>>   * 21cfbeae7d7c^ works
>>   * v6.19 fails
>>   * v6.19 + 'git revert 5323186e2e8d' works
>>
>> I don't have more information at the moment.
> Let me add some more info to my previous report.
>
> The function failing is clk_set_rate() in rockchip_i2s_tdm_hw_params().
>
> I added some logging to rockchip_i2s_tdm_hw_params() and here's the output:
>
>    On v6.19-11566-g254edc893f3a (current Linux master)
>
>    At boot:
>      [    3.324622] rockchip-i2s-tdm ff320000.i2s: The target mclk_tx freq is: 50176000
>      [    3.324720] rockchip-i2s-tdm ff320000.i2s: using zero-initialized flat cache, this may cause unexpected behavior
>    When launching arecord:
>      [   18.429389] rockchip_i2s_tdm_hw_params:653 i2s_tdm->is_master_mode = 1
>      [   18.430154] rockchip_i2s_tdm_hw_params:654 i2s_tdm->mclk_tx clk_i2s2_8ch_tx
>      [   18.430815] rockchip_i2s_tdm_hw_params:655 i2s_tdm->mclk_rx clk_i2s2_8ch_rx
>      [   18.431465] rockchip_i2s_tdm_hw_params:656 i2s_tdm->mclk_tx_freq 50176000
>      [   18.432097] rockchip_i2s_tdm_hw_params:657 i2s_tdm->mclk_rx_freq 0
>      [   18.432677] rockchip_i2s_tdm_hw_params:658 i2s_tdm->clk_trcm 0
>      [   18.433230] rockchip_i2s_tdm_hw_params:659 substream->stream 1
>      [   18.433858] rockchip_i2s_tdm_hw_params:678 mclk clk_i2s2_8ch_rx
>      [   18.434423] rockchip_i2s_tdm_hw_params:679 mclk_rate 0
>      [   18.434977] rockchip_i2s_tdm_hw_params:682 clk_set_rate(clk_i2s2_8ch_rx, 0) returned -22
>      [   18.435743] rockchip-i2s-tdm ff320000.i2s: ASoC error (-22): at snd_soc_dai_hw_params() on ff320000.i2s
>      [   18.436699]  ff320000.i2s-rk3308-hifi: ASoC error (-22): at __soc_pcm_hw_params() on ff320000.i2s-rk3308-hifi
>
>    On v6.19-11566-g254edc893f3a + 'git revert 5323186e2e8d'
>
>    At boot:
>      [    3.283853] rockchip-i2s-tdm ff320000.i2s: using zero-initialized flat cache, this may cause unexpected behavior
>    When launching arecord:
>      [   11.952290] rockchip_i2s_tdm_hw_params:631 i2s_tdm->is_master_mode = 1
>      [   11.952955] rockchip_i2s_tdm_hw_params:632 i2s_tdm->mclk_tx clk_i2s2_8ch_tx
>      [   11.953615] rockchip_i2s_tdm_hw_params:633 i2s_tdm->mclk_rx clk_i2s2_8ch_rx
>      [   11.954355] rockchip_i2s_tdm_hw_params:636 i2s_tdm->clk_trcm 0
>      [   11.954917] rockchip_i2s_tdm_hw_params:637 substream->stream 1
>      [   11.955477] rockchip_i2s_tdm_hw_params:652 mclk clk_i2s2_8ch_rx
>      [   11.956048] rockchip_i2s_tdm_hw_params:653 params_rate(params) 96000
>      [   11.956769] rockchip_i2s_tdm_hw_params:656 clk_set_rate(clk_i2s2_8ch_rx, 24576000) returned 0
>
> I cannot point to the right fix but the value 0 in i2s_tdm->mclk_rx_freq
> appears as a good clue, perhaps it has never been set.
>
> I hope this helps.
>
> Luca
>
> --
> Luca Ceresoli, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

[-- Attachment #2: 0001-sound-rockchip-Use-param-freq-if-not-provided-by-set.patch --]
[-- Type: text/x-patch, Size: 1268 bytes --]

From f2441859c368fe25512a71e82160b0a307ed4429 Mon Sep 17 00:00:00 2001
From: Detlev Casanova <detlev.casanova@collabora.com>
Date: Wed, 18 Feb 2026 10:46:50 -0500
Subject: [PATCH] sound: rockchip: Use param freq if not provided by set_sysclk

Hi Lucas, sorry for breaking support on your board.
I'm also surprised that you were not added in CC of the original patch.

Can you try to keep the patch and apply this instead ?

Drivers will not always set the sysclk.
If that is the case, use the clock set in the params multiplied by the
default mclk-fs.

---
 sound/soc/rockchip/rockchip_i2s_tdm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/sound/soc/rockchip/rockchip_i2s_tdm.c b/sound/soc/rockchip/rockchip_i2s_tdm.c
index 770b9bfbb384..f2cda7e64b74 100644
--- a/sound/soc/rockchip/rockchip_i2s_tdm.c
+++ b/sound/soc/rockchip/rockchip_i2s_tdm.c
@@ -665,6 +665,12 @@ static int rockchip_i2s_tdm_hw_params(struct snd_pcm_substream *substream,
 			mclk_rate = i2s_tdm->mclk_rx_freq;
 		}
 
+		/* If the dai/component driver didn't call set_sysclk(), simply
+		 * use the freq from the params.
+		 */
+		if (!mclk_rate)
+			mclk_rate = DEFAULT_MCLK_FS * params_rate(params);
+
 		err = clk_set_rate(mclk, mclk_rate);
 		if (err)
 			return err;
-- 
2.53.0