[alsa-devel] [PATCH 0/3] ad193x set_pll, fix register setting and typo
Hi Mark/Liam, The following 3 patches add new features or fix bugs in ad193x drivers. 1/3. add set_pll entry to support different clock input 2/3. fix wrong register setting in ad193x_set_dai_fmt 3/3. fix typo, delete redundant space
Thanks Barry
Signed-off-by: Barry Song 21cnbao@gmail.com --- sound/soc/codecs/ad193x.c | 31 +++++++++++++++++++++++++++++++ sound/soc/codecs/ad193x.h | 5 +++++ 2 files changed, 36 insertions(+), 0 deletions(-)
diff --git a/sound/soc/codecs/ad193x.c b/sound/soc/codecs/ad193x.c index 7ed787e..8a099ac 100644 --- a/sound/soc/codecs/ad193x.c +++ b/sound/soc/codecs/ad193x.c @@ -249,6 +249,36 @@ static int ad193x_set_dai_fmt(struct snd_soc_dai *codec_dai, return 0; }
+static int ad193x_set_dai_pll(struct snd_soc_dai *codec_dai, + int pll_id, int source, unsigned int freq_in, unsigned int freq_out) +{ + struct snd_soc_codec *codec = codec_dai->codec; + int reg; + + reg = snd_soc_read(codec, AD193X_PLL_CLK_CTRL0); + + switch (freq_in) { + case 12288000: + reg = (reg & AD193X_PLL_INPUT_MASK) | AD193X_PLL_INPUT_256; + break; + case 18432000: + reg = (reg & AD193X_PLL_INPUT_MASK) | AD193X_PLL_INPUT_384; + break; + case 24576000: + reg = (reg & AD193X_PLL_INPUT_MASK) | AD193X_PLL_INPUT_512; + break; + case 36864000: + reg = (reg & AD193X_PLL_INPUT_MASK) | AD193X_PLL_INPUT_768; + break; + default: + dev_err(codec->dev, "ad193x_set_dai_pll: unsupported pll input freq:%d", freq_in); + return -EINVAL; + } + + snd_soc_write(codec, AD193X_PLL_CLK_CTRL0, reg); + return 0; +} + static int ad193x_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *dai) @@ -382,6 +412,7 @@ static struct snd_soc_dai_ops ad193x_dai_ops = { .digital_mute = ad193x_mute, .set_tdm_slot = ad193x_set_tdm_slot, .set_fmt = ad193x_set_dai_fmt, + .set_pll = ad193x_set_dai_pll, };
/* codec DAI instance */ diff --git a/sound/soc/codecs/ad193x.h b/sound/soc/codecs/ad193x.h index a03c880..654ba64 100644 --- a/sound/soc/codecs/ad193x.h +++ b/sound/soc/codecs/ad193x.h @@ -11,6 +11,11 @@
#define AD193X_PLL_CLK_CTRL0 0x800 #define AD193X_PLL_POWERDOWN 0x01 +#define AD193X_PLL_INPUT_MASK (~0x6) +#define AD193X_PLL_INPUT_256 (0 << 1) +#define AD193X_PLL_INPUT_384 (1 << 1) +#define AD193X_PLL_INPUT_512 (2 << 1) +#define AD193X_PLL_INPUT_768 (3 << 1) #define AD193X_PLL_CLK_CTRL1 0x801 #define AD193X_DAC_CTRL0 0x802 #define AD193X_DAC_POWERDOWN 0x01
Signed-off-by: Barry Song 21cnbao@gmail.com --- sound/soc/codecs/ad193x.c | 48 +++++++++++++++++++++++--------------------- 1 files changed, 25 insertions(+), 23 deletions(-)
diff --git a/sound/soc/codecs/ad193x.c b/sound/soc/codecs/ad193x.c index 8a099ac..e79788c 100644 --- a/sound/soc/codecs/ad193x.c +++ b/sound/soc/codecs/ad193x.c @@ -163,9 +163,10 @@ static int ad193x_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt) { struct snd_soc_codec *codec = codec_dai->codec; - int adc_reg, dac_reg; + int adc_reg1, adc_reg2, dac_reg;
- adc_reg = snd_soc_read(codec, AD193X_ADC_CTRL2); + adc_reg1 = snd_soc_read(codec, AD193X_ADC_CTRL1); + adc_reg2 = snd_soc_read(codec, AD193X_ADC_CTRL2); dac_reg = snd_soc_read(codec, AD193X_DAC_CTRL1);
/* At present, the driver only support AUX ADC mode(SND_SOC_DAIFMT_I2S @@ -173,12 +174,12 @@ static int ad193x_set_dai_fmt(struct snd_soc_dai *codec_dai, */ switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) { case SND_SOC_DAIFMT_I2S: - adc_reg &= ~AD193X_ADC_SERFMT_MASK; - adc_reg |= AD193X_ADC_SERFMT_TDM; + adc_reg1 &= ~AD193X_ADC_SERFMT_MASK; + adc_reg1 |= AD193X_ADC_SERFMT_TDM; break; case SND_SOC_DAIFMT_DSP_A: - adc_reg &= ~AD193X_ADC_SERFMT_MASK; - adc_reg |= AD193X_ADC_SERFMT_AUX; + adc_reg1 &= ~AD193X_ADC_SERFMT_MASK; + adc_reg1 |= AD193X_ADC_SERFMT_AUX; break; default: return -EINVAL; @@ -186,27 +187,27 @@ static int ad193x_set_dai_fmt(struct snd_soc_dai *codec_dai,
switch (fmt & SND_SOC_DAIFMT_INV_MASK) { case SND_SOC_DAIFMT_NB_NF: /* normal bit clock + frame */ - adc_reg &= ~AD193X_ADC_LEFT_HIGH; - adc_reg &= ~AD193X_ADC_BCLK_INV; + adc_reg2 &= ~AD193X_ADC_LEFT_HIGH; + adc_reg2 &= ~AD193X_ADC_BCLK_INV; dac_reg &= ~AD193X_DAC_LEFT_HIGH; dac_reg &= ~AD193X_DAC_BCLK_INV; break; case SND_SOC_DAIFMT_NB_IF: /* normal bclk + invert frm */ - adc_reg |= AD193X_ADC_LEFT_HIGH; - adc_reg &= ~AD193X_ADC_BCLK_INV; + adc_reg2 |= AD193X_ADC_LEFT_HIGH; + adc_reg2 &= ~AD193X_ADC_BCLK_INV; dac_reg |= AD193X_DAC_LEFT_HIGH; dac_reg &= ~AD193X_DAC_BCLK_INV; break; case SND_SOC_DAIFMT_IB_NF: /* invert bclk + normal frm */ - adc_reg &= ~AD193X_ADC_LEFT_HIGH; - adc_reg |= AD193X_ADC_BCLK_INV; + adc_reg2 &= ~AD193X_ADC_LEFT_HIGH; + adc_reg2 |= AD193X_ADC_BCLK_INV; dac_reg &= ~AD193X_DAC_LEFT_HIGH; dac_reg |= AD193X_DAC_BCLK_INV; break;
case SND_SOC_DAIFMT_IB_IF: /* invert bclk + frm */ - adc_reg |= AD193X_ADC_LEFT_HIGH; - adc_reg |= AD193X_ADC_BCLK_INV; + adc_reg2 |= AD193X_ADC_LEFT_HIGH; + adc_reg2 |= AD193X_ADC_BCLK_INV; dac_reg |= AD193X_DAC_LEFT_HIGH; dac_reg |= AD193X_DAC_BCLK_INV; break; @@ -216,26 +217,26 @@ static int ad193x_set_dai_fmt(struct snd_soc_dai *codec_dai,
switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) { case SND_SOC_DAIFMT_CBM_CFM: /* codec clk & frm master */ - adc_reg |= AD193X_ADC_LCR_MASTER; - adc_reg |= AD193X_ADC_BCLK_MASTER; + adc_reg2 |= AD193X_ADC_LCR_MASTER; + adc_reg2 |= AD193X_ADC_BCLK_MASTER; dac_reg |= AD193X_DAC_LCR_MASTER; dac_reg |= AD193X_DAC_BCLK_MASTER; break; case SND_SOC_DAIFMT_CBS_CFM: /* codec clk slave & frm master */ - adc_reg |= AD193X_ADC_LCR_MASTER; - adc_reg &= ~AD193X_ADC_BCLK_MASTER; + adc_reg2 |= AD193X_ADC_LCR_MASTER; + adc_reg2 &= ~AD193X_ADC_BCLK_MASTER; dac_reg |= AD193X_DAC_LCR_MASTER; dac_reg &= ~AD193X_DAC_BCLK_MASTER; break; case SND_SOC_DAIFMT_CBM_CFS: /* codec clk master & frame slave */ - adc_reg &= ~AD193X_ADC_LCR_MASTER; - adc_reg |= AD193X_ADC_BCLK_MASTER; + adc_reg2 &= ~AD193X_ADC_LCR_MASTER; + adc_reg2 |= AD193X_ADC_BCLK_MASTER; dac_reg &= ~AD193X_DAC_LCR_MASTER; dac_reg |= AD193X_DAC_BCLK_MASTER; break; case SND_SOC_DAIFMT_CBS_CFS: /* codec clk & frm slave */ - adc_reg &= ~AD193X_ADC_LCR_MASTER; - adc_reg &= ~AD193X_ADC_BCLK_MASTER; + adc_reg2 &= ~AD193X_ADC_LCR_MASTER; + adc_reg2 &= ~AD193X_ADC_BCLK_MASTER; dac_reg &= ~AD193X_DAC_LCR_MASTER; dac_reg &= ~AD193X_DAC_BCLK_MASTER; break; @@ -243,7 +244,8 @@ static int ad193x_set_dai_fmt(struct snd_soc_dai *codec_dai, return -EINVAL; }
- snd_soc_write(codec, AD193X_ADC_CTRL2, adc_reg); + snd_soc_write(codec, AD193X_ADC_CTRL1, adc_reg1); + snd_soc_write(codec, AD193X_ADC_CTRL2, adc_reg2); snd_soc_write(codec, AD193X_DAC_CTRL1, dac_reg);
return 0;
Signed-off-by: Barry Song 21cnbao@gmail.com --- sound/soc/codecs/ad193x.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/soc/codecs/ad193x.c b/sound/soc/codecs/ad193x.c index e79788c..d4089b8 100644 --- a/sound/soc/codecs/ad193x.c +++ b/sound/soc/codecs/ad193x.c @@ -46,13 +46,13 @@ static const struct soc_enum ad193x_deemp_enum =
static const struct snd_kcontrol_new ad193x_snd_controls[] = { /* DAC volume control */ - SOC_DOUBLE_R("DAC1 Volume", AD193X_DAC_L1_VOL, + SOC_DOUBLE_R("DAC1 Volume", AD193X_DAC_L1_VOL, AD193X_DAC_R1_VOL, 0, 0xFF, 1), - SOC_DOUBLE_R("DAC2 Volume", AD193X_DAC_L2_VOL, + SOC_DOUBLE_R("DAC2 Volume", AD193X_DAC_L2_VOL, AD193X_DAC_R2_VOL, 0, 0xFF, 1), - SOC_DOUBLE_R("DAC3 Volume", AD193X_DAC_L3_VOL, + SOC_DOUBLE_R("DAC3 Volume", AD193X_DAC_L3_VOL, AD193X_DAC_R3_VOL, 0, 0xFF, 1), - SOC_DOUBLE_R("DAC4 Volume", AD193X_DAC_L4_VOL, + SOC_DOUBLE_R("DAC4 Volume", AD193X_DAC_L4_VOL, AD193X_DAC_R4_VOL, 0, 0xFF, 1),
/* ADC switch control */
On Wed, Apr 21, 2010 at 05:36:49PM +0800, Barry Song wrote:
Signed-off-by: Barry Song 21cnbao@gmail.com
Applied, thanks.
On Wed, Apr 21, 2010 at 05:36:48PM +0800, Barry Song wrote:
Signed-off-by: Barry Song 21cnbao@gmail.com
Please provide more detailed changelogs. What is wrong with the register settings? I've applied the patch for 2.6.35 since it doesn't apply on 2.6.34 but it'd have been very much more helpful if you'd said what the problem you were trying to fix is.
On Fri, Apr 23, 2010 at 11:14 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Wed, Apr 21, 2010 at 05:36:48PM +0800, Barry Song wrote:
Signed-off-by: Barry Song 21cnbao@gmail.com
Please provide more detailed changelogs. What is wrong with the register settings? I've applied the patch for 2.6.35 since it doesn't apply on 2.6.34 but it'd have been very much more helpful if you'd said what the problem you were trying to fix is.
There was an error before. DAIFMT_FORMAT is controlled by adc_ctrl1 reg and DAIFMT_INV is controlled by adc_ctrl2 reg in ad193x. Old codes set all by adc_ctrl2, then sometimes, when running "arecord | aplay", ad193x is set to wrong mode, we can't hear anything.
On Wed, Apr 21, 2010 at 05:36:47PM +0800, Barry Song wrote:
Signed-off-by: Barry Song 21cnbao@gmail.com
+static int ad193x_set_dai_pll(struct snd_soc_dai *codec_dai,
int pll_id, int source, unsigned int freq_in, unsigned int freq_out)+{
- struct snd_soc_codec *codec = codec_dai->codec;
- int reg;
- reg = snd_soc_read(codec, AD193X_PLL_CLK_CTRL0);
- switch (freq_in) {
- case 12288000:
reg = (reg & AD193X_PLL_INPUT_MASK) | AD193X_PLL_INPUT_256;break;
This all looks like you should just implement set_sysclk() not set_pll() - from the user point of view the fact that a PLL ends up getting tuned is immaterial here, they can't control the output frequency at all.
On Fri, Apr 23, 2010 at 11:06 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Wed, Apr 21, 2010 at 05:36:47PM +0800, Barry Song wrote:
Signed-off-by: Barry Song 21cnbao@gmail.com
+static int ad193x_set_dai_pll(struct snd_soc_dai *codec_dai,
- int pll_id, int source, unsigned int freq_in, unsigned int freq_out)
+{
- struct snd_soc_codec *codec = codec_dai->codec;
- int reg;
- reg = snd_soc_read(codec, AD193X_PLL_CLK_CTRL0);
- switch (freq_in) {
- case 12288000:
- reg = (reg & AD193X_PLL_INPUT_MASK) | AD193X_PLL_INPUT_256;
- break;
This all looks like you should just implement set_sysclk() not set_pll()
- from the user point of view the fact that a PLL ends up getting tuned
is immaterial here, they can't control the output frequency at all. From their point of view they just specify the clock going into the CODEC and the driver works out everything else for them.
Sorry for I have not noticed this entry. Then i'll use set_sysclk() to set the MCLK instead of set_pll().
On Wed, 2010-04-21 at 17:36 +0800, Barry Song wrote:
Hi Mark/Liam, The following 3 patches add new features or fix bugs in ad193x drivers. 1/3. add set_pll entry to support different clock input 2/3. fix wrong register setting in ad193x_set_dai_fmt 3/3. fix typo, delete redundant space
All look fine to me.
All Acked-by: Liam Girdwood lrg@slimlogic.co.uk
participants (3)
-
Barry Song -
Liam Girdwood -
Mark Brown