Skip to contentSkip to conversation

ASoC: Add support for GPIOs driven amplifiers

PATCH In review Herve Codina 3 participants 24 messages May 13 – May 19

Subsystems (6)
Device tree bindings Sound ASoC GPIO +2 more subsystems
lore
Share & tools
lore

Herve Codina

author maintainer
#0 lore raw

v3 cover ASoC: Add support for GPIOs driven amplifiers

On some embedded system boards, audio amplifiers are designed using discrete components such as op-amp, several resistors and switches to either adjust the gain (switching resistors) or fully switch the audio signal path (mute and/or bypass features).

Those switches are usually driven by simple GPIOs.

This kind of amplifiers are not handled in ASoC and the fallback is to let the user-space handle those GPIOs out of the ALSA world.

In order to have those kind of amplifiers fully integrated in the audio stack, this series introduces the audio-gpio-amp to handle them.

This new ASoC component allows to have the amplifiers seen as ASoC auxiliarty devices and so it allows to control them through audio mixer controls.

Compared to previous iteration, this v3 series takes into account feedback received from Mark and also some comments from Shashiko.

In order to ease the review, I choose to split modifications related to the merge of the gpio-audio-amp part into the simple-amplfier driver in several commits.

Best regards,
Hervé

---
Changes v2 -> v3
v2: https://lore.kernel.org/all/[email protected]/

Patch 2:
- Fix a typo s/rignt/right/ (Sashiko)
- Use the 'int32-matrix' type for gain-ranges property. Indeed the gain value can be a negative value (Sashiko).
- Add 'Reviewed-by: Rob Herring'

Patch 8:
- Fix error message

Patch 11:
- Clamp the value received in simple_amp_single_kctrl_put() in order to have a more robust code (Sashiko)

Patch 14:
- Return 'unsigned int *' in simple_amp_alloc_tlv_ranges()
- Check for existing ranges entirely inside the new one in simple_amp_check_new_range().
- Don't allow an empty ranges array. nb_ranges cannot be set to 0 anymore (Sahiko).

Patch 15:
- Don't allow an empty labels array. nb_labels cannot be set to 0 anymore (Sahiko).

Other patches:
No changes

Changes v1 -> v2
v1: https://lore.kernel.org/all/[email protected]/

Rebase on top of v7.1-rc1.

Patch 1:
- Add 'Acked-by: Rob Herring'

Patch 2:
- Use gpio-audio-amp-* instead of audio-gpio-amp
- Introduce mono/stereo channel distinction with gpio-audio-amp-mono and gpio-audio-amp-stereo compatible strings (needed if the "simple-amplifier" compatible string is deprecated in the near future).
- Reduce the maximum numbers of gpios used for gain setting to 16.
- Use amplifier-N instead of amplifierN in examples
- Introduce gpio-ranges property and remove the no more needed gpio-points and gpio-range properties

Other patches (full rework):
- Merge the new audio-gpio-amp driver provided in v1 into the existing simple-amplifier driver.
- Slit modification in several commits.
- Reduce the maximum number of gpios used for gain setting to 16.
- Fix a memory leak in case of error in simple_amp_multi_add_kcontrol() (Reported by Shahiko).
- Handle the new gpio-ranges property and drop the support for the no more needed properties (gpio-points and gpio-range).
- Update author and copyright.

Herve Codina (17):
of: Introduce of_property_read_s32_index()
ASoC: dt-bindings: Add support for the GPIOs driven amplifier
ASoC: simple-amplifier: Remove DRV_NAME defined value
ASoC: simple-amplifier: Add missing headers
ASoC: simple-amplifier: Remove CONFIG_OF flag and of_match_ptr()
ASoC: simple-amplifier: Rename drv_event() function
ASoC: simple-amplifier: Use 'simple_amp' variable name instead of

    'priv'

ASoC: simple-amplifier: Remove DAPM widgets and routes from the ASoC
component driver
ASoC: simple-amplifier: Introduce support for gpio-audio-amp
ASoC: simple-amplifier: gpio-audio-amp: Add support for extra power
supplies
ASoC: simple-amplifier: gpio-audio-amp: Add support for mute gpio
ASoC: simple-amplifier: gpio-audio-amp: Add support for bypass gpio
ASoC: simple-amplifier: gpio-audio-amp: Add support for basic gain
ASoC: simple-amplifier: gpio-audio-amp: Add support for gain-ranges
ASoC: simple-amplifier: gpio-audio-amp: Add support for gain-labels
ASoC: simple-amplifier: Update author and copyright
MAINTAINERS: Add the ASoC gpio audio amplifier entry

 .../bindings/sound/gpio-audio-amp.yaml        | 270 ++++++
 MAINTAINERS                                   |   7 +
 include/linux/of.h                            |   7 +
 sound/soc/codecs/simple-amplifier.c           | 918 +++++++++++++++++-
 4 files changed, 1176 insertions(+), 26 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/gpio-audio-amp.yaml
Signature · 1 line
2.54.0
author maintainer
#1 lore raw

PATCH v3 1/17 of: Introduce of_property_read_s32_index()

Signed integers can be read from single value properties using of_property_read_s32() but nothing exist to read signed integers from multi-value properties.

Fix this lack adding of_property_read_s32_index().

---
 include/linux/of.h | 7 +++++++
 1 file changed, 7 insertions(+)
include/linux/of.h +7 -0
index 959786f8f196..28153616e616 100644
@@ -1476,6 +1476,13 @@ static inline int of_property_read_s32(const struct device_node *np,
 	return of_property_read_u32(np, propname, (u32*) out_value);
 }
 
+static inline int of_property_read_s32_index(const struct device_node *np,
+					     const char *propname, u32 index,
+					     s32 *out_value)
+{
+	return of_property_read_u32_index(np, propname, index, (u32 *)out_value);
+}
+
 #define of_for_each_phandle(it, err, np, ln, cn, cc)			\
 	for (of_phandle_iterator_init((it), (np), (ln), (cn), (cc)),	\
 	     err = of_phandle_iterator_next(it);			\
Signature · 1 line
2.54.0
author maintainer
#2 lore raw

PATCH v3 2/17 ASoC: dt-bindings: Add support for the GPIOs driven amplifier

In reply to Herve Codina

Some amplifiers based on analog switches and op-amps can be present in the audio path and can be driven by GPIOs in order to control their gain value, their mute and/or bypass functions.

Those components needs to be viewed as audio components in order to be fully integrated in the audio path.

gpio-audio-amp allows to consider these GPIO driven amplifiers as auxiliary audio devices.

---
 .../bindings/sound/gpio-audio-amp.yaml        | 270 ++++++++++++++++++
 1 file changed, 270 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/gpio-audio-amp.yaml
Documentation/devicetree/bindings/sound/gpio-audio-amp.yaml new file +270 -0
new file mode 100644
index 000000000000..3690f3d1628c
@@ -0,0 +1,270 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/gpio-audio-amp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Audio amplifier driven by GPIOs
+
+maintainers:
+  - Herve Codina <[email protected]>
+
+description: |
+  Audio GPIO amplifiers are driven by GPIO in order to control the gain value
+  of the amplifier, its mute function and/or its bypass function.
+
+  Those amplifiers are based on discrete components (analog switches, op-amps
+  and more) where some of them, mostly analog switches, are controlled by GPIOs
+  to adjust the gain value of the whole amplifier and/or to control
+  the mute and/or bypass function.
+
+  For instance, the following piece of hardware is a GPIO amplifier
+
+                                         +5VA
+                                           ^
+                                        |\ |
+                                        | \
+        Vin >---------------------------|+ \
+                                        |   +-------+-----> Vout
+                .--\/\/\/--+------------|- /        |
+                |          |            | /         |
+                v          |            |/ |        |
+               GND         o               v        |
+                            \             GND       |
+       gpio >----------->    \                      |
+                         o    o                     |
+                         |    |                     |
+                         |    '--\/\/\/--.          |
+                         |               +--\/\/\/--'
+                         '---------------'
+
+properties:
+  compatible:
+    oneOf:
+      - const: gpio-audio-amp-mono
+        description:
+          A single channel amplifier. All features apply to this sole channel.
+
+      - const: gpio-audio-amp-stereo
+        description:
+          A dual channel amplifier (left and right). All features apply to both
+          channels producing the same effect on both channels at the same time.
+
+  vdd-supply:
+    description: Main power supply of the amplifier
+
+  vddio-supply:
+    description: Power supply related to the control path
+
+  vdda1-supply:
+    description: Analog power supply
+
+  vdda2-supply:
+    description: Additional analog power supply
+
+  mute-gpios:
+    description: GPIO to control the mute function
+    maxItems: 1
+
+  bypass-gpios:
+    description: GPIO to control the bypass function
+    maxItems: 1
+
+  gain-gpios:
+    description: |
+      GPIOs to control the amplifier gain
+
+      The gain value is computed from GPIOs value from 0 to 2^N-1 with N the
+      number of GPIO described. The first GPIO described is the lsb of the gain
+      value.
+
+      For instance assuming 2 gpios
+         gain-gpios = <&gpio1 GPIO_ACTIVE_HIGH> <&gpio2 GPIO_ACTIVE_HIGH>;
+      The gain value will be the following:
+
+          gpio1 | gpio2 | gain
+          ------+-------+-----
+            0   |    0  | 0b00 -> 0
+            1   |    0  | 0b01 -> 1
+            0   |    1  | 0b10 -> 2
+            1   |    1  | 0b11 -> 3
+          ------+-------+-----
+
+      Note: The gain value, bits set to 1 or 0, indicate the state active (bit
+            set) or the state inactive (bit unset) of the related GPIO. The
+            physical voltage corresponding to this active/inactive state is
+            given by the GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags.
+
+    minItems: 1
+    maxItems: 16
+
+  gain-ranges:
+    $ref: /schemas/types.yaml#/definitions/int32-matrix
+    description: |
+      A list of one or more ranges of possible values. Each range is defined by
+      the first and last point in the range. Each point is defined by the pair
+      (GPIOs value, Gain in 0.01 dB unit).
+
+      Ranges can be contiguous or holes can be present between ranges if some
+      gpios value should not be used. Also in a range the first point and the
+      last point can be identical. In that case, the range contains only one
+      item, the given point.
+
+    items:
+      items:
+        - description: GPIOs value of the first point in the range
+        - description: Gain in 0.01 dB unit of the first point in the range
+        - description: GPIOs value of the last point in the range
+        - description: Gain in 0.01 dB unit of the last point in the range
+      description: |
+        A range defines a linear function (linear in dB) from the first point
+        to the last point, both included. The number of items in the range is
+          N = abs(first_point.gpio_value - last_point.gpio_value) + 1
+
+        It allows to define the gain range from the first_point.gain to
+        the last_point.gain, both points included.
+
+             Gain (0.01 dB unit)
+               ^
+               |                      last
+               +- - - - - - - - - - + point
+               |                 +  .
+               |              +     .
+               |           +        .
+               +- - - - +           .
+               |  first .           .
+               |  point .           .
+               |        .           .
+               +--------+-----------+---> gpios
+                                          value
+
+        Note: Even if first_point.gpio_value is lower than last_point.gpio_value
+              and first_point.gain is lower than last_point.gain in the above
+              graphic, all combination of values are supported leading to an
+              increasing or a decreasing linear segment.
+
+    minItems: 1
+    maxItems: 65536
+
+  gain-labels:
+    $ref: /schemas/types.yaml#/definitions/string-array
+    minItems: 2
+    maxItems: 65536
+    description: |
+      List of the gain labels attached to the combination of GPIOs controlling
+      the gain. The first label is related to the gain value 0, the second label
+      is related to the gain value 1 and so on.
+
+      With 2 GPIOs controlling the gain, GPIOs value can be 0, 1, 2 and 3.
+      Assuming that gain value set the hardware according to the following
+      table:
+
+         GPIOs | Hardware
+         value | amplification
+         ------+--------------
+           0   | Low
+           1   | Middle
+           2   | High
+           3   | Max
+         ------+--------------
+
+      The description using gain labels can be:
+        gain-labels = "Low", "Middle", "High", "Max";
+
+dependencies:
+  gain-ranges: [ gain-gpios ]
+  gain-labels: [ gain-gpios ]
+
+required:
+  - compatible
+  - vdd-supply
+
+anyOf:
+  - required:
+      - gain-gpios
+  - required:
+      - mute-gpios
+  - required:
+      - bypass-gpios
+
+allOf:
+  - $ref: component-common.yaml#
+  - if:
+      required:
+        - gain-ranges
+    then:
+      properties:
+        gain-labels: false
+  - if:
+      required:
+        - gain-labels
+    then:
+      properties:
+        gain-ranges: false
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+
+    /* Gain controlled by gpios */
+    amplifier-0 {
+        compatible = "gpio-audio-amp-mono";
+        vdd-supply = <&regulator>;
+        gain-gpios = <&gpio 0 GPIO_ACTIVE_HIGH>, <&gpio 1 GPIO_ACTIVE_HIGH>;
+    };
+
+    /* Gain controlled by gpio using a simple range on a stereo amplifier */
+    amplifier-1 {
+        compatible = "gpio-audio-amp-stereo";
+        vdd-supply = <&regulator>;
+        gain-gpios = <&gpio 0 GPIO_ACTIVE_HIGH>, <&gpio 1 GPIO_ACTIVE_HIGH>;
+        gain-ranges = <0 (-300) 3 600>;
+    };
+
+    /* Gain controlled by gpio with labels */
+    amplifier-3 {
+        compatible = "gpio-audio-amp-mono";
+        vdd-supply = <&regulator>;
+        gain-gpios = <&gpio 0 GPIO_ACTIVE_HIGH>;
+        gain-labels = "Low", "High";
+    };
+
+    /* A mutable stereo amplifier without any gain control */
+    amplifier-4 {
+        compatible = "gpio-audio-amp-stereo";
+        vdd-supply = <&regulator>;
+        mute-gpios = <&gpio 0 GPIO_ACTIVE_HIGH>;
+    };
+
+    /*
+     * Several supplies, gain controlled using more complex ranges, mute and
+     * bypass.
+     *
+     * Assuming 3 gpios for controlling the gain with the following table
+     *   gpios value    Gain
+     *      0b000       Do not use (gpios value not allowed)
+     *      0b001       - 3dB
+     *      0b010       + 3dB
+     *      0b011       + 10dB
+     *      0b100       Do not use (gpios value not allowed)
+     *      0b101       + 6dB
+     *      0b110       + 7dB
+     *      0b111       + 8dB
+     */
+    amplifier-5 {
+        compatible = "gpio-audio-amp-mono";
+        vdd-supply = <&regulator>;
+        vddio-supply = <&regulator1>;
+        vdda1-supply = <&regulator2>;
+        gain-gpios = <&gpio 0 GPIO_ACTIVE_HIGH>,
+                     <&gpio 1 GPIO_ACTIVE_HIGH>,
+                     <&gpio 2 GPIO_ACTIVE_HIGH>;
+        gain-ranges = <1 (-300) 2 300>,
+                      <3 1000   3 1000>,
+                      <5 600    7 800>;
+        mute-gpios = <&gpio 3 GPIO_ACTIVE_HIGH>;
+        bypass-gpios = <&gpio 4 GPIO_ACTIVE_HIGH>;
+    };
+...
Signature · 1 line
2.54.0
author maintainer
#3 lore raw

PATCH v3 3/17 ASoC: simple-amplifier: Remove DRV_NAME defined value

In reply to Herve Codina

DRV_NAME is defined and used only in the simple-amplifier driver declaration.

Remove the useless defined and use directly the value in the driver declaration itself.

---
 sound/soc/codecs/simple-amplifier.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
sound/soc/codecs/simple-amplifier.c +1 -3
index d306c585b52b..41485445727d 100644
@@ -9,8 +9,6 @@
 #include <linux/regulator/consumer.h>
 #include <sound/soc.h>
 
-#define DRV_NAME "simple-amplifier"
-
 struct simple_amp {
 	struct gpio_desc *gpiod_enable;
 };
@@ -97,7 +95,7 @@ MODULE_DEVICE_TABLE(of, simple_amp_ids);
 
 static struct platform_driver simple_amp_driver = {
 	.driver = {
-		.name = DRV_NAME,
+		.name = "simple-amplifier",
 		.of_match_table = of_match_ptr(simple_amp_ids),
 	},
 	.probe = simple_amp_probe,
Signature · 1 line
2.54.0
author maintainer
#4 lore raw

PATCH v3 4/17 ASoC: simple-amplifier: Add missing headers

In reply to Herve Codina

The simple-amplifier driver is a platform device driver.

Add missing include files related to this kind of driver.

---
 sound/soc/codecs/simple-amplifier.c | 2 ++
 1 file changed, 2 insertions(+)
sound/soc/codecs/simple-amplifier.c +2 -0
index 41485445727d..5b44bcfef49e 100644
@@ -5,7 +5,9 @@
  */
 
 #include <linux/gpio/consumer.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
+#include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 #include <sound/soc.h>
 
Signature · 1 line
2.54.0
author maintainer
#5 lore raw

PATCH v3 5/17 ASoC: simple-amplifier: Remove CONFIG_OF flag and of_match_ptr()

In reply to Herve Codina

The simple-amplifier Use CONFIG_OF flag for its of_device_id table and of_match_ptr() when it assigns the table in the driver declaration.

This is no more needed. Drop them.

---
 sound/soc/codecs/simple-amplifier.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
sound/soc/codecs/simple-amplifier.c +1 -3
index 5b44bcfef49e..215318ff62fc 100644
@@ -86,19 +86,17 @@ static int simple_amp_probe(struct platform_device *pdev)
 					       NULL, 0);
 }
 
