[alsa-devel] [PATCH 1/2] ASoC: rt5651: Add support for external amplifier enable GPIO
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(+)
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;
The rt5651 does not have a built-in speaker amplifier, so it is often used together with an external amplifier. On Cherry Trail boards this external amplifier's enable pin is driven through a GPIO, which is given as the first GPIO in the ACPI resources of the codec fwnode.
This commit adds a mapping from the first GPIO to "ext-amp-enable-gpios" to the codec device, so that the codec driver can find and contorl the GPIO.
Signed-off-by: Hans de Goede hdegoede@redhat.com --- sound/soc/intel/boards/bytcr_rt5651.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/sound/soc/intel/boards/bytcr_rt5651.c b/sound/soc/intel/boards/bytcr_rt5651.c index 987720e203f9..925dfb46b4fe 100644 --- a/sound/soc/intel/boards/bytcr_rt5651.c +++ b/sound/soc/intel/boards/bytcr_rt5651.c @@ -26,8 +26,10 @@ #include <linux/clk.h> #include <linux/device.h> #include <linux/dmi.h> +#include <linux/gpio/machine.h> #include <linux/slab.h> #include <asm/cpu_device_id.h> +#include <asm/intel-family.h> #include <asm/platform_sst_audio.h> #include <sound/pcm.h> #include <sound/pcm_params.h> @@ -414,6 +416,18 @@ static const struct dmi_system_id byt_rt5651_quirk_table[] = { {} };
+static const struct x86_cpu_id cherrytrail_cpu_ids[] = { + { X86_VENDOR_INTEL, 6, INTEL_FAM6_ATOM_AIRMONT }, /* Braswell, CHT */ + {} +}; + +static const struct acpi_gpio_params ext_amp_enable_gpios = { 0, 0, false }; + +static const struct acpi_gpio_mapping byt_rt5651_gpios[] = { + { "ext-amp-enable-gpios", &ext_amp_enable_gpios, 1 }, + { }, +}; + /* * Note this MUST be called before snd_soc_register_card(), so that the props * are in place before the codec component driver's probe function parses them. @@ -441,8 +455,15 @@ static int byt_rt5651_add_codec_device_props(const char *i2c_dev_name) props[cnt++] = PROPERTY_ENTRY_BOOL("realtek,dmic-en");
ret = device_add_properties(i2c_dev, props); - put_device(i2c_dev); + if (ret) + goto out;
+ /* Cherry Trail devices use an external amplifier enable gpio */ + if (x86_match_cpu(cherrytrail_cpu_ids)) + ret = devm_acpi_dev_add_driver_gpios(i2c_dev, byt_rt5651_gpios); + + put_device(i2c_dev); +out: return ret; }
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.
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
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.
participants (2)
-
Hans de Goede -
Mark Brown