Skip to contentSkip to conversation

ASoC: simple-card: Add multi-CODEC support

PATCH Applied Jean-Francois Moine 2 participants 8 messages Nov 25, 2014 – Dec 29, 2014

Subsystems (4)

v5 of 6 Applied Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v5 cover ASoC: simple-card: Add multi-CODEC support

This patch set adds multi-CODEC support to the simple card.

v5: accept only "sound-dai" as the dai link property (Arnd Bergmann)
v4: (Mark Brown)
- document the new core function
- calculation of the number of codecs moved to the core function
- split the patch about simple-card.c
- less code changes
- remove the cast of pointers
- less changes in the DT binding example 2
v3: rebase on broonie git 2014/11/03
v2: accept list of CODECs in 'sound-dais' (Benoit Cousson)

Jean-Francois Moine (4):
ASoC: core: add multi-codec support in DT
ASoC: simple-card: Remove useless function argument
ASoC: simple-card: add multi-CODECs in DT
ASoC: simple-card: Remove useless check

 .../devicetree/bindings/sound/simple-card.txt      |  18 +---
 include/sound/soc.h                                |   3 +
 sound/soc/generic/simple-card.c                    |  87 ++++++++--------
 sound/soc/soc-core.c                               | 110 ++++++++++++++++++---
 4 files changed, 149 insertions(+), 69 deletions(-)
Signature · 6 lines
2.1.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#1 lore raw

PATCH v5 1/4 ASoC: core: add multi-codec support in DT

This patch exports a core function which handles the DT description of multi-codec links (as: "sound-dai = <&hdmi 0>, <&spdif_codec>;") and creates a CODEC component array in the DAI link.

---
 include/sound/soc.h  |   3 ++
 sound/soc/soc-core.c | 110 ++++++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 98 insertions(+), 15 deletions(-)
include/sound/soc.h +3 -0
index 13e1648..8426a4f 100644
@@ -1492,6 +1492,9 @@ unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
 				     struct device_node **framemaster);
 int snd_soc_of_get_dai_name(struct device_node *of_node,
 			    const char **dai_name);
+int snd_soc_of_get_dai_link_codecs(struct device *dev,
+				   struct device_node *of_node,
+				   struct snd_soc_dai_link *dai_link);
 
 #include <sound/soc-dai.h>
 
sound/soc/soc-core.c +95 -15

143 lines not on this page · raw message

Signature · 1 line
2.1.3
maintainer Applied
#2 lore raw

Re: [PATCH v5 1/4] ASoC: core: add multi-codec support in DT

Quoted 3 lines · Jean-Francois Moine

This patch exports a core function which handles the DT description of multi-codec links (as: "sound-dai = <&hdmi 0>, <&spdif_codec>;") and creates a CODEC component array in the DAI link.

Applied, thanks.

#3 lore raw

PATCH v5 2/4 ASoC: simple-card: Remove useless function argument

In reply to Jean-Francois Moine

The device node pointer 'cpu' is not used in the function asoc_simple_card_parse_daifmt().

---
 sound/soc/generic/simple-card.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
sound/soc/generic/simple-card.c +1 -2
index 3e3fec4..ece22d5 100644
@@ -232,7 +232,6 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
 
 static int asoc_simple_card_parse_daifmt(struct device_node *node,
 					 struct simple_card_data *priv,
-					 struct device_node *cpu,
 					 struct device_node *codec,
 					 char *prefix, int idx)
 {
@@ -309,7 +308,7 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
 	}
 
 	ret = asoc_simple_card_parse_daifmt(node, priv,
-					    cpu, codec, prefix, idx);
+					    codec, prefix, idx);
 	if (ret < 0)
 		goto dai_link_of_err;
 
Signature · 1 line
2.1.3
maintainer Applied
#4 lore raw

Re: [PATCH v5 2/4] ASoC: simple-card: Remove useless function argument

Quoted 2 lines · Jean-Francois Moine

The device node pointer 'cpu' is not used in the function asoc_simple_card_parse_daifmt().

Applied, thanks.

#5 lore raw

PATCH v5 3/4 ASoC: simple-card: add multi-CODECs in DT

In reply to Jean-Francois Moine

This patch allows many CODECs per link to be defined in the device tree.

---
 .../devicetree/bindings/sound/simple-card.txt      | 18 ++---
 sound/soc/generic/simple-card.c                    | 81 ++++++++++++----------
 2 files changed, 51 insertions(+), 48 deletions(-)