-#ifdef CONFIG_OF
 static const struct of_device_id simple_amp_ids[] = {
 	{ .compatible = "dioo,dio2125", },
 	{ .compatible = "simple-audio-amplifier", },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, simple_amp_ids);
-#endif
 
 static struct platform_driver simple_amp_driver = {
 	.driver = {
 		.name = "simple-amplifier",
-		.of_match_table = of_match_ptr(simple_amp_ids),
+		.of_match_table = simple_amp_ids,
 	},
 	.probe = simple_amp_probe,
 };
Signature · 1 line
2.54.0
author maintainer
#6 lore raw

PATCH v3 6/17 ASoC: simple-amplifier: Rename drv_event() function

In reply to Herve Codina

The drv_event() is used to handle power events related to the DRV item.

Later, with the support for gpio-audio-amp, this function will be also used to handle power events related to the PGA item.

Also, more functions will be added in the driver and it is a common usage to prefix functions based on the driver name.

Rename the drv_event() function to simple_amp_power_event() to follow common usage and get rid of the 'drv' term.

---
 sound/soc/codecs/simple-amplifier.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
sound/soc/codecs/simple-amplifier.c +3 -3
index 215318ff62fc..8f2daec55134 100644
@@ -15,8 +15,8 @@ struct simple_amp {
 	struct gpio_desc *gpiod_enable;
 };
 
