[PATCH 0/4] ASoC: qcom: Parse "pin-switches" and "widgets" from DT

[PATCH 0/4] ASoC: qcom: Parse "pin-switches" and "widgets" from DT

[Stephan Gerhold]

Some sound card setups might require extra pin switches to allow 
turning off certain audio components. simple-card supports this
already using the "pin-switches" and "widgets" device tree property. 
This series makes it possible to use the same properties for the Qcom 
sound cards.

To implement that, the function that parses the "pin-switches" property 
in simple-card-utils.c is first moved into the ASoC core. Then two 
simple function calls are added to the common Qcom sound card DT parser.
Finally there is a small patch for the msm8916-wcd-analog codec to make
it possible to model sound card setups used in some MSM8916 smartphones.
(See PATCH 2/4 for an explanation of some real example use cases.)

Using pin switches rather than patching codec drivers with switches was
originally suggested by Mark Brown on a patch for the tfa989x codec:
https://lore.kernel.org/alsa-devel/YXaMVHo9drCIuD3u@sirena.org.uk/

Stephan Gerhold (4):
  ASoC: core: Add snd_soc_of_parse_pin_switches() from simple-card-utils
  ASoC: dt-bindings: qcom: sm8250: Document "pin-switches" and "widgets"
  ASoC: qcom: common: Parse "pin-switches" and "widgets" from DT
  ASoC: msm8916-wcd-analog: Use separate outputs for HPH_L/HPH_R

 .../bindings/sound/qcom,sm8250.yaml           | 16 ++++++
 include/sound/soc.h                           |  1 +
 sound/soc/codecs/msm8916-wcd-analog.c         |  7 +--
 sound/soc/generic/simple-card-utils.c         | 45 +----------------
 sound/soc/qcom/common.c                       | 10 ++++
 sound/soc/soc-core.c                          | 50 +++++++++++++++++++
 6 files changed, 82 insertions(+), 47 deletions(-)

-- 
2.34.1

[PATCH 1/4] ASoC: core: Add snd_soc_of_parse_pin_switches() from simple-card-utils

[Stephan Gerhold]

The ASoC core already has several helpers to parse card properties
from the device tree. Move the parsing code for "pin-switches" from
simple-card-utils to a shared snd_soc_of_parse_pin_switches() function
so other drivers can also use it to set up pin switches configured in
the device tree.

Cc: Paul Cercueil <paul@crapouillou.net>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 include/sound/soc.h                   |  1 +
 sound/soc/generic/simple-card-utils.c | 45 +-----------------------
 sound/soc/soc-core.c                  | 50 +++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 44 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 8e6dd8a257c5..061570048484 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1213,6 +1213,7 @@ int snd_soc_of_parse_card_name(struct snd_soc_card *card,
 			       const char *propname);
 int snd_soc_of_parse_audio_simple_widgets(struct snd_soc_card *card,
 					  const char *propname);
+int snd_soc_of_parse_pin_switches(struct snd_soc_card *card, const char *prop);
 int snd_soc_of_get_slot_mask(struct device_node *np,
 			     const char *prop_name,
 			     unsigned int *mask);
diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index 850e968677f1..a81323d1691d 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -499,57 +499,14 @@ EXPORT_SYMBOL_GPL(asoc_simple_parse_widgets);
 int asoc_simple_parse_pin_switches(struct snd_soc_card *card,
 				   char *prefix)
 {
-	const unsigned int nb_controls_max = 16;
-	const char **strings, *control_name;
-	struct snd_kcontrol_new *controls;
-	struct device *dev = card->dev;
-	unsigned int i, nb_controls;
 	char prop[128];
-	int ret;
 
 	if (!prefix)
 		prefix = "";
 
 	snprintf(prop, sizeof(prop), "%s%s", prefix, "pin-switches");
 
-	if (!of_property_read_bool(dev->of_node, prop))
-		return 0;
-
-	strings = devm_kcalloc(dev, nb_controls_max,
-			       sizeof(*strings), GFP_KERNEL);
-	if (!strings)
-		return -ENOMEM;
-
-	ret = of_property_read_string_array(dev->of_node, prop,
-					    strings, nb_controls_max);
-	if (ret < 0)
-		return ret;
-
-	nb_controls = (unsigned int)ret;
-
-	controls = devm_kcalloc(dev, nb_controls,
-				sizeof(*controls), GFP_KERNEL);
-	if (!controls)
-		return -ENOMEM;
-
-	for (i = 0; i < nb_controls; i++) {
-		control_name = devm_kasprintf(dev, GFP_KERNEL,
-					      "%s Switch", strings[i]);
-		if (!control_name)
-			return -ENOMEM;
-
-		controls[i].iface = SNDRV_CTL_ELEM_IFACE_MIXER;
-		controls[i].name = control_name;
-		controls[i].info = snd_soc_dapm_info_pin_switch;
-		controls[i].get = snd_soc_dapm_get_pin_switch;
-		controls[i].put = snd_soc_dapm_put_pin_switch;
-		controls[i].private_value = (unsigned long)strings[i];
-	}
-
-	card->controls = controls;
-	card->num_controls = nb_controls;
-
-	return 0;
+	return snd_soc_of_parse_pin_switches(card, prop);
 }
 EXPORT_SYMBOL_GPL(asoc_simple_parse_pin_switches);
 
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index dcf6be4c4aaa..c714b0f63fcc 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2824,6 +2824,56 @@ int snd_soc_of_parse_audio_simple_widgets(struct snd_soc_card *card,
 }
 EXPORT_SYMBOL_GPL(snd_soc_of_parse_audio_simple_widgets);
 
+int snd_soc_of_parse_pin_switches(struct snd_soc_card *card, const char *prop)
+{
+	const unsigned int nb_controls_max = 16;
+	const char **strings, *control_name;
+	struct snd_kcontrol_new *controls;
+	struct device *dev = card->dev;
+	unsigned int i, nb_controls;
+	int ret;
+
+	if (!of_property_read_bool(dev->of_node, prop))
+		return 0;
+
+	strings = devm_kcalloc(dev, nb_controls_max,
+			       sizeof(*strings), GFP_KERNEL);
+	if (!strings)
+		return -ENOMEM;
+
+	ret = of_property_read_string_array(dev->of_node, prop,
+					    strings, nb_controls_max);
+	if (ret < 0)
+		return ret;
+
+	nb_controls = (unsigned int)ret;
+
+	controls = devm_kcalloc(dev, nb_controls,
+				sizeof(*controls), GFP_KERNEL);
+	if (!controls)
+		return -ENOMEM;
+
+	for (i = 0; i < nb_controls; i++) {
+		control_name = devm_kasprintf(dev, GFP_KERNEL,
+					      "%s Switch", strings[i]);
+		if (!control_name)
+			return -ENOMEM;
+
+		controls[i].iface = SNDRV_CTL_ELEM_IFACE_MIXER;
+		controls[i].name = control_name;
+		controls[i].info = snd_soc_dapm_info_pin_switch;
+		controls[i].get = snd_soc_dapm_get_pin_switch;
+		controls[i].put = snd_soc_dapm_put_pin_switch;
+		controls[i].private_value = (unsigned long)strings[i];
+	}
+
+	card->controls = controls;
+	card->num_controls = nb_controls;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_soc_of_parse_pin_switches);
+
 int snd_soc_of_get_slot_mask(struct device_node *np,
 			     const char *prop_name,
 			     unsigned int *mask)
-- 
2.34.1

[PATCH 2/4] ASoC: dt-bindings: qcom: sm8250: Document "pin-switches" and "widgets"

[Stephan Gerhold]

Some sound card setups might require extra pin switches to allow
turning off certain audio components. There are two real examples for
this in smartphones/tablets based on MSM8916:

  1. Analog speaker amplifiers connected to headphone outputs.

     The MSM8916 analog codec does not have a separate "Line Out" port
     so some devices have an analog speaker amplifier connected to one
     of the headphone outputs. A pin switch is necessary to allow
     playback on headphones without also activating the speaker.

  2. External speaker codec also used as earpiece.

     Some smartphones have two front-facing (stereo) speakers that can
     be also configured to act as an earpiece during voice calls. A pin
     switch is needed to allow disabling the second speaker during
     voice calls.

There are existing bindings that allow setting up such pin switches in
simple-card.yaml. Document the same for Qcom sound cards.

