[PATCH v5] ASoC: wm8978: add missing BCLK divider setup

Brian Sune posted 1 patch 4 months ago
There is a newer version of this series
sound/soc/codecs/wm8978.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
[PATCH v5] ASoC: wm8978: add missing BCLK divider setup
Posted by Brian Sune 4 months ago
In previous WM8978 codec driver versions, wm8978_set_dai_clkdiv
might not have been called for BCLK, leaving the bit clock
divider unconfigured. This could cause incorrect or unstable audio
clocks depending on sample rate and word length.

This patch adds a check in wm8978_hw_params: if the BCLK divider
has not been set via wm8978_set_dai_clkdiv, it is dynamically
calculated and configured at runtime.
This ensures that BCLK is always correctly set, whether the
machine driver configures it explicitly or not.

Apart from this core patch, due to request from Mark Brown and
Charles Keepax. Overclock BCLK setup is applied, and dropped the
possible lowest error BCLK result. On top of the overclocking,
warning message is given to user as a reminding.
This patch author do not agree with this design nor
concept from first place!

Signed-off-by: Brian Sune <briansune@gmail.com>
---
 sound/soc/codecs/wm8978.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/sound/soc/codecs/wm8978.c b/sound/soc/codecs/wm8978.c
index 8c45ba6fc4c3..73ceca9782ae 100644
--- a/sound/soc/codecs/wm8978.c
+++ b/sound/soc/codecs/wm8978.c
@@ -99,6 +99,7 @@ struct wm8978_priv {
 	unsigned int f_mclk;
 	unsigned int f_256fs;
 	unsigned int f_opclk;
+	bool bclk_set;
 	int mclk_idx;
 	enum wm8978_sysclk_src sysclk;
 };