-static int drv_event(struct snd_soc_dapm_widget *w,
-		     struct snd_kcontrol *control, int event)
+static int simple_amp_power_event(struct snd_soc_dapm_widget *w,
+				  struct snd_kcontrol *control, int event)
 {
 	struct snd_soc_component *c = snd_soc_dapm_to_component(w->dapm);
 	struct simple_amp *priv = snd_soc_component_get_drvdata(c);
@@ -42,7 +42,7 @@ static int drv_event(struct snd_soc_dapm_widget *w,
 static const struct snd_soc_dapm_widget simple_amp_dapm_widgets[] = {
 	SND_SOC_DAPM_INPUT("INL"),
 	SND_SOC_DAPM_INPUT("INR"),
-	SND_SOC_DAPM_OUT_DRV_E("DRV", SND_SOC_NOPM, 0, 0, NULL, 0, drv_event,
+	SND_SOC_DAPM_OUT_DRV_E("DRV", SND_SOC_NOPM, 0, 0, NULL, 0, simple_amp_power_event,
 			       (SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD)),
 	SND_SOC_DAPM_OUTPUT("OUTL"),
 	SND_SOC_DAPM_OUTPUT("OUTR"),
Signature · 1 line
2.54.0
author maintainer
#7 lore raw

PATCH v3 7/17 ASoC: simple-amplifier: Use 'simple_amp' variable name instead of 'priv'

In reply to Herve Codina

The simple-amplifier driver use 'priv' as variable name for its private data (struct simple_amp).

With the support for gpio-audio-amp, more functions and data structures will be added.

Those future additions will add more complexity in data manipulation and will make the 'priv' term error prone.

In order to clearly identify the struct simple_amp private data, use 'simple_amp' as variable name when this structure is involved.

---
 sound/soc/codecs/simple-amplifier.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)
sound/soc/codecs/simple-amplifier.c +10 -10
index 8f2daec55134..231e84ab4c0e 100644
@@ -19,7 +19,7 @@ static int simple_amp_power_event(struct snd_soc_dapm_widget *w,
 				  struct snd_kcontrol *control, int event)
 {
 	struct snd_soc_component *c = snd_soc_dapm_to_component(w->dapm);
-	struct simple_amp *priv = snd_soc_component_get_drvdata(c);
+	struct simple_amp *simple_amp = snd_soc_component_get_drvdata(c);
 	int val;
 
 	switch (event) {
@@ -34,7 +34,7 @@ static int simple_amp_power_event(struct snd_soc_dapm_widget *w,
 		return -EINVAL;
 	}
 
-	gpiod_set_value_cansleep(priv->gpiod_enable, val);
+	gpiod_set_value_cansleep(simple_amp->gpiod_enable, val);
 
 	return 0;
 }
@@ -68,17 +68,17 @@ static const struct snd_soc_component_driver simple_amp_component_driver = {
 static int simple_amp_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct simple_amp *priv;
+	struct simple_amp *simple_amp;
 
-	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
-	if (priv == NULL)
+	simple_amp = devm_kzalloc(dev, sizeof(*simple_amp), GFP_KERNEL);
+	if (!simple_amp)
 		return -ENOMEM;
-	platform_set_drvdata(pdev, priv);
+	platform_set_drvdata(pdev, simple_amp);
 
-	priv->gpiod_enable = devm_gpiod_get_optional(dev, "enable",
-						     GPIOD_OUT_LOW);
-	if (IS_ERR(priv->gpiod_enable))
-		return dev_err_probe(dev, PTR_ERR(priv->gpiod_enable),
+	simple_amp->gpiod_enable = devm_gpiod_get_optional(dev, "enable",
+							   GPIOD_OUT_LOW);
+	if (IS_ERR(simple_amp->gpiod_enable))
+		return dev_err_probe(dev, PTR_ERR(simple_amp->gpiod_enable),
 				     "Failed to get 'enable' gpio");
 
 	return devm_snd_soc_register_component(dev,
Signature · 1 line
2.54.0
author maintainer
#8 lore raw

PATCH v3 8/17 ASoC: simple-amplifier: Remove DAPM widgets and routes from the ASoC component driver

In reply to Herve Codina

The simple-amplifier set the DAPM wigets and routes table in the ASoC component driver. This is perfectly fine when the component has well known DAPM tables.

The simple-amplifier is going to handle several kind of components based on the driver compatible string. The DAPM table will not be the same for all components supported by the driver.

In order to have different DAPM table based on matching compatible strings, move those tables from the ASoC component driver to the device compatible string matching data.

Add those DAPM widgets and routes dynamically during the ASoC component probe operation.

---
 sound/soc/codecs/simple-amplifier.c | 59 ++++++++++++++++++++++++++---
 1 file changed, 53 insertions(+), 6 deletions(-)
sound/soc/codecs/simple-amplifier.c +53 -6
index 231e84ab4c0e..3e644c1c2696 100644
@@ -11,7 +11,15 @@
 #include <linux/regulator/consumer.h>
 #include <sound/soc.h>
 
+struct simple_amp_data {
+	const struct snd_soc_dapm_widget *dapm_widgets;
+	unsigned int num_dapm_widgets;
+	const struct snd_soc_dapm_route *dapm_routes;
+	unsigned int num_dapm_routes;
+};
+
 struct simple_amp {
+	const struct simple_amp_data *data;
 	struct gpio_desc *gpiod_enable;
 };
 
@@ -58,11 +66,39 @@ static const struct snd_soc_dapm_route simple_amp_dapm_routes[] = {
 	{ "OUTR", NULL, "DRV" },
 };
 
+static int simple_amp_add_basic_dapm(struct snd_soc_component *component)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_component_to_dapm(component);
+	struct simple_amp *simple_amp = snd_soc_component_get_drvdata(component);
+	struct device *dev = component->dev;
+	int ret;
+
+	/* Add basic dapm widgets and routes */
+	ret = snd_soc_dapm_new_controls(dapm, simple_amp->data->dapm_widgets,
+					simple_amp->data->num_dapm_widgets);
+	if (ret) {
+		dev_err(dev, "Failed to add basic dapm widgets (%d)\n", ret);
+		return ret;
+	}
+
+	ret = snd_soc_dapm_add_routes(dapm, simple_amp->data->dapm_routes,
+				      simple_amp->data->num_dapm_routes);
+	if (ret) {
+		dev_err(dev, "Failed to add basic dapm routes (%d)\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int simple_amp_component_probe(struct snd_soc_component *component)
+{
+	/* Add basic dapm widgets and routes */
+	return simple_amp_add_basic_dapm(component);
+}
+
 static const struct snd_soc_component_driver simple_amp_component_driver = {
-	.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),
+	.probe = simple_amp_component_probe,
 };
 
 static int simple_amp_probe(struct platform_device *pdev)
@@ -75,6 +111,10 @@ static int simple_amp_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	platform_set_drvdata(pdev, simple_amp);
 
+	simple_amp->data = of_device_get_match_data(dev);
+	if (!simple_amp->data)
+		return -EINVAL;
+
 	simple_amp->gpiod_enable = devm_gpiod_get_optional(dev, "enable",
 							   GPIOD_OUT_LOW);
 	if (IS_ERR(simple_amp->gpiod_enable))
@@ -86,9 +126,16 @@ static int simple_amp_probe(struct platform_device *pdev)
 					       NULL, 0);
 }
 
+static const struct simple_amp_data simple_audio_amplifier_data = {
+	.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 const struct of_device_id simple_amp_ids[] = {
-	{ .compatible = "dioo,dio2125", },
-	{ .compatible = "simple-audio-amplifier", },
+	{ .compatible = "dioo,dio2125",		  .data = &simple_audio_amplifier_data},
+	{ .compatible = "simple-audio-amplifier", .data = &simple_audio_amplifier_data},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, simple_amp_ids);
Signature · 1 line
2.54.0
#9 lore raw

Re: [PATCH v3 08/17] ASoC: simple-amplifier: Remove DAPM widgets and routes from the ASoC component driver

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Probe fails unconditionally for non-Device Tree instantiations (platform board files, ACPI…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Probe fails unconditionally for non-Device Tree instantiations (platform board files, ACPI PRP0001, or CONFIG_OF disabled). --

commit a564ff83aa42e00d34fe9812df9996695059dbc5
Author: Herve Codina <[email protected]>

ASoC: simple-amplifier: Remove DAPM widgets and routes from the ASoC component driver

This patch moves DAPM tables from the ASoC component driver to the device compatible string matching data. This allows the simple-amplifier driver to configure different DAPM widgets and routes dynamically during probe based on the compatible string.

Quoted 4 lines
sound/soc/codecs/simple-amplifier.c +0 -0
index 231e84ab4c0e3..3e644c1c26960 100644

[ ... ]

sound/soc/codecs/simple-amplifier.c static int simple_amp_probe(struct platform_device *pdev) · line 117

Quoted 8 lines
sound/soc/codecs/simple-amplifier.c +4 -0
@@ -75,6 +111,10 @@ static int simple_amp_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	platform_set_drvdata(pdev, simple_amp);
 
+	simple_amp->data = of_device_get_match_data(dev);
+	if (!simple_amp->data)
+		return -EINVAL;
+

Will this cause the probe function to fail unconditionally on non-Device Tree platforms?

Since the driver does not depend on CONFIG_OF, it can be built and used on systems without Device Tree. If CONFIG_OF is disabled, or if the device is instantiated via ACPI PRP0001 where dev->of_node is NULL, the call to of_device_get_match_data() will evaluate to NULL.

Because probe now returns -EINVAL when the match data is NULL, this looks like it will prevent the driver from probing entirely on those platforms.

Could this use device_get_match_data() instead to safely support non-DT systems?

Quoted 3 lines
sound/soc/codecs/simple-amplifier.c +0 -0
 	simple_amp->gpiod_enable = devm_gpiod_get_optional(dev, "enable",
 							   GPIOD_OUT_LOW);
 	if (IS_ERR(simple_amp->gpiod_enable))
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8
author maintainer
#10 lore raw

PATCH v3 9/17 ASoC: simple-amplifier: Introduce support for gpio-audio-amp

In reply to Herve Codina

Improve the simple-amplifier introducing preliminary support for gpio-audio-amp.

Those amplifiers are amplifiers driven by gpios.

This support introduction doesn't handle any GPIO yet but introduces the compatible strings and the related DAPM table.

Two gpio-audio-amp are available: A mono and a stereo version.

The mono version has only one audio channel and gpio settings impact features such as the gain or mute of this sole channel.

The stereo version has two channels (left and right). Gpio settings impact both channels in the same manner and at the same time. For instance, the gain setting set the gain of both channels as well as the mute setting mutes both channels.

---
 sound/soc/codecs/simple-amplifier.c | 54 +++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)
sound/soc/codecs/simple-amplifier.c +54 -0
index 3e644c1c2696..1704cdbb7de5 100644
@@ -4,6 +4,7 @@
  * Author: Jerome Brunet <[email protected]>
  */
 
+#include <linux/bits.h>
 #include <linux/gpio/consumer.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
@@ -12,6 +13,9 @@
 #include <sound/soc.h>
 
 struct simple_amp_data {
+	unsigned int supports;
+#define SIMPLE_AUDIO_SUPPORT_PGA		BIT(0)
+
 	const struct snd_soc_dapm_widget *dapm_widgets;
 	unsigned int num_dapm_widgets;
 	const struct snd_soc_dapm_route *dapm_routes;
@@ -66,6 +70,38 @@ static const struct snd_soc_dapm_route simple_amp_dapm_routes[] = {
 	{ "OUTR", NULL, "DRV" },
 };
 
+static const struct snd_soc_dapm_widget simple_amp_mono_pga_dapm_widgets[] = {
+	SND_SOC_DAPM_INPUT("IN"),
+	SND_SOC_DAPM_OUTPUT("OUT"),
+	SND_SOC_DAPM_PGA_E("PGA", SND_SOC_NOPM, 0, 0, NULL, 0, simple_amp_power_event,
+			   (SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD)),
+	SND_SOC_DAPM_REGULATOR_SUPPLY("vdd", 0, 0),
+};
+
+static const struct snd_soc_dapm_route simple_amp_mono_pga_dapm_routes[] = {
+	{ "PGA", NULL, "IN" },
+	{ "PGA", NULL, "vdd" },
+	{ "OUT", NULL, "PGA" },
+};
+
+static const struct snd_soc_dapm_widget simple_amp_stereo_pga_dapm_widgets[] = {
+	SND_SOC_DAPM_INPUT("INL"),
+	SND_SOC_DAPM_INPUT("INR"),
+	SND_SOC_DAPM_OUTPUT("OUTL"),
+	SND_SOC_DAPM_OUTPUT("OUTR"),
+	SND_SOC_DAPM_PGA_E("PGA", SND_SOC_NOPM, 0, 0, NULL, 0, simple_amp_power_event,
+			   (SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD)),
+	SND_SOC_DAPM_REGULATOR_SUPPLY("vdd", 0, 0),
+};
+
+static const struct snd_soc_dapm_route simple_amp_stereo_pga_dapm_routes[] = {
+	{ "PGA", NULL, "INL" },
+	{ "PGA", NULL, "INR" },
+	{ "PGA", NULL, "vdd" },
+	{ "OUTL", NULL, "PGA" },
+	{ "OUTR", NULL, "PGA" },
+};
+
 static int simple_amp_add_basic_dapm(struct snd_soc_component *component)
 {
 	struct snd_soc_dapm_context *dapm = snd_soc_component_to_dapm(component);
@@ -133,9 +169,27 @@ static const struct simple_amp_data simple_audio_amplifier_data = {
 	.num_dapm_routes	= ARRAY_SIZE(simple_amp_dapm_routes),
 };
 
+static const struct simple_amp_data simple_audio_mono_pga_data = {
+	.supports		= SIMPLE_AUDIO_SUPPORT_PGA,
+	.dapm_widgets		= simple_amp_mono_pga_dapm_widgets,
+	.num_dapm_widgets	= ARRAY_SIZE(simple_amp_mono_pga_dapm_widgets),
+	.dapm_routes		= simple_amp_mono_pga_dapm_routes,
+	.num_dapm_routes	= ARRAY_SIZE(simple_amp_mono_pga_dapm_routes),
+};
+
+static const struct simple_amp_data simple_audio_stereo_pga_data = {
+	.supports		= SIMPLE_AUDIO_SUPPORT_PGA,
+	.dapm_widgets		= simple_amp_stereo_pga_dapm_widgets,
+	.num_dapm_widgets	= ARRAY_SIZE(simple_amp_stereo_pga_dapm_widgets),
+	.dapm_routes		= simple_amp_stereo_pga_dapm_routes,
+	.num_dapm_routes	= ARRAY_SIZE(simple_amp_stereo_pga_dapm_routes),
+};
+
 static const struct of_device_id simple_amp_ids[] = {
 	{ .compatible = "dioo,dio2125",		  .data = &simple_audio_amplifier_data},
 	{ .compatible = "simple-audio-amplifier", .data = &simple_audio_amplifier_data},
+	{ .compatible = "gpio-audio-amp-mono",	  .data = &simple_audio_mono_pga_data},
+	{ .compatible = "gpio-audio-amp-stereo",  .data = &simple_audio_stereo_pga_data},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, simple_amp_ids);
Signature · 1 line
2.54.0
#11 lore raw

Re: [PATCH v3 09/17] ASoC: simple-amplifier: Introduce support for gpio-audio-amp

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Mixing of independent stereo audio paths in the DAPM graph due to the use of a single PGA…

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Mixing of independent stereo audio paths in the DAPM graph due to the use of a single PGA widget.
- [Low] Preprocessor macro #define SIMPLE_AUDIO_SUPPORT_PGA is defined inside a struct definition. --

commit d8f2282bd54ac87735c5b891e08ae94ea9bfa366
Author: Herve Codina <[email protected]>

ASoC: simple-amplifier: Introduce support for gpio-audio-amp

This commit introduces preliminary support for GPIO-driven audio amplifiers. It adds compatible strings and DAPM tables for both mono and stereo variants.

sound/soc/codecs/simple-amplifier.c line 18

Quoted 11 lines
sound/soc/codecs/simple-amplifier.c +3 -0
index 3e644c1c26960..1704cdbb7de5d 100644
@@ -12,6 +13,9 @@
 #include <sound/soc.h>
 
 struct simple_amp_data {
+	unsigned int supports;
+#define SIMPLE_AUDIO_SUPPORT_PGA		BIT(0)
+

This isn't a bug, but should this macro be defined outside the struct?

Macros are handled by the preprocessor and have file-wide scope, so placing it inside the struct can be misleading and goes against standard kernel coding style.

Quoted 1 line
sound/soc/codecs/simple-amplifier.c +0 -0
 	const struct snd_soc_dapm_widget *dapm_widgets;

[ ... ]

sound/soc/codecs/simple-amplifier.c static const struct snd_soc_dapm_route simple_amp_dapm_routes[] = { · line 103

Quoted 7 lines
sound/soc/codecs/simple-amplifier.c +7 -0
+static const struct snd_soc_dapm_route simple_amp_stereo_pga_dapm_routes[] = {
+	{ "PGA", NULL, "INL" },
+	{ "PGA", NULL, "INR" },
+	{ "PGA", NULL, "vdd" },
+	{ "OUTL", NULL, "PGA" },
+	{ "OUTR", NULL, "PGA" },
+};

Does this routing mix the independent stereo audio paths?

By routing both INL and INR into a single PGA widget, and then routing that PGA to both OUTL and OUTR, DAPM will see valid audio paths crossing over (INL to OUTR, and INR to OUTL).

Could this cause incorrect power management behavior where playing audio on just the left channel keeps the right channel's downstream widgets powered up unnecessarily?

[ ... ]

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9
author maintainer
#12 lore raw

PATCH v3 10/17 ASoC: simple-amplifier: gpio-audio-amp: Add support for extra power supplies

In reply to Herve Codina

The gpio-audio-amp devices can use additional power supplies:
- vddio,
- vdda1,
- vdda2

Add support for those additional power supplies.

---
 sound/soc/codecs/simple-amplifier.c | 85 ++++++++++++++++++++++++++++-
 1 file changed, 82 insertions(+), 3 deletions(-)
sound/soc/codecs/simple-amplifier.c +82 -3
index 1704cdbb7de5..89e7e9e53b03 100644
@@ -15,6 +15,7 @@
 struct simple_amp_data {
 	unsigned int supports;
 #define SIMPLE_AUDIO_SUPPORT_PGA		BIT(0)
+#define SIMPLE_AUDIO_SUPPORT_POWER_SUPPLIES	BIT(1)
 
 	const struct snd_soc_dapm_widget *dapm_widgets;
 	unsigned int num_dapm_widgets;
@@ -127,10 +128,86 @@ static int simple_amp_add_basic_dapm(struct snd_soc_component *component)
 	return 0;
 }
 
+struct simple_amp_supply {
+	const char *prop_name;
+	const struct snd_soc_dapm_widget dapm_widget;
+	const struct snd_soc_dapm_route dapm_route;
+};
+
+static const struct simple_amp_supply simple_amp_supplies[] = {
+	{
+		.prop_name = "vddio-supply",
+		.dapm_widget = SND_SOC_DAPM_REGULATOR_SUPPLY("vddio", 0, 0),
+		.dapm_route = { "PGA", NULL, "vddio" },
+	}, {
+		.prop_name = "vdda1-supply",
+		.dapm_widget = SND_SOC_DAPM_REGULATOR_SUPPLY("vdda1", 0, 0),
+		.dapm_route = { "PGA", NULL, "vdda1" },
+	}, {
+		.prop_name = "vdda2-supply",
+		.dapm_widget = SND_SOC_DAPM_REGULATOR_SUPPLY("vdda2", 0, 0),
+		.dapm_route = { "PGA", NULL, "vdda2" },
+	},
+	{ /* End of list */}
+};
+
+static int simple_amp_add_power_supplies(struct snd_soc_component *component)
+{
+	struct snd_soc_dapm_context *dapm = snd_soc_component_to_dapm(component);
+	struct simple_amp *simple_amp = snd_soc_component_get_drvdata(component);
+	const struct simple_amp_supply *supply;
+	struct device *dev = component->dev;
+	int ret;
+
+	/*
+	 * Those additional power supplies are attached to the PGA.
+	 * If PGA is not supported, simply skipped them.
+	 */
+	if (!(simple_amp->data->supports & SIMPLE_AUDIO_SUPPORT_PGA)) {
+		dev_err(dev, "Extra power supplied need PGA\n");
+		return -EINVAL;
+	}
+
+	supply = simple_amp_supplies;
+	do {
+		if (!of_property_present(dev->of_node, supply->prop_name))
+			continue;
+
+		ret = snd_soc_dapm_new_controls(dapm, &supply->dapm_widget, 1);
+		if (ret) {
+			dev_err(dev, "Failed to add control for '%s' (%d)\n",
+				supply->prop_name, ret);
+			return ret;
+		}
+		ret = snd_soc_dapm_add_routes(dapm, &supply->dapm_route, 1);
+		if (ret) {
+			dev_err(dev, "Failed to add route for '%s' (%d)\n",
+				supply->prop_name, ret);
+			return ret;
+		}
+	} while ((++supply)->prop_name);
+
+	return 0;
+}
+
 static int simple_amp_component_probe(struct snd_soc_component *component)
 {
+	struct simple_amp *simple_amp = snd_soc_component_get_drvdata(component);
+	int ret;
+
 	/* Add basic dapm widgets and routes */
-	return simple_amp_add_basic_dapm(component);
+	ret = simple_amp_add_basic_dapm(component);
+	if (ret)
+		return ret;
+
+	/* Add additional power supplies */
+	if (simple_amp->data->supports & SIMPLE_AUDIO_SUPPORT_POWER_SUPPLIES) {
+		ret = simple_amp_add_power_supplies(component);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
 }
 
 static const struct snd_soc_component_driver simple_amp_component_driver = {
@@ -170,7 +247,8 @@ static const struct simple_amp_data simple_audio_amplifier_data = {
 };
 
 static const struct simple_amp_data simple_audio_mono_pga_data = {
-	.supports		= SIMPLE_AUDIO_SUPPORT_PGA,
+	.supports		= SIMPLE_AUDIO_SUPPORT_PGA |
+				  SIMPLE_AUDIO_SUPPORT_POWER_SUPPLIES,
 	.dapm_widgets		= simple_amp_mono_pga_dapm_widgets,
 	.num_dapm_widgets	= ARRAY_SIZE(simple_amp_mono_pga_dapm_widgets),
 	.dapm_routes		= simple_amp_mono_pga_dapm_routes,
@@ -178,7 +256,8 @@ static const struct simple_amp_data simple_audio_mono_pga_data = {
 };
 
 static const struct simple_amp_data simple_audio_stereo_pga_data = {
-	.supports		= SIMPLE_AUDIO_SUPPORT_PGA,
+	.supports		= SIMPLE_AUDIO_SUPPORT_PGA |
+				  SIMPLE_AUDIO_SUPPORT_POWER_SUPPLIES,
 	.dapm_widgets		= simple_amp_stereo_pga_dapm_widgets,
 	.num_dapm_widgets	= ARRAY_SIZE(simple_amp_stereo_pga_dapm_widgets),
 	.dapm_routes		= simple_amp_stereo_pga_dapm_routes,
Signature · 1 line
2.54.0
author maintainer
#13 lore raw

PATCH v3 11/17 ASoC: simple-amplifier: gpio-audio-amp: Add support for mute gpio

In reply to Herve Codina

A gpio can be used to control the amplifier mute feature.

Add support for this mute gpio.

---
 sound/soc/codecs/simple-amplifier.c | 127 +++++++++++++++++++++++++++-
 1 file changed, 125 insertions(+), 2 deletions(-)
sound/soc/codecs/simple-amplifier.c +125 -2
index 89e7e9e53b03..c8991e340def 100644
@@ -12,10 +12,18 @@
 #include <linux/regulator/consumer.h>
 #include <sound/soc.h>
 
+struct simple_amp_single {
+	struct gpio_desc *gpio;
+	bool is_inverted;
+	int kctrl_val;
+	const char *control_name;
+};
+
 struct simple_amp_data {
 	unsigned int supports;
 #define SIMPLE_AUDIO_SUPPORT_PGA		BIT(0)
 #define SIMPLE_AUDIO_SUPPORT_POWER_SUPPLIES	BIT(1)
+#define SIMPLE_AUDIO_SUPPORT_MUTE		BIT(2)
 
 	const struct snd_soc_dapm_widget *dapm_widgets;
 	unsigned int num_dapm_widgets;
@@ -26,6 +34,7 @@ struct simple_amp_data {
 struct simple_amp {
 	const struct simple_amp_data *data;
 	struct gpio_desc *gpiod_enable;
+	struct simple_amp_single mute;
 };
 
 static int simple_amp_power_event(struct snd_soc_dapm_widget *w,
@@ -103,6 +112,78 @@ static const struct snd_soc_dapm_route simple_amp_stereo_pga_dapm_routes[] = {
 	{ "OUTR", NULL, "PGA" },
 };
 
+static int simple_amp_single_kctrl_write_gpio(struct simple_amp_single *single,
+					      int kctrl_val)
+{
+	int gpio_val;
+
+	gpio_val = single->is_inverted ? !kctrl_val : kctrl_val;
+
+	return gpiod_set_value_cansleep(single->gpio, gpio_val);
+}
+
+static int simple_amp_single_kctrl_info(struct snd_kcontrol *kcontrol,
+					struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->count = 1;
+	uinfo->value.integer.min = 0;
+	uinfo->value.integer.max = 1;
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_BOOLEAN;
+	return 0;
+}
+
+static int simple_amp_single_kctrl_get(struct snd_kcontrol *kcontrol,
+				       struct snd_ctl_elem_value *ucontrol)
+{
+	struct simple_amp_single *single = (struct simple_amp_single *)kcontrol->private_value;
+
+	ucontrol->value.integer.value[0] = single->kctrl_val;
+
+	return 0;
+}
+
+static int simple_amp_single_kctrl_put(struct snd_kcontrol *kcontrol,
+				       struct snd_ctl_elem_value *ucontrol)
+{
+	struct simple_amp_single *single = (struct simple_amp_single *)kcontrol->private_value;
+	int kctrl_val;
+	int err;
+
+	kctrl_val = ucontrol->value.integer.value[0] ? 1 : 0;
+
+	if (kctrl_val == single->kctrl_val)
+		return 0;
+
+	err = simple_amp_single_kctrl_write_gpio(single, kctrl_val);
+	if (err)
+		return err;
+
+	single->kctrl_val = kctrl_val;
+
+	return 1; /* The value changed */
+}
+
+static int simple_amp_single_add_kcontrol(struct snd_soc_component *component,
+					  struct simple_amp_single *single)
+{
+	struct snd_kcontrol_new control = {
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = single->control_name,
+		.info = simple_amp_single_kctrl_info,
+		.get = simple_amp_single_kctrl_get,
+		.put = simple_amp_single_kctrl_put,
+		.private_value = (unsigned long)single,
+	};
+	int ret;
+
+	/* Be consistent between single->kctrl_val value and the GPIO value */
+	ret = simple_amp_single_kctrl_write_gpio(single, single->kctrl_val);
+	if (ret)
+		return ret;
+
+	return snd_soc_add_component_controls(component, &control, 1);
+}
+
 static int simple_amp_add_basic_dapm(struct snd_soc_component *component)
 {
 	struct snd_soc_dapm_context *dapm = snd_soc_component_to_dapm(component);
@@ -207,6 +288,21 @@ static int simple_amp_component_probe(struct snd_soc_component *component)
 			return ret;
 	}
 
+	if (simple_amp->mute.gpio) {
+		/*
+		 * The name of the GPIO used is mute. According to this name, 1
+		 * means muted and 0 means un-muted.
+		 *
+		 * An inversion is expected by ALSA. Indeed from ALSA point of
+		 * view, 1 means 'on' (un-muted) and 0 means 'off' (muted).
+		 */
+		simple_amp->mute.is_inverted = true;
+		simple_amp->mute.kctrl_val = 1; /* Un-muted */
+		ret = simple_amp_single_add_kcontrol(component, &simple_amp->mute);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -214,10 +310,26 @@ static const struct snd_soc_component_driver simple_amp_component_driver = {
 	.probe = simple_amp_component_probe,
 };
 
+static int simple_amp_parse_single_gpio(struct device *dev,
+					struct simple_amp_single *single,
+					const char *gpio_property)
+{
+	/* Start with the inactive value */
+	single->is_inverted = false;
+	single->kctrl_val = 0;
+	single->gpio = devm_gpiod_get_optional(dev, gpio_property, GPIOD_OUT_LOW);
+	if (IS_ERR(single->gpio))
+		return dev_err_probe(dev, PTR_ERR(single->gpio),
+				     "Failed to get '%s' gpio\n",
+				     gpio_property);
+	return 0;
+}
+
 static int simple_amp_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct simple_amp *simple_amp;
+	int ret;
 
 	simple_amp = devm_kzalloc(dev, sizeof(*simple_amp), GFP_KERNEL);
 	if (!simple_amp)
@@ -234,6 +346,15 @@ static int simple_amp_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(simple_amp->gpiod_enable),
 				     "Failed to get 'enable' gpio");
 
+	if (simple_amp->data->supports & SIMPLE_AUDIO_SUPPORT_MUTE) {
+		ret = simple_amp_parse_single_gpio(dev, &simple_amp->mute, "mute");
+		if (ret)
+			return ret;
+	}
+
+	/* Set controls name */
+	simple_amp->mute.control_name = "Switch";
+
 	return devm_snd_soc_register_component(dev,
 					       &simple_amp_component_driver,
 					       NULL, 0);
@@ -248,7 +369,8 @@ static const struct simple_amp_data simple_audio_amplifier_data = {
 
 static const struct simple_amp_data simple_audio_mono_pga_data = {
 	.supports		= SIMPLE_AUDIO_SUPPORT_PGA |
-				  SIMPLE_AUDIO_SUPPORT_POWER_SUPPLIES,
+				  SIMPLE_AUDIO_SUPPORT_POWER_SUPPLIES |
+				  SIMPLE_AUDIO_SUPPORT_MUTE,
 	.dapm_widgets		= simple_amp_mono_pga_dapm_widgets,
 	.num_dapm_widgets	= ARRAY_SIZE(simple_amp_mono_pga_dapm_widgets),
 	.dapm_routes		= simple_amp_mono_pga_dapm_routes,
@@ -257,7 +379,8 @@ static const struct simple_amp_data simple_audio_mono_pga_data = {
 
 static const struct simple_amp_data simple_audio_stereo_pga_data = {
 	.supports		= SIMPLE_AUDIO_SUPPORT_PGA |
-				  SIMPLE_AUDIO_SUPPORT_POWER_SUPPLIES,
+				  SIMPLE_AUDIO_SUPPORT_POWER_SUPPLIES |
+				  SIMPLE_AUDIO_SUPPORT_MUTE,
 	.dapm_widgets		= simple_amp_stereo_pga_dapm_widgets,
 	.num_dapm_widgets	= ARRAY_SIZE(simple_amp_stereo_pga_dapm_widgets),
 	.dapm_routes		= simple_amp_stereo_pga_dapm_routes,
Signature · 1 line
2.54.0
author maintainer
#14 lore raw

PATCH v3 12/17 ASoC: simple-amplifier: gpio-audio-amp: Add support for bypass gpio

In reply to Herve Codina

A gpio can be used to control the amplifier bypass feature.

Add support for this bypass gpio in the same way as it has been done for the mute gpio.

---
 sound/soc/codecs/simple-amplifier.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)
sound/soc/codecs/simple-amplifier.c +19 -2
index c8991e340def..b9c09a04832a 100644
@@ -24,6 +24,7 @@ struct simple_amp_data {
 #define SIMPLE_AUDIO_SUPPORT_PGA		BIT(0)
 #define SIMPLE_AUDIO_SUPPORT_POWER_SUPPLIES	BIT(1)
 #define SIMPLE_AUDIO_SUPPORT_MUTE		BIT(2)
+#define SIMPLE_AUDIO_SUPPORT_BYPASS		BIT(3)
 
 	const struct snd_soc_dapm_widget *dapm_widgets;
 	unsigned int num_dapm_widgets;
@@ -35,6 +36,7 @@ struct simple_amp {
 	const struct simple_amp_data *data;
 	struct gpio_desc *gpiod_enable;
 	struct simple_amp_single mute;
+	struct simple_amp_single bypass;
 };
 
 static int simple_amp_power_event(struct snd_soc_dapm_widget *w,
@@ -303,6 +305,12 @@ static int simple_amp_component_probe(struct snd_soc_component *component)
 			return ret;
 	}
 
+	if (simple_amp->bypass.gpio) {
+		ret = simple_amp_single_add_kcontrol(component, &simple_amp->bypass);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -352,8 +360,15 @@ static int simple_amp_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	if (simple_amp->data->supports & SIMPLE_AUDIO_SUPPORT_BYPASS) {
+		ret = simple_amp_parse_single_gpio(dev, &simple_amp->bypass, "bypass");
+		if (ret)
+			return ret;
+	}
+
 	/* Set controls name */
 	simple_amp->mute.control_name = "Switch";
+	simple_amp->bypass.control_name = "Bypass Switch";
 
 	return devm_snd_soc_register_component(dev,
 					       &simple_amp_component_driver,
@@ -370,7 +385,8 @@ static const struct simple_amp_data simple_audio_amplifier_data = {
 static const struct simple_amp_data simple_audio_mono_pga_data = {
 	.supports		= SIMPLE_AUDIO_SUPPORT_PGA |
 				  SIMPLE_AUDIO_SUPPORT_POWER_SUPPLIES |
-				  SIMPLE_AUDIO_SUPPORT_MUTE,
+				  SIMPLE_AUDIO_SUPPORT_MUTE |
+				  SIMPLE_AUDIO_SUPPORT_BYPASS,
 	.dapm_widgets		= simple_amp_mono_pga_dapm_widgets,
 	.num_dapm_widgets	= ARRAY_SIZE(simple_amp_mono_pga_dapm_widgets),
 	.dapm_routes		= simple_amp_mono_pga_dapm_routes,
@@ -380,7 +396,8 @@ static const struct simple_amp_data simple_audio_mono_pga_data = {
 static const struct simple_amp_data simple_audio_stereo_pga_data = {
 	.supports		= SIMPLE_AUDIO_SUPPORT_PGA |
 				  SIMPLE_AUDIO_SUPPORT_POWER_SUPPLIES |
-				  SIMPLE_AUDIO_SUPPORT_MUTE,
+				  SIMPLE_AUDIO_SUPPORT_MUTE |
+				  SIMPLE_AUDIO_SUPPORT_BYPASS,
 	.dapm_widgets		= simple_amp_stereo_pga_dapm_widgets,
 	.num_dapm_widgets	= ARRAY_SIZE(simple_amp_stereo_pga_dapm_widgets),
 	.dapm_routes		= simple_amp_stereo_pga_dapm_routes,
Signature · 1 line
2.54.0
#15 lore raw

Re: [PATCH v3 12/17] ASoC: simple-amplifier: gpio-audio-amp: Add support for bypass gpio

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The bypass GPIO can drive a physical logic HIGH into an unpowered amplifier chip, creating a…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The bypass GPIO can drive a physical logic HIGH into an unpowered amplifier chip, creating a severe risk of back-powering and permanent hardware damage. --

commit 802f9d8e2a1a80c9fdc791af2e538c251cfdc8e9
Author: Herve Codina <[email protected]>

ASoC: simple-amplifier: gpio-audio-amp: Add support for bypass gpio

This commit adds support for controlling an amplifier bypass feature using a GPIO. It duplicates the initialization pattern used for the mute GPIO.

Quoted 4 lines
sound/soc/codecs/simple-amplifier.c +0 -0
index c8991e340def5..b9c09a04832a8 100644

[ ... ]

sound/soc/codecs/simple-amplifier.c static int simple_amp_probe(struct platform_device *pdev) · line 364

Quoted 6 lines
sound/soc/codecs/simple-amplifier.c +2 -0
@@ -352,8 +360,15 @@ static int simple_amp_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	if (simple_amp->data->supports & SIMPLE_AUDIO_SUPPORT_BYPASS) {
+		ret = simple_amp_parse_single_gpio(dev, &simple_amp->bypass, "bypass");

Looking at simple_amp_parse_single_gpio(), it requests the GPIO synchronously with GPIOD_OUT_LOW:

sound/soc/codecs/simple-amplifier.c:simple_amp_parse_single_gpio() {
    ...
    single->gpio = devm_gpiod_get_optional(dev, gpio_property, GPIOD_OUT_LOW);
    ...
}

Since this is called during probe, before the DAPM powers up the device, can this cause hardware damage if the device tree defines the GPIO as active low?

If the GPIO is defined as active low, gpiolib will immediately drive the pin to a physical high voltage. Because the amplifier's power supplies and enable pin are managed by DAPM and disabled at probe time, driving a physical high signal into an unpowered CMOS device might forward-bias the internal ESD protection diodes.

This could back-power the chip from the SoC's GPIO pad and cause latch-up. Would it be safer to strictly sequence the GPIO state changes with the device's power state? For example, by deferring the GPIO output configuration until DAPM powers up the device, or implementing the controls as DAPM widgets.

Note that this identical behavior appears to exist for the mute GPIO as well, which this bypass GPIO implementation is modeled after.

Quoted 7 lines
sound/soc/codecs/simple-amplifier.c +5 -0
+		if (ret)
+			return ret;
+	}
+
 	/* Set controls name */
 	simple_amp->mute.control_name = "Switch";
+	simple_amp->bypass.control_name = "Bypass Switch";
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12
author maintainer
#16 lore raw

PATCH v3 13/17 ASoC: simple-amplifier: gpio-audio-amp: Add support for basic gain

In reply to Herve Codina

Several gpios can be used to control the amplifier gain.

Add basic support for those gpios.

This basic support doesn't include any mapping between the GPIOs value and the physical gain value (dB).

The support for this kind of mapping will be added later on.

---
 sound/soc/codecs/simple-amplifier.c | 125 ++++++++++++++++++++++++++++
 1 file changed, 125 insertions(+)
sound/soc/codecs/simple-amplifier.c +125 -0
index b9c09a04832a..5b172e520dcd 100644
@@ -4,6 +4,7 @@
  * Author: Jerome Brunet <[email protected]>
  */
 
+#include <linux/bitmap.h>
 #include <linux/bits.h>
 #include <linux/gpio/consumer.h>
 #include <linux/mod_devicetable.h>
@@ -19,6 +20,13 @@ struct simple_amp_single {
 	const char *control_name;
 };
 
+struct simple_amp_multi {
+	struct gpio_descs *gpios;
+	u32 kctrl_val;
+	u32 kctrl_max;
+	const char *control_name;
+};
+
 struct simple_amp_data {
 	unsigned int supports;
 #define SIMPLE_AUDIO_SUPPORT_PGA		BIT(0)
@@ -37,6 +45,7 @@ struct simple_amp {
 	struct gpio_desc *gpiod_enable;
 	struct simple_amp_single mute;
 	struct simple_amp_single bypass;
+	struct simple_amp_multi gain;
 };
 
 static int simple_amp_power_event(struct snd_soc_dapm_widget *w,
@@ -186,6 +195,84 @@ static int simple_amp_single_add_kcontrol(struct snd_soc_component *component,
 	return snd_soc_add_component_controls(component, &control, 1);
 }
 
+static int simple_amp_multi_kctrl_write_gpios(struct simple_amp_multi *multi,
+					      u32 kctrl_val)
+{
+	DECLARE_BITMAP(bm, 32);
+	u32 gpio_val;
+
+	if (kctrl_val > multi->kctrl_max)
+		return -EINVAL;
+
+	gpio_val = kctrl_val;
+	bitmap_from_arr32(bm, &gpio_val, multi->gpios->ndescs);
+
+	return gpiod_multi_set_value_cansleep(multi->gpios, bm);
+}
+
+static int simple_amp_multi_kctrl_int_info(struct snd_kcontrol *kcontrol,
+					   struct snd_ctl_elem_info *uinfo)
+{
+	struct simple_amp_multi *multi = (struct simple_amp_multi *)kcontrol->private_value;
+
+	uinfo->count = 1;
+	uinfo->value.integer.min = 0;
+	uinfo->value.integer.max = multi->kctrl_max;
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	return 0;
+}
+
+static int simple_amp_multi_kctrl_int_get(struct snd_kcontrol *kcontrol,
+					  struct snd_ctl_elem_value *ucontrol)
+{
+	struct simple_amp_multi *multi = (struct simple_amp_multi *)kcontrol->private_value;
+
+	ucontrol->value.integer.value[0] = multi->kctrl_val;
+	return 0;
+}
+
+static int simple_amp_multi_kctrl_int_put(struct snd_kcontrol *kcontrol,
+					  struct snd_ctl_elem_value *ucontrol)
+{
+	struct simple_amp_multi *multi = (struct simple_amp_multi *)kcontrol->private_value;
+	u32 kctrl_val;
+	int ret;
+
+	kctrl_val = ucontrol->value.integer.value[0];
+
+	if (kctrl_val == multi->kctrl_val)
+		return 0;
+
+	ret = simple_amp_multi_kctrl_write_gpios(multi, kctrl_val);
+	if (ret)
+		return ret;
+
+	multi->kctrl_val = kctrl_val;
+
+	return 1; /* The value changed */
+}
+
+static int simple_amp_multi_add_kcontrol(struct snd_soc_component *component,
+					 struct simple_amp_multi *multi)
+{
+	struct snd_kcontrol_new control = {
+		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
+		.name = multi->control_name,
+		.info = simple_amp_multi_kctrl_int_info,
+		.get = simple_amp_multi_kctrl_int_get,
+		.put = simple_amp_multi_kctrl_int_put,
+		.private_value = (unsigned long)multi,
+	};
+	int ret;
+
+	/* Be consistent between multi->kctrl_val value and the GPIOs value */
+	ret = simple_amp_multi_kctrl_write_gpios(multi, multi->kctrl_val);
+	if (ret)
+		return ret;
+
+	return snd_soc_add_component_controls(component, &control, 1);
+}
+
 static int simple_amp_add_basic_dapm(struct snd_soc_component *component)
 {
 	struct snd_soc_dapm_context *dapm = snd_soc_component_to_dapm(component);
@@ -311,6 +398,12 @@ static int simple_amp_component_probe(struct snd_soc_component *component)
 			return ret;
 	}
 
+	if (simple_amp->gain.gpios) {
+		ret = simple_amp_multi_add_kcontrol(component, &simple_amp->gain);
+		if (ret)
+			return ret;
+	}
+
 	return 0;
 }
 
@@ -333,6 +426,31 @@ static int simple_amp_parse_single_gpio(struct device *dev,
 	return 0;
 }
 
+static int simple_amp_parse_multi_gpio(struct device *dev,
+				       struct simple_amp_multi *multi,
+				       const char *gpios_property)
+{
+	/* Start with the value 0 (GPIO inactive). Can be changed later */
+	multi->kctrl_val = 0;
+	multi->gpios = devm_gpiod_get_array_optional(dev, gpios_property, GPIOD_OUT_LOW);
+	if (IS_ERR(multi->gpios))
+		return dev_err_probe(dev, PTR_ERR(multi->gpios),
+				     "Failed to get '%s' gpios\n",
+				     gpios_property);
+	if (!multi->gpios)
+		return 0;
+
+	if (multi->gpios->ndescs > 16)
+		return dev_err_probe(dev, -EINVAL,
+				     "Number of '%s' gpios limited to 16\n",
+				     gpios_property);
+
+	/* Set default value for the kctrl_max. Can be changed later */
+	multi->kctrl_max = (1 << multi->gpios->ndescs) - 1;
+
+	return 0;
+}
+
 static int simple_amp_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -366,7 +484,14 @@ static int simple_amp_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	if (simple_amp->data->supports & SIMPLE_AUDIO_SUPPORT_PGA) {
+		ret = simple_amp_parse_multi_gpio(dev, &simple_amp->gain, "gain");
+		if (ret)
+			return ret;
+	}
+
 	/* Set controls name */
+	simple_amp->gain.control_name = "Volume";
 	simple_amp->mute.control_name = "Switch";
 	simple_amp->bypass.control_name = "Bypass Switch";
 
Signature · 1 line
2.54.0
author maintainer
#17 lore raw

PATCH v3 14/17 ASoC: simple-amplifier: gpio-audio-amp: Add support for gain-ranges

In reply to Herve Codina

The mapping between physical gain values and gpio values can be expressed using ranges described in the gain-ranges property.

This gain-ranges property is an array of ranges.

Each range in the array is defined by the first point and last point in the range. Those points are a pair of values, the gpios value and the related gain (dB) value.

With that, a given range defines N possible items (from the first point gpios value to the last point gpios value) in order to set a gain from the first point gain value to the last point gain value.

Handle this description and the related kcontrol.

---
 sound/soc/codecs/simple-amplifier.c | 317 +++++++++++++++++++++++++++-
 1 file changed, 312 insertions(+), 5 deletions(-)
sound/soc/codecs/simple-amplifier.c +312 -5
index 5b172e520dcd..5759f9bc2f4f 100644
@@ -7,11 +7,16 @@
 #include <linux/bitmap.h>
 #include <linux/bits.h>
 #include <linux/gpio/consumer.h>
+#include <linux/math.h>
+#include <linux/minmax.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
+#include <linux/slab.h>
 #include <sound/soc.h>
+#include <linux/sort.h>
+#include <sound/tlv.h>
 
 struct simple_amp_single {
 	struct gpio_desc *gpio;
@@ -20,11 +25,35 @@ struct simple_amp_single {
 	const char *control_name;
 };
 
+struct simple_amp_point {
+	u32 gpio_val;
+	int gain_db;
+};
+
+struct simple_amp_range {
+	unsigned int nb_points;
+	struct simple_amp_point min;
+	struct simple_amp_point max;
+};
+
+struct simple_amp_ranges {
+	unsigned int nb_ranges;
+	struct simple_amp_range *tab_ranges;
+};
+
+enum simple_amp_mode {
+	SIMPLE_AMP_MODE_NONE,
+	SIMPLE_AMP_MODE_RANGES,
+};
+
 struct simple_amp_multi {
 	struct gpio_descs *gpios;
 	u32 kctrl_val;
 	u32 kctrl_max;
 	const char *control_name;
+	unsigned int *tlv_array;
+	enum simple_amp_mode mode;
+	struct simple_amp_ranges ranges;
 };
 
 struct simple_amp_data {
@@ -195,6 +224,32 @@ static int simple_amp_single_add_kcontrol(struct snd_soc_component *component,
 	return snd_soc_add_component_controls(component, &control, 1);
 }
 
+static u32 simple_amp_multi_ranges_kctrl_to_gpio(u32 kctrl_val,
+						 struct simple_amp_ranges *ranges)
+{
+	struct simple_amp_range *range;
+	u32 index = kctrl_val;
+	unsigned int i;
+
+	for (i = 0; i < ranges->nb_ranges; i++) {
+		range = &ranges->tab_ranges[i];
+
+		if (index < range->nb_points)
+			return (range->max.gpio_val >= range->min.gpio_val) ?
+				range->min.gpio_val + index :
+				range->min.gpio_val - index;
+
+		index -= range->nb_points;
+	}
+
+	/*
+	 * Given index out of possible ranges. This is shouldn't happen.
+	 * Signal the issue and return the maximum value
+	 */
+	WARN(1, "kctrl_val %u out of ranges\n", kctrl_val);
+	return ranges->tab_ranges[ranges->nb_ranges - 1].max.gpio_val;
+}
+
 static int simple_amp_multi_kctrl_write_gpios(struct simple_amp_multi *multi,
 					      u32 kctrl_val)
 {
@@ -204,7 +259,12 @@ static int simple_amp_multi_kctrl_write_gpios(struct simple_amp_multi *multi,
 	if (kctrl_val > multi->kctrl_max)
 		return -EINVAL;
 
-	gpio_val = kctrl_val;
+	if (multi->mode == SIMPLE_AMP_MODE_RANGES)
+		gpio_val = simple_amp_multi_ranges_kctrl_to_gpio(kctrl_val,
+								 &multi->ranges);
+	else
+		gpio_val = kctrl_val;
+
 	bitmap_from_arr32(bm, &gpio_val, multi->gpios->ndescs);
 
 	return gpiod_multi_set_value_cansleep(multi->gpios, bm);
@@ -252,6 +312,38 @@ static int simple_amp_multi_kctrl_int_put(struct snd_kcontrol *kcontrol,
 	return 1; /* The value changed */
 }
 
+static unsigned int *simple_amp_alloc_tlv_ranges(const struct simple_amp_ranges *ranges)
+{
+	unsigned int index;
+	unsigned int *tlv;
+	unsigned int *t;
+	unsigned int i;
+
+	tlv = kzalloc_objs(*tlv, 2 + ranges->nb_ranges * 6, GFP_KERNEL);
+	if (!tlv)
+		return NULL;
+
+	t = tlv;
+
+	/* Fill first TLV */
+	*t++ = SNDRV_CTL_TLVT_DB_RANGE; /* Tag */
+	*t++ = ranges->nb_ranges * 6 * sizeof(*tlv); /* Len */
+	/* Ranges are sorted from lower to higher value */
+	index = 0;
+	for (i = 0; i < ranges->nb_ranges; i++) {
+		/* Fill range item i */
+		*t++ = index;  /* min */
+		index += ranges->tab_ranges[i].nb_points;
+		*t++ = index - 1;  /* max */
+		*t++ = SNDRV_CTL_TLVT_DB_MINMAX; /* Tag */
+		*t++ = 2 * sizeof(*tlv); /* Len */
+		*t++ = ranges->tab_ranges[i].min.gain_db; /* min_dB */
+		*t++ = ranges->tab_ranges[i].max.gain_db; /* max_dB */
+	}
+
+	return tlv;
+}
+
 static int simple_amp_multi_add_kcontrol(struct snd_soc_component *component,
 					 struct simple_amp_multi *multi)
 {
@@ -265,12 +357,39 @@ static int simple_amp_multi_add_kcontrol(struct snd_soc_component *component,
 	};
 	int ret;
 
+	switch (multi->mode) {
+	case SIMPLE_AMP_MODE_RANGES:
+		multi->tlv_array = simple_amp_alloc_tlv_ranges(&multi->ranges);
+		if (!multi->tlv_array)
+			return -ENOMEM;
+
+		control.access = SNDRV_CTL_ELEM_ACCESS_TLV_READ |
+				 SNDRV_CTL_ELEM_ACCESS_READWRITE;
+		control.tlv.p = multi->tlv_array;
+		break;
+
+	case SIMPLE_AMP_MODE_NONE:
+		/* Already set control configuration is enough */
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
 	/* Be consistent between multi->kctrl_val value and the GPIOs value */
 	ret = simple_amp_multi_kctrl_write_gpios(multi, multi->kctrl_val);
 	if (ret)
-		return ret;
+		goto err_free_tlv_array;
 
-	return snd_soc_add_component_controls(component, &control, 1);
+	ret = snd_soc_add_component_controls(component, &control, 1);
+	if (ret)
+		goto err_free_tlv_array;
+
+	return 0;
+
+err_free_tlv_array:
+	kfree(multi->tlv_array);
+	return ret;
 }
 
 static int simple_amp_add_basic_dapm(struct snd_soc_component *component)
@@ -407,8 +526,17 @@ static int simple_amp_component_probe(struct snd_soc_component *component)
 	return 0;
 }
 
+static void simple_amp_component_remove(struct snd_soc_component *component)
+{
+	struct simple_amp *simple_amp = snd_soc_component_get_drvdata(component);
+
+	kfree(simple_amp->gain.tlv_array);
+	simple_amp->gain.tlv_array = NULL;
+}
+
 static const struct snd_soc_component_driver simple_amp_component_driver = {
 	.probe = simple_amp_component_probe,
+	.remove = simple_amp_component_remove,
 };
 
 static int simple_amp_parse_single_gpio(struct device *dev,
@@ -426,10 +554,179 @@ static int simple_amp_parse_single_gpio(struct device *dev,
 	return 0;
 }
 
+static int simple_amp_cmp_ranges(const void *a, const void *b)
+{
+	const struct simple_amp_range *a_range = a;
+	const struct simple_amp_range *b_range = b;
+
+	/* Ranges a and b don't overlap. This has been already checked */
+
+	return a_range->min.gain_db - b_range->max.gain_db;
+}
+
+static int simple_amp_check_new_range(const struct simple_amp_range *new_range,
+				      const struct simple_amp_range *tab_ranges,
+				      unsigned int nb_ranges)
+{
+	unsigned int i;
+
+	for (i = 0; i < nb_ranges; i++) {
+		/* Check for range overlaps */
+		if (new_range->min.gain_db >= tab_ranges[i].min.gain_db &&
+		    new_range->min.gain_db <= tab_ranges[i].max.gain_db)
+			return -EINVAL;
+
+		if (new_range->max.gain_db >= tab_ranges[i].min.gain_db &&
+		    new_range->max.gain_db <= tab_ranges[i].max.gain_db)
+			return -EINVAL;
+
+		if (new_range->min.gain_db <= tab_ranges[i].min.gain_db &&
+		    new_range->max.gain_db >= tab_ranges[i].max.gain_db)
+			return -EINVAL;
+	}
+	return 0;
+}
+
+static int simple_amp_parse_ranges(struct device *dev,
+				   struct simple_amp_multi *multi,
+				   const char *ranges_property)
+{
+	struct simple_amp_ranges *ranges = &multi->ranges;
+	struct simple_amp_range *range;
+	struct device_node *np = dev->of_node;
+	struct simple_amp_point first_point;
+	unsigned int max_gpio_val;
+	unsigned int i;
+	int ret;
+	u32 u;
+	s32 s;
+
+	max_gpio_val = (1 << multi->gpios->ndescs) - 1;
+
+	ret = of_property_count_u32_elems(np, ranges_property);
+	if (ret < 0)
+		return ret;
+
+	/* The ranges array cannot be empty */
+	if (ret == 0)
+		return -EINVAL;
+	/*
+	 * One range item is composed of 2 points and each point is composed of
+	 * 2 values.
+	 */
+	if (ret % 4)
+		return -EINVAL;
+
+	ranges->nb_ranges = ret / 4;
+
+	/* The worst case is one range per possible gpio value */
+	if (ranges->nb_ranges > max_gpio_val + 1)
+		return -EINVAL;
+
+	ranges->tab_ranges = devm_kcalloc(dev, ranges->nb_ranges,
+					  sizeof(*ranges->tab_ranges),
+					  GFP_KERNEL);
+	if (!ranges->tab_ranges)
+		return -ENOMEM;
+
+	multi->kctrl_max = 0;
+	for (i = 0; i < ranges->nb_ranges; i++) {
+		range = &ranges->tab_ranges[i];
+
+		/* First gpios value */
+		ret = of_property_read_u32_index(np, ranges_property, i * 4, &u);
+		if (ret)
+			return ret;
+		if (u > max_gpio_val)
+			return -EINVAL;
+
+		range->min.gpio_val = u;
+
+		/* First Gain value */
+		ret = of_property_read_s32_index(np, ranges_property, i * 4 + 1, &s);
+		if (ret)
+			return ret;
+
+		range->min.gain_db = s;
+
+		/* Second gpios value */
+		ret = of_property_read_u32_index(np, ranges_property, i * 4 + 2, &u);
+		if (ret)
+			return ret;
+		if (u > max_gpio_val)
+			return -EINVAL;
+
+		range->max.gpio_val = u;
+
+		/* Second Gain value */
+		ret = of_property_read_s32_index(np, ranges_property, i * 4 + 3, &s);
+		if (ret)
+			return ret;
+
+		range->max.gain_db = s;
+
+		/* Save the first point for later usage */
+		if (i == 0)
+			first_point = range->min;
+
+		/* Fix min and max if needed */
+		if (range->min.gain_db > range->max.gain_db)
+			swap(range->min, range->max);
+
+		ret = simple_amp_check_new_range(range, ranges->tab_ranges, i);
+		if (ret)
+			return ret;
+
+		range->nb_points = abs_diff(range->min.gpio_val,
+					    range->max.gpio_val) + 1;
+
+		multi->kctrl_max += range->nb_points;
+	}
+
+	multi->kctrl_max -= 1;
+
+	/* Sort the tab_range array by gain_db value */
+	sort(ranges->tab_ranges, ranges->nb_ranges, sizeof(*ranges->tab_ranges),
+	     simple_amp_cmp_ranges, NULL);
+
+	/*
+	 * multi->kctrl_val is the index in tab_ranges.
+	 *
+	 * Choose to have the initial amplification value set to the first point
+	 * available in the first range available in the tab_ranges array before
+	 * sorting.
+	 *
+	 * This first point has been identified before sorting. Search for it in
+	 * the sorted array in order to set the multi->kctrl_val initial value.
+	 */
+	multi->kctrl_val = 0;
+	for (i = 0; i < ranges->nb_ranges; i++) {
+		range = &ranges->tab_ranges[i];
+
+		if (range->min.gpio_val == first_point.gpio_val &&
+		    range->min.gain_db == first_point.gain_db)
+			break;
+
+		multi->kctrl_val += range->nb_points;
+
+		if (range->max.gpio_val == first_point.gpio_val &&
+		    range->max.gain_db == first_point.gain_db) {
+			multi->kctrl_val--;
+			break;
+		}
+	}
+
+	return 0;
+}
+
 static int simple_amp_parse_multi_gpio(struct device *dev,
 				       struct simple_amp_multi *multi,
-				       const char *gpios_property)
+				       const char *gpios_property,
+				       const char *ranges_property)
 {
+	struct device_node *np = dev->of_node;
+	int ret;
+
 	/* Start with the value 0 (GPIO inactive). Can be changed later */
 	multi->kctrl_val = 0;
 	multi->gpios = devm_gpiod_get_array_optional(dev, gpios_property, GPIOD_OUT_LOW);
@@ -448,6 +745,15 @@ static int simple_amp_parse_multi_gpio(struct device *dev,
 	/* Set default value for the kctrl_max. Can be changed later */
 	multi->kctrl_max = (1 << multi->gpios->ndescs) - 1;
 
+	multi->mode = SIMPLE_AMP_MODE_NONE;
+	if (of_property_present(np, ranges_property)) {
+		ret = simple_amp_parse_ranges(dev, multi, ranges_property);
+		if (ret < 0)
+			return dev_err_probe(dev, ret, "Failed to parse '%s'\n",
+					     ranges_property);
+		multi->mode = SIMPLE_AMP_MODE_RANGES;
+	}
+
 	return 0;
 }
 
@@ -485,7 +791,8 @@ static int simple_amp_probe(struct platform_device *pdev)
 	}
 
 	if (simple_amp->data->supports & SIMPLE_AUDIO_SUPPORT_PGA) {
-		ret = simple_amp_parse_multi_gpio(dev, &simple_amp->gain, "gain");
+		ret = simple_amp_parse_multi_gpio(dev, &simple_amp->gain, "gain",
+						  "gain-ranges");
 		if (ret)
 			return ret;
 	}
Signature · 1 line
2.54.0
#18 lore raw

Re: [PATCH v3 14/17] ASoC: simple-amplifier: gpio-audio-amp: Add support for gain-ranges

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] Unsynchronized freeing of dynamically allocated TLV array causes a Use-After-Free during…

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Unsynchronized freeing of dynamically allocated TLV array causes a Use-After-Free during concurrent ALSA TLV read.
- [Medium] Integer overflow in kctrl_max accumulation leads to validation bypass and a reachable WARN(1).
- [Medium] Integer overflow in simple_amp_cmp_ranges can corrupt the sorted array.
- [Medium] Unprotected state modifications in ALSA control put handlers lead to a TOCTOU race and hardware state desynchronization. --

commit 24a3721afe8e210f60da48c3439973f4c9f7e094
Author: Herve Codina <[email protected]>

ASoC: simple-amplifier: gpio-audio-amp: Add support for gain-ranges

This commit introduces support for gain-ranges in the simple-amplifier driver, mapping physical gain values to GPIO values through an array of ranges and exporting them via an ALSA kcontrol with TLV range information.

sound/soc/codecs/simple-amplifier.c +0 -0
index 5b172e520dcd0..5759f9bc2f4ff 100644

[ ... ]

Review static int simple_amp_multi_kctrl_int_put(struct snd_kcontrol *kcontrol, · line 313

Quoted 3 lines
(quoted diff) +0 -0
@@ -252,6 +312,38 @@ static int simple_amp_multi_kctrl_int_put(struct snd_kcontrol *kcontrol,
 	return 1; /* The value changed */
 }

Can concurrent calls to simple_amp_multi_kctrl_int_put() and simple_amp_single_kctrl_put() lead to hardware state desynchronization?

These handlers read the cached software state, issue hardware I/O via gpiod_multi_set_value_cansleep(), and update the cached state without driver-level locking.

The OSS emulation layer only holds a read lock when invoking kctl->put(). Two racing threads could pass the equality check and interleave their hardware writes, causing the cached software state to permanently desynchronize from the actual hardware GPIO state.

[ ... ]

Review static int simple_amp_component_probe(struct snd_soc_component *component) · line 535

Quoted 11 lines
(quoted diff) +7 -0
@@ -407,8 +526,17 @@ static int simple_amp_component_probe(struct snd_soc_component *component)
 	return 0;
 }
 
+static void simple_amp_component_remove(struct snd_soc_component *component)
+{
+	struct simple_amp *simple_amp = snd_soc_component_get_drvdata(component);
+
+	kfree(simple_amp->gain.tlv_array);
+	simple_amp->gain.tlv_array = NULL;
+}

Does this kfree() cause a use-after-free during concurrent ALSA TLV reads?

ALSA controls remain accessible as long as a user holds an open file descriptor, and snd_card_disconnect() does not drain active ioctls. A concurrent SNDRV_CTL_IOCTL_TLV_READ could be blocked in copy_to_user() reading control.tlv.p when this memory is synchronously freed.

Should the array's lifetime instead be bound to the kcontrol via the kcontrol->private_free callback?

[ ... ]

Review static int simple_amp_parse_single_gpio(struct device *dev, · line 565

Quoted 13 lines
(quoted diff) +9 -0
@@ -426,10 +554,179 @@ static int simple_amp_parse_single_gpio(struct device *dev,
 	return 0;
 }
 
+static int simple_amp_cmp_ranges(const void *a, const void *b)
+{
+	const struct simple_amp_range *a_range = a;
+	const struct simple_amp_range *b_range = b;
+
+	/* Ranges a and b don't overlap. This has been already checked */
+
+	return a_range->min.gain_db - b_range->max.gain_db;
+}

Can this subtraction mathematically overflow?

If highly negative and highly positive s32 values are provided via the Device Tree, this operation could reverse the intended sign and break the strict weak ordering required by sort(), corrupting the resulting array state.

Could this use safe inequality checks instead of subtraction?

[ ... ]

Quoted 4 lines
(quoted diff) +4 -0
+static int simple_amp_parse_ranges(struct device *dev,
+				   struct simple_amp_multi *multi,
+				   const char *ranges_property)
+{

[ ... ]

Review

Quoted 7 lines
(quoted diff) +7 -0
+		range->nb_points = abs_diff(range->min.gpio_val,
+					    range->max.gpio_val) + 1;
+
+		multi->kctrl_max += range->nb_points;
+	}
+
+	multi->kctrl_max -= 1;

Does this code overflow multi->kctrl_max?

A malformed Device Tree could define up to 65,536 ranges, each mapping to 65,536 points. This summation would reach exactly 4,294,967,296, completely overflowing the u32 back to 0.

The subsequent multi->kctrl_max -= 1 operation would then underflow it to 0xFFFFFFFF, bypassing the bounds check in simple_amp_multi_kctrl_write_gpios() and triggering a reachable WARN(1) in simple_amp_multi_ranges_kctrl_to_gpio() which could act as a denial of service if panic_on_warn is set.

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=14
author maintainer
#19 lore raw

PATCH v3 15/17 ASoC: simple-amplifier: gpio-audio-amp: Add support for gain-labels

In reply to Herve Codina

The possible gain values can be described using labels instead of gain values in dB.

Those different labels are attached to a gpio values using the gain-labels property.

Using the gain-labels description is mutually exclusive with gain-ranges description used to describe the relationship between gpios values and gain values.

Handle the gain-labels description and the related kcontrol.

---
 sound/soc/codecs/simple-amplifier.c | 116 +++++++++++++++++++++++++++-
 1 file changed, 113 insertions(+), 3 deletions(-)
sound/soc/codecs/simple-amplifier.c +113 -3
index 5759f9bc2f4f..3785044235bb 100644
@@ -41,9 +41,15 @@ struct simple_amp_ranges {
 	struct simple_amp_range *tab_ranges;
 };
 
+struct simple_amp_labels {
+	unsigned int nb_labels;
+	const char **tab_labels;
+};
+
 enum simple_amp_mode {
 	SIMPLE_AMP_MODE_NONE,
 	SIMPLE_AMP_MODE_RANGES,
+	SIMPLE_AMP_MODE_LABELS,
 };
 
 struct simple_amp_multi {
@@ -53,7 +59,10 @@ struct simple_amp_multi {
 	const char *control_name;
 	unsigned int *tlv_array;
 	enum simple_amp_mode mode;
-	struct simple_amp_ranges ranges;
+	union {
+		struct simple_amp_ranges ranges;
+		struct simple_amp_labels labels;
+	};
 };
 
 struct simple_amp_data {
@@ -312,6 +321,45 @@ static int simple_amp_multi_kctrl_int_put(struct snd_kcontrol *kcontrol,
 	return 1; /* The value changed */
 }
 
+static int simple_amp_multi_kctrl_enum_info(struct snd_kcontrol *kcontrol,
+					    struct snd_ctl_elem_info *uinfo)
+{
+	struct simple_amp_multi *multi = (struct simple_amp_multi *)kcontrol->private_value;
+
+	return snd_ctl_enum_info(uinfo, 1, multi->labels.nb_labels,
+				 multi->labels.tab_labels);
+}
+
+static int simple_amp_multi_kctrl_enum_get(struct snd_kcontrol *kcontrol,
+					   struct snd_ctl_elem_value *ucontrol)
+{
+	struct simple_amp_multi *multi = (struct simple_amp_multi *)kcontrol->private_value;
+
+	ucontrol->value.enumerated.item[0] = multi->kctrl_val;
+	return 0;
+}
+
+static int simple_amp_multi_kctrl_enum_put(struct snd_kcontrol *kcontrol,
+					   struct snd_ctl_elem_value *ucontrol)
+{
+	struct simple_amp_multi *multi = (struct simple_amp_multi *)kcontrol->private_value;
+	u32 kctrl_val;
+	int ret;
+
+	kctrl_val = ucontrol->value.enumerated.item[0];
+
+	if (kctrl_val == multi->kctrl_val)
+		return 0;
+
+	ret = simple_amp_multi_kctrl_write_gpios(multi, kctrl_val);
+	if (ret)
+		return ret;
+
+	multi->kctrl_val = kctrl_val;
+
+	return 1; /* The value changed */
+}
+
 static unsigned int *simple_amp_alloc_tlv_ranges(const struct simple_amp_ranges *ranges)
 {
 	unsigned int index;
@@ -368,6 +416,13 @@ static int simple_amp_multi_add_kcontrol(struct snd_soc_component *component,
 		control.tlv.p = multi->tlv_array;
 		break;
 
+	case SIMPLE_AMP_MODE_LABELS:
+		/* Use enumerated values */
+		control.info = simple_amp_multi_kctrl_enum_info;
+		control.get = simple_amp_multi_kctrl_enum_get;
+		control.put = simple_amp_multi_kctrl_enum_put;
+		break;
+
 	case SIMPLE_AMP_MODE_NONE:
 		/* Already set control configuration is enough */
 		break;
@@ -719,10 +774,44 @@ static int simple_amp_parse_ranges(struct device *dev,
 	return 0;
 }
 
+static int simple_amp_parse_labels(struct device *dev,
+				   struct simple_amp_multi *multi,
+				   const char *labels_property)
+{
+	struct simple_amp_labels *labels = &multi->labels;
+	struct device_node *np = dev->of_node;
+	int ret;
+
+	ret = of_property_count_strings(np, labels_property);
+	if (ret < 0)
+		return ret;
+
+	/* The labels array cannot be empty */
+	if (ret == 0)
+		return -EINVAL;
+
+	labels->nb_labels = ret;
+	if (labels->nb_labels > (1 << multi->gpios->ndescs))
+		return -EINVAL;
+
+	labels->tab_labels = devm_kcalloc(dev, labels->nb_labels,
+					  sizeof(*labels->tab_labels),
+					  GFP_KERNEL);
+	if (!labels->tab_labels)
+		return -ENOMEM;
+
+	multi->kctrl_max = labels->nb_labels - 1;
+	multi->kctrl_val = 0;
+
+	return of_property_read_string_array(np, labels_property, labels->tab_labels,
+					     labels->nb_labels);
+}
+
 static int simple_amp_parse_multi_gpio(struct device *dev,
 				       struct simple_amp_multi *multi,
 				       const char *gpios_property,
-				       const char *ranges_property)
+				       const char *ranges_property,
+				       const char *labels_property)
 {
 	struct device_node *np = dev->of_node;
 	int ret;
@@ -752,6 +841,13 @@ static int simple_amp_parse_multi_gpio(struct device *dev,
 			return dev_err_probe(dev, ret, "Failed to parse '%s'\n",
 					     ranges_property);
 		multi->mode = SIMPLE_AMP_MODE_RANGES;
+	} else if (of_property_present(np, labels_property)) {
+		ret = simple_amp_parse_labels(dev, multi, labels_property);
+		if (ret < 0)
+			return dev_err_probe(dev, ret, "Failed to parse '%s'\n",
+					     labels_property);
+
+		multi->mode = SIMPLE_AMP_MODE_LABELS;
 	}
 
 	return 0;
@@ -792,7 +888,7 @@ static int simple_amp_probe(struct platform_device *pdev)
 
 	if (simple_amp->data->supports & SIMPLE_AUDIO_SUPPORT_PGA) {
 		ret = simple_amp_parse_multi_gpio(dev, &simple_amp->gain, "gain",
-						  "gain-ranges");
+						  "gain-ranges", "gain-labels");
 		if (ret)
 			return ret;
 	}
@@ -802,6 +898,20 @@ static int simple_amp_probe(struct platform_device *pdev)
 	simple_amp->mute.control_name = "Switch";
 	simple_amp->bypass.control_name = "Bypass Switch";
 
+	if (simple_amp->gain.mode == SIMPLE_AMP_MODE_LABELS) {
+		/*
+		 * The gain widget control will use enumerated values.
+		 *
+		 * Having just "Voltage" and "Switch" widget names with
+		 * enumerated values and boolean value can confuse ALSA in terms
+		 * of possible values (strings).
+		 *
+		 * Make things clear and avoid the just "Switch" name in that
+		 * case.
+		 */
+		simple_amp->mute.control_name = "Out Switch";
+	}
+
 	return devm_snd_soc_register_component(dev,
 					       &simple_amp_component_driver,
 					       NULL, 0);
Signature · 1 line
2.54.0
author maintainer
#20 lore raw

PATCH v3 16/17 ASoC: simple-amplifier: Update author and copyright

In reply to Herve Codina

After reworking the simple-amplifier driver and adding support for gpio-audio-amp in the driver, add myself as the author of the gpio-audio-amp part of the driver and add a related copyright.

---
 sound/soc/codecs/simple-amplifier.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
sound/soc/codecs/simple-amplifier.c +8 -2
index 3785044235bb..ca0e6ce8cc37 100644
@@ -1,7 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * Copyright (c) 2017 BayLibre, SAS.
- * Author: Jerome Brunet <[email protected]>
+ * Support for gpio amplifier
+ *   Copyright 2026 CS GROUP France
+ *   Author: Herve Codina <[email protected]>
+ *
+ * Basic simple amplifier driver
+ *   Copyright (c) 2017 BayLibre, SAS.
+ *   Author: Jerome Brunet <[email protected]>
  */
 
 #include <linux/bitmap.h>
@@ -967,4 +972,5 @@ module_platform_driver(simple_amp_driver);
 
 MODULE_DESCRIPTION("ASoC Simple Audio Amplifier driver");
 MODULE_AUTHOR("Jerome Brunet <[email protected]>");
+MODULE_AUTHOR("Herve Codina <[email protected]>");
 MODULE_LICENSE("GPL");
Signature · 1 line
2.54.0
#21 lore raw

Re: [PATCH v3 16/17] ASoC: simple-amplifier: Update author and copyright

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Copyright and authorship changes should be squashed into the relevant code commits. --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Copyright and authorship changes should be squashed into the relevant code commits. --

commit 8c20a26d5c232c4d2ed2b81d9a71f2b38091a58b
Author: Herve Codina <[email protected]>

ASoC: simple-amplifier: Update author and copyright

This commit updates the author and copyright information for the simple-amplifier driver to reflect the recent addition of gpio-audio-amp support.

Quoted 3 lines

After reworking the simple-amplifier driver and adding support for gpio-audio-amp in the driver, add myself as the author of the gpio-audio-amp part of the driver and add a related copyright.

This isn't a bug, but should these copyright and authorship updates be squashed directly into the commit that introduces the gpio-audio-amp support?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=16
author maintainer
#22 lore raw

PATCH v3 17/17 MAINTAINERS: Add the ASoC gpio audio amplifier entry

In reply to Herve Codina

After contributing the component, add myself as the maintainer for the ASoC gpio audio amplifier component.

---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)
MAINTAINERS +7 -0
index 2fb1c75afd16..4bd360f7aa80 100644
@@ -24985,6 +24985,13 @@ F:	include/sound/dmaengine_pcm.h
 F:	sound/core/pcm_dmaengine.c
 F:	sound/soc/soc-generic-dmaengine-pcm.c
 
+SOUND - SOC LAYER / GPIO AUDIO AMPLIFIER
+M:	Herve Codina <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	Documentation/devicetree/bindings/sound/gpio-audio-amp.yaml
+F:	sound/soc/codecs/simple-amplifier.c
+
 SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEMENT (ASoC)
 M:	Liam Girdwood <[email protected]>
 M:	Mark Brown <[email protected]>
Signature · 1 line
2.54.0
maintainer
#23 lore raw
In reply to Herve Codina
Quoted 8 lines · Herve Codina

ASoC: Add support for GPIOs driven amplifiers

On some embedded system boards, audio amplifiers are designed using discrete components such as op-amp, several resistors and switches to either adjust the gain (switching resistors) or fully switch the audio signal path (mute and/or bypass features).

[...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.2

Thanks!

[01/17] of: Introduce of_property_read_s32_index()

        https://git.kernel.org/broonie/sound/c/37dba38ac8d0
[02/17] ASoC: dt-bindings: Add support for the GPIOs driven amplifier
        https://git.kernel.org/broonie/sound/c/04a1fe8e6582
[03/17] ASoC: simple-amplifier: Remove DRV_NAME defined value
        https://git.kernel.org/broonie/sound/c/28188a6d8869
[04/17] ASoC: simple-amplifier: Add missing headers
        https://git.kernel.org/broonie/sound/c/23cf9e3ca498
[05/17] ASoC: simple-amplifier: Remove CONFIG_OF flag and of_match_ptr()
        https://git.kernel.org/broonie/sound/c/2c562adb159e
[06/17] ASoC: simple-amplifier: Rename drv_event() function
        https://git.kernel.org/broonie/sound/c/dc8beaad2926
[07/17] ASoC: simple-amplifier: Use 'simple_amp' variable name instead of 'priv'
        https://git.kernel.org/broonie/sound/c/34ddd2d368c3
[08/17] ASoC: simple-amplifier: Remove DAPM widgets and routes from the ASoC component driver
        https://git.kernel.org/broonie/sound/c/4d84b75e5eec
[09/17] ASoC: simple-amplifier: Introduce support for gpio-audio-amp
        https://git.kernel.org/broonie/sound/c/41e3ebbfcab1
[10/17] ASoC: simple-amplifier: gpio-audio-amp: Add support for extra power supplies
        https://git.kernel.org/broonie/sound/c/f68933d0eb67
[11/17] ASoC: simple-amplifier: gpio-audio-amp: Add support for mute gpio
        https://git.kernel.org/broonie/sound/c/432331970eca
[12/17] ASoC: simple-amplifier: gpio-audio-amp: Add support for bypass gpio
        https://git.kernel.org/broonie/sound/c/46c21e55c3bf
[13/17] ASoC: simple-amplifier: gpio-audio-amp: Add support for basic gain
        https://git.kernel.org/broonie/sound/c/5796026b8e90
[14/17] ASoC: simple-amplifier: gpio-audio-amp: Add support for gain-ranges
        https://git.kernel.org/broonie/sound/c/d25e00ecd5c5
[15/17] ASoC: simple-amplifier: gpio-audio-amp: Add support for gain-labels
        https://git.kernel.org/broonie/sound/c/fb757dfa039f
[16/17] ASoC: simple-amplifier: Update author and copyright
        https://git.kernel.org/broonie/sound/c/7b295a6fc632
[17/17] MAINTAINERS: Add the ASoC gpio audio amplifier entry
        https://git.kernel.org/broonie/sound/c/5bd8c4b7d3bb

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks,
Mark