linux-kernel - Re: [PATCH v3] ASoC: wm8524: enable constraints when sysclk is configured.

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <aFUbGFNAISmamfv8@opensource.cirrus.com>
Date: Fri, 20 Jun 2025 09:26:00 +0100
From: Charles Keepax <ckeepax@...nsource.cirrus.com>
To: Shengjiu Wang <shengjiu.wang@....com>
Cc: lgirdwood@...il.com, broonie@...nel.org, perex@...ex.cz, tiwai@...e.com,
        patches@...nsource.cirrus.com, linux-sound@...r.kernel.org,
        linux-kernel@...r.kernel.org, shengjiu.wang@...il.com
Subject: Re: [PATCH v3] ASoC: wm8524: enable constraints when sysclk is
 configured.

On Fri, Jun 20, 2025 at 10:14:03AM +0800, Shengjiu Wang wrote:
> In some cases, the sysclk won't be configured on init, and sysclk can be
> changed in hw_params() according to different sample rate, for example,
> for 44kHz sample rate, the sysclk is 11.2896MHz, for 48kHz sample rate,
> the sysclk is 12.288MHz.
> 
> In order to support the above case, only enable constraints when sysclk
> is configured, and check the rate in hw_params.
> 
> So overall there are three cases that need to be considered:
> - call set_sysclk() on init, then constraints will be initialized.
> - don't call set_sysclk() on init, but call it after startup(), then
>   constraints will be configured, the constraints can be cleared with
>   call set_sysclk() again in shutdown().
> - don't call set_sysclk() in the whole flow, then there are no any
>   constraints. The clocks depend on cpu dai.
> 
> Enlarge the WM8524_NUM_RATES to 12, as the supported rate range is 8kHz
> to 192kHz.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@....com>
> ---
> +static int wm8524_hw_params(struct snd_pcm_substream *substream,
> +			    struct snd_pcm_hw_params *params,
> +			    struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_component *component = dai->component;
> +	struct wm8524_priv *wm8524 = snd_soc_component_get_drvdata(component);
> +	int i;
> +
> +	/* If sysclk is not configured, no need to check the rate */
> +	if (!wm8524->sysclk)
> +		return 0;

This is kinda the opposite of what I was hoping we could do. The
idea was to make sure we returned an error if we can't support
the given rate. So if we don't have the constraint, we check the
value in hw_params. This looks like it checks in hw_params only
in the case the constraint existed, but in that case there is no
need to check because we had the constraint.

Thanks,
Charles

> +	/* Find a supported LRCLK rate */
> +	for (i = 0; i < wm8524->rate_constraint.count; i++) {
> +		if (wm8524->rate_constraint.list[i] == params_rate(params))
> +			break;
> +	}
> +
> +	if (i == wm8524->rate_constraint.count) {
> +		dev_err(component->dev, "LRCLK %d unsupported with MCLK %d\n",
> +			params_rate(params), wm8524->sysclk);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}