| Message ID | 20220301123742.72146-3-stephan@gerhold.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ASoC: codecs: Add Awinic AW8738 audio amplifier driver | expand |
On Tue, Mar 01, 2022 at 01:37:42PM +0100, Stephan Gerhold wrote: > From: Jonathan Albrieux <jonathan.albrieux@gmail.com> > > The Awinic AW8738 is a simple audio amplifier using an enable GPIO. > The main difference to simple-amplifier is that there is a "one-wire > pulse control" that allows configuring the amplifier to one of a few > pre-defined modes. I only have this patch, nothing else from the rest of the series. What's the story with dependencies?
On Tue, Mar 01, 2022 at 12:47:10PM +0000, Mark Brown wrote: > On Tue, Mar 01, 2022 at 01:37:42PM +0100, Stephan Gerhold wrote: > > From: Jonathan Albrieux <jonathan.albrieux@gmail.com> > > > > The Awinic AW8738 is a simple audio amplifier using an enable GPIO. > > The main difference to simple-amplifier is that there is a "one-wire > > pulse control" that allows configuring the amplifier to one of a few > > pre-defined modes. > > I only have this patch, nothing else from the rest of the series. > What's the story with dependencies? Hmm, I definitely sent the whole series to you. Let's wait a bit longer to see if it will still arrive, otherwise let me know and I can try to send it again. It seems to have arrived fully on the mailing list: https://lore.kernel.org/alsa-devel/20220301123742.72146-1-stephan@gerhold.net/ Thanks, Stephan
On Tue, Mar 01, 2022 at 01:53:12PM +0100, Stephan Gerhold wrote: > On Tue, Mar 01, 2022 at 12:47:10PM +0000, Mark Brown wrote: > > I only have this patch, nothing else from the rest of the series. > > What's the story with dependencies? > Hmm, I definitely sent the whole series to you. Let's wait a bit longer > to see if it will still arrive, otherwise let me know and I can try to > send it again. Yes, it's turned up OK now - wonder what delayed the earlier bits.
On Tue, Mar 01, 2022 at 01:37:42PM +0100, Stephan Gerhold wrote: > The Awinic AW8738 is a simple audio amplifier using an enable GPIO. > The main difference to simple-amplifier is that there is a "one-wire > pulse control" that allows configuring the amplifier to one of a few > pre-defined modes. What exactly are the modes here? Looking at the web site for the part it seems like it's selecting a power limit for the speaker so it makes sense that the mode would be fixed in DT but it's not clear from the driver. > + aw->gpiod_enable = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW); > + if (IS_ERR(aw->gpiod_enable)) > + return dev_err_probe(dev, PTR_ERR(aw->gpiod_enable), > + "Failed to get 'enable' gpio"); Are we sure that enable is the best name for this pin? It's more complex than just an enable since it's the 1 wire data - according to what's on the awinic web site it looks like the actual label is /SHDN which is similarly misleading though :/
On Tue, Mar 01, 2022 at 01:45:01PM +0000, Mark Brown wrote: > On Tue, Mar 01, 2022 at 01:37:42PM +0100, Stephan Gerhold wrote: > > > The Awinic AW8738 is a simple audio amplifier using an enable GPIO. > > The main difference to simple-amplifier is that there is a "one-wire > > pulse control" that allows configuring the amplifier to one of a few > > pre-defined modes. > > What exactly are the modes here? Looking at the web site for the part > it seems like it's selecting a power limit for the speaker so it makes > sense that the mode would be fixed in DT but it's not clear from the > driver. > It seems to be mostly a power limit but not only. E.g. on AW8738 mode 3/4 and 5/6 seem to have the same power limit but select between a "NCN function" or "Multi-Level AGC function", which seems to control how the amplifier behaves if the power limit is reached. The exact effect of the modes varies greatly between different Awinic parts, but since I don't really see a use case for changing those options dynamically I think it's best to just load it from DT. > > + aw->gpiod_enable = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW); > > + if (IS_ERR(aw->gpiod_enable)) > > + return dev_err_probe(dev, PTR_ERR(aw->gpiod_enable), > > + "Failed to get 'enable' gpio"); > > Are we sure that enable is the best name for this pin? It's more > complex than just an enable since it's the 1 wire data - according to > what's on the awinic web site it looks like the actual label is /SHDN > which is similarly misleading though :/ Yeah, I was considering to call it "shdn" instead but given the negation that seemed even more confusing. I ended up using "enable" since this is the name used in the mode table of the datasheet (which will probably be the main reference when setting up the amplifier in the DT). Thanks, Stephan
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 4de029ae377c..0e35d33f8590 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -53,6 +53,7 @@ config SND_SOC_ALL_CODECS imply SND_SOC_AK5558 imply SND_SOC_ALC5623 imply SND_SOC_ALC5632 + imply SND_SOC_AW8738 imply SND_SOC_BT_SCO imply SND_SOC_BD28623 imply SND_SOC_CQ0093VC @@ -579,6 +580,15 @@ config SND_SOC_ALC5632 tristate depends on I2C +config SND_SOC_AW8738 + tristate "Awinic AW8738 Audio Amplifier" + select GPIOLIB + help + Enable support for the Awinic AW8738 audio amplifier (or similar). + The driver supports simple audio amplifiers similar to + SND_SOC_SIMPLE_AMPLIFIER, but additionally allows setting the + amplifier mode using the Awinic-specific one-wire pulse control. + config SND_SOC_BD28623 tristate "ROHM BD28623 CODEC" help diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index c3c6059a5f8a..8637e9e869e3 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -45,6 +45,7 @@ snd-soc-ak4671-objs := ak4671.o snd-soc-ak5386-objs := ak5386.o snd-soc-ak5558-objs := ak5558.o snd-soc-arizona-objs := arizona.o arizona-jack.o +snd-soc-aw8738-objs := aw8738.o snd-soc-bd28623-objs := bd28623.o snd-soc-bt-sco-objs := bt-sco.o snd-soc-cpcap-objs := cpcap.o @@ -388,6 +389,7 @@ obj-$(CONFIG_SND_SOC_AK5558) += snd-soc-ak5558.o obj-$(CONFIG_SND_SOC_ALC5623) += snd-soc-alc5623.o obj-$(CONFIG_SND_SOC_ALC5632) += snd-soc-alc5632.o obj-$(CONFIG_SND_SOC_ARIZONA) += snd-soc-arizona.o +obj-$(CONFIG_SND_SOC_AW8738) += snd-soc-aw8738.o obj-$(CONFIG_SND_SOC_BD28623) += snd-soc-bd28623.o obj-$(CONFIG_SND_SOC_BT_SCO) += snd-soc-bt-sco.o obj-$(CONFIG_SND_SOC_CQ0093VC) += snd-soc-cq93vc.o diff --git a/sound/soc/codecs/aw8738.c b/sound/soc/codecs/aw8738.c new file mode 100644 index 000000000000..b075c7c0efd0 --- /dev/null +++ b/sound/soc/codecs/aw8738.c @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: GPL-2.0-only + +#include <linux/gpio/consumer.h> +#include <linux/module.h> +#include <linux/regulator/consumer.h> +#include <sound/soc.h> + +struct aw8738_priv { + struct gpio_desc *gpiod_enable; + unsigned int mode; +}; + +static int aw8738_drv_event(struct snd_soc_dapm_widget *w, + struct snd_kcontrol *kcontrol, int event) +{ + struct snd_soc_component *c = snd_soc_dapm_to_component(w->dapm); + struct aw8738_priv *aw = snd_soc_component_get_drvdata(c); + int i; + + switch (event) { + case SND_SOC_DAPM_POST_PMU: + for (i = 0; i < aw->mode; i++) { + gpiod_set_value_cansleep(aw->gpiod_enable, 0); + udelay(2); + gpiod_set_value_cansleep(aw->gpiod_enable, 1); + udelay(2); + } + msleep(40); + break; + case SND_SOC_DAPM_PRE_PMD: + gpiod_set_value_cansleep(aw->gpiod_enable, 0); + usleep_range(1000, 2000); + break; + default: + WARN(1, "Unexpected event"); + return -EINVAL; + } + + return 0; +} + +static const struct snd_soc_dapm_widget aw8738_dapm_widgets[] = { + SND_SOC_DAPM_INPUT("IN"), + SND_SOC_DAPM_OUT_DRV_E("DRV", SND_SOC_NOPM, 0, 0, NULL, 0, aw8738_drv_event, + SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD), + SND_SOC_DAPM_OUTPUT("OUT"), +}; + +static const struct snd_soc_dapm_route aw8738_dapm_routes[] = { + { "DRV", NULL, "IN" }, + { "OUT", NULL, "DRV" }, +}; + +static const struct snd_soc_component_driver aw8738_component_driver = { + .dapm_widgets = aw8738_dapm_widgets, + .num_dapm_widgets = ARRAY_SIZE(aw8738_dapm_widgets), + .dapm_routes = aw8738_dapm_routes, + .num_dapm_routes = ARRAY_SIZE(aw8738_dapm_routes), +}; + +static int aw8738_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct aw8738_priv *aw; + int ret; + + aw = devm_kzalloc(dev, sizeof(*aw), GFP_KERNEL); + if (!aw) + return -ENOMEM; + platform_set_drvdata(pdev, aw); + + aw->gpiod_enable = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW); + if (IS_ERR(aw->gpiod_enable)) + return dev_err_probe(dev, PTR_ERR(aw->gpiod_enable), + "Failed to get 'enable' gpio"); + + ret = device_property_read_u32(dev, "awinic,mode", &aw->mode); + if (ret) + return -EINVAL; + + return devm_snd_soc_register_component(&pdev->dev, + &aw8738_component_driver, + NULL, 0); +} + +#ifdef CONFIG_OF +static const struct of_device_id aw8738_of_match[] = { + { .compatible = "awinic,aw8738" }, + { } +}; +MODULE_DEVICE_TABLE(of, aw8738_of_match); +#endif + +static struct platform_driver aw8738_driver = { + .probe = aw8738_probe, + .driver = { + .name = "aw8738", + .of_match_table = of_match_ptr(aw8738_of_match), + }, +}; +module_platform_driver(aw8738_driver); + +MODULE_DESCRIPTION("Awinic AW8738 Amplifier Driver"); +MODULE_LICENSE("GPL v2");