[PATCH] Add support for tlv320aic3007 to tlv320aic3x driver

[PATCH] Add support for tlv320aic3007 to tlv320aic3x driver

[Randolph Chung]

[-- Attachment #1: Type: text/plain, Size: 373 bytes --]

The attached patch adds support for the tlv320aic3007 codec to the
tlv320aic3x driver.

The tlv320aic3007 is similar to the aic31, but has an additional
class-D speaker amp. The speaker amp control register overlaps with
the mono output register of other codecs in this family, so we allow
the board driver to pass in a model id to identify the codec being
used.

randolph

[-- Attachment #2: tlv320aic3x.diff --]
[-- Type: application/octet-stream, Size: 6112 bytes --]

Add support for tlv320aic3007 codec.

Signed-off-by: Randolph Chung <tausq@parisc-linux.org>

diff --git a/include/sound/tlv320aic3x.h b/include/sound/tlv320aic3x.h
index b1a5f34..0c29303 100644
--- a/include/sound/tlv320aic3x.h
+++ b/include/sound/tlv320aic3x.h
@@ -12,6 +12,8 @@
 
 struct aic3x_pdata {
 	int gpio_reset; /* < 0 if not used */
+#define AIC3X_MODEL_3007 1  /* tlv320aic3007 */
+    u16 model;
 };
 
 #endif
diff --git a/sound/soc/codecs/tlv320aic3x.c b/sound/soc/codecs/tlv320aic3x.c
index 63cad43..3fa1d6e 100644
--- a/sound/soc/codecs/tlv320aic3x.c
+++ b/sound/soc/codecs/tlv320aic3x.c
@@ -12,11 +12,11 @@
  *
  * Notes:
  *  The AIC3X is a driver for a low power stereo audio
- *  codecs aic31, aic32, aic33.
+ *  codecs aic31, aic32, aic33, aic3007.
  *
  *  It supports full aic33 codec functionality.
  *  The compatibility with aic32, aic31 is as follows:
- *        aic32        |        aic31
+ *    aic32/aic3007    |        aic31
  *  ---------------------------------------
  *   MONO_LOUT -> N/A  |  MONO_LOUT -> N/A
  *                     |  IN1L -> LINE1L
@@ -68,6 +68,7 @@ struct aic3x_priv {
 	unsigned int sysclk;
 	int master;
 	int gpio_reset;
+    u16 model;
 };
 
 /*
@@ -241,6 +242,7 @@ static const char *aic3x_right_hpcom_mux[] =
 static const char *aic3x_linein_mode_mux[] = { "single-ended", "differential" };
 static const char *aic3x_adc_hpf[] =
     { "Disabled", "0.0045xFs", "0.0125xFs", "0.025xFs" };
+static const char *aic3x_classd_amp_gain[] = { "0dB", "6dB", "12dB", "18dB" };
 
 #define LDAC_ENUM	0
 #define RDAC_ENUM	1
@@ -264,6 +266,9 @@ static const struct soc_enum aic3x_enum[] = {
 	SOC_ENUM_DOUBLE(AIC3X_CODEC_DFILT_CTRL, 6, 4, 4, aic3x_adc_hpf),
 };
 
+static const struct soc_enum aic3x_classd_amp_gain_enum =
+	SOC_ENUM_DOUBLE(CLASSD_CTRL, 6, 4, 4, aic3x_classd_amp_gain);
+
 /*
  * DAC digital volumes. From -63.5 to 0 dB in 0.5 dB steps
  */
@@ -359,6 +364,9 @@ static const struct snd_kcontrol_new aic3x_snd_controls[] = {
 	SOC_ENUM("ADC HPF Cut-off", aic3x_enum[ADC_HPF_ENUM]),
 };
 
+static const struct snd_kcontrol_new aic3x_classd_amp_gain_ctrl =
+	SOC_ENUM("Class-D Amplifier Gain", aic3x_classd_amp_gain_enum);
+
 /* Left DAC Mux */
 static const struct snd_kcontrol_new aic3x_left_dac_mux_controls =
 SOC_DAPM_ENUM("Route", aic3x_enum[LDAC_ENUM]);
@@ -587,6 +595,15 @@ static const struct snd_soc_dapm_widget aic3x_dapm_widgets[] = {
 	SND_SOC_DAPM_INPUT("LINE2R"),
 };
 
+static const struct snd_soc_dapm_widget aic3007_dapm_widgets[] = {
+	/* Class-D outputs */
+	SND_SOC_DAPM_PGA("Left Class-D Out", CLASSD_CTRL, 3, 0, NULL, 0),
+	SND_SOC_DAPM_PGA("Right Class-D Out", CLASSD_CTRL, 2, 0, NULL, 0),
+
+	SND_SOC_DAPM_OUTPUT("SPOP"),
+	SND_SOC_DAPM_OUTPUT("SPOM"),
+};
+
 static const struct snd_soc_dapm_route intercon[] = {
 	/* Left Output */
 	{"Left DAC Mux", "DAC_L1", "Left DAC"},
@@ -757,14 +774,30 @@ static const struct snd_soc_dapm_route intercon[] = {
 	{"GPIO1 dmic modclk", NULL, "DMic Rate 32"},
 };
 
+static const struct snd_soc_dapm_route intercon_3007[] = {
+	/* Class-D outputs */
+	{"Left Class-D Out", NULL, "Left Line Out"},
+	{"Right Class-D Out", NULL, "Left Line Out"},
+	{"SPOP", NULL, "Left Class-D Out"},
+	{"SPOM", NULL, "Right Class-D Out"},
+};
+
 static int aic3x_add_widgets(struct snd_soc_codec *codec)
 {
+	struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
+
 	snd_soc_dapm_new_controls(codec, aic3x_dapm_widgets,
 				  ARRAY_SIZE(aic3x_dapm_widgets));
 
 	/* set up audio path interconnects */
 	snd_soc_dapm_add_routes(codec, intercon, ARRAY_SIZE(intercon));
 
+	if (aic3x->model == AIC3X_MODEL_3007) {
+		snd_soc_dapm_new_controls(codec, aic3007_dapm_widgets,
+			ARRAY_SIZE(aic3007_dapm_widgets));
+		snd_soc_dapm_add_routes(codec, intercon_3007, ARRAY_SIZE(intercon_3007));
+    }
+
 	return 0;
 }
 
@@ -1156,6 +1189,7 @@ static int aic3x_resume(struct platform_device *pdev)
  */
 static int aic3x_init(struct snd_soc_codec *codec)
 {
+	struct aic3x_priv *aic3x = snd_soc_codec_get_drvdata(codec);
 	int reg;
 
 	mutex_init(&codec->mutex);
@@ -1240,6 +1274,16 @@ static int aic3x_init(struct snd_soc_codec *codec)
 	aic3x_write(codec, LINE2L_2_MONOLOPM_VOL, DEFAULT_VOL);
 	aic3x_write(codec, LINE2R_2_MONOLOPM_VOL, DEFAULT_VOL);
 
+	if (aic3x->model == AIC3X_MODEL_3007) {
+		/* Class-D speaker driver init; datasheet p. 46 */
+		aic3x_write(codec, AIC3X_PAGE_SELECT, 0x0D);
+		aic3x_write(codec, 0xD, 0x0D);
+		aic3x_write(codec, 0x8, 0x5C);
+		aic3x_write(codec, 0x8, 0x5D);
+		aic3x_write(codec, 0x8, 0x5C);
+		aic3x_write(codec, AIC3X_PAGE_SELECT, 0x00);
+	}
+
 	/* off, with power on */
 	aic3x_set_bias_level(codec, SND_SOC_BIAS_STANDBY);
 
@@ -1336,6 +1380,8 @@ static int aic3x_i2c_probe(struct i2c_client *i2c,
 		aic3x->gpio_reset = pdata->gpio_reset;
 		gpio_direction_output(aic3x->gpio_reset, 0);
 	}
+	if (pdata)
+		aic3x->model = pdata->model;
 
 	for (i = 0; i < ARRAY_SIZE(aic3x->supplies); i++)
 		aic3x->supplies[i].supply = aic3x_supply_names[i];
@@ -1420,6 +1466,7 @@ static int aic3x_probe(struct platform_device *pdev)
 	struct snd_soc_device *socdev = platform_get_drvdata(pdev);
 	struct aic3x_setup_data *setup;
 	struct snd_soc_codec *codec;
+	struct aic3x_priv *aic3x;
 	int ret = 0;
 
 	codec = aic3x_codec;
@@ -1449,6 +1496,9 @@ static int aic3x_probe(struct platform_device *pdev)
 	snd_soc_add_controls(codec, aic3x_snd_controls,
 			     ARRAY_SIZE(aic3x_snd_controls));
 
+	aic3x = snd_soc_codec_get_drvdata(codec);
+	if (aic3x->model == AIC3X_MODEL_3007)
+		snd_soc_add_controls(codec, &aic3x_classd_amp_gain_ctrl, 1);
 	aic3x_add_widgets(codec);
 
 	return ret;
diff --git a/sound/soc/codecs/tlv320aic3x.h b/sound/soc/codecs/tlv320aic3x.h
index 9af1c88..19ef7dc 100644
--- a/sound/soc/codecs/tlv320aic3x.h
+++ b/sound/soc/codecs/tlv320aic3x.h
@@ -111,6 +111,8 @@
 #define DACL1_2_MONOLOPM_VOL		75
 #define DACR1_2_MONOLOPM_VOL		78
 #define MONOLOPM_CTRL			79
+/* Class-D speaker driver on tlv320aic3007 */
+#define CLASSD_CTRL			73
 /* Line Output Plus/Minus control registers */
 #define LINE2L_2_LLOPM_VOL		80
 #define LINE2L_2_RLOPM_VOL		87

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH] Add support for tlv320aic3007 to tlv320aic3x driver

[Mark Brown]

On Thu, Aug 19, 2010 at 02:19:16PM +0800, Randolph Chung wrote:
> The attached patch adds support for the tlv320aic3007 codec to the
> tlv320aic3x driver.
> 
> The tlv320aic3007 is similar to the aic31, but has an additional
> class-D speaker amp. The speaker amp control register overlaps with
> the mono output register of other codecs in this family, so we allow
> the board driver to pass in a model id to identify the codec being
> used.

Rather than passing something in the platform data you should use the
I2C device ID table for the device and have the board register the
device as a tlv320aic3007.

I would review the patch in more detail but you attached it as a base64
encoded attachment which means my MUA hasn't quoted it for me and won't
even display it unless I explicitly open it...  The main thing I noticed
was that you're using an enum instead of a TLV control for the amp
volume.

As I reminded you last time you always need to CC maintainers on
patches.  This is very helpful for ensuring that we actually see things
on busy lists and is helpful for keeping track of what needs reviewing.

Re: [PATCH] Add support for tlv320aic3007 to tlv320aic3x driver

[Randolph Chung]

> Rather than passing something in the platform data you should use the
> I2C device ID table for the device and have the board register the
> device as a tlv320aic3007.

Yes, that will work too - if that's the preference I will rework the
patch that way.

> I would review the patch in more detail but you attached it as a base64
> encoded attachment which means my MUA hasn't quoted it for me and won't
> even display it unless I explicitly open it...

My apologies - since you reported whitespace damage in my previous
post I included the patch as an attachment and apparently my mail
client decided to base64 encode it. I will post followups from another
MUA.

> The main thing I noticed
> was that you're using an enum instead of a TLV control for the amp
> volume.

The amp volume (gain) only has 4 steps (0, 6dB, 12dB, 18dB). I can
rework as a TLV if that's the preference. TI has a (non-published?)
driver for this codec that used an enum so this part was copied over.

> As I reminded you last time you always need to CC maintainers on
> patches.  This is very helpful for ensuring that we actually see things
> on busy lists and is helpful for keeping track of what needs reviewing.

You copied my last patch to lrg (and he acked the patch), so I copied
my patch to lrg. Previous patches to this driver were also signed off
by him. Should I copy to somebody else?

randolph

Re: [PATCH] Add support for tlv320aic3007 to tlv320aic3x driver

[Mark Brown]

On Thu, Aug 19, 2010 at 06:00:29PM +0800, Randolph Chung wrote:
> > Rather than passing something in the platform data you should use the
> > I2C device ID table for the device and have the board register the
> > device as a tlv320aic3007.

> Yes, that will work too - if that's the preference I will rework the
> patch that way.

Yes, please.

> > I would review the patch in more detail but you attached it as a base64
> > encoded attachment which means my MUA hasn't quoted it for me and won't
> > even display it unless I explicitly open it...

> My apologies - since you reported whitespace damage in my previous
> post I included the patch as an attachment and apparently my mail
> client decided to base64 encode it. I will post followups from another
> MUA.

git send-email is usually the way forward.

> > The main thing I noticed
> > was that you're using an enum instead of a TLV control for the amp
> > volume.

> The amp volume (gain) only has 4 steps (0, 6dB, 12dB, 18dB). I can
> rework as a TLV if that's the preference. TI has a (non-published?)
> driver for this codec that used an enum so this part was copied over.

The number of steps doesn't make any difference - the advantage of a
proper " Volume" control with TLV information is that applications can
understand and interpret the control, displaying it appropriately in UIs
and so on.

Non-mainline drivers aren't a terribly reliable guide since they haven't
been reviewed.

> > As I reminded you last time you always need to CC maintainers on
> > patches.  This is very helpful for ensuring that we actually see things
> > on busy lists and is helpful for keeping track of what needs reviewing.

> You copied my last patch to lrg (and he acked the patch), so I copied
> my patch to lrg. Previous patches to this driver were also signed off
> by him. Should I copy to somebody else?

Me.  Liam and myself are comaintainers for ASoC.  See MAINTAINERS (there
is a script scripts/get_maintainers.pl in the kernel which can parse
this automatically).