[PATCH 0/3] ad193x set_pll, fix register setting and typo

[PATCH 0/3] ad193x set_pll, fix register setting and typo

[Barry Song]

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

[PATCH 1/3] ad193x: add set_pll entry to support different clock input

[Barry Song]

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
-- 
1.5.6.3

[PATCH 2/3] ad193x: fix wrong register setting in ad193x_set_dai_fmt

[Barry Song]

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;
-- 
1.5.6.3

[PATCH 3/3] ad193x: fix typo, delete redundant space

[Barry Song]

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 */
-- 
1.5.6.3

Re: [PATCH 0/3] ad193x set_pll, fix register setting and typo

[Liam Girdwood]

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>

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

Re: [PATCH 1/3] ad193x: add set_pll entry to support different clock input

[Mark Brown]

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.

Re: [PATCH 2/3] ad193x: fix wrong register setting in ad193x_set_dai_fmt

[Mark Brown]

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.

Re: [PATCH 3/3] ad193x: fix typo, delete redundant space

[Mark Brown]

On Wed, Apr 21, 2010 at 05:36:49PM +0800, Barry Song wrote:
> Signed-off-by: Barry Song <21cnbao@gmail.com>

Applied, thanks.

Re: [PATCH 2/3] ad193x: fix wrong register setting in ad193x_set_dai_fmt

[Barry Song]

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.
>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 1/3] ad193x: add set_pll entry to support different clock input

[Barry Song]

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().

>
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel