[PATCH] sound/soc/codecs/wm8978: add missing BCLK divider setup

[PATCH] sound/soc/codecs/wm8978: add missing BCLK divider setup

[Brian Sune]

The original WM8978 codec driver did not set the BCLK (bit clock)
divider, which can cause audio clocks to be incorrect or unstable
depending on the sample rate and word length.

This patch adds proper calculation and configuration of the BCLK
divider based on the sample rate and word width, ensuring the
codec generates the correct bit clock for all supported rates.

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

diff --git a/sound/soc/codecs/wm8978.c b/sound/soc/codecs/wm8978.c
index 8c45ba6fc4c3..2109c84f33df 100644
--- a/sound/soc/codecs/wm8978.c
+++ b/sound/soc/codecs/wm8978.c
@@ -717,6 +717,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 +825,21 @@ 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);
 
+	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] sound/soc/codecs/wm8978: add missing BCLK divider setup

[Charles Keepax]

On Fri, Oct 03, 2025 at 05:13:04PM +0800, Brian Sune wrote:

Patch title should be tweaked to match the other patches to this
driver:

ASoC: wm8978: ...

> The original WM8978 codec driver did not set the BCLK (bit clock)
> divider, which can cause audio clocks to be incorrect or unstable
> depending on the sample rate and word length.

This isn't totally accurate, the driver expects it will be set
through the set_clkdiv callback. Which one could probably argue
is a bit of a more legacy approach, but probably worth calling
that out here.

> This patch adds proper calculation and configuration of the BCLK
> divider based on the sample rate and word width, ensuring the
> codec generates the correct bit clock for all supported rates.
> 
> Signed-off-by: Brian Sune <briansune@gmail.com>
> ---
>  sound/soc/codecs/wm8978.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/sound/soc/codecs/wm8978.c b/sound/soc/codecs/wm8978.c
> index 8c45ba6fc4c3..2109c84f33df 100644
> --- a/sound/soc/codecs/wm8978.c
> +++ b/sound/soc/codecs/wm8978.c
> @@ -717,6 +717,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;

Probably better to use snd_soc_params_to_bclk or similar helper.

> +	/* 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 +825,21 @@ 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);
>  
> +	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);

We should probably add something to handle the interaction with
set_clkdiv and skip this if a fixed divider has been set. Well
either that or remove the set_clkdiv option, although that is a
little braver.

Thanks,
Charles

Re: [PATCH] sound/soc/codecs/wm8978: add missing BCLK divider setup

[Sune Brian]

Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月7日 週二 下午6:30寫道：
>
> On Fri, Oct 03, 2025 at 05:13:04PM +0800, Brian Sune wrote:
>
> Patch title should be tweaked to match the other patches to this
> driver:
>
> ASoC: wm8978: ...

Will submit update patch, thank you.

>
> > The original WM8978 codec driver did not set the BCLK (bit clock)
> > divider, which can cause audio clocks to be incorrect or unstable
> > depending on the sample rate and word length.
>
> This isn't totally accurate, the driver expects it will be set
> through the set_clkdiv callback. Which one could probably argue
> is a bit of a more legacy approach, but probably worth calling
> that out here.
>

According to actual hardware tests and the WM8978 driver study.
There are no bclk register setup in the entire driver. So I am not sure
How could this even set through the callback? The IC itself requires
2-wires register load and this can't be done via software level.

> > This patch adds proper calculation and configuration of the BCLK
> > divider based on the sample rate and word width, ensuring the
> > codec generates the correct bit clock for all supported rates.
> >
> > Signed-off-by: Brian Sune <briansune@gmail.com>
> > ---
> >  sound/soc/codecs/wm8978.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/sound/soc/codecs/wm8978.c b/sound/soc/codecs/wm8978.c
> > index 8c45ba6fc4c3..2109c84f33df 100644
> > --- a/sound/soc/codecs/wm8978.c
> > +++ b/sound/soc/codecs/wm8978.c
> > @@ -717,6 +717,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;
>
> Probably better to use snd_soc_params_to_bclk or similar helper.
>
> > +     /* 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 +825,21 @@ 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);
> >
> > +     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);
>
> We should probably add something to handle the interaction with
> set_clkdiv and skip this if a fixed divider has been set. Well
> either that or remove the set_clkdiv option, although that is a
> little braver.

Not too understand this idea but the current problem or bug is just
the bclk is missing.
Which had tested via this patch and provide good result. Meanwhile,
the mclk divider
is restricted to actual IC clock design so this is why it is a fixed table.

>
> Thanks,
> Charles

Re: [PATCH] sound/soc/codecs/wm8978: add missing BCLK divider setup

[Charles Keepax]

On Tue, Oct 07, 2025 at 07:22:10PM +0800, Sune Brian wrote:
> Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月7日 週二 下午6:30寫道：
> > On Fri, Oct 03, 2025 at 05:13:04PM +0800, Brian Sune wrote:
> > > The original WM8978 codec driver did not set the BCLK (bit clock)
> > > divider, which can cause audio clocks to be incorrect or unstable
> > > depending on the sample rate and word length.
> >
> > This isn't totally accurate, the driver expects it will be set
> > through the set_clkdiv callback. Which one could probably argue
> > is a bit of a more legacy approach, but probably worth calling
> > that out here.
> 
> According to actual hardware tests and the WM8978 driver study.
> There are no bclk register setup in the entire driver. So I am not sure
> How could this even set through the callback? The IC itself requires
> 2-wires register load and this can't be done via software level.

/*
 * Configure WM8978 clock dividers.
 */
static int wm8978_set_dai_clkdiv(struct snd_soc_dai *codec_dai,
				 int div_id, int div)
{
	struct snd_soc_component *component = codec_dai->component;
	struct wm8978_priv *wm8978 = snd_soc_component_get_drvdata(component);
	int ret = 0;

	switch (div_id) {
	case WM8978_OPCLKRATE:
		...
	case WM8978_BCLKDIV:
		if (div & ~0x1c)
			return -EINVAL;
		snd_soc_component_update_bits(component, WM8978_CLOCKING, 0x1c, div); <<---- HERE
		break;
	default:
		return -EINVAL;
	}

	dev_dbg(component->dev, "%s: ID %d, value %u\n", __func__, div_id, div);

	return ret;
}

Its not missing its right there. That said your way is probably
slightly more standard these days, but we should take care of the
interaction between the two.

Thanks,
Charles

Re: [PATCH] sound/soc/codecs/wm8978: add missing BCLK divider setup

[Mark Brown]

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

On Tue, Oct 07, 2025 at 01:11:24PM +0100, Charles Keepax wrote:

> Its not missing its right there. That said your way is probably
> slightly more standard these days, but we should take care of the
> interaction between the two.

Where we've had users using the manual divider configuration that we
cared about in the past we've suppressed automatic configuration if
manual configuration has ever been done by the machine driver.

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

Re: [PATCH] sound/soc/codecs/wm8978: add missing BCLK divider setup

[Charles Keepax]

On Tue, Oct 07, 2025 at 01:18:02PM +0100, Mark Brown wrote:
> On Tue, Oct 07, 2025 at 01:11:24PM +0100, Charles Keepax wrote:
> 
> > Its not missing its right there. That said your way is probably
> > slightly more standard these days, but we should take care of the
> > interaction between the two.
> 
> Where we've had users using the manual divider configuration that we
> cared about in the past we've suppressed automatic configuration if
> manual configuration has ever been done by the machine driver.

I don't see any current users for the manual divider configuration
at least in mainline, so we could probably just drop the manual
stuff. I don't think I have a strong opinion between that
and blocking the automatic configuration if it was previously
set manually, but we should do one of them to keep things clear.

Thanks,
Charles

Re: [PATCH] sound/soc/codecs/wm8978: add missing BCLK divider setup

[Mark Brown]

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

On Tue, Oct 07, 2025 at 01:26:52PM +0100, Charles Keepax wrote:

> I don't see any current users for the manual divider configuration
> at least in mainline, so we could probably just drop the manual
> stuff. I don't think I have a strong opinion between that
> and blocking the automatic configuration if it was previously
> set manually, but we should do one of them to keep things clear.

Yes, definitely.

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

Re: [PATCH] sound/soc/codecs/wm8978: add missing BCLK divider setup

[Sune Brian]

Mark Brown <broonie@kernel.org> 於 2025年10月7日 週二 下午8:30寫道：
>
> On Tue, Oct 07, 2025 at 01:26:52PM +0100, Charles Keepax wrote:
>
> > I don't see any current users for the manual divider configuration
> > at least in mainline, so we could probably just drop the manual
> > stuff. I don't think I have a strong opinion between that
> > and blocking the automatic configuration if it was previously
> > set manually, but we should do one of them to keep things clear.
>
> Yes, definitely.

if you read that code more clearly. It is a case that is select between
WM8978_OPCLKRATE and WM8978_BCLKDIV.
With the latest test on the dts file.
When running such codec on master mode the bclk had never set to correct
bclk rate. As such even you see that codes exist. It never run as expected.
I am not sure any adjust is required on dts but with proper lrclk and
mclk output.
The bclk is wrong on that version.

This is why I propose the patch of the bclk.

Re: [PATCH] sound/soc/codecs/wm8978: add missing BCLK divider setup

[Sune Brian]

Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月7日 週二 下午8:11寫道：
>
> On Tue, Oct 07, 2025 at 07:22:10PM +0800, Sune Brian wrote:
> > Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月7日 週二 下午6:30寫道：
> > > On Fri, Oct 03, 2025 at 05:13:04PM +0800, Brian Sune wrote:
> > > > The original WM8978 codec driver did not set the BCLK (bit clock)
> > > > divider, which can cause audio clocks to be incorrect or unstable
> > > > depending on the sample rate and word length.
> > >
> > > This isn't totally accurate, the driver expects it will be set
> > > through the set_clkdiv callback. Which one could probably argue
> > > is a bit of a more legacy approach, but probably worth calling
> > > that out here.
> >
> > According to actual hardware tests and the WM8978 driver study.
> > There are no bclk register setup in the entire driver. So I am not sure
> > How could this even set through the callback? The IC itself requires
> > 2-wires register load and this can't be done via software level.
>
> /*
>  * Configure WM8978 clock dividers.
>  */
> static int wm8978_set_dai_clkdiv(struct snd_soc_dai *codec_dai,
>                                  int div_id, int div)
> {
>         struct snd_soc_component *component = codec_dai->component;
>         struct wm8978_priv *wm8978 = snd_soc_component_get_drvdata(component);
>         int ret = 0;
>
>         switch (div_id) {
>         case WM8978_OPCLKRATE:
>                 ...
>         case WM8978_BCLKDIV:
>                 if (div & ~0x1c)
>                         return -EINVAL;
>                 snd_soc_component_update_bits(component, WM8978_CLOCKING, 0x1c, div); <<---- HERE
>                 break;
>         default:
>                 return -EINVAL;
>         }
>
>         dev_dbg(component->dev, "%s: ID %d, value %u\n", __func__, div_id, div);
>
>         return ret;
> }
>
> Its not missing its right there. That said your way is probably
> slightly more standard these days, but we should take care of the
> interaction between the two.

What my missing meant is if run with DEBUG flag on that case had never
behave as expected.
MCLK and LRCLK both is correctly outputted. While the current
unpatched version will generate
wrong BCLK complete break the codec. As such I proposed the BCLK patch.
I had not investigate deep why it never calls but the "int div" is
loaded and computed by where is a bit puzzling.
And the loaded it simply with div on actual mclk/2/bit_per_channel is
also incorrect.
As mentioned in previous explanations, the clock register is a fix
table on dividing # that is a LUT with restricted # allowed.

>
> Thanks,
> Charles

Re: [PATCH] sound/soc/codecs/wm8978: add missing BCLK divider setup

[Charles Keepax]

On Tue, Oct 07, 2025 at 08:48:40PM +0800, Sune Brian wrote:
> Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月7日 週二 下午8:11寫道：
> > On Tue, Oct 07, 2025 at 07:22:10PM +0800, Sune Brian wrote:
> > > Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月7日 週二 下午6:30寫道：
> > > > On Fri, Oct 03, 2025 at 05:13:04PM +0800, Brian Sune wrote:
> > > > > The original WM8978 codec driver did not set the BCLK (bit clock)
> > Its not missing its right there. That said your way is probably
> > slightly more standard these days, but we should take care of the
> > interaction between the two.
> 
> What my missing meant is if run with DEBUG flag on that case had never
> behave as expected.
> MCLK and LRCLK both is correctly outputted. While the current
> unpatched version will generate
> wrong BCLK complete break the codec. As such I proposed the BCLK patch.
> I had not investigate deep why it never calls but the "int div" is
> loaded and computed by where is a bit puzzling.
> And the loaded it simply with div on actual mclk/2/bit_per_channel is
> also incorrect.
> As mentioned in previous explanations, the clock register is a fix
> table on dividing # that is a LUT with restricted # allowed.

