From: Charles Keepax <ckeepax@opensource.cirrus.com>
To: Shengjiu Wang <shengjiu.wang@nxp.com>
Cc: <lgirdwood@gmail.com>, <broonie@kernel.org>, <perex@perex.cz>,
<tiwai@suse.com>, <kuninori.morimoto.gx@renesas.com>,
<patches@opensource.cirrus.com>, <alsa-devel@alsa-project.org>,
<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ASoC: wm8960: Fix clock configuration on slave mode
Date: Tue, 7 Sep 2021 10:33:37 +0000 [thread overview]
Message-ID: <20210907103337.GO9223@ediswmail.ad.cirrus.com> (raw)
In-Reply-To: <1631005869-7308-1-git-send-email-shengjiu.wang@nxp.com>
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@nxp.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
next prev parent reply other threads:[~2021-09-07 10:34 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-07 9:11 [PATCH] ASoC: wm8960: Fix clock configuration on slave mode Shengjiu Wang
2021-09-07 10:33 ` Charles Keepax [this message]
2021-10-13 5:24 S.j. Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210907103337.GO9223@ediswmail.ad.cirrus.com \
--to=ckeepax@opensource.cirrus.com \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=kuninori.morimoto.gx@renesas.com \
--cc=lgirdwood@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=patches@opensource.cirrus.com \
--cc=perex@perex.cz \
--cc=shengjiu.wang@nxp.com \
--cc=tiwai@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).