[PATCH] ASoC: stm32: sai: manage context in set_sysclk callback

[PATCH] ASoC: stm32: sai: manage context in set_sysclk callback

[Olivier Moysan]

The mclk direction now needs to be specified in endpoint node with
"system-clock-direction-out" property. However some calls to the
set_sysclk callback, related to CPU DAI clock, result in unbalanced
calls to clock API.
The set_sysclk callback in STM32 SAI driver is intended only for mclk
management. So it is relevant to ensure that calls to set_sysclk are
related to mclk only.
Since the master clock is handled only at runtime, skip the calls to
set_sysclk in the initialization phase.

Signed-off-by: Olivier Moysan <olivier.moysan@foss.st.com>

---

Here is feedback regarding commit 5725bce709db1c001140d79398581e067e28c031
ASoC: simple-card-utils: Unify clock direction by clk_direction

I have observed some impacts on the STM32 SAI driver behavior.

To accommodate the change introduced by this commit, I added the property
"system-clock-direction-out" in the SAI device tree node.
The SAI nodes typically look as follows:

&sai2 {
	sai2a: audio-controller@4400b004 {
		#clock-cells = <0>;
		clocks = <&rcc SAI2_K>;
		clock-names = "sai_ck";
		status = "okay";

		sai2a_port: port {
			sai2a_endpoint: endpoint {
				mclk-fs = <256>;
				system-clock-direction-out;
			};
		};
	};
};
However, I noticed a change in the driver behavior:

* Before the change:
- Initialization:
simple_init_dai() -> set_sysclk(id=0, freq=sai_ck freq, dir=out)
	Calls clk_set_rate_exclusive()
simple_util_shutdown() -> set_sysclk(id=0, freq=0, dir=out)
	Calls clk_rate_exclusive_put() (releases the mclk clock)

Comments:
At initialization, the mclk rate is set with the kernel clock frequency.

- Runtime:
simple_util_hw_params() -> set_sysclk(id=0, freq=mclk freq, dir=out)

Comments:
At runtime, the mclk rate is set with the mclk clock frequency.

* After the change:
- Initialization:
simple_init_dai() -> set_sysclk(id=0, freq=sai_ck freq, dir=out)
	Calls clk_set_rate_exclusive()
simple_util_shutdown() -> set_sysclk(id=0, freq=0, dir=in)
	clk_rate_exclusive_put() NOT called (mclk clock is not released)

Comments:
The set_sysclk() is called with input direction, resulting in unbalanced
calls. This seems to be an unexpected behavior.

- Runtime:
simple_util_hw_params() -> set_sysclk(id=0, freq=mclk freq, dir=out)

This incorrect behavior made me realize that set_sysclk should not be
called when the frequency corresponds to the kernel clock frequency.
Here, the SAI driver needs a way to discriminate between the kernel clock
and the master clock.

I identified the following possibilities to achieve this:

- Check execution context:
Assuming the requested frequency is correct for mclk at runtime,
the execution context may be used as a discriminator.
The snd_soc_dai_stream_active() or snd_soc_card_is_instantiated() functions
can help determine the execution context.
For example, the following check can be added in STM32 SAI set_sysclk:

if (!snd_soc_card_is_instantiated(cpu_dai->component->card))
    return 0;

This approach fixes the issue but looks more like a workaround.

- Check clock direction:
I expected the kernel clock to remain tagged as an input clock.
I am not sure what the correct behavior is yet.
This may have been a way to differentiate clocks in this particular case,
but I don't think it is a robust method anyway.

- Check clk_id:
This seems the most relevant way to identify clocks.
However, clk_id is still set to 0 in simple card set_sysclk calls,
So, it does not seem to be a valid option at this time.
---
 sound/soc/stm/stm32_sai_sub.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/sound/soc/stm/stm32_sai_sub.c b/sound/soc/stm/stm32_sai_sub.c