Yeah the existing code expects the machine driver to call
snd_soc_dai_set_clkdiv. I am guessing you are using something
like simple card that doesn't do this.

To be clear the bulk of your patch is good, updating this in
hw_params is probably more normal these days. But we need to
make sure the two paths don't interfere with each other. Think
of a system that is already calling snd_soc_dai_set_clkdiv() to
set BCLKDIV, after your patch BCLKDIV will be set twice
potentially to two different values and would generate no error
messages.

I think you have two options:

1) Remove WM8978_BCLKDIV from wm8978_set_dai_clkdiv. There are no
   upstream users that I can see, so this should be fine. This
   would mean an out of tree user of snd_soc_dai_set_clkdiv would
   now get an error so they know they need to fix something.
2) Only run your dynamic BCLK code if wm8978_set_dai_clkdiv
   hasn't been called. This would mean any out of tree users of
   snd_soc_dai_set_clkdiv would have no problems everything would
   keep working as before, but at the cost of a little complexity
   in the code.

I am happy with either approach so which ever you prefer is fine
with me.

Thanks,
Charles

Re: [PATCH] sound/soc/codecs/wm8978: add missing BCLK divider setup

[Sune Brian]

Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月7日 週二 下午9:15寫道：
>
> On Tue, Oct 07, 2025 at 08:48:40PM +0800, Sune Brian wrote:
> > Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月7日 週二 下午8:11寫道：
> > > On Tue, Oct 07, 2025 at 07:22:10PM +0800, Sune Brian wrote:
> > > > Charles Keepax <ckeepax@opensource.cirrus.com> 於 2025年10月7日 週二 下午6:30寫道：
> > > > > On Fri, Oct 03, 2025 at 05:13:04PM +0800, Brian Sune wrote:
> > > > > > The original WM8978 codec driver did not set the BCLK (bit clock)
> > > Its not missing its right there. That said your way is probably
> > > slightly more standard these days, but we should take care of the
> > > interaction between the two.
> >
> > What my missing meant is if run with DEBUG flag on that case had never
> > behave as expected.
> > MCLK and LRCLK both is correctly outputted. While the current
> > unpatched version will generate
> > wrong BCLK complete break the codec. As such I proposed the BCLK patch.
> > I had not investigate deep why it never calls but the "int div" is
> > loaded and computed by where is a bit puzzling.
> > And the loaded it simply with div on actual mclk/2/bit_per_channel is
> > also incorrect.
> > As mentioned in previous explanations, the clock register is a fix
> > table on dividing # that is a LUT with restricted # allowed.
>
> Yeah the existing code expects the machine driver to call
> snd_soc_dai_set_clkdiv. I am guessing you are using something
> like simple card that doesn't do this.
>
> To be clear the bulk of your patch is good, updating this in
> hw_params is probably more normal these days. But we need to
> make sure the two paths don't interfere with each other. Think
> of a system that is already calling snd_soc_dai_set_clkdiv() to
> set BCLKDIV, after your patch BCLKDIV will be set twice
> potentially to two different values and would generate no error
> messages.

Before the below action started. I need more background, if possible.
If my understand is correct on your ideas. Do you mean the default driver calls
the bclkdiv on other places? But how could that bclkdiv # be correct
from first place?
As I had mentioned the div # ifself like this codec is a LUT rather
than the actual divided #?
For example /32 from LUT is a 3b101?
For example what if MCLK is set higher than /64 could even out of the
LUT from first place.
The external div # passed in, how to ensure it is codec LUT compatible
from first place?

