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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aOTrsYllMst3oR03@opensource.cirrus.com>
Date: Tue, 7 Oct 2025 11:30:09 +0100
From: Charles Keepax <ckeepax@...nsource.cirrus.com>
To: Brian Sune <briansune@...il.com>
Cc: Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
        Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>,
        linux-sound@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] sound/soc/codecs/wm8978: add missing BCLK divider setup

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@...il.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