From: Stefano Radaelli <stefano.r@variscite.com>

When simple-audio-card programs sysclk for CPU and codec DAIs during
hw_params, the ordering of these calls may matter on some platforms.

Some CPU DAIs finalize or adjust the MCLK rate as part of their
set_sysclk() callback (for example by calling clk_set_rate()). If the
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 value.

Such situations can arise depending on the clock provider/consumer
relationship between the CPU DAI and the codec.

Introduce an explicit sysclk ordering enum in simple-card-utils and use
it to control the order of snd_soc_dai_set_sysclk() calls in the mclk-fs
handling path. The default behaviour remains unchanged (codec-first)
to avoid regressions.

Signed-off-by: Stefano Radaelli <stefano.r@variscite.com>
---
v3:
 - DT flag not applicable, not a hardware configuration
 - Introduced enum to support order configuration

v2:
 - Do not change the default sysclk ordering
 - Make the ordering selectable via DT flag
 - Add DT binding documentation
Link: https://patchwork.kernel.org/project/alsa-devel/patch/20260210164506.161810-2-stefano.r@variscite.com/

v1:
 - Changed sysclk order, cpu first, then codec
Link: https://patchwork.kernel.org/project/alsa-devel/patch/20260206134014.143057-1-stefano.r@variscite.com/

 include/sound/simple_card_utils.h     |  7 +++++
 sound/soc/generic/simple-card-utils.c | 44 ++++++++++++++++++++-------
 2 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
index 69a9c9c4d0e9..137eb795f587 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -54,6 +54,12 @@ struct prop_nums {
 	int platforms;
 };
 
+enum simple_util_sysclk_order {
+	SIMPLE_SYSCLK_ORDER_UNSPEC = 0,
+	SIMPLE_SYSCLK_ORDER_CODEC_FIRST,
+	SIMPLE_SYSCLK_ORDER_CPU_FIRST,
+};
+
 struct simple_util_priv {
 	struct snd_soc_card snd_card;
 	struct simple_dai_props {
@@ -63,6 +69,7 @@ struct simple_util_priv {
 		struct snd_soc_codec_conf *codec_conf;
 		struct prop_nums num;
 		unsigned int mclk_fs;
+		enum simple_util_sysclk_order sysclk_order;
 	} *dai_props;
 	struct simple_util_jack hp_jack;
 	struct simple_util_jack mic_jack;
diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index bdc02e85b089..66030f2bdece 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -468,9 +468,13 @@ int simple_util_hw_params(struct snd_pcm_substream *substream,
 	struct snd_soc_dai *sdai;
 	struct simple_util_priv *priv = snd_soc_card_get_drvdata(rtd->card);
 	struct simple_dai_props *props = runtime_simple_priv_to_props(priv, rtd);
+	enum simple_util_sysclk_order order = props->sysclk_order;
 	unsigned int mclk, mclk_fs = 0;
 	int i, ret;
 
+	if (order == SIMPLE_SYSCLK_ORDER_UNSPEC)
+		order = SIMPLE_SYSCLK_ORDER_CODEC_FIRST;
+
 	if (props->mclk_fs)
 		mclk_fs = props->mclk_fs;
 
@@ -501,18 +505,36 @@ 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);
-			ret = snd_soc_dai_set_sysclk(sdai, 0, mclk, pdai->clk_direction);
-			if (ret && ret != -ENOTSUPP)
-				goto end;
-		}
+		if (order == SIMPLE_SYSCLK_ORDER_CPU_FIRST) {
+			/* CPU first */
+			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);
-			ret = snd_soc_dai_set_sysclk(sdai, 0, mclk, pdai->clk_direction);
-			if (ret && ret != -ENOTSUPP)
-				goto end;
+			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;
+			}
+		} else {
+			/* default: codec first */
+			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;
+			}
+
+			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;
+			}
 		}
 	}
 

base-commit: b7ff7151e653aa296ab6c5495b2c1ab7c21eb250
-- 
2.47.3

Hi Stefano

Thank you for your patch

> When simple-audio-card programs sysclk for CPU and codec DAIs during
> hw_params, the ordering of these calls may matter on some platforms.
> 
> Some CPU DAIs finalize or adjust the MCLK rate as part of their
> set_sysclk() callback (for example by calling clk_set_rate()). If the
> 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 value.
> 
> Such situations can arise depending on the clock provider/consumer
> relationship between the CPU DAI and the codec.
> 
> Introduce an explicit sysclk ordering enum in simple-card-utils and use
> it to control the order of snd_soc_dai_set_sysclk() calls in the mclk-fs
> handling path. The default behaviour remains unchanged (codec-first)
> to avoid regressions.
> 
> Signed-off-by: Stefano Radaelli <stefano.r@variscite.com>
> ---
(snip)
> +enum simple_util_sysclk_order {
> +	SIMPLE_SYSCLK_ORDER_UNSPEC = 0,
> +	SIMPLE_SYSCLK_ORDER_CODEC_FIRST,
> +	SIMPLE_SYSCLK_ORDER_CPU_FIRST,
> +};