>
> I think you have two options:
>
> 1) Remove WM8978_BCLKDIV from wm8978_set_dai_clkdiv. There are no
>    upstream users that I can see, so this should be fine. This
>    would mean an out of tree user of snd_soc_dai_set_clkdiv would
>    now get an error so they know they need to fix something.
> 2) Only run your dynamic BCLK code if wm8978_set_dai_clkdiv
>    hasn't been called. This would mean any out of tree users of
>    snd_soc_dai_set_clkdiv would have no problems everything would
>    keep working as before, but at the cost of a little complexity
>    in the code.
>
> I am happy with either approach so which ever you prefer is fine
> with me.
>
> Thanks,
> Charles

Re: [PATCH] sound/soc/codecs/wm8978: add missing BCLK divider setup

[Mark Brown]

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

On Tue, Oct 07, 2025 at 09:39:59PM +0800, Sune Brian wrote:

> Before the below action started. I need more background, if possible.
> If my understand is correct on your ideas. Do you mean the default driver calls
> the bclkdiv on other places? But how could that bclkdiv # be correct
> from first place?
> As I had mentioned the div # ifself like this codec is a LUT rather
> than the actual divided #?
> For example /32 from LUT is a 3b101?
> For example what if MCLK is set higher than /64 could even out of the
> LUT from first place.
> The external div # passed in, how to ensure it is codec LUT compatible
> from first place?

Systems doing this are using a machine driver written for that specific
machine and will know exactly which CODEC and other components they are
dealing with.

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

Re: [PATCH] sound/soc/codecs/wm8978: add missing BCLK divider setup

[Sune Brian]

Mark Brown <broonie@kernel.org> 於 2025年10月7日 週二 下午9:48寫道：

> Systems doing this are using a machine driver written for that specific
> machine and will know exactly which CODEC and other components they are
> dealing with.

With such idea, this is not CODEC controlled from first place to
restrict the possible configurations.
This could introduce many possible hazard, but not this patch trying
to fix from first place.

As for Charles, I will try to patch with option 2 unless it is too
tedious to do so.

I think you have two options:

1) Remove WM8978_BCLKDIV from wm8978_set_dai_clkdiv. There are no
   upstream users that I can see, so this should be fine. This
   would mean an out of tree user of snd_soc_dai_set_clkdiv would
   now get an error so they know they need to fix something.
2) Only run your dynamic BCLK code if wm8978_set_dai_clkdiv
   hasn't been called. This would mean any out of tree users of
   snd_soc_dai_set_clkdiv would have no problems everything would
   keep working as before, but at the cost of a little complexity
   in the code.

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

[Brian Sune]

The original WM8978 codec driver did not set the BCLK (bit clock)
divider, which can cause audio clocks to be incorrect or unstable
depending on the sample rate and word length.

This patch adds proper calculation and configuration of the BCLK
divider based on the sample rate and word width, ensuring the
codec generates the correct bit clock for all supported rates.

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

diff --git a/sound/soc/codecs/wm8978.c b/sound/soc/codecs/wm8978.c
index 8c45ba6fc4c3..2109c84f33df 100644
--- a/sound/soc/codecs/wm8978.c
+++ b/sound/soc/codecs/wm8978.c
@@ -717,6 +717,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 +825,21 @@ 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);
 
+	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 v2] ASoC: wm8978: add missing BCLK divider setup

[Mark Brown]

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

On Tue, Oct 07, 2025 at 07:33:05PM +0800, Brian Sune wrote:
> The original WM8978 codec driver did not set the BCLK (bit clock)
> divider, which can cause audio clocks to be incorrect or unstable
> depending on the sample rate and word length.

Please don't send new patches in reply to old patches or serieses, this
makes it harder for both people and tools to understand what is going
on - it can bury things in mailboxes and make it difficult to keep track
of what current patches are, both for the new patches and the old ones.

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

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

[Sune Brian]

Mark Brown <broonie@kernel.org> 於 2025年10月7日 週二 下午7:44寫道：
>
> On Tue, Oct 07, 2025 at 07:33:05PM +0800, Brian Sune wrote:
> > The original WM8978 codec driver did not set the BCLK (bit clock)
> > divider, which can cause audio clocks to be incorrect or unstable
> > depending on the sample rate and word length.
>
> Please don't send new patches in reply to old patches or serieses, this
> makes it harder for both people and tools to understand what is going
> on - it can bury things in mailboxes and make it difficult to keep track
> of what current patches are, both for the new patches and the old ones.

