[alsa-devel] [PATCH 1/2] ASoC: add generic simple-amplifier support
Chen-Yu Tsai
wens at csie.org
Mon Jun 25 14:28:32 CEST 2018
On Mon, Jun 25, 2018 at 7:31 PM, Jerome Brunet <jbrunet at baylibre.com> wrote:
> On Mon, 2018-06-25 at 12:21 +0100, Mark Brown wrote:
>> On Mon, Jun 25, 2018 at 01:06:16PM +0200, Nicolò Veronese wrote:
>> > Many boards have speaker amplifier attached on an analog output.
>> > Most of them are only managed by a mute or shutdown GPIO.
>> > In order to reduce duplicate driver, this patch adds generic/simple-amplifier.
>>
>> Copying in Hans who has a machine that needs something like this, not
>> deleting context for his benefit.
>>
>> This looks good to me, one coding style thing below but otherwise the
>> main thing I'm thinking is that we might want to add a mono channel as
>> well for simplicity when wiring up a system using a mono amplifier.
>> That could always be done later though.
>>
>> >
>> > Signed-off-by: Nicolò Veronese <nicveronese at gmail.com>
>
> Hi Nicolo,
>
> Same comment as on IRC, you driver looks very similar to the one I posted a
> while back (sound/soc/codecs/dio2125.c)
>
> Could you just add your compatible to the list in the existing driver ?
> Is there anything I missed which makes them incompatible ?
I think slightly different semantics apply for "shutdown" vs "mute"
pins / controls.
"shutdown" implies a low power, if not powered off altogether, mode which might
require a slight delay to charge up when the amp is enabled. "mute" is
just that,
disconnecting the output, which would be useful in masking pops or other noises,
but does nothing to help power consumption.
Not sure if the subtle differences can be handled in DAPM, but it would be nice
to have. I've seen devices that have both control pins.
Regards
ChenYu
> Regards
> Jerome
>
>> > ---
>> > sound/soc/generic/Kconfig | 5 ++
>> > sound/soc/generic/Makefile | 2 +
>> > sound/soc/generic/simple-amplifier.c | 109 +++++++++++++++++++++++++++++++++++
>> > 3 files changed, 116 insertions(+)
>> > create mode 100644 sound/soc/generic/simple-amplifier.c
>> >
>> > diff --git a/sound/soc/generic/Kconfig b/sound/soc/generic/Kconfig
>> > index c954be0..ace17c3a 100644
>> > --- a/sound/soc/generic/Kconfig
>> > +++ b/sound/soc/generic/Kconfig
>> > @@ -7,6 +7,11 @@ config SND_SIMPLE_CARD
>> > help
>> > This option enables generic simple sound card support
>> >
>> > +config SND_SIMPLE_AMPLIFIER
>> > + tristate "ASoC Simple Amplifier support"
>> > + help
>> > + This option enables generic simple amplifier support
>> > +
>> > config SND_SIMPLE_SCU_CARD
>> > tristate "ASoC Simple SCU sound card support"
>> > depends on OF
>> > diff --git a/sound/soc/generic/Makefile b/sound/soc/generic/Makefile
>> > index 9dec293..805a746 100644
>> > --- a/sound/soc/generic/Makefile
>> > +++ b/sound/soc/generic/Makefile
>> > @@ -1,12 +1,14 @@
>> > # SPDX-License-Identifier: GPL-2.0
>> > snd-soc-simple-card-utils-objs := simple-card-utils.o
>> > snd-soc-simple-card-objs := simple-card.o
>> > +snd-soc-simple-amplifier-objs := simple-amplifier.o
>> > snd-soc-simple-scu-card-objs := simple-scu-card.o
>> > snd-soc-audio-graph-card-objs := audio-graph-card.o
>> > snd-soc-audio-graph-scu-card-objs := audio-graph-scu-card.o
>> >
>> > obj-$(CONFIG_SND_SIMPLE_CARD_UTILS) += snd-soc-simple-card-utils.o
>> > obj-$(CONFIG_SND_SIMPLE_CARD) += snd-soc-simple-card.o
>> > +obj-$(CONFIG_SND_SIMPLE_AMPLIFIER) += snd-soc-simple-amplifier.o
>> > obj-$(CONFIG_SND_SIMPLE_SCU_CARD) += snd-soc-simple-scu-card.o
>> > obj-$(CONFIG_SND_AUDIO_GRAPH_CARD) += snd-soc-audio-graph-card.o
>> > obj-$(CONFIG_SND_AUDIO_GRAPH_SCU_CARD) += snd-soc-audio-graph-scu-card.o
>> > diff --git a/sound/soc/generic/simple-amplifier.c b/sound/soc/generic/simple-amplifier.c
>> > new file mode 100644
>> > index 0000000..e135a43
>> > --- /dev/null
>> > +++ b/sound/soc/generic/simple-amplifier.c
>> > @@ -0,0 +1,109 @@
>> > +// SPDX-License-Identifier: GPL-2.0
>> > +// ALSA SoC simple amplifier driver
>> > +//
>> > +// Copyright 2018
>> > +// Author: Nicolò Veronese <nicveronese at gmail.com>
>> > +
>> > +#include <linux/module.h>
>> > +#include <linux/platform_device.h>
>> > +#include <linux/gpio/consumer.h>
>> > +#include <linux/regulator/consumer.h>
>> > +
>> > +#include <sound/soc.h>
>> > +#include <sound/soc-dapm.h>
>> > +
>> > +/* This struct is used to save the context */
>> > +struct simple_amp_data {
>> > + struct device *dev;
>> > + struct regmap *regmap;
>>
>> You don't need a regmap here.
>>
>> > + struct gpio_desc *power_gpio;
>> > +};
>> > +
>> > +static int simple_amp_power_event(struct snd_soc_dapm_widget *widget,
>> > + struct snd_kcontrol *kctrl, int event)
>> > +{
>> > + struct snd_soc_component *c = snd_soc_dapm_to_component(widget->dapm);
>> > + struct simple_amp_data *data = snd_soc_component_get_drvdata(c);
>> > +
>> > + /* Shutdown GPIO */
>> > + if (data->power_gpio)
>> > + gpiod_set_value_cansleep(data->power_gpio, SND_SOC_DAPM_EVENT_ON(event));
>> > +
>> > + return 0;
>> > +}
>> > +
>> > +static const struct snd_soc_dapm_widget simple_amp_dapm_widgets[] = {
>> > + SND_SOC_DAPM_INPUT("LEFT AMP IN"),
>> > + SND_SOC_DAPM_INPUT("RIGHT AMP IN"),
>> > +
>> > + SND_SOC_DAPM_OUTPUT("SPK LEFT"),
>> > + SND_SOC_DAPM_OUTPUT("SPK RIGHT"),
>> > +
>> > + SND_SOC_DAPM_REGULATOR_SUPPLY("VCC", 20, 0),
>> > + SND_SOC_DAPM_SPK("MUTE", simple_amp_power_event),
>> > +};
>> > +
>> > +static const struct snd_soc_dapm_route simple_amp_dapm_routes[] = {
>> > + { "MUTE", NULL, "LEFT AMP IN" },
>> > + { "MUTE", NULL, "RIGHT AMP IN" },
>> > +
>> > + { "SPK LEFT", NULL, "VCC" },
>> > + { "SPK RIGHT", NULL, "VCC" },
>> > +
>> > + { "SPK LEFT", NULL, "MUTE" },
>> > + { "SPK RIGHT", NULL, "MUTE" },
>> > +};
>> > +
>> > +static const struct snd_soc_component_driver simple_amp_component_driver = {
>> > + .name = "simple-amplifier",
>> > + .dapm_widgets = simple_amp_dapm_widgets,
>> > + .num_dapm_widgets = ARRAY_SIZE(simple_amp_dapm_widgets),
>> > + .dapm_routes = simple_amp_dapm_routes,
>> > + .num_dapm_routes = ARRAY_SIZE(simple_amp_dapm_routes),
>> > +};
>> > +
>> > +static int asoc_simple_amp_probe(struct platform_device *pdev)
>> > +{
>> > + struct simple_amp_data *data;
>> > + struct device *dev = &pdev->dev;
>> > + int ret;
>> > +
>> > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> > + if (!data)
>> > + return -ENOMEM;
>> > + data->dev = &pdev->dev;
>> > + platform_set_drvdata(pdev, data);
>> > +
>> > + if(of_property_read_bool(pdev->dev.of_node,"shutdown-gpios")) {
>>
>> Coding style: should be if (
>>
>> > + data->power_gpio = devm_gpiod_get_optional(&pdev->dev, "shutdown", GPIOD_OUT_LOW);
>> > +
>> > + if (IS_ERR(data->power_gpio)) {
>> > + ret = PTR_ERR(data->power_gpio);
>> > + dev_err(&pdev->dev, "Failed to get shutdown gpio: %d\n", ret);
>> > + return ret;
>> > + }
>> > + }
>> > +
>> > + return devm_snd_soc_register_component(dev,
>> > + &simple_amp_component_driver, NULL, 0);
>> > +}
>> > +
>> > +static const struct of_device_id asoc_simple_of_match[] = {
>> > + { .compatible = "simple-audio-amplifier", },
>> > + {},
>> > +};
>> > +MODULE_DEVICE_TABLE(of, asoc_simple_of_match);
>> > +
>> > +static struct platform_driver asoc_simple_amp = {
>> > + .driver = {
>> > + .name = "asoc-simple-amplifier",
>> > + .of_match_table = of_match_ptr(asoc_simple_of_match),
>> > + },
>> > + .probe = asoc_simple_amp_probe,
>> > +};
>> > +
>> > +module_platform_driver(asoc_simple_amp);
>> > +
>> > +MODULE_LICENSE("GPL v2");
>> > +MODULE_DESCRIPTION("ASoC Simple Amplifier");
>> > +MODULE_AUTHOR("Nicolò Veronese <nicveronese at gmail.com>");
>> > --
>> > 2.7.4
>> >
>>
>> _______________________________________________
>> Alsa-devel mailing list
>> Alsa-devel at alsa-project.org
>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
More information about the Alsa-devel
mailing list