One variant of example 1 above is added to the examples in the DT
schema: There is an analog speaker amplifier connected to the HPH_R
(right headphone channel) output. Adding a "Speaker" pin switch and
widget allows turning off the speaker when audio should be only played
via the connected headphones.

Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 .../devicetree/bindings/sound/qcom,sm8250.yaml   | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
index e50964c54bb9..4bfda04b4608 100644
--- a/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
+++ b/Documentation/devicetree/bindings/sound/qcom,sm8250.yaml
@@ -39,6 +39,14 @@ properties:
     $ref: /schemas/types.yaml#/definitions/string
     description: User visible long sound card name
 
+  pin-switches:
+    description: List of widget names for which pin switches should be created.
+    $ref: /schemas/types.yaml#/definitions/string-array
+
+  widgets:
+    description: User specified audio sound widgets.
+    $ref: /schemas/types.yaml#/definitions/non-unique-string-array
+
   # Only valid for some compatibles (see allOf if below)
   reg: true
   reg-names: true
@@ -251,7 +259,15 @@ examples:
         reg-names = "mic-iomux", "spkr-iomux";
 
         model = "msm8916";
+        widgets =
+            "Speaker", "Speaker",
+            "Headphone", "Headphones";
+        pin-switches = "Speaker";
         audio-routing =
+            "Speaker", "Speaker Amp OUT",
+            "Speaker Amp IN", "HPH_R",
+            "Headphones", "HPH_L",
+            "Headphones", "HPH_R",
             "AMIC1", "MIC BIAS Internal1",
             "AMIC2", "MIC BIAS Internal2",
             "AMIC3", "MIC BIAS Internal3";
-- 
2.34.1

Re: [PATCH 2/4] ASoC: dt-bindings: qcom: sm8250: Document "pin-switches" and "widgets"

[Rob Herring]

On Tue, Dec 14, 2021 at 03:20:47PM +0100, Stephan Gerhold wrote:
> Some sound card setups might require extra pin switches to allow
> turning off certain audio components. There are two real examples for
> this in smartphones/tablets based on MSM8916:
> 
>   1. Analog speaker amplifiers connected to headphone outputs.
> 
>      The MSM8916 analog codec does not have a separate "Line Out" port
>      so some devices have an analog speaker amplifier connected to one
>      of the headphone outputs. A pin switch is necessary to allow
>      playback on headphones without also activating the speaker.
> 
>   2. External speaker codec also used as earpiece.
> 
>      Some smartphones have two front-facing (stereo) speakers that can
>      be also configured to act as an earpiece during voice calls. A pin
>      switch is needed to allow disabling the second speaker during
>      voice calls.

This all makes sense, but how that translates to the DT properties I 
don't have a clue.

> There are existing bindings that allow setting up such pin switches in
> simple-card.yaml. Document the same for Qcom sound cards.

And that description is equally as bad.

> One variant of example 1 above is added to the examples in the DT
> schema: There is an analog speaker amplifier connected to the HPH_R
> (right headphone channel) output. Adding a "Speaker" pin switch and
> widget allows turning off the speaker when audio should be only played
> via the connected headphones.
> 
> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> ---
>  .../devicetree/bindings/sound/qcom,sm8250.yaml   | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)

But nothing new here really, so

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 3/4] ASoC: qcom: common: Parse "pin-switches" and "widgets" from DT

[Stephan Gerhold]

Use the DT helpers in the ASoC core to parse the "pin-switches" and
"widgets" properties from the device tree. This allows adding extra
mixers to disable e.g. an extra speaker amplifier that would be
normally powered on automatically because it is connected to a shared
output pin.

Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 sound/soc/qcom/common.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/sound/soc/qcom/common.c b/sound/soc/qcom/common.c
index 2e1c618f7529..eff134785051 100644
--- a/sound/soc/qcom/common.c
+++ b/sound/soc/qcom/common.c
@@ -26,6 +26,12 @@ int qcom_snd_parse_of(struct snd_soc_card *card)
 		return ret;
 	}
 
+	if (of_property_read_bool(dev->of_node, "widgets")) {
+		ret = snd_soc_of_parse_audio_simple_widgets(card, "widgets");
+		if (ret)
+			return ret;
+	}
+
 	/* DAPM routes */
 	if (of_property_read_bool(dev->of_node, "audio-routing")) {
 		ret = snd_soc_of_parse_audio_routing(card, "audio-routing");
@@ -39,6 +45,10 @@ int qcom_snd_parse_of(struct snd_soc_card *card)
 			return ret;
 	}
 