@@ -590,6 +591,7 @@ static int wm8978_set_dai_clkdiv(struct snd_soc_dai *codec_dai,
 	case WM8978_BCLKDIV:
 		if (div & ~0x1c)
 			return -EINVAL;
+		wm8978->bclk_set = true;
 		snd_soc_component_update_bits(component, WM8978_CLOCKING, 0x1c, div);
 		break;
 	default:
@@ -717,6 +719,11 @@ static int wm8978_hw_params(struct snd_pcm_substream *substream,
 			    struct snd_pcm_hw_params *params,
 			    struct snd_soc_dai *dai)
 {
+	unsigned int bclk, bclkdiv = 0, min_diff = UINT_MAX;
+	unsigned int target_bclk = params_rate(params) * params_width(params) * 2;
+	/* WM8978 supports divisors */
+	static const int bclk_divs[] = {1, 2, 4, 8, 16, 32};
+
 	struct snd_soc_component *component = dai->component;
 	struct wm8978_priv *wm8978 = snd_soc_component_get_drvdata(component);
 	/* Word length mask = 0x60 */
@@ -820,6 +827,32 @@ static int wm8978_hw_params(struct snd_pcm_substream *substream,
 	/* MCLK divisor mask = 0xe0 */
 	snd_soc_component_update_bits(component, WM8978_CLOCKING, 0xe0, best << 5);
 
+	if (!wm8978->bclk_set) {
+		for (i = 0; i < ARRAY_SIZE(bclk_divs); i++) {
+			bclk = wm8978->f_256fs / bclk_divs[i];
+
+			if (bclk < target_bclk) {
+				if (min_diff != 0)
+					dev_warn(component->dev,
+						 "Auto BCLK cannot fit, BCLK using: #%u\n",
+						 wm8978->f_256fs / bclk_divs[bclkdiv]);
+				break;
+			}
+
+			if (abs(bclk - target_bclk) < min_diff) {
+				min_diff = abs(bclk - target_bclk);
+				bclkdiv = i;
+			}
+		}
+
+		dev_dbg(component->dev, "%s: fs=%u width=%u -> target BCLK=%u, using div #%u\n",
+			__func__, params_rate(params), params_width(params), target_bclk,
+			bclk_divs[bclkdiv]);
+
+		/* BCLKDIV divisor mask = 0x1c */
+		snd_soc_component_update_bits(component, WM8978_CLOCKING, 0x1c, bclkdiv << 2);
+	}
+
 	snd_soc_component_write(component, WM8978_AUDIO_INTERFACE, iface_ctl);
 	snd_soc_component_write(component, WM8978_ADDITIONAL_CONTROL, add_ctl);
 
-- 
2.47.1.windows.1
Re: [PATCH v5] ASoC: wm8978: add missing BCLK divider setup
Posted by Charles Keepax 4 months ago
On Thu, Oct 09, 2025 at 12:27:19AM +0800, Brian Sune wrote:
> Apart from this core patch, due to request from Mark Brown and
> Charles Keepax. Overclock BCLK setup is applied, and dropped the
> possible lowest error BCLK result. On top of the overclocking,
> warning message is given to user as a reminding.
> This patch author do not agree with this design nor
> concept from first place!

Please read section 6.2 of the I2S specification[1], particularly
the last sentence before the note.

But if you are so concerned about the bclk being wrong, just
change your patch to only accept an exact bclk match? I am fine
with that, it's still an improvement on the current driver and
someone else can add the inexact matches later if they need them.

> +		for (i = 0; i < ARRAY_SIZE(bclk_divs); i++) {
> +			bclk = wm8978->f_256fs / bclk_divs[i];
> +
> +			if (bclk < target_bclk) {
> +				if (min_diff != 0)
> +					dev_warn(component->dev,
> +						 "Auto BCLK cannot fit, BCLK using: #%u\n",
> +						 wm8978->f_256fs / bclk_divs[bclkdiv]);
> +				break;
> +			}
> +
> +			if (abs(bclk - target_bclk) < min_diff) {
> +				min_diff = abs(bclk - target_bclk);
> +				bclkdiv = i;
> +			}

What you missing here is that your bclk_divs are sorted so all
this min_diff is wasted effort. Each time you move to a higher
divider there are only two options:

1) You are closer to the target bclk than last time.
2) You are under the target bclk and the system won't work.

So the last value that gives a bclk above the target is the
min_diff by definition.

Thanks,
Charles

[1] https://www.nxp.com/docs/en/user-manual/UM11732.pdf
Re: [PATCH v5] ASoC: wm8978: add missing BCLK divider setup
Posted by Sune Brian 4 months ago
Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月9日 週四 上午1:16寫道:

> [1] https://www.nxp.com/docs/en/user-manual/UM11732.pdf

At the end of the day. After refreshing myself a bit,
I want to cleanup what this document trying to proposed.
From beginning I am very clear on what I am understood.
Along the discussions I am been mislead  by this parties
as well.
What the standard about extra bit is ignore is not because

1: BCLK clock rate allow faster
2: BCLK is not relayed by MCLK and LRCLK

Explains

What IIS inherent design.
1: in order to form a 24 bit counter 6 flops is required.
As such then counting 32 / channel hence 64 bit.

2:To divide BCLK to form LRCLK then this 64 ticks are
ran. As such when a 24  bit data format is used, extra
8 bit is ignored as such division can form proper bounded S.R.

3: With this ignore bit method less die size power on VLSI.
Simplified 8,12,16,24,32 data format while LRCLK vs BCLK is
bounded.

4: As such, this is no the concept of overclocking
BCLK to fit all those how argue about BCLK is faster to etc.

So agree to disagree. I never accept the concept of
overclock on IIS. And this is not what bit is ignored intended!

Brian
Re: [PATCH v5] ASoC: wm8978: add missing BCLK divider setup
Posted by Sune Brian 4 months ago
Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月9日 週四 上午1:16寫道:

> What you missing here is that your bclk_divs are sorted so all
> this min_diff is wasted effort. Each time you move to a higher
> divider there are only two options:
>
> 1) You are closer to the target bclk than last time.
> 2) You are under the target bclk and the system won't work.
>
> So the last value that gives a bclk above the target is the
> min_diff by definition.
>
> Thanks,
> Charles
>
> [1] https://www.nxp.com/docs/en/user-manual/UM11732.pdf