I think UNSPEC is not needed ?

	enum simple_util_sysclk_order {
		SIMPLE_SYSCLK_ORDER_CODEC_FIRST = 0,
		SIMPLE_SYSCLK_ORDER_CPU_FIRST,
	};

Do you set this "order" via custome driver if you want ot use CPU_FIRST ?

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Hi Stefano

> > When simple-audio-card programs sysclk for CPU and codec DAIs during
> > hw_params, the ordering of these calls may matter on some platforms.
> > 
> > Some CPU DAIs finalize or adjust the MCLK rate as part of their
> > set_sysclk() callback (for example by calling clk_set_rate()). If the
> > 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 value.
> > 
> > Such situations can arise depending on the clock provider/consumer
> > relationship between the CPU DAI and the codec.
> > 
> > Introduce an explicit sysclk ordering enum in simple-card-utils and use
> > it to control the order of snd_soc_dai_set_sysclk() calls in the mclk-fs
> > handling path. The default behaviour remains unchanged (codec-first)
> > to avoid regressions.
> > 
> > Signed-off-by: Stefano Radaelli <stefano.r@variscite.com>
> > ---
> (snip)
> > +enum simple_util_sysclk_order {
> > +	SIMPLE_SYSCLK_ORDER_UNSPEC = 0,
> > +	SIMPLE_SYSCLK_ORDER_CODEC_FIRST,
> > +	SIMPLE_SYSCLK_ORDER_CPU_FIRST,
> > +};
> 
> I think UNSPEC is not needed ?
> 
> 	enum simple_util_sysclk_order {
> 		SIMPLE_SYSCLK_ORDER_CODEC_FIRST = 0,
> 		SIMPLE_SYSCLK_ORDER_CPU_FIRST,
> 	};
> 
> Do you set this "order" via custome driver if you want ot use CPU_FIRST ?

Or, not use flag, but check whether "clock master" ?

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Hi Kuninori,
> 
> > (snip)
> > > +enum simple_util_sysclk_order {
> > > +	SIMPLE_SYSCLK_ORDER_UNSPEC = 0,
> > > +	SIMPLE_SYSCLK_ORDER_CODEC_FIRST,
> > > +	SIMPLE_SYSCLK_ORDER_CPU_FIRST,
> > > +};
> > 
> > I think UNSPEC is not needed ?
> > 
> > 	enum simple_util_sysclk_order {
> > 		SIMPLE_SYSCLK_ORDER_CODEC_FIRST = 0,
> > 		SIMPLE_SYSCLK_ORDER_CPU_FIRST,
> > 	};
> > 
> > Do you set this "order" via custome driver if you want ot use CPU_FIRST ?
> 
> Or, not use flag, but check whether "clock master" ?
> 

thanks for the feedback!

Yes, my initial idea was to set CPU_FIRST from our downstream/custom
kernel (e.g. via a small local change) when needed.

Also thanks for the suggestion about checking the clock master.
I had a look at it, but unfortunately the information we can reliably
derive is about BCLK/FS (bitclock-master and frame-master).
These properties describe who provides the serial bus clocks, but they
do not imply anything about MCLK.

In my case, for example, the codec is both frame-master and bitclock-master,
yet I still need to call CPU DAI .set_sysclk() first because the CPU DAI may
finalize/adjust the actual MCLK rate (e.g. via clk_set_rate()) and the codec
may otherwise configure its internal clocking based on a non-final MCLK value.

So selecting the sysclk ordering based on BCLK/FS master would not
cover this case.

So, do you think the solution I proposed would be acceptable if I remove
SIMPLE_SYSCLK_ORDER_UNSPEC and keep only:

enum simple_util_sysclk_order {
        SIMPLE_SYSCLK_ORDER_CODEC_FIRST = 0,
        SIMPLE_SYSCLK_ORDER_CPU_FIRST,
};

with codec-first as the default behaviour?

Many thanks and best regards,
Stefano Radaelli

Hi Stefano

> Do you set this "order" via custome driver if you want ot use CPU_FIRST ?
(snip)
> Yes, my initial idea was to set CPU_FIRST from our downstream/custom
> kernel (e.g. via a small local change) when needed.

OK, nice to know.
I'm not sure how it helps you, but Audio-Graph-Card1/2 allows custom
driver. You can call audio_graph[2]_parse_of() from it.
These are sample code for you
	sound/soc/tegra/tegra_audio_graph_card.c		// for Audio-Graph-Card
	sound/soc/generic/audio-graph-card2-custom-sample.c	// for Audio-Graph-Card2

> enum simple_util_sysclk_order {
>         SIMPLE_SYSCLK_ORDER_CODEC_FIRST = 0,
>         SIMPLE_SYSCLK_ORDER_CPU_FIRST,
> };
> 
> with codec-first as the default behaviour?

Looks good

Thank you for your help !!

Best regards
---
Kuninori Morimoto