linux-kernel - Re: [PATCH] ASoC: wm8960: Fix clock configuration on slave mode

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>] [day] [month] [year] [list]

Date:   Wed, 13 Oct 2021 05:24:22 +0000
From:   "S.j. Wang" <shengjiu.wang@....com>
To:     Charles Keepax <ckeepax@...nsource.cirrus.com>
CC:     "lgirdwood@...il.com" <lgirdwood@...il.com>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "perex@...ex.cz" <perex@...ex.cz>,
        "tiwai@...e.com" <tiwai@...e.com>,
        "kuninori.morimoto.gx@...esas.com" <kuninori.morimoto.gx@...esas.com>,
        "patches@...nsource.cirrus.com" <patches@...nsource.cirrus.com>,
        "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ASoC: wm8960: Fix clock configuration on slave mode

> On Tue, Sep 07, 2021 at 05:11:09PM +0800, Shengjiu Wang wrote:
> > There is a noise issue for 8kHz sample rate on slave mode.
> > Compared with master mode, the difference is the DACDIV setting, after
> > correcting the DACDIV, the noise is gone.
> >
> > There is no noise issue for 48kHz sample rate, because the default
> > value of DACDIV is correct for 48kHz.
> >
> > So wm8960_configure_clocking() should be functional for ADC and DAC
> > function even if it is slave mode.
> >
> > In order to be compatible for old use case, just add condition for
> > checking that sysclk is zero with slave mode.
> >
> > Fixes: 0e50b51aa22f ("ASoC: wm8960: Let wm8960 driver configure its
> > bit clock and frame clock")
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@....com>
> > ---
> >  sound/soc/codecs/wm8960.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/sound/soc/codecs/wm8960.c b/sound/soc/codecs/wm8960.c
> > index 9e621a254392..9c6af76a60fd 100644
> > --- a/sound/soc/codecs/wm8960.c
> > +++ b/sound/soc/codecs/wm8960.c
> > @@ -742,7 +742,7 @@ static int wm8960_configure_clocking(struct
> snd_soc_component *component)
> >       int i, j, k;
> >       int ret;
> >
> > -     if (!(iface1 & (1<<6))) {
> > +     if (!(iface1 & (1 << 6)) && !wm8960->sysclk) {
> >               dev_dbg(component->dev,
> >                       "Codec is slave mode, no need to configure
> > clock\n");
> 
> Looking through the datasheet it just looks like this if statement has always
> been non-sense, it looks pretty clear the clocking should still be configured in
> slave mode (apart from BCLKDIV which is presumably ignored in slave mode).
> 
> I would be slightly inclined to suggest it would be better to just fixup any
> systems not setting sysclk for slave mode, but I am assuming you are talking
> primarily about out of tree systems. So I think we need to at least update the
> message here as well, it should probably change to a warning and state that
> the we are proceeding with no clock configuration, rather than erroneously
> saying it doesn't need one.
> 
> Thanks,
> Charles

Sorry for missing your email. I will update it in v2.
Thanks.

Best regards
Wang Shengjiu