index 463a2b7d023b..0ae1eae2a59e 100644
--- a/sound/soc/stm/stm32_sai_sub.c
+++ b/sound/soc/stm/stm32_sai_sub.c
@@ -672,6 +672,14 @@ static int stm32_sai_set_sysclk(struct snd_soc_dai *cpu_dai,
 	struct stm32_sai_sub_data *sai = snd_soc_dai_get_drvdata(cpu_dai);
 	int ret;
 
+	/*
+	 * The mclk rate is determined at runtime from the audio stream rate.
+	 * Skip calls to the set_sysclk callback that are not relevant during the
+	 * initialization phase.
+	 */
+	if (!snd_soc_card_is_instantiated(cpu_dai->component->card))
+		return 0;
+
 	if (dir == SND_SOC_CLOCK_OUT && sai->sai_mclk) {
 		ret = stm32_sai_sub_reg_up(sai, STM_SAI_CR1_REGX,
 					   SAI_XCR1_NODIV,
-- 
2.25.1

Re: [PATCH] ASoC: stm32: sai: manage context in set_sysclk callback

[Kuninori Morimoto]

Hi Olivier

Thank you for your feedback

> Here is feedback regarding commit 5725bce709db1c001140d79398581e067e28c031
> ASoC: simple-card-utils: Unify clock direction by clk_direction
(snip)
> To accommodate the change introduced by this commit, I added the property
> "system-clock-direction-out" in the SAI device tree node.
(snip)
> * Before the change:
> - Initialization:
> simple_init_dai() -> set_sysclk(id=0, freq=sai_ck freq, dir=out)
> 	Calls clk_set_rate_exclusive()
> simple_util_shutdown() -> set_sysclk(id=0, freq=0, dir=out)
> 	Calls clk_rate_exclusive_put() (releases the mclk clock)

Here, about "Before the change". Does this "change" mean "before adding
system-clock-direction-out" or "before commit 5725bce709db1..." ?

> * After the change:
> - Initialization:
> simple_init_dai() -> set_sysclk(id=0, freq=sai_ck freq, dir=out)
> 	Calls clk_set_rate_exclusive()
> simple_util_shutdown() -> set_sysclk(id=0, freq=0, dir=in)
> 	clk_rate_exclusive_put() NOT called (mclk clock is not released)

Hmm...
If it was latest kernel, and if you added "system-clock-direction-out" in DT,
dir should be "out" in my understanding. dir=in means it doesn't have
"system-clock-direction-out".

And, dir will not be changed (out/in) in init / shutdown.
Are these same DAI ? Both "struct snd_soc_dai" and "struct simple_util_dai"
have "*name". Could you please double-check it ?

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH] ASoC: stm32: sai: manage context in set_sysclk callback

[Olivier MOYSAN]

Hi Kuninori,

On 9/17/25 02:32, Kuninori Morimoto wrote:
> 
> Hi Olivier
> 
> Thank you for your feedback
> 
>> Here is feedback regarding commit 5725bce709db1c001140d79398581e067e28c031
>> ASoC: simple-card-utils: Unify clock direction by clk_direction
> (snip)
>> To accommodate the change introduced by this commit, I added the property
>> "system-clock-direction-out" in the SAI device tree node.
> (snip)
>> * Before the change:
>> - Initialization:
>> simple_init_dai() -> set_sysclk(id=0, freq=sai_ck freq, dir=out)
>> 	Calls clk_set_rate_exclusive()
>> simple_util_shutdown() -> set_sysclk(id=0, freq=0, dir=out)
>> 	Calls clk_rate_exclusive_put() (releases the mclk clock)
> 
> Here, about "Before the change". Does this "change" mean "before adding
> system-clock-direction-out" or "before commit 5725bce709db1..." ?
> 

Oh yes, I mean both in fact. Namely, before having the commit and the 
system-clock-direction-out property. Typically, I tested on a v6.10 and 
on a v6.17 (or later) where I noticed a change in the SAI driver behavior.

>> * After the change:
>> - Initialization:
>> simple_init_dai() -> set_sysclk(id=0, freq=sai_ck freq, dir=out)
>> 	Calls clk_set_rate_exclusive()
>> simple_util_shutdown() -> set_sysclk(id=0, freq=0, dir=in)
>> 	clk_rate_exclusive_put() NOT called (mclk clock is not released)
> 
> Hmm...
> If it was latest kernel, and if you added "system-clock-direction-out" in DT,
> dir should be "out" in my understanding. dir=in means it doesn't have
> "system-clock-direction-out".
> 
> And, dir will not be changed (out/in) in init / shutdown.
> Are these same DAI ? Both "struct snd_soc_dai" and "struct simple_util_dai"
> have "*name". Could you please double-check it ?
> 

I did supplementary checks, keeping only one DAI and adding additional 
traces. It seems I reached a wrong conclusion regarding the cause of 
unbalanced calls. Sorry for the confusion.
Looking at the traces (see below), initially, we had one call with the 
'in' direction (kernel clock frequency) and one call with the 'out' 
direction.
Now, both calls have the 'out' direction when the property 
'system-clock-direction-out' is set. This seems more consistent with the 
changes from commit 5725bce709db1.
In my setup, the kernel clock is an input clock. Maybe it should be 
tagged as an input clock. However, we have no 
'system-clock-direction-in' property yet.
Anyway, the patch in the STM32 SAI driver is still valid.

v6.10

simple_init_dai - dai: 0x110b6418, dai name: cs42l51-hifi,
simple_init_dai - dai: 0x6204899f, dai name: 4400b004.audio-controller,
stm32_sai_set_sysclk - dai: 0x6204899f, dai name: 
4400b004.audio-controller, id: 0, freq: 29700000, dir: 0
stm32_sai_set_sysclk - dai: 0x6204899f, dai name: 
4400b004.audio-controller, id: 0, freq: 12288000, dir: 1
stm32_sai_set_sysclk - dai: 0x6204899f, [set_rate] current freq: 0, 
request freq: 12288000
simple_util_shutdown - dai: 0x6204899f, dai name: 
4400b004.audio-controller, dir: 0
stm32_sai_set_sysclk - dai: 0x6204899f, dai name: 
4400b004.audio-controller, id: 0, freq: 0, dir: 1
stm32_sai_set_sysclk - dai: 0x6204899f, [put_rate]

v6.17

simple_init_dai - dai: 0xbca889eb, dai name: cs42l51-hifi,
simple_init_dai - dai: 0x4c0382ed, dai name: 4400b004.audio-controller,
stm32_sai_set_sysclk - dai: 0x4c0382ed, dai name: 
4400b004.audio-controller, id: 0, freq: 29700000, dir: 1
stm32_sai_set_sysclk - dai: 0x4c0382ed, [set_rate] current freq: 0, 
request freq: 29700000
stm32_sai_set_sysclk - dai: 0x4c0382ed, dai name: 
4400b004.audio-controller, id: 0, freq: 12288000, dir: 1
stm32_sai_set_sysclk - dai: 0x4c0382ed, [set_rate] current freq: 
24573875, request freq: 12288000
stm32_sai_set_sysclk - dai: 0x4c0382ed, dai name: 
4400b004.audio-controller, id: 0, freq: 12288000, dir: 1
stm32_sai_set_sysclk - dai: 0x4c0382ed, [set_rate] current freq: 
24573875, request freq: 12288000
stm32_sai_set_sysclk - dai: 0x4c0382ed, dai name: 
4400b004.audio-controller, id: 0, freq: 12288000, dir: 1
stm32_sai_set_sysclk - dai: 0x4c0382ed, [set_rate] current freq: 
24573875, request freq: 12288000
stm32_sai_set_sysclk - dai: 0x4c0382ed, dai name: 
4400b004.audio-controller, id: 0, freq: 12288000, dir: 1
stm32_sai_set_sysclk - dai: 0x4c0382ed, [set_rate] current freq: 
24573875, request freq: 12288000
simple_util_shutdown - dai: 0x4c0382ed, dai name: 
4400b004.audio-controller, dir: 1
stm32_sai_set_sysclk - dai: 0x4c0382ed, dai name: 
4400b004.audio-controller, id: 0, freq: 0, dir: 1
stm32_sai_set_sysclk - dai: 0x4c0382ed, [put_rate]

I hope this can be helpful.

Thanks and Best regards
Olivier

> Thank you for your help !!
> 
> Best regards
> ---
> Kuninori Morimoto

Re: [PATCH] ASoC: stm32: sai: manage context in set_sysclk callback

[Kuninori Morimoto]

Hi Olivier

Thank you for clarify the situation

> Looking at the traces (see below), initially, we had one call with the 
> 'in' direction (kernel clock frequency) and one call with the 'out' 
> direction.
(snip)
> Now, both calls have the 'out' direction when the property 
> 'system-clock-direction-out' is set. This seems more consistent with the 
> changes from commit 5725bce709db1.

Ah.. simple-card-utils calls snd_soc_dai_set_sysclk() from 3 functions

	- simple_init_dai()
	- simple_util_hw_params()
	- simple_util_shutdown()

but its "direction" is...

Step0:
	Before
	a728f56094e7cf60a1dc0642fe86901d1a4dfb4e
	 ("ASoC: make clock direction configurable in asoc-simple")

	- simple_init_dai()		fixed: 0
	- simple_util_hw_params()	fixed: SND_SOC_CLOCK_xx
	- simple_util_shutdown()	fixed: SND_SOC_CLOCK_xx

Step1:
	After
	a728f56094e7cf60a1dc0642fe86901d1a4dfb4e
	 ("ASoC: make clock direction configurable in asoc-simple")

	and before

	5725bce709db1c001140d79398581e067e28c031
	("ASoC: simple-card-utils: Unify clock direction by clk_direction")


	- simple_init_dai()		dai->clk_direction
	- simple_util_hw_params()	fixed: SND_SOC_CLOCK_xx
	- simple_util_shutdown()	fixed: SND_SOC_CLOCK_xx

Step2:
	After
	5725bce709db1c001140d79398581e067e28c031
	("ASoC: simple-card-utils: Unify clock direction by clk_direction")

	- simple_init_dai()		dai->clk_direction
	- simple_util_hw_params()	dai->clk_direction
	- simple_util_shutdown()	dai->clk_direction

In Step0 and Step1, your dirver is called with both dir IN/OUT ?

> In my setup, the kernel clock is an input clock. Maybe it should be 
> tagged as an input clock. However, we have no 
> 'system-clock-direction-in' property yet.
> Anyway, the patch in the STM32 SAI driver is still valid.

OK

Or maybe reset simple_dai->sysclk somehow (not sure...) ?
(simple_init_dai() will call snd_soc_dai_set_sysclk() if it has
simple_dai->sysclk)

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH] ASoC: stm32: sai: manage context in set_sysclk callback

[Olivier MOYSAN]

Hi Kuninori,

On 9/18/25 03:34, Kuninori Morimoto wrote:
> 
> Hi Olivier
> 
> Thank you for clarify the situation
> 
>> Looking at the traces (see below), initially, we had one call with the
>> 'in' direction (kernel clock frequency) and one call with the 'out'
>> direction.
> (snip)
>> Now, both calls have the 'out' direction when the property
>> 'system-clock-direction-out' is set. This seems more consistent with the
>> changes from commit 5725bce709db1.
> 
> Ah.. simple-card-utils calls snd_soc_dai_set_sysclk() from 3 functions
> 
> 	- simple_init_dai()
> 	- simple_util_hw_params()
> 	- simple_util_shutdown()
> 
> but its "direction" is...
> 
> Step0:
> 	Before
> 	a728f56094e7cf60a1dc0642fe86901d1a4dfb4e
> 	 ("ASoC: make clock direction configurable in asoc-simple")
> 
> 	- simple_init_dai()		fixed: 0
> 	- simple_util_hw_params()	fixed: SND_SOC_CLOCK_xx
> 	- simple_util_shutdown()	fixed: SND_SOC_CLOCK_xx
> 
> Step1:
> 	After
> 	a728f56094e7cf60a1dc0642fe86901d1a4dfb4e
> 	 ("ASoC: make clock direction configurable in asoc-simple")
> 
> 	and before
> 
> 	5725bce709db1c001140d79398581e067e28c031
> 	("ASoC: simple-card-utils: Unify clock direction by clk_direction")
> 
> 
> 	- simple_init_dai()		dai->clk_direction
> 	- simple_util_hw_params()	fixed: SND_SOC_CLOCK_xx
> 	- simple_util_shutdown()	fixed: SND_SOC_CLOCK_xx
> 
> Step2:
> 	After
> 	5725bce709db1c001140d79398581e067e28c031
> 	("ASoC: simple-card-utils: Unify clock direction by clk_direction")
> 
> 	- simple_init_dai()		dai->clk_direction
> 	- simple_util_hw_params()	dai->clk_direction
> 	- simple_util_shutdown()	dai->clk_direction
> 
> In Step0 and Step1, your dirver is called with both dir IN/OUT ?
> 

Yes, based on the trace, this appears to be the case.

>> In my setup, the kernel clock is an input clock. Maybe it should be
>> tagged as an input clock. However, we have no
>> 'system-clock-direction-in' property yet.
>> Anyway, the patch in the STM32 SAI driver is still valid.
> 
> OK
> 
> Or maybe reset simple_dai->sysclk somehow (not sure...) ?
> (simple_init_dai() will call snd_soc_dai_set_sysclk() if it has
> simple_dai->sysclk)
> 

Yes, resetting simple_dai->sysclk may also be an option, but I could not 
find a way to do it in practice. Requesting a reset of 
simple_dai->sysclk from the CPU DAI .probe callback might be the right 
time I think, but there is no service that allows this.
Another way would be to set "system-clock-frequency = <0>" in the CPU DT 
node. However, this is not possible, as it conflicts with the following 
check in the simple_set_clk_rate() function:
if (simple_dai->clk_fixed && rate != simple_dai->sysclk) {
    dev_err(...)
}

So, the current patch still seems like the better option to me.

Best regards
Olivier

> Thank you for your help !!
> 
> Best regards
> ---
> Kuninori Morimoto

Re: [PATCH] ASoC: stm32: sai: manage context in set_sysclk callback

[Kuninori Morimoto]

Hi Olivier

> > Or maybe reset simple_dai->sysclk somehow (not sure...) ?
> > (simple_init_dai() will call snd_soc_dai_set_sysclk() if it has
> > simple_dai->sysclk)
> > 
> 
> Yes, resetting simple_dai->sysclk may also be an option, but I could not 
> find a way to do it in practice. Requesting a reset of 
> simple_dai->sysclk from the CPU DAI .probe callback might be the right 
> time I think, but there is no service that allows this.
> Another way would be to set "system-clock-frequency = <0>" in the CPU DT 
> node. However, this is not possible, as it conflicts with the following 
> check in the simple_set_clk_rate() function:
> if (simple_dai->clk_fixed && rate != simple_dai->sysclk) {
>     dev_err(...)
> }

OK, thank you for confirming the situation.

Generalize closk setting is one of complex code, and some driver might get
damage if framework was updated, like you.

> So, the current patch still seems like the better option to me.

Yeah, it seems.


Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH] ASoC: stm32: sai: manage context in set_sysclk callback

[Mark Brown]

On Tue, 16 Sep 2025 14:31:18 +0200, Olivier Moysan wrote:
> The mclk direction now needs to be specified in endpoint node with
> "system-clock-direction-out" property. However some calls to the
> set_sysclk callback, related to CPU DAI clock, result in unbalanced
> calls to clock API.
> The set_sysclk callback in STM32 SAI driver is intended only for mclk
> management. So it is relevant to ensure that calls to set_sysclk are
> related to mclk only.
> Since the master clock is handled only at runtime, skip the calls to
> set_sysclk in the initialization phase.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/1] ASoC: stm32: sai: manage context in set_sysclk callback
      commit: 27fa1a8b2803dfd88c39f03b0969c55f667cdc43

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark