linux-kernel - Re: [PATCH v2] 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: <CAA+D8AMQ5pAw6uVGQEYK+oPHJAojqnPbB+hsMWbmQOwmE9AFvw@mail.gmail.com>
Date: Wed, 18 Jun 2025 15:45:48 +0800
From: Shengjiu Wang <shengjiu.wang@...il.com>
To: Charles Keepax <ckeepax@...nsource.cirrus.com>
Cc: Shengjiu Wang <shengjiu.wang@....com>, 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
Subject: Re: [PATCH v2] ASoC: wm8524: enable constraints when sysclk is configured.

On Tue, Jun 17, 2025 at 4:55 PM Charles Keepax
<ckeepax@...nsource.cirrus.com> wrote:
>
> On Tue, Jun 17, 2025 at 03:18:33PM +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.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@....com>
> > ---
> > changes in v2
> > - Don't remove constraints, but only enable constraints when sysclk
> >   is configured.
> >
> >  sound/soc/codecs/wm8524.c | 17 +++++++----------
> >  1 file changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/sound/soc/codecs/wm8524.c b/sound/soc/codecs/wm8524.c
> > index 403e513f3fa8..5d8fb45db272 100644
> > --- a/sound/soc/codecs/wm8524.c
> > +++ b/sound/soc/codecs/wm8524.c
> > @@ -63,17 +63,12 @@ static int wm8524_startup(struct snd_pcm_substream *substream,
> >       struct wm8524_priv *wm8524 = snd_soc_component_get_drvdata(component);
> >
> >       /* The set of sample rates that can be supported depends on the
> > -      * MCLK supplied to the CODEC - enforce this.
> > +      * MCLK supplied to the CODEC.
> >        */
> > -     if (!wm8524->sysclk) {
> > -             dev_err(component->dev,
> > -                     "No MCLK configured, call set_sysclk() on init\n");
> > -             return -EINVAL;
> > -     }
> > -
> > -     snd_pcm_hw_constraint_list(substream->runtime, 0,
> > -                                SNDRV_PCM_HW_PARAM_RATE,
> > -                                &wm8524->rate_constraint);
> > +     if (wm8524->sysclk)
> > +             snd_pcm_hw_constraint_list(substream->runtime, 0,
> > +                                        SNDRV_PCM_HW_PARAM_RATE,
> > +                                        &wm8524->rate_constraint);
> >
> >       gpiod_set_value_cansleep(wm8524->mute, 1);
> >
>
> If we are allow the clock to be set at hw_params time, we
> should probably also double check the rates in hw_params and
> return an error if its not supported.

Ok, will add the check in hw_params.

>
> > @@ -98,6 +93,8 @@ static int wm8524_set_dai_sysclk(struct snd_soc_dai *codec_dai,
> >       int i, j = 0;
> >
> >       wm8524->sysclk = freq;
> > +     if (!wm8524->sysclk)
> > +             return 0;
> >
> >       wm8524->rate_constraint.count = 0;
> >       for (i = 0; i < ARRAY_SIZE(lrclk_ratios); i++) {
>
> We probably should clear rate_constraint.count in the clearing
> sysclk case, and we should probably also check if the DAI is
> active. The user shoudln't be allowed to change clock there is
> audio already happening.

Seems we can't check if the DAI is active or not in sysclk(),
because startup()->set_sysclk(),  in sysclk() the dai is always
active.  if we allow the clock changed in hw_params time, then
we can't check if the DAI is active.

best regards
Shengjiu Wang

>
> Thanks,
> Charles