+	ret = snd_soc_of_parse_pin_switches(card, "pin-switches");
+	if (ret)
+		return ret;
+
 	ret = snd_soc_of_parse_aux_devs(card, "aux-devs");
 	if (ret)
 		return ret;
-- 
2.34.1

[PATCH 4/4] ASoC: msm8916-wcd-analog: Use separate outputs for HPH_L/HPH_R

[Stephan Gerhold]

The analog codec has separate output paths for the left headphone channel
(HPH_L) and the right headphone channel (HPH_R). While they are usually
used together for actual headphones output, some devices also have an
analog speaker amplifier connected to one of the headphone channels.

To allow modelling that properly (and to avoid powering on the unneeded
output path), HPH_L and HPH_R should be represented by separate outputs
rather than a shared HEADPHONE output that always activates both paths.

Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
 sound/soc/codecs/msm8916-wcd-analog.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/msm8916-wcd-analog.c b/sound/soc/codecs/msm8916-wcd-analog.c
index 3ddd822240e3..485cda46dbb9 100644
--- a/sound/soc/codecs/msm8916-wcd-analog.c
+++ b/sound/soc/codecs/msm8916-wcd-analog.c
@@ -822,8 +822,8 @@ static const struct snd_soc_dapm_route pm8916_wcd_analog_audio_map[] = {
 	{"EAR PA", NULL, "EAR CP"},
 
 	/* Headset (RX MIX1 and RX MIX2) */
-	{"HEADPHONE", NULL, "HPHL PA"},
-	{"HEADPHONE", NULL, "HPHR PA"},
+	{"HPH_L", NULL, "HPHL PA"},
+	{"HPH_R", NULL, "HPHR PA"},
 
 	{"HPHL DAC", NULL, "EAR_HPHL_CLK"},
 	{"HPHR DAC", NULL, "EAR_HPHR_CLK"},
@@ -870,7 +870,8 @@ static const struct snd_soc_dapm_widget pm8916_wcd_analog_dapm_widgets[] = {
 	SND_SOC_DAPM_INPUT("AMIC3"),
 	SND_SOC_DAPM_INPUT("AMIC2"),
 	SND_SOC_DAPM_OUTPUT("EAR"),
-	SND_SOC_DAPM_OUTPUT("HEADPHONE"),
+	SND_SOC_DAPM_OUTPUT("HPH_L"),
+	SND_SOC_DAPM_OUTPUT("HPH_R"),
 
 	/* RX stuff */
 	SND_SOC_DAPM_SUPPLY("INT_LDO_H", SND_SOC_NOPM, 1, 0, NULL, 0),
-- 
2.34.1

Re: [PATCH 0/4] ASoC: qcom: Parse "pin-switches" and "widgets" from DT

[Mark Brown]

On Tue, 14 Dec 2021 15:20:45 +0100, Stephan Gerhold wrote:
> Some sound card setups might require extra pin switches to allow
> turning off certain audio components. simple-card supports this
> already using the "pin-switches" and "widgets" device tree property.
> This series makes it possible to use the same properties for the Qcom
> sound cards.
> 
> To implement that, the function that parses the "pin-switches" property
> in simple-card-utils.c is first moved into the ASoC core. Then two
> simple function calls are added to the common Qcom sound card DT parser.
> Finally there is a small patch for the msm8916-wcd-analog codec to make
> it possible to model sound card setups used in some MSM8916 smartphones.
> (See PATCH 2/4 for an explanation of some real example use cases.)
> 
> [...]

Applied to

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

Thanks!

[1/4] ASoC: core: Add snd_soc_of_parse_pin_switches() from simple-card-utils
      commit: 3d4641a42ccf1593b3f3a474ee7541727acbb8e0
[2/4] ASoC: dt-bindings: qcom: sm8250: Document "pin-switches" and "widgets"
      commit: 37a49da9a7d5ac1f7128000de42ff222da46ba7a
[3/4] ASoC: qcom: common: Parse "pin-switches" and "widgets" from DT
      commit: 2623e66de125ba153e41be6a0b8af24cae8aa436
[4/4] ASoC: msm8916-wcd-analog: Use separate outputs for HPH_L/HPH_R
      commit: 319a05330f4ff3f951f9c42094958c6cdef393b3

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