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

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

[Brian Sune]

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.

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

diff --git a/sound/soc/codecs/wm8978.c b/sound/soc/codecs/wm8978.c
index 8c45ba6fc4c3..8dfce6ede8cd 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,23 @@ 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 (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 v3] ASoC: wm8978: add missing BCLK divider setup

[Charles Keepax]

On Tue, Oct 07, 2025 at 10:50:28PM +0800, Brian Sune wrote:
> 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.
> 
> Signed-off-by: Brian Sune <briansune@gmail.com>
> ---
>  sound/soc/codecs/wm8978.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/sound/soc/codecs/wm8978.c b/sound/soc/codecs/wm8978.c
> index 8c45ba6fc4c3..8dfce6ede8cd 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,23 @@ 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) {

Yeah that looks good.

> +		for (i = 0; i < ARRAY_SIZE(bclk_divs); i++) {
> +			bclk = wm8978->f_256fs / bclk_divs[i];
> +			if (abs(bclk - target_bclk) < min_diff) {
> +				min_diff = abs(bclk - target_bclk);
> +				bclkdiv = i;
> +			}
> +		}

Apologies but just realised there is still one small problem here.
You want to match the closest BCLK that is over your target rate,
if the BCLK is too slow the system won't work. As your bclk_divs
array is sorted I think you can do something like:

	for (i = 0; i < ARRAY_SIZE(bclk_divs); i++) {
		bclk = wm8978->f_256fs / bclk_divs[i];

		if (bclk < target_bclk)
			break;

		bclkdiv = i;
	}

Thanks,
Charles

Re: [PATCH v3] ASoC: wm8978: add missing BCLK divider setup

[Sune Brian]

Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月7日 週二 下午11:21寫道：

> Apologies but just realised there is still one small problem here.
> You want to match the closest BCLK that is over your target rate,
> if the BCLK is too slow the system won't work. As your bclk_divs
> array is sorted I think you can do something like:
>
>         for (i = 0; i < ARRAY_SIZE(bclk_divs); i++) {
>                 bclk = wm8978->f_256fs / bclk_divs[i];
>
>                 if (bclk < target_bclk)
>                         break;
>
>                 bclkdiv = i;
>         }
>
> Thanks,
> Charles

Not too understand what is the issue.
The idea is setting the initial value to max any diff that is smaller
will update bclkdiv index,
If the maximum LUT 32 still not met then it is what it is.
mclk / 32 is the bclk, unless you want an error message.

Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月7日 週二 下午11:21寫道：
>
> On Tue, Oct 07, 2025 at 10:50:28PM +0800, Brian Sune wrote:
> > 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.
> >
> > Signed-off-by: Brian Sune <briansune@gmail.com>
> > ---
> >  sound/soc/codecs/wm8978.c | 24 ++++++++++++++++++++++++
> >  1 file changed, 24 insertions(+)
> >
> > diff --git a/sound/soc/codecs/wm8978.c b/sound/soc/codecs/wm8978.c
> > index 8c45ba6fc4c3..8dfce6ede8cd 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,23 @@ 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) {
>
> Yeah that looks good.
>
> > +             for (i = 0; i < ARRAY_SIZE(bclk_divs); i++) {
> > +                     bclk = wm8978->f_256fs / bclk_divs[i];
> > +                     if (abs(bclk - target_bclk) < min_diff) {
> > +                             min_diff = abs(bclk - target_bclk);
> > +                             bclkdiv = i;
> > +                     }
> > +             }
>
> Apologies but just realised there is still one small problem here.
> You want to match the closest BCLK that is over your target rate,
> if the BCLK is too slow the system won't work. As your bclk_divs
> array is sorted I think you can do something like:
>
>         for (i = 0; i < ARRAY_SIZE(bclk_divs); i++) {
>                 bclk = wm8978->f_256fs / bclk_divs[i];
>
>                 if (bclk < target_bclk)
>                         break;
>
>                 bclkdiv = i;
>         }
>
> Thanks,
> Charles

Re: [PATCH v3] ASoC: wm8978: add missing BCLK divider setup

[Mark Brown]

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

On Tue, Oct 07, 2025 at 11:55:29PM +0800, Sune Brian wrote:
> Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月7日 週二 下午11:21寫道：

> > Apologies but just realised there is still one small problem here.
> > You want to match the closest BCLK that is over your target rate,
> > if the BCLK is too slow the system won't work. As your bclk_divs
> > array is sorted I think you can do something like:

> Not too understand what is the issue.
> The idea is setting the initial value to max any diff that is smaller
> will update bclkdiv index,
> If the maximum LUT 32 still not met then it is what it is.
> mclk / 32 is the bclk, unless you want an error message.

Consider the case where the BCLK needed is 100 and SYSCLK is 198.
Dividing by 1 will result in an absolute difference of 98 and a BCLK of
198 while dividing by 2 will result in an absolute difference 1 and a
BCLK of 99 which is lower than the required BCLK.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3] ASoC: wm8978: add missing BCLK divider setup