Documentation/devicetree/bindings/sound/simple-card.txt +5 -13
index c3cba60..461abf0 100644
@@ -65,7 +65,8 @@ properties should also be placed in the codec node if needed.
 
 Required CPU/CODEC subnodes properties:
 
-- sound-dai				: phandle and port of CPU/CODEC
+- sound-dai				: phandle and port of CPU
+					  or list of phandle and port of CODECs
 
 Optional CPU/CODEC subnodes properties:
 
@@ -119,7 +120,7 @@ sh_fsi2: sh_fsi2@ec230000 {
 	interrupts = <0 146 0x4>;
 };
 
-Example 2 - many DAI links:
+Example 2 - many DAI links and multi-CODECs:
 
 sound {
 	compatible = "simple-audio-card";
@@ -135,21 +136,12 @@ sound {
 		};
 	};
 
-	simple-audio-card,dai-link@1 {		/* S/PDIF - HDMI */
+	simple-audio-card,dai-link@1 {		/* S/PDIF - HDMI & S/PDIF */
 		cpu {
 			sound-dai = <&audio1 1>;
 		};
 		codec {
-			sound-dai = <&tda998x 1>;
-		};
-	};
-
-	simple-audio-card,dai-link@2 {		/* S/PDIF - S/PDIF */
-		cpu {
-			sound-dai = <&audio1 1>;
-		};
-		codec {
-			sound-dai = <&spdif_codec>;
+			sound-dai = <&tda998x 1>, <&spdif_codec>;
 		};
 	};
 };
sound/soc/generic/simple-card.c +46 -35

151 lines not on this page · raw message

Signature · 6 lines
2.1.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
maintainer
#6 lore raw

Re: [PATCH v5 3/4] ASoC: simple-card: add multi-CODECs in DT

Quoted 1 line · Jean-Francois Moine

This patch allows many CODECs per link to be defined in the device tree.

It's also quite big and fiddly and hard to read, the changes that are being made aren't blindingly obvious and there's quite a few of them. As I've said before it's really importat that changes are clear and easy to read, if the code is complex or surprising then the changelog needs to be that bit more detailed to make thigs clear. Things like talking about why the code is being moved out and how it is being transformed would be really helpful with this one, it's not enough to know the overall goal of the patch, I also need to know how the patch is intended to achieve that.

I think this is mostly OK but a couple of things...

Documentation/devicetree/bindings/sound/simple-card.txt sh_fsi2: sh_fsi2@ec230000 { · line 123

Quoted 2 lines
Documentation/devicetree/bindings/sound/simple-card.txt +1 -1
-Example 2 - many DAI links:
+Example 2 - many DAI links and multi-CODECs:

I'd be much happier with a new example here rather than modifying the old one.

sound/soc/generic/simple-card.c static int asoc_simple_card_dai_link_of(struct device_node *node, · line 364

Quoted 7 lines
sound/soc/generic/simple-card.c +1 -0
@@ -365,8 +359,18 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
 	 */
 	if (!cpu_args)
 		dai_link->cpu_dai_name = NULL;
+	goto out;
 
 dai_link_of_err:

This goto out thing here is messy, it's not our normal coding style and is error prone - better to just duplicate a small amount of cleanup.

sound/soc/generic/simple-card.c static int asoc_simple_card_dai_link_of(struct device_node *node, · line 369

Quoted 5 lines
sound/soc/generic/simple-card.c +5 -0
+	for (i = 0, component = dai_link->codecs;
+	     i < dai_link->num_codecs;
+	     i++, component++) {
+		if (!component->of_node)
+			break;

What's this break doing here, why might we be missing a node and why should we skip all remaining components rather than just this one as a result - a continue would be less surprising.

#7 lore raw

PATCH v5 4/4 ASoC: simple-card: Remove useless check

In reply to Jean-Francois Moine

The CPU and CODEC names are checked when getting the DAI names. There is no need to check them once more.

---
 sound/soc/generic/simple-card.c | 5 -----
 1 file changed, 5 deletions(-)
sound/soc/generic/simple-card.c +0 -5
index 61f9500..c1ba78b 100644
@@ -319,11 +319,6 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
 	if (ret < 0)
 		goto dai_link_of_err;
 
-	if (!dai_link->cpu_dai_name || !dai_link->codec_dai_name) {
-		ret = -EINVAL;
-		goto dai_link_of_err;
-	}
-
 	/* Simple Card assumes platform == cpu */
 	dai_link->platform_of_node = dai_link->cpu_of_node;
 
Signature · 1 line
2.1.3