Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1425624 > unrolled thread

[PATCH 4/4] ASoC: tpa6130a2: Add DAPM support

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 4/4] ASoC: tpa6130a2: Add DAPM support Helen Koike <helen.koike@collabora.co.uk> - 2016-06-18 05:40 +0200
    Re: [PATCH 4/4] ASoC: tpa6130a2: Add DAPM support Sebastian Reichel <sre@kernel.org> - 2016-06-19 01:30 +0200
      Re: [PATCH 4/4] ASoC: tpa6130a2: Add DAPM support Lars-Peter Clausen <lars@metafoo.de> - 2016-06-19 11:40 +0200
        Re: [PATCH 4/4] ASoC: tpa6130a2: Add DAPM support Sebastian Reichel <sre@kernel.org> - 2016-06-19 22:10 +0200
    Re: [PATCH 4/4] ASoC: tpa6130a2: Add DAPM support Peter Ujfalusi <peter.ujfalusi@ti.com> - 2016-06-20 10:40 +0200

#1425624 — [PATCH 4/4] ASoC: tpa6130a2: Add DAPM support

Add DAPM support and updated rx51 accordingly.
As a consequence:
- the exported function tpa6130a2_stereo_enable is not needed anymore
- the mutex is dealt in the DAPM
- the power state is tracked by the DAPM

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
[koike: port for upstream]
Signed-off-by: Helen Koike <helen.koike@collabora.co.uk>
---
 sound/soc/codecs/tpa6130a2.c | 184 +++++++++++++++++--------------------------
 sound/soc/codecs/tpa6130a2.h |  11 +--
 sound/soc/omap/rx51.c        |  22 ++----
 3 files changed, 86 insertions(+), 131 deletions(-)

diff --git a/sound/soc/codecs/tpa6130a2.c b/sound/soc/codecs/tpa6130a2.c
index dc7bfc9..7cb5066 100644
--- a/sound/soc/codecs/tpa6130a2.c
+++ b/sound/soc/codecs/tpa6130a2.c
@@ -41,79 +41,71 @@ enum tpa_model {
 	TPA6140A2,
 };
 
-static struct i2c_client *tpa6130a2_client;
-
 /* This struct is used to save the context */
 struct tpa6130a2_data {
-	struct mutex mutex;
 	struct regmap *regmap;
 	struct regulator *supply;
 	int power_gpio;
-	u8 power_state:1;
 	enum tpa_model id;
 };
 
