sound/soc/codecs/wm8978.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
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.1On 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
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
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.
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?
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.
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
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.
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.
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
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.
© 2016 - 2026 Red Hat, Inc.