Sorry for this action. But this patch is just a title fixe according
to previous comment.
Patch body is fully aligned to previous revision.

May I know what is the proper way to amend the title?

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

[Mark Brown]

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

On Tue, Oct 07, 2025 at 07:48:11PM +0800, Sune Brian wrote:
> Mark Brown <broonie@kernel.org> 於 2025年10月7日 週二 下午7:44寫道：

> > Please don't send new patches in reply to old patches or serieses, this
> > makes it harder for both people and tools to understand what is going
> > on - it can bury things in mailboxes and make it difficult to keep track
> > of what current patches are, both for the new patches and the old ones.

> Sorry for this action. But this patch is just a title fixe according
> to previous comment.
> Patch body is fully aligned to previous revision.

> May I know what is the proper way to amend the title?

The title is fine, the above is about sending the patch as a reply.

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

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

[Sune Brian]

Mark Brown <broonie@kernel.org> 於 2025年10月7日 週二 下午7:52寫道：
>
> On Tue, Oct 07, 2025 at 07:48:11PM +0800, Sune Brian wrote:
> > Mark Brown <broonie@kernel.org> 於 2025年10月7日 週二 下午7:44寫道：
>
> > > Please don't send new patches in reply to old patches or serieses, this
> > > makes it harder for both people and tools to understand what is going
> > > on - it can bury things in mailboxes and make it difficult to keep track
> > > of what current patches are, both for the new patches and the old ones.
>
> > Sorry for this action. But this patch is just a title fixe according
> > to previous comment.
> > Patch body is fully aligned to previous revision.
>
> > May I know what is the proper way to amend the title?
>
> The title is fine, the above is about sending the patch as a reply.

Got it, when title amended version is sent, need no reply with
previous revision ID.
Thank you

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

[Mark Brown]

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

On Tue, Oct 07, 2025 at 07:56:31PM +0800, Sune Brian wrote:
> Mark Brown <broonie@kernel.org> 於 2025年10月7日 週二 下午7:52寫道：

> > > > Please don't send new patches in reply to old patches or serieses, this
> > > > makes it harder for both people and tools to understand what is going
> > > > on - it can bury things in mailboxes and make it difficult to keep track
> > > > of what current patches are, both for the new patches and the old ones.

> > The title is fine, the above is about sending the patch as a reply.

> Got it, when title amended version is sent, need no reply with
> previous revision ID.

No.  You sent the new version as a reply to the prior verison which
nearly got it binned.  You should send new versions as new threads, not
as replies to other serieses.

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

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

[Sune Brian]

Understood, so in this situation.
What is the proper way to fix it?
Leave it as such or create a new thread?

Thank you

Mark Brown <broonie@kernel.org> 於 2025年10月7日 週二 下午8:10寫道：
>
> On Tue, Oct 07, 2025 at 07:56:31PM +0800, Sune Brian wrote:
> > Mark Brown <broonie@kernel.org> 於 2025年10月7日 週二 下午7:52寫道：
>
> > > > > Please don't send new patches in reply to old patches or serieses, this
> > > > > makes it harder for both people and tools to understand what is going
> > > > > on - it can bury things in mailboxes and make it difficult to keep track
> > > > > of what current patches are, both for the new patches and the old ones.
>
> > > The title is fine, the above is about sending the patch as a reply.
>
> > Got it, when title amended version is sent, need no reply with
> > previous revision ID.
>
> No.  You sent the new version as a reply to the prior verison which
> nearly got it binned.  You should send new versions as new threads, not
> as replies to other serieses.

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

[Mark Brown]

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

On Tue, Oct 07, 2025 at 09:03:37PM +0800, Sune Brian wrote:
> Understood, so in this situation.
> What is the proper way to fix it?
> Leave it as such or create a new thread?

It's fine, just don't do this for any future versions you post.

Please don't top post, reply in line with needed context.  This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.

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

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

[Sune Brian]

Mark Brown <broonie@kernel.org> 於 2025年10月7日 週二 下午9:15寫道：
> Please don't top post, reply in line with needed context.  This allows
> readers to readily follow the flow of conversation and understand what
> you are talking about and also helps ensure that everything in the
> discussion is being addressed.

Understood crop previous reply to the content that this is mail replying.
Will soon repatch another version on Charles comments.
Thank you