-static int tpa6130a2_power(u8 power)
+static int tpa6130a2_power(struct device *dev, bool enable)
 {
-	struct	tpa6130a2_data *data;
-	int	ret = 0;
-
-	if (WARN_ON(!tpa6130a2_client))
-		return -EINVAL;
-	data = i2c_get_clientdata(tpa6130a2_client);
-
-	mutex_lock(&data->mutex);
-	if (power == data->power_state)
-		goto exit;
+	struct tpa6130a2_data *data = dev_get_drvdata(dev);
+	int ret;
 
-	if (power) {
+	if (enable) {
 		ret = regulator_enable(data->supply);
 		if (ret != 0) {
-			dev_err(&tpa6130a2_client->dev,
-				"Failed to enable supply: %d\n", ret);
-			goto exit;
+			dev_err(dev, "Failed to enable supply: %d\n", ret);
+			return ret;
 		}
 		/* Power on */
 		if (data->power_gpio >= 0)
 			gpio_set_value(data->power_gpio, 1);
-
-		data->power_state = 1;
-		ret = regcache_sync(data->regmap);
-		if (ret < 0) {
-			dev_err(&tpa6130a2_client->dev,
-				"Failed to initialize chip\n");
-			if (data->power_gpio >= 0)
-				gpio_set_value(data->power_gpio, 0);
-			regulator_disable(data->supply);
-			data->power_state = 0;
-			goto exit;
-		}
 	} else {
-		/* set SWS */
-		regmap_update_bits(data->regmap, TPA6130A2_REG_CONTROL,
-			TPA6130A2_SWS, TPA6130A2_SWS);
-
 		/* Power off */
 		if (data->power_gpio >= 0)
 			gpio_set_value(data->power_gpio, 0);
 
 		ret = regulator_disable(data->supply);
 		if (ret != 0) {
-			dev_err(&tpa6130a2_client->dev,
-				"Failed to disable supply: %d\n", ret);
-			goto exit;
+			dev_err(dev, "Failed to disable supply: %d\n", ret);
+			return ret;
 		}
 
-		data->power_state = 0;
 		/* device regs does not match the cache state anymore */
 		regcache_mark_dirty(data->regmap);
 	}
 
-exit:
-	mutex_unlock(&data->mutex);
 	return ret;
 }
 
+static int tpa6130a2_power_event(struct snd_soc_dapm_widget *w,
+				 struct snd_kcontrol *kctrl, int event)
+{
+	struct snd_soc_component *c = snd_soc_dapm_to_component(w->dapm);
+	struct tpa6130a2_data *data = snd_soc_component_get_drvdata(c);
+	int ret;
+
+	/* before widget power up */
+	if (SND_SOC_DAPM_EVENT_ON(event)) {
+		/* Turn on the chip */
+		tpa6130a2_power(c->dev, true);
+		/* Sync the registers */
+		ret = regcache_sync(data->regmap);
+		if (ret < 0) {
+			dev_err(c->dev, "Failed to initialize chip\n");
+			tpa6130a2_power(c->dev, false);
+			return ret;
+		}
+	/* after widget power down */
+	} else
+		tpa6130a2_power(c->dev, false);
+
+	return 0;
+}
+
 /*
  * TPA6130 volume. From -59.5 to 4 dB with increasing step size when going
  * down in gain.
@@ -149,57 +141,6 @@ static const struct snd_kcontrol_new tpa6140a2_controls[] = {
 		       tpa6140_tlv),
 };
 
-/*
- * Enable or disable channel (left or right)
- * The bit number for mute and amplifier are the same per channel:
- * bit 6: Right channel
- * bit 7: Left channel
- * in both registers.
- */
-static void tpa6130a2_channel_enable(u8 channel, int enable)
-{
-	struct tpa6130a2_data *data = i2c_get_clientdata(tpa6130a2_client);
-
-	if (enable) {
-		/* Enable channel */
-		/* Enable amplifier */
-		regmap_update_bits(data->regmap, TPA6130A2_REG_CONTROL,
-			channel | TPA6130A2_SWS, channel & ~TPA6130A2_SWS);
-
-		/* Unmute channel */
-		regmap_update_bits(data->regmap, TPA6130A2_REG_VOL_MUTE,
-				   channel, 0);
-	} else {
-		/* Disable channel */
-		/* Mute channel */
-		regmap_update_bits(data->regmap, TPA6130A2_REG_VOL_MUTE,
-				   channel, channel);
-
-		/* Disable amplifier */
-		regmap_update_bits(data->regmap, TPA6130A2_REG_CONTROL,
-				   channel, 0);
-	}
-}
-
-int tpa6130a2_stereo_enable(struct snd_soc_codec *codec, int enable)
-{
-	int ret = 0;
-	if (enable) {
-		ret = tpa6130a2_power(1);
-		if (ret < 0)
-			return ret;
-		tpa6130a2_channel_enable(TPA6130A2_HP_EN_R | TPA6130A2_HP_EN_L,
-					 1);
-	} else {
-		tpa6130a2_channel_enable(TPA6130A2_HP_EN_R | TPA6130A2_HP_EN_L,
-					 0);
-		ret = tpa6130a2_power(0);
-	}
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(tpa6130a2_stereo_enable);
-
 static int tpa6130a2_component_probe(struct snd_soc_component *component)
 {
 	struct tpa6130a2_data *data = snd_soc_component_get_drvdata(component);
@@ -212,9 +153,47 @@ static int tpa6130a2_component_probe(struct snd_soc_component *component)
 			tpa6130a2_controls, ARRAY_SIZE(tpa6130a2_controls));
 }
 
+static const struct snd_soc_dapm_widget tpa6130a2_dapm_widgets[] = {
+	SND_SOC_DAPM_INPUT("LEFTIN"),
+	SND_SOC_DAPM_INPUT("RIGHTIN"),
+	SND_SOC_DAPM_OUTPUT("HPLEFT"),
+	SND_SOC_DAPM_OUTPUT("HPRIGHT"),
+
+	SND_SOC_DAPM_PGA("Left Mute", TPA6130A2_REG_VOL_MUTE,
+			 TPA6130A2_HP_EN_L_SHIFT, 1, NULL, 0),
+	SND_SOC_DAPM_PGA("Right Mute", TPA6130A2_REG_VOL_MUTE,
+			 TPA6130A2_HP_EN_R_SHIFT, 1, NULL, 0),
+	SND_SOC_DAPM_PGA("Left PGA", TPA6130A2_REG_CONTROL,
+			 TPA6130A2_HP_EN_L_SHIFT, 0, NULL, 0),
+	SND_SOC_DAPM_PGA("Right PGA", TPA6130A2_REG_CONTROL,
+			 TPA6130A2_HP_EN_R_SHIFT, 0, NULL, 0),
+
+	SND_SOC_DAPM_SUPPLY("Power", TPA6130A2_REG_CONTROL,
+			    TPA6130A2_SWS_SHIFT, 1, tpa6130a2_power_event,
+			    SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+};
+
+static const struct snd_soc_dapm_route tpa6130a2_dapm_routes[] = {
+	{ "Left PGA", NULL, "LEFTIN" },
+	{ "Right PGA", NULL, "RIGHTIN" },
+
+	{ "Left Mute", NULL, "Left PGA" },
+	{ "Right Mute", NULL, "Right PGA" },
+
+	{ "HPLEFT", NULL, "Left Mute" },
+	{ "HPRIGHT", NULL, "Right Mute" },
+
+	{ "Left PGA", NULL, "Power" },
+	{ "Right PGA", NULL, "Power" },
+};
+
 struct snd_soc_component_driver tpa6130a2_component_driver = {
 	.name = "tpa6130a2",
 	.probe = tpa6130a2_component_probe,
+	.dapm_widgets = tpa6130a2_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(tpa6130a2_dapm_widgets),
+	.dapm_routes = tpa6130a2_dapm_routes,
+	.num_dapm_routes = ARRAY_SIZE(tpa6130a2_dapm_routes),
 };
 
 static const struct reg_default tpa6130a2_reg_defaults[] = {
@@ -262,14 +241,10 @@ static int tpa6130a2_probe(struct i2c_client *client,
 		return -ENODEV;
 	}
 
-	tpa6130a2_client = client;
-
-	i2c_set_clientdata(tpa6130a2_client, data);
+	i2c_set_clientdata(client, data);
 
 	data->id = id->driver_data;
 
-	mutex_init(&data->mutex);
-
 	if (data->power_gpio >= 0) {
 		ret = devm_gpio_request(dev, data->power_gpio,
 					"tpa6130a2 enable");
@@ -300,7 +275,7 @@ static int tpa6130a2_probe(struct i2c_client *client,
 		goto err_gpio;
 	}
 
-	ret = tpa6130a2_power(1);
+	ret = tpa6130a2_power(dev, true);
 	if (ret != 0)
 		goto err_gpio;
 
@@ -312,7 +287,7 @@ static int tpa6130a2_probe(struct i2c_client *client,
 		dev_warn(dev, "UNTESTED version detected (%d)\n", version);
 
 	/* Disable the chip */
-	ret = tpa6130a2_power(0);
+	ret = tpa6130a2_power(dev, false);
 	if (ret != 0)
 		goto err_gpio;
 
@@ -320,19 +295,9 @@ static int tpa6130a2_probe(struct i2c_client *client,
 			&tpa6130a2_component_driver, NULL, 0);
 
 err_gpio:
-	tpa6130a2_client = NULL;
-
 	return ret;
 }
 
-static int tpa6130a2_remove(struct i2c_client *client)
-{
-	tpa6130a2_power(0);
-	tpa6130a2_client = NULL;
-
-	return 0;
-}
-
 static const struct i2c_device_id tpa6130a2_id[] = {
 	{ "tpa6130a2", TPA6130A2 },
 	{ "tpa6140a2", TPA6140A2 },
@@ -355,7 +320,6 @@ static struct i2c_driver tpa6130a2_i2c_driver = {
 		.of_match_table = of_match_ptr(tpa6130a2_of_match),
 	},
 	.probe = tpa6130a2_probe,
-	.remove = tpa6130a2_remove,
 	.id_table = tpa6130a2_id,
 };
 
diff --git a/sound/soc/codecs/tpa6130a2.h b/sound/soc/codecs/tpa6130a2.h
index ef05a3f..f19cad5 100644
--- a/sound/soc/codecs/tpa6130a2.h
+++ b/sound/soc/codecs/tpa6130a2.h
@@ -32,15 +32,18 @@
 
 /* Register bits */
 /* TPA6130A2_REG_CONTROL (0x01) */
-#define TPA6130A2_SWS			(0x01 << 0)
+#define TPA6130A2_SWS_SHIFT		0
+#define TPA6130A2_SWS			(0x01 << TPA6130A2_SWS_SHIFT)
 #define TPA6130A2_TERMAL		(0x01 << 1)
 #define TPA6130A2_MODE(x)		(x << 4)
 #define TPA6130A2_MODE_STEREO		(0x00)
 #define TPA6130A2_MODE_DUAL_MONO	(0x01)
 #define TPA6130A2_MODE_BRIDGE		(0x02)
 #define TPA6130A2_MODE_MASK		(0x03)
-#define TPA6130A2_HP_EN_R		(0x01 << 6)
-#define TPA6130A2_HP_EN_L		(0x01 << 7)
+#define TPA6130A2_HP_EN_R_SHIFT		6
+#define TPA6130A2_HP_EN_R		(0x01 << TPA6130A2_HP_EN_R_SHIFT)
+#define TPA6130A2_HP_EN_L_SHIFT		7
+#define TPA6130A2_HP_EN_L		(0x01 << TPA6130A2_HP_EN_L_SHIFT)
 
 /* TPA6130A2_REG_VOL_MUTE (0x02) */
 #define TPA6130A2_VOLUME(x)		((x & 0x3f) << 0)
@@ -54,6 +57,4 @@
 /* TPA6130A2_REG_VERSION (0x04) */
 #define TPA6130A2_VERSION_MASK		(0x0f)
 
-extern int tpa6130a2_stereo_enable(struct snd_soc_codec *codec, int enable);
-
 #endif /* __TPA6130A2_H__ */
diff --git a/sound/soc/omap/rx51.c b/sound/soc/omap/rx51.c
index b59cf89..8c29a34 100644
--- a/sound/soc/omap/rx51.c
+++ b/sound/soc/omap/rx51.c
@@ -164,19 +164,6 @@ static int rx51_spk_event(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
-static int rx51_hp_event(struct snd_soc_dapm_widget *w,
-			 struct snd_kcontrol *k, int event)
-{
-	struct snd_soc_codec *codec = snd_soc_dapm_to_codec(w->dapm);
-
-	if (SND_SOC_DAPM_EVENT_ON(event))
-		tpa6130a2_stereo_enable(codec, 1);
-	else
-		tpa6130a2_stereo_enable(codec, 0);
-
-	return 0;
-}
-
 static int rx51_get_input(struct snd_kcontrol *kcontrol,
 			  struct snd_ctl_elem_value *ucontrol)
 {
@@ -235,7 +222,7 @@ static struct snd_soc_jack_gpio rx51_av_jack_gpios[] = {
 static const struct snd_soc_dapm_widget aic34_dapm_widgets[] = {
 	SND_SOC_DAPM_SPK("Ext Spk", rx51_spk_event),
 	SND_SOC_DAPM_MIC("DMic", NULL),
-	SND_SOC_DAPM_HP("Headphone Jack", rx51_hp_event),
+	SND_SOC_DAPM_HP("Headphone Jack", NULL),
 	SND_SOC_DAPM_MIC("HS Mic", NULL),
 	SND_SOC_DAPM_LINE("FM Transmitter", NULL),
 	SND_SOC_DAPM_SPK("Earphone", NULL),
@@ -246,11 +233,14 @@ static const struct snd_soc_dapm_route audio_map[] = {
 	{"Ext Spk", NULL, "HPROUT"},
 	{"Ext Spk", NULL, "HPLCOM"},
 	{"Ext Spk", NULL, "HPRCOM"},
-	{"Headphone Jack", NULL, "LLOUT"},
-	{"Headphone Jack", NULL, "RLOUT"},
 	{"FM Transmitter", NULL, "LLOUT"},
 	{"FM Transmitter", NULL, "RLOUT"},
 
+	{"Headphone Jack", NULL, "TPA6130A2 HPLEFT"},
+	{"Headphone Jack", NULL, "TPA6130A2 HPRIGHT"},
+	{"TPA6130A2 HPLEFT", NULL, "LLOUT"},
+	{"TPA6130A2 HPRIGHT", NULL, "RLOUT"},
+
 	{"DMic Rate 64", NULL, "DMic"},
 	{"DMic", NULL, "Mic Bias"},
 
-- 
1.9.1

[toc] | [next] | [standalone]

#1425874

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Sat, Jun 18, 2016 at 12:26:31AM -0300, Helen Koike wrote:
> Add DAPM support and updated rx51 accordingly.
> As a consequence:
> - the exported function tpa6130a2_stereo_enable is not needed anymore
> - the mutex is dealt in the DAPM
> - the power state is tracked by the DAPM

This breaks headphone on N900. So far I only checked, that
tpa6130a2_power_event() is not called. I guess the DAPM
graph is wrong.

-- Sebastian

[toc] | [prev] | [next] | [standalone]

#1425952

[Multipart message — attachments visible in raw view] — view raw

On 06/19/2016 01:21 AM, Sebastian Reichel wrote:
> Hi,
> 
> On Sat, Jun 18, 2016 at 12:26:31AM -0300, Helen Koike wrote:
>> Add DAPM support and updated rx51 accordingly.
>> As a consequence:
>> - the exported function tpa6130a2_stereo_enable is not needed anymore
>> - the mutex is dealt in the DAPM
>> - the power state is tracked by the DAPM
> 
> This breaks headphone on N900. So far I only checked, that
> tpa6130a2_power_event() is not called. I guess the DAPM
> graph is wrong.

Can you try with

+	{"TPA6130A2 HPLEFT", NULL, "LLOUT"},
+	{"TPA6130A2 HPRIGHT", NULL, "RLOUT"}

being replaced with

+       {"TPA6130A2 LEFTIN", NULL, "LLOUT"},
+       {"TPA6130A2 RIGHTIN", NULL, "RLOUT"},

Thanks,
- Lars

[toc] | [prev] | [next] | [standalone]

#1426031

[Multipart message — attachments visible in raw view] — view raw

Hi,

On Sun, Jun 19, 2016 at 11:39:15AM +0200, Lars-Peter Clausen wrote:
> On 06/19/2016 01:21 AM, Sebastian Reichel wrote:
> > On Sat, Jun 18, 2016 at 12:26:31AM -0300, Helen Koike wrote:
> >> Add DAPM support and updated rx51 accordingly.
> >> As a consequence:
> >> - the exported function tpa6130a2_stereo_enable is not needed anymore
> >> - the mutex is dealt in the DAPM
> >> - the power state is tracked by the DAPM
> > 
> > This breaks headphone on N900. So far I only checked, that
> > tpa6130a2_power_event() is not called. I guess the DAPM
> > graph is wrong.
> 
> Can you try with
> 
> +	{"TPA6130A2 HPLEFT", NULL, "LLOUT"},
> +	{"TPA6130A2 HPRIGHT", NULL, "RLOUT"}
> 
> being replaced with
> 
> +       {"TPA6130A2 LEFTIN", NULL, "LLOUT"},
> +       {"TPA6130A2 RIGHTIN", NULL, "RLOUT"},

With that change:

Tested-By: Sebastian Reichel <sre@kernel.org>
Reviewed-By: Sebastian Reichel <sre@kernel.org>

-- Sebastian

[toc] | [prev] | [next] | [standalone]

#1426330

On 06/18/2016 07:42 AM, Helen Koike wrote:
> Add DAPM support and updated rx51 accordingly.
> As a consequence:
> - the exported function tpa6130a2_stereo_enable is not needed anymore
> - the mutex is dealt in the DAPM
> - the power state is tracked by the DAPM
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> [koike: port for upstream]
> Signed-off-by: Helen Koike <helen.koike@collabora.co.uk>
> ---
>  sound/soc/codecs/tpa6130a2.c | 184 +++++++++++++++++--------------------------
>  sound/soc/codecs/tpa6130a2.h |  11 +--
>  sound/soc/omap/rx51.c        |  22 ++----
>  3 files changed, 86 insertions(+), 131 deletions(-)
> 
> diff --git a/sound/soc/codecs/tpa6130a2.c b/sound/soc/codecs/tpa6130a2.c
> index dc7bfc9..7cb5066 100644
> --- a/sound/soc/codecs/tpa6130a2.c
> +++ b/sound/soc/codecs/tpa6130a2.c
> @@ -41,79 +41,71 @@ enum tpa_model {
>  	TPA6140A2,
>  };
>  
> -static struct i2c_client *tpa6130a2_client;
> -
>  /* This struct is used to save the context */
>  struct tpa6130a2_data {
> -	struct mutex mutex;
>  	struct regmap *regmap;
>  	struct regulator *supply;
>  	int power_gpio;
> -	u8 power_state:1;
>  	enum tpa_model id;
>  };
>  
> -static int tpa6130a2_power(u8 power)
> +static int tpa6130a2_power(struct device *dev, bool enable)

I would rather add 'struct device *dev' to tpa6130a2_data and pass the pointer
to the private struct instead to the device.

>  {
> -	struct	tpa6130a2_data *data;
> -	int	ret = 0;
> -
> -	if (WARN_ON(!tpa6130a2_client))
> -		return -EINVAL;
> -	data = i2c_get_clientdata(tpa6130a2_client);
> -
> -	mutex_lock(&data->mutex);
> -	if (power == data->power_state)
> -		goto exit;
> +	struct tpa6130a2_data *data = dev_get_drvdata(dev);
> +	int ret;
>  
> -	if (power) {
> +	if (enable) {

-- 
Péter

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web