[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20120314143941.GS3133@opensource.wolfsonmicro.com>
Date: Wed, 14 Mar 2012 14:39:41 +0000
From: Mark Brown <broonie@...nsource.wolfsonmicro.com>
To: Tomoya MORINAGA <tomoya.rohm@...il.com>
Cc: Liam Girdwood <lrg@...com>, Jaroslav Kysela <perex@...ex.cz>,
Takashi Iwai <tiwai@...e.de>, alsa-devel@...a-project.org,
linux-kernel@...r.kernel.org, qi.wang@...el.com,
yong.y.wang@...el.com, joel.clark@...el.com, kok.howg.ewe@...el.com
Subject: Re: [PATCH v7] sound/soc/codecs: add LAPIS Semiconductor ML26124
On Fri, Mar 09, 2012 at 03:37:06PM +0900, Tomoya MORINAGA wrote:
A substantial improvement on previous versions but there's still a few
issues, though they're now fairly small.
> + /* SAI configuration */
> + if (priv->channels == 1) {
> + ml26124_update_bits(codec, ML26124_SAI_TRANS_CTL, mask,
> + ML26124_SAI_NO_DELAY | ML26124_SAI_FRAME_SYNC);
> + ml26124_update_bits(codec, ML26124_SAI_RCV_CTL, mask,
> + ML26124_SAI_NO_DELAY | ML26124_SAI_FRAME_SYNC);
> + } else {
> + ml26124_update_bits(codec, ML26124_SAI_TRANS_CTL, mask,
> + ML26124_SAI_NO_DELAY);
> + ml26124_update_bits(codec, ML26124_SAI_RCV_CTL, mask,
> + ML26124_SAI_NO_DELAY);
> + }
What happens if the system switches between 1 channel mode and
multi-channel mode at runtime? It looks like _SYNC will never be
cleared.
> +static int ml26124_set_dai_sysclk(struct snd_soc_dai *codec_dai,
> + int clk_id, unsigned int freq, int dir)
> +{
> + struct snd_soc_codec *codec = codec_dai->codec;
> + struct ml26124_priv *priv = snd_soc_codec_get_drvdata(codec);
> +
> + switch (clk_id) {
> + case ML26124_USE_PLL:
> + ml26124_update_bits(codec, ML26124_CLK_CTL,
> + BIT(0) | BIT(1), 0);
> + break;
> + case ML26124_USE_MCLKI_256FS:
> + ml26124_update_bits(codec, ML26124_CLK_CTL,
> + BIT(0) | BIT(1), 1);
> + break;
> + case ML26124_USE_MCLKI_512FS:
> + ml26124_update_bits(codec, ML26124_CLK_CTL,
> + BIT(0) | BIT(1), 2);
> + break;
> + case ML26124_USE_MCLKI_1024FS:
> + ml26124_update_bits(codec, ML26124_CLK_CTL,
> + BIT(0) | BIT(1), 3);
> + break;
Why not just specify the MCLK rate and then calculate the division based
on the sample rate?
> + switch (freq) {
> + case 12288000:
> + priv->mclk = freq;
> + break;
This is especially true given that only one MCLK rate is supported.
> + case SND_SOC_BIAS_STANDBY:
> + /* VMID ON */
> + ml26124_update_bits(codec, ML26124_PW_REF_PW_MNG,
> + ML26124_VMID, ML26124_VMID);
> + msleep(500);
This will sleep for 500ms when powering down which probably isn't what's
desired...
Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)
Powered by blists - more mailing lists