diff mbox

[1/2] ASoC: rt5651: Add support for external amplifier enable GPIO

Message ID 20180624130247.18150-1-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hans de Goede June 24, 2018, 1:02 p.m. UTC 
The rt5651 does not have a built-in speaker amplifier, so it is often
used together with an external amplifier. On some boards the external
amplifier's enable pin is driven through a GPIO, add support for this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 sound/soc/codecs/rt5651.c | 43 +++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/rt5651.h |  2 ++
 2 files changed, 45 insertions(+)

Comments

Mark Brown June 25, 2018, 10:45 a.m. UTC | #1 
On Sun, Jun 24, 2018 at 03:02:46PM +0200, Hans de Goede wrote:
> The rt5651 does not have a built-in speaker amplifier, so it is often
> used together with an external amplifier. On some boards the external
> amplifier's enable pin is driven through a GPIO, add support for this.

This obviously shouldn't be part of the CODEC driver since it's not part
of the device, you should make a driver for GPIO controlled amplifiers
and add it to the card.  There's actually a guy I've been talking to a
bit on IRC recently who's just written one actually and will hopefully
be posting it soon.
Hans de Goede June 25, 2018, 12:19 p.m. UTC | #2 
Hi,

On 25-06-18 12:45, Mark Brown wrote:
> On Sun, Jun 24, 2018 at 03:02:46PM +0200, Hans de Goede wrote:
>> The rt5651 does not have a built-in speaker amplifier, so it is often
>> used together with an external amplifier. On some boards the external
>> amplifier's enable pin is driven through a GPIO, add support for this.
> 
> This obviously shouldn't be part of the CODEC driver since it's not part
> of the device, you should make a driver for GPIO controlled amplifiers
> and add it to the card.  There's actually a guy I've been talking to a
> bit on IRC recently who's just written one actually and will hopefully
> be posting it soon.

Hmm, that seems like a complicated solution to use for non DT platforms
I already need to deal with this in the sound/soc/intel/boards/bytcr_rt5651.c
machine driver (call devm_acpi_dev_add_driver_gpios), so I'm tempted to
model this a supply for the speaker (which it in essence is) inside the
machine driver and make that supply directly control the GPIO, would
that be acceptable?

Regards,

Hans
Mark Brown June 25, 2018, 12:40 p.m. UTC | #3 
On Mon, Jun 25, 2018 at 02:19:02PM +0200, Hans de Goede wrote:

> Hmm, that seems like a complicated solution to use for non DT platforms

It should be fine for board file systems as well.  I think it's just
ACPI that's painful here :(

> I already need to deal with this in the sound/soc/intel/boards/bytcr_rt5651.c
> machine driver (call devm_acpi_dev_add_driver_gpios), so I'm tempted to
> model this a supply for the speaker (which it in essence is) inside the
> machine driver and make that supply directly control the GPIO, would
> that be acceptable?

Sure.
diff mbox

Patch

diff --git a/sound/soc/codecs/rt5651.c b/sound/soc/codecs/rt5651.c
index 6b5669f3e85d..e5db8883ee9f 100644
--- a/sound/soc/codecs/rt5651.c
+++ b/sound/soc/codecs/rt5651.c
@@ -19,6 +19,7 @@ 
 #include <linux/platform_device.h>
 #include <linux/spi/spi.h>
 #include <linux/acpi.h>
+#include <linux/gpio/consumer.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -717,6 +718,26 @@  static int rt5651_amp_power_event(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
+static int rt5651_ext_amp_power_event(struct snd_soc_dapm_widget *w,
+	struct snd_kcontrol *kcontrol, int event)
+{
+	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
+	struct rt5651_priv *rt5651 = snd_soc_component_get_drvdata(component);
+
+	switch (event) {
+	case SND_SOC_DAPM_POST_PMU:
+		gpiod_set_value_cansleep(rt5651->ext_amp_gpio, 1);
+		break;
+	case SND_SOC_DAPM_PRE_PMD:
+		gpiod_set_value_cansleep(rt5651->ext_amp_gpio, 0);
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
 static int rt5651_hp_event(struct snd_soc_dapm_widget *w,
 	struct snd_kcontrol *kcontrol, int event)
 {
@@ -1057,6 +1078,9 @@  static const struct snd_soc_dapm_widget rt5651_dapm_widgets[] = {
 	SND_SOC_DAPM_SUPPLY("Amp Power", RT5651_PWR_ANLG1,
 			    RT5651_PWR_HA_BIT, 0, rt5651_amp_power_event,
 			    SND_SOC_DAPM_POST_PMU),
+	SND_SOC_DAPM_SUPPLY("Ext Amp Power", SND_SOC_NOPM, 0, 0,
+			    rt5651_ext_amp_power_event,
+			    SND_SOC_DAPM_PRE_PMD | SND_SOC_DAPM_POST_PMU),
 	SND_SOC_DAPM_PGA_S("HP Amp", 1, SND_SOC_NOPM, 0, 0, rt5651_hp_event,
 			   SND_SOC_DAPM_PRE_PMD | SND_SOC_DAPM_POST_PMU),
 	SND_SOC_DAPM_SWITCH("HPO L Playback", SND_SOC_NOPM, 0, 0,
@@ -1272,8 +1296,10 @@  static const struct snd_soc_dapm_route rt5651_dapm_routes[] = {
 	{"LOUT R Playback", "Switch", "LOUT MIX"},
 	{"LOUTL", NULL, "LOUT L Playback"},
 	{"LOUTL", NULL, "Amp Power"},
+	{"LOUTL", NULL, "Ext Amp Power"},
 	{"LOUTR", NULL, "LOUT R Playback"},
 	{"LOUTR", NULL, "Amp Power"},
+	{"LOUTR", NULL, "Ext Amp Power"},
 
 	{"PDML", NULL, "PDM L Mux"},
 	{"PDMR", NULL, "PDM R Mux"},
@@ -1857,6 +1883,23 @@  static int rt5651_probe(struct snd_soc_component *component)
 
 	rt5651_apply_properties(component);
 
+	/*
+	 * Needs to be done from component-probe() and not i2c-probe(), so that
+	 * the platform driver can add GPIO mappings if necessary.
+	 */
+	rt5651->ext_amp_gpio = devm_gpiod_get_optional(component->dev,
+						       "ext-amp-enable",
+						       GPIOD_OUT_LOW);
+	if (IS_ERR(rt5651->ext_amp_gpio)) {
+		int err = PTR_ERR(rt5651->ext_amp_gpio);
+
+		if (err != -EPROBE_DEFER)
+			dev_err(component->dev, "Failed to get ext-amp GPIO: %d\n",
+				err);
+
+		return err;
+	}
+
 	return 0;
 }
 
diff --git a/sound/soc/codecs/rt5651.h b/sound/soc/codecs/rt5651.h
index 3a0968c53fde..267a71395bd7 100644
--- a/sound/soc/codecs/rt5651.h
+++ b/sound/soc/codecs/rt5651.h
@@ -12,6 +12,7 @@ 
 #ifndef __RT5651_H__
 #define __RT5651_H__
 
+#include <linux/gpio/consumer.h>
 #include <dt-bindings/sound/rt5651.h>
 
 /* Info */
@@ -2071,6 +2072,7 @@  struct rt5651_pll_code {
 struct rt5651_priv {
 	struct snd_soc_component *component;
 	struct regmap *regmap;
+	struct gpio_desc *ext_amp_gpio;
 	struct snd_soc_jack *hp_jack;
 	struct work_struct jack_detect_work;
 	unsigned int jd_src;