Document had read and indeed it mentioned it will be ignored.
I apologized on my strong words about IIS standard about extra bit clocks.
However, it just mentioned if there are additional bit happens will be ignored.
It never said this is a way to relay between MCLK LRCLK and BCLK.
As such I am still don't convinced this is a proper way to reach the
targeted S.R.
And my stand of MCLK <-> BCLK <-> LRCLK relationship still holds strong.
No matter how you do it, it will only result in a close result but not
match result.
Doing faster BCLK but unnecessary just make setup/slack timing issues.
Any specifications on how much you can overrun from first place?
Which make zero sense and reason to do so from first place.
If BCLK is slow then the final S.R. is slowed this is what it is.
Same as BCLK is fast then S.R. is fasted as it is.

As for this patch do the current version introduce any error or bug.
If not then I will stop the patching.
Leave it to other to patch the approximated BCLK.

Brian
Re: [PATCH v5] ASoC: wm8978: add missing BCLK divider setup
Posted by Charles Keepax 4 months ago
On Thu, Oct 09, 2025 at 02:27:19AM +0800, Sune Brian wrote:
> Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月9日 週四 上午1:16寫道:
> Document had read and indeed it mentioned it will be ignored.
> I apologized on my strong words about IIS standard about extra bit clocks.
> However, it just mentioned if there are additional bit happens will be ignored.
> It never said this is a way to relay between MCLK LRCLK and BCLK.
> As such I am still don't convinced this is a proper way to reach the
> targeted S.R.
> And my stand of MCLK <-> BCLK <-> LRCLK relationship still holds strong.

Ah, I see what you are getting at. Yes if the LRCLK is generated
from the BCLK then the LRCLK would stretch/squash to always
provide enough bandwidth.

However, I believe most of the Wolfson parts from this era
generate the LRCLK from the internal SYSCLK directly. By the
looks of the datasheet on this part there is a fixed 256 divide
from SYSCLK to the LRCLK.

That said though, this part does somewhat pre-date me so I could
be wrong. If you have hardware and an oscilloscope this should
be pretty simple to verify in practice?

> No matter how you do it, it will only result in a close result but not
> match result.
> Doing faster BCLK but unnecessary just make setup/slack timing issues.
> Any specifications on how much you can overrun from first place?
> Which make zero sense and reason to do so from first place.

There are generally two reasons to run a faster than needed BCLK:

1) Because you can't generate the actual BCLK.
2) Because you are TDMing multiple devices onto the bus. ie.
   something else is using those extra bits.

> If BCLK is slow then the final S.R. is slowed this is what it is.
> Same as BCLK is fast then S.R. is fasted as it is.
> As for this patch do the current version introduce any error or bug.
> If not then I will stop the patching.
> Leave it to other to patch the approximated BCLK.

That is absolutely your choice. The patch should I think work
now, but could still use a little tidy up to remove the unneeded
code and big warning for something that isn't an issue.

Thanks,
Charles
Re: [PATCH v5] ASoC: wm8978: add missing BCLK divider setup
Posted by Sune Brian 4 months ago
Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月9日 週四 上午4:18寫道:

> 1) Because you can't generate the actual BCLK.

If you can't generate actual BCLK can you generate actual LRCLK
on this codec through the same MCLK?
Re: [PATCH v5] ASoC: wm8978: add missing BCLK divider setup
Posted by Sune Brian 4 months ago
Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月9日 週四 上午4:18寫道:

> That said though, this part does somewhat pre-date me so I could
> be wrong. If you have hardware and an oscilloscope this should
> be pretty simple to verify in practice?

Have gear but not worth to waste time on this test.
Well remind this part is EOL far way ago.


> There are generally two reasons to run a faster than needed BCLK:
>
> 1) Because you can't generate the actual BCLK.

Then let it be as delta-BCLK that is minimum error is better same as
PPM concept.
There must be +ve and -ve errors on margin.

> 2) Because you are TDMing multiple devices onto the bus. ie.
>    something else is using those extra bits.

Is this device supported?
I am not too familiar on TDM things are series things on same IIS
sound too system deepened,
But of cause this make rooms for future improvement or support.

> That is absolutely your choice. The patch should I think work
> now, but could still use a little tidy up to remove the unneeded
> code and big warning for something that isn't an issue.

Tired and lazy to check out all these things again and again.
Every patch i run on actual HW before sent.

Brian
Re: [PATCH v5] ASoC: wm8978: add missing BCLK divider setup
Posted by Mark Brown 4 months ago
On Wed, Oct 08, 2025 at 09:18:18PM +0100, Charles Keepax wrote:

> There are generally two reasons to run a faster than needed BCLK:

> 1) Because you can't generate the actual BCLK.
> 2) Because you are TDMing multiple devices onto the bus. ie.
>    something else is using those extra bits.

Another one I've seen is that you're using the BCLK as the MCLK for
another part (a few devices even require this), you might want to run
BCLK at 256fs or whatever for the MCLK even though it's not needed for
when used as BCLK.
Re: [PATCH v5] ASoC: wm8978: add missing BCLK divider setup
Posted by Sune Brian 4 months ago
Mark Brown <broonie@kernel.org> 於 2025年10月9日 週四 上午4:26寫道:

> Another one I've seen is that you're using the BCLK as the MCLK for
> another part (a few devices even require this), you might want to run
> BCLK at 256fs or whatever for the MCLK even though it's not needed for
> when used as BCLK.

If you need BCLK as MCLK same clock rate why you need to use BCLK from
first place?
Re: [PATCH v5] ASoC: wm8978: add missing BCLK divider setup
Posted by Mark Brown 4 months ago
On Thu, Oct 09, 2025 at 05:27:58AM +0800, Sune Brian wrote:
> Mark Brown <broonie@kernel.org> 於 2025年10月9日 週四 上午4:26寫道:

> > Another one I've seen is that you're using the BCLK as the MCLK for
> > another part (a few devices even require this), you might want to run
> > BCLK at 256fs or whatever for the MCLK even though it's not needed for
> > when used as BCLK.

> If you need BCLK as MCLK same clock rate why you need to use BCLK from
> first place?

This is often partly a pinmuxing/routing question - if the CODEC is the
clock provider (and perhaps you're using a CODEC PLL so the clock isn't
visible without being output by the CODEC) then using a fast BCLK can
either be needed due to a lack of other places to output the MCLK or
just seem convenient to the board designer due to where the available
pins are.
Re: [PATCH v5] ASoC: wm8978: add missing BCLK divider setup
Posted by Sune Brian 4 months ago
Mark Brown <broonie@kernel.org> 於 2025年10月9日 週四 上午5:44寫道:

> This is often partly a pinmuxing/routing question - if the CODEC is the
> clock provider (and perhaps you're using a CODEC PLL so the clock isn't
> visible without being output by the CODEC) then using a fast BCLK can
> either be needed due to a lack of other places to output the MCLK or
> just seem convenient to the board designer due to where the available
> pins are.

Then why this codec need to support this uncongenial design from first place?
Are this is general kernel driver trunk?
As such, this is just trying to argue and make
BCLK LRCLK noncorrelated reasonable.

If a HW designer choice to use a codec as clock generator rather than audio
bases. Why this driver got to support on that.
As such all these discussion just want to argue and make thing reasonable
w/o practical application ground.

From all these discussions only this specific codec I cannot see reasonable
application field that you run lrclk on audio S.R. but bclk on same as mclk.
While using such configuration still generate auditable sound?
Re: [PATCH v5] ASoC: wm8978: add missing BCLK divider setup
Posted by Sune Brian 4 months ago
Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月9日 週四 上午1:16寫道:

> [1] https://www.nxp.com/docs/en/user-manual/UM11732.pdf

I am curious. With this codec WM8978.
Can you set a LRCLK rate that is not relied on MCLK+BCLK ratio from
first place when codec is in master mode.
Can you request a sample rate that requires the concept of extra bit
clock arbitrary higher in one extra order?
Very curious on such idea.

If possible maybe some examples can help.