[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <873439o1xt.wl-kuninori.morimoto.gx@renesas.com>
Date: Tue, 10 Feb 2026 00:51:58 +0000
From: Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>
To: Stefano Radaelli <stefano.radaelli21@...il.com>
Cc: Mark Brown <broonie@...nel.org>,
Liam Girdwood <lgirdwood@...il.com>,
Jaroslav Kysela <perex@...ex.cz>,
Takashi Iwai <tiwai@...e.com>,
linux-sound@...r.kernel.org,
linux-kernel@...r.kernel.org,
alexander.h@...iscite.com,
pierluigi.p@...iscite.com,
Stefano Radaelli <stefano.r@...iscite.com>
Subject: Re: [PATCH v1] ASoC: simple-card-utils: set CPU sysclk before codec for mclk-fs
Hi Stefano
> When simple-audio-card uses simple-audio-card,mclk-fs,
> simple_util_hw_params() programs sysclk for codec DAIs and CPU DAIs.
>
> Some CPU DAIs may apply the final MCLK rate as part of their .set_sysclk()
> callback (e.g. by changing the underlying clock rate via clk_set_rate()).
> If codec sysclk is configured before the CPU DAI applies the final
> MCLK rate, the codec may configure its internal clocking based on a
> non-final MCLK setting, leading to an incorrect clocking state on the
> first playback after boot.
>
> This behaviour was observed on i.MX95 systems using fsl_sai with
> downstream kernels, which made the issue visible.
> However, the problem is generic and can affect any setup where the
> CPU DAI applies the final MCLK rate in set_sysclk() while
> simple-audio-card uses mclk-fs.
>
> A reproducible symptom is that the first playback runs at the wrong speed
> ("slow" audio), while subsequent playbacks work correctly.
>
> Example setup:
> - CPU DAI: fsl_sai (i.MX95)
> - Codec: wm8904
> - simple-audio-card,mclk-fs enabled
>
> Reproducer:
> aplay -D hw:wm8904audio /home/Front_Center_8k.wav
>
> Debug evidence:
> ftrace showed snd_soc_dai_set_sysclk() called from simple_util_hw_params
> for both codec and CPU DAIs, and confirmed wm8904_set_sysclk() ran before
> the CPU DAI sysclk call, and the CPU DAI then applied the final
> MCLK settings.
>
> Fix this by setting the CPU DAI sysclk before the codec sysclk
> when mclk-fs is used, so that the codec always configures its internal
> clocking based on the final MCLK rate.
>
> Signed-off-by: Stefano Radaelli <stefano.r@...iscite.com>
> ---
> sound/soc/generic/simple-card-utils.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
> index bdc02e85b089..50f60e4996a5 100644
> --- a/sound/soc/generic/simple-card-utils.c
> +++ b/sound/soc/generic/simple-card-utils.c
> @@ -501,15 +501,21 @@ int simple_util_hw_params(struct snd_pcm_substream *substream,
> goto end;
> }
>
> - for_each_rtd_codec_dais(rtd, i, sdai) {
> - pdai = simple_props_to_dai_codec(props, i);
> + /*
> + * When mclk-fs is used, some CPU DAIs may change the
> + * actual MCLK rate in set_sysclk() (e.g. via clk_set_rate()).
> + * Configure CPU DAI first so codec clocking/FLL sees
> + * the final MCLK rate.
> + */
> + for_each_rtd_cpu_dais(rtd, i, sdai) {
> + pdai = simple_props_to_dai_cpu(props, i);
> ret = snd_soc_dai_set_sysclk(sdai, 0, mclk, pdai->clk_direction);
> if (ret && ret != -ENOTSUPP)
> goto end;
> }
>
> - for_each_rtd_cpu_dais(rtd, i, sdai) {
> - pdai = simple_props_to_dai_cpu(props, i);
> + for_each_rtd_codec_dais(rtd, i, sdai) {
> + pdai = simple_props_to_dai_codec(props, i);
> ret = snd_soc_dai_set_sysclk(sdai, 0, mclk, pdai->clk_direction);
> if (ret && ret != -ENOTSUPP)
> goto end;
Actually, this is one of my concern (= which should be worked first
(CPU or Codec)). I don't remember why current code calls Codec first,
but maybe original code was.
I guess, some other Card needs to call as-is (= Codec first), and
you want to call opposite (= CPU first). It depends on clock
provider/consumer settings, instead of mclk-fs, but based on each Card.
Not same but similar issue we had before about how we want to call
.trigger.
356caf663deee8dc46ff3168ec0b24bcbeb00b28
("ASoC: add new trigger ordering method")
It was fixed order, but is now controlled by flag.
Maybe we want to have similar method ?
// unreliable name
#define XX_CLK_START_CODEC_FIRST 0
#define XX_CLK_START_CPU_FIRST 1
if (xx_clk_start_first == XX_CLK_START_CODEC_FIRST) {
// same as current code
start_codec_clk()
start_cpu_clk()
} else {
// opposite
start_cpu_clk()
start_codec_clk()
}
Thank you for your help !!
Best regards
---
Kuninori Morimoto
Powered by blists - more mailing lists