[Sune Brian]

Mark Brown <broonie@kernel.org> 於 2025年10月8日 週三 下午8:12寫道：

> Consider the case where the BCLK needed is 100 and SYSCLK is 198.
> Dividing by 1 will result in an absolute difference of 98 and a BCLK of
> 198 while dividing by 2 will result in an absolute difference 1 and a
> BCLK of 99 which is lower than the required BCLK.

for (i = 0; i < ARRAY_SIZE(bclk_divs); i++) {
bclk = wm8978->f_256fs / bclk_divs[i];
if (abs(bclk - target_bclk) < min_diff) {
min_diff = abs(bclk - target_bclk);
bclkdiv = i;
}
}

Cycle 0: min_diff=0xffff_ffff
bclk = 198/bclk_divs[0] = 198/1=198
bclk-target_bclk = 198-100 = 98
98 < 0xffff_ffff
min_diff = 98
bclkdiv=0

Cycle 1: min_diff = 98
bclk = 198/bclk_divs[1] = 198/2=99
bclk-target_bclk = 99-100 = 1
1 < 98
min_diff = 1
bclk_div=1

Cycle 2: min_diff = 1
bclk = 198/bclk_divs[2] = 198/4=49
bclk-target_bclk = 49-1 = 48
48 < 1 skip

loop done

Final bclk_div=1 LUT=/2 198/2 ~= 100

what it the issue here or I messed up?

Re: [PATCH v3] ASoC: wm8978: add missing BCLK divider setup

[Mark Brown]

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

On Wed, Oct 08, 2025 at 09:26:16PM +0800, Sune Brian wrote:
> Mark Brown <broonie@kernel.org> 於 2025年10月8日 週三 下午8:12寫道：

> > Consider the case where the BCLK needed is 100 and SYSCLK is 198.
> > Dividing by 1 will result in an absolute difference of 98 and a BCLK of
> > 198 while dividing by 2 will result in an absolute difference 1 and a
> > BCLK of 99 which is lower than the required BCLK.
> 
> for (i = 0; i < ARRAY_SIZE(bclk_divs); i++) {
> bclk = wm8978->f_256fs / bclk_divs[i];
> if (abs(bclk - target_bclk) < min_diff) {
> min_diff = abs(bclk - target_bclk);
> bclkdiv = i;
> }
> }

Your mail client has made this extremely hard to read.

> Cycle 0: min_diff=0xffff_ffff
> bclk = 198/bclk_divs[0] = 198/1=198
> bclk-target_bclk = 198-100 = 98
> 98 < 0xffff_ffff
> min_diff = 98
> bclkdiv=0

This is OK, BCLK is 198.

> Cycle 1: min_diff = 98
> bclk = 198/bclk_divs[1] = 198/2=99
> bclk-target_bclk = 99-100 = 1
> 1 < 98
> min_diff = 1
> bclk_div=1

This is not OK, BCLK is 99 which is less than 100 so there are not
enough BCLK cycles to clock the samples.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3] ASoC: wm8978: add missing BCLK divider setup

[Sune Brian]

Mark Brown <broonie@kernel.org> 於 2025年10月8日 週三 下午9:40寫道：

> This is not OK, BCLK is 99 which is less than 100 so there are not
> enough BCLK cycles to clock the samples.

With all do respect.
This is not how IIS, left/right-just works my friend.
LRCLK and BCLK must follows.
This is no pure sample rate concept.
I hate to explain my self.
If this patch is not good I just give up.

Re: [PATCH v3] ASoC: wm8978: add missing BCLK divider setup

[Mark Brown]

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

On Wed, Oct 08, 2025 at 09:46:07PM +0800, Sune Brian wrote:
> Mark Brown <broonie@kernel.org> 於 2025年10月8日 週三 下午9:40寫道：

> > This is not OK, BCLK is 99 which is less than 100 so there are not
> > enough BCLK cycles to clock the samples.

> With all do respect.
> This is not how IIS, left/right-just works my friend.
> LRCLK and BCLK must follows.
> This is no pure sample rate concept.
> I hate to explain my self.
> If this patch is not good I just give up.

Many devices (including all the Wolfson ones of that era IIRC) will
happily just ignore extra cycles on BCLK, the issue here is handling of
a f_256fs which is a bit off what it should be for some reason.  You're
assuming that the device is clocked at an exact and suitable multiple of
the sample rate like it's supposed to be but in practice these devices
work well enough for the system's purposes when the clocking is merely
close, they tend not to be particularly fragile and users perhaps not so
deeply concerned with audio fidelity.  Note the warning the driver will
generate about considering using the PLL to fix up such misclocking.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3] ASoC: wm8978: add missing BCLK divider setup

[Sune Brian]

Mark Brown <broonie@kernel.org> 於 2025年10月8日 週三 下午10:20寫道：

> Many devices (including all the Wolfson ones of that era IIRC) will
> happily just ignore extra cycles on BCLK, the issue here is handling of
> a f_256fs which is a bit off what it should be for some reason.  You're
> assuming that the device is clocked at an exact and suitable multiple of
> the sample rate like it's supposed to be but in practice these devices
> work well enough for the system's purposes when the clocking is merely
> close, they tend not to be particularly fragile and users perhaps not so
> deeply concerned with audio fidelity.  Note the warning the driver will
> generate about considering using the PLL to fix up such misclocking.

If this concept holds.
Are you telling me a operatable case audio can output just quicker or
slower on such w/o issue?
Are you telling me the left/right channel bits  are able to load
correctly on fixed lrclk while bclk is overclock?
If so why this patch required from first place?
Running bclk quick and just feed in the left/right channel data on
1bit delay on both channel should works on IIS.
Just make bclk /2 or /4 from mclk always works while LRCLK just fixed
to 44.1k 48k or any audio SR?
Datasheet WM8978: Figure 38 I2 S Audio Interface (assuming n-bit word length)

Why we got to argue this things?

What merely close is based on bclk and lrclk is correlated properly?
For example bclk is fasted then bclk/bit_size/channel_num = lrclk
faster then this is still holds.
Do LRCLK is fixed but BCLK is far faster still runs properly?

I think if the patch is not good enough just drop this idea.
I cannot understand what operation scope you are expecting.

Brian

Re: [PATCH v3] ASoC: wm8978: add missing BCLK divider setup

[Mark Brown]

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

On Wed, Oct 08, 2025 at 10:37:03PM +0800, Sune Brian wrote:

> If this concept holds.
> Are you telling me a operatable case audio can output just quicker or
> slower on such w/o issue?

Yeah, it depends a lot on the application - if you're playing music your
quality requirements are probably going to be a lot higher than if for
example you are playing warning tones.  The warning tones probably just
need to be consistent, accuracy is more of a nice to have.

> Are you telling me the left/right channel bits  are able to load
> correctly on fixed lrclk while bclk is overclock?

Yes, for a lot of devices - the most common case where extra clocks are
an issue is when a device is faking I2S support and only paying
attention to the leading edge of the LRCLK (ie, DSP mode).  In that case
extra dead BCLK cycles between the left and right channels will break.

> If so why this patch required from first place?

Aside from the interoperability issues there's also likely to be some
very marginal power gain, and slower buses tend to be more electically
robust.

> Running bclk quick and just feed in the left/right channel data on
> 1bit delay on both channel should works on IIS.
> Just make bclk /2 or /4 from mclk always works while LRCLK just fixed
> to 44.1k 48k or any audio SR?
> Datasheet WM8978: Figure 38 I2 S Audio Interface (assuming n-bit word length)

The most common case where it might matter is the above one with fake
I2S implementations (usually switching to DSP mode works around it fine,
but some devices don't do DSP mode).

> Do LRCLK is fixed but BCLK is far faster still runs properly?

Usually, yes, so long as the BCLK isn't run so fast that it's exceeding
limits the device has or otherwise causing electrical issues.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3] ASoC: wm8978: add missing BCLK divider setup

[Sune Brian]

Mark Brown <broonie@kernel.org> 於 2025年10月8日 週三 下午9:40寫道：

> This is not OK, BCLK is 99 which is less than 100 so there are not
> enough BCLK cycles to clock the samples.

What you are questioning won't even fits under normal operation scope
of this codec IC.
This is simply arguing and I really hate these type of conversions.
This is a patch for specific codec IC architecture.
Such automated bclk seeker just runs on expected operatable scope of this codec.
And such patch is to fix missing proper bclk register load from first place.
If there are cases that is this codec is allow but could generate
wrong result on this patch.
Willing to update this patch and learn from my errors.

Any one could do what you are expecting and could runs on this codec.
Happy to learn as much as I can.

Brian