[PATCH RFC v2 0/2] Move dai-link level properties away from dai subnodes

[PATCH RFC v2 0/2] Move dai-link level properties away from dai subnodes

[Jyri Sarha]

This patch is implemented on top of late patches from Jean-Francois
Moine [1].

These patches implement the main part of the simple-card changes
discussed in alsa-devel mailing list [2].

Since RFC version of the patches:
- ASoC: core: Update snd_soc_of_parse_daifmt() interface
  - Make minimal changes to simple-card for it to work with the updated
    snd_soc_of_parse_daifmt(). (Kuninori Morimoto's comment [3]).
- ASoC: simple-card: Move dai-link level properties away from dai subnodes
  - Fix typo in DT binding document (Jean-Francois Moine's comment [4]).
  - Remove #define DEBUG
  - Remove accidental line wrap at simple-audio-card,name property descripition
    in the DT binding document

[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2014-March/074592.html
[2] http://mailman.alsa-project.org/pipermail/alsa-devel/2014-March/074388.html
[3] http://mailman.alsa-project.org/pipermail/alsa-devel/2014-March/074699.html
[4] http://mailman.alsa-project.org/pipermail/alsa-devel/2014-March/074697.html

Best regards,
Jyri

Jyri Sarha (2):
  ASoC: core: Update snd_soc_of_parse_daifmt() interface
  ASoC: simple-card: Move dai-link level properties away from dai
    subnodes

 .../devicetree/bindings/sound/simple-card.txt      |   88 ++++----
 include/sound/soc.h                                |    4 +-
 sound/soc/generic/simple-card.c                    |  236 ++++++++++++--------
 sound/soc/soc-core.c                               |    8 +-
 4 files changed, 198 insertions(+), 138 deletions(-)

-- 
1.7.9.5

[PATCH RFC v2 1/2] ASoC: core: Update snd_soc_of_parse_daifmt() interface

[Jyri Sarha]

Adds struct device_node **bitclkmaster and struct device_node **framemaster
function parameters. With the new syntax bitclock-master and frame-master
properties can explicitly indicate the dai-link bit-clock and frame masters
with a phandle. This patch also makes the minimal changes to simple-card
for it to work with the updated snd_soc_of_parse_daifmt(). Simple-card appears
to be the only user of snd_soc_of_parse_daifmt() for now.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 include/sound/soc.h             |    4 +++-
 sound/soc/generic/simple-card.c |    4 ++--
 sound/soc/soc-core.c            |    8 +++++++-
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index f7de629..e40713a 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1228,7 +1228,9 @@ int snd_soc_of_parse_tdm_slot(struct device_node *np,
 int snd_soc_of_parse_audio_routing(struct snd_soc_card *card,
 				   const char *propname);
 unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
-				     const char *prefix);
+				     const char *prefix,
+				     struct device_node **bitclkmaster,
+				     struct device_node **framemaster);
 int snd_soc_of_get_dai_name(struct device_node *of_node,
 			    const char **dai_name);
 
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 21f1ccb..e0cc53b 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -121,7 +121,7 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
 	 * bitclock-master,    frame-master
 	 * and specific "format" if it has
 	 */
-	dai->fmt = snd_soc_of_parse_daifmt(np, NULL);
+	dai->fmt = snd_soc_of_parse_daifmt(np, NULL, NULL, NULL);
 	dai->fmt |= daifmt;
 
 	/*
@@ -201,7 +201,7 @@ static int asoc_simple_card_parse_of(struct device_node *node,
 	snd_soc_of_parse_card_name(&priv->snd_card, "simple-audio-card,name");
 
 	/* get CPU/CODEC common format via simple-audio-card,format */
-	daifmt = snd_soc_of_parse_daifmt(node, "simple-audio-card,") &
+	daifmt = snd_soc_of_parse_daifmt(node, "simple-audio-card,", NULL, NULL)&
 		(SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK);
 
 	/* off-codec widgets */
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index b322cf2..7979dcc 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -4560,7 +4560,9 @@ int snd_soc_of_parse_audio_routing(struct snd_soc_card *card,
 EXPORT_SYMBOL_GPL(snd_soc_of_parse_audio_routing);
 
 unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
-				     const char *prefix)
+				     const char *prefix,
+				     struct device_node **bitclkmaster,
+				     struct device_node **framemaster)
 {
 	int ret, i;
 	char prop[128];
@@ -4643,9 +4645,13 @@ unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
 	 */
 	snprintf(prop, sizeof(prop), "%sbitclock-master", prefix);
 	bit = !!of_get_property(np, prop, NULL);
+	if (bit && bitclkmaster)
+		*bitclkmaster = of_parse_phandle(np, prop, 0);
 
 	snprintf(prop, sizeof(prop), "%sframe-master", prefix);
 	frame = !!of_get_property(np, prop, NULL);
+	if (frame && framemaster)
+		*framemaster = of_parse_phandle(np, prop, 0);
 
 	switch ((bit << 4) + frame) {
 	case 0x11:
-- 
1.7.9.5

[PATCH RFC v2 2/2] ASoC: simple-card: Move dai-link level properties away from dai subnodes

[Jyri Sarha]

The properties like format, bitclock-master, frame-master,
bitclock-inversion, and frame-inversion should be common to the dais
connected with a dai-link. For bitclock-master and frame-master
properties to be unambiguous they need to indicate the mastering dai
node with a phandle.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 .../devicetree/bindings/sound/simple-card.txt      |   88 ++++----
 sound/soc/generic/simple-card.c                    |  236 ++++++++++++--------
 2 files changed, 188 insertions(+), 136 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
index 131aa2a..59b842b 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.txt
+++ b/Documentation/devicetree/bindings/sound/simple-card.txt
@@ -1,6 +1,6 @@
 Simple-Card:
 
-Simple-Card specifies audio DAI connection of SoC <-> codec.
+Simple-Card specifies audio DAI connections of SoC <-> codec.
 
 Required properties:
 
@@ -10,26 +10,51 @@ Optional properties:
 
 - simple-audio-card,name		: User specified audio sound card name, one string
 					  property.
-- simple-audio-card,format		: CPU/CODEC common audio format.
-					  "i2s", "right_j", "left_j" , "dsp_a"
-					  "dsp_b", "ac97", "pdm", "msb", "lsb"
 - simple-audio-card,widgets		: Please refer to widgets.txt.
 - simple-audio-card,routing		: A list of the connections between audio components.
 					  Each entry is a pair of strings, the first being the
 					  connection's sink, the second being the connection's
 					  source.
-- dai-tdm-slot-num			: Please refer to tdm-slot.txt.
-- dai-tdm-slot-width			: Please refer to tdm-slot.txt.
+Optional subnodes:
+
+- simple-audio-card,dai-link		: Container for dai-link level
+					  properties and the CPU and CODEC
+					  sub-nodes. This container may be
+					  omitted when the card has only one
+					  DAI link. See the examples and the
+					  section bellow.
+
+Dai-link subnode properties and subnodes:
+
+If dai-link subnode is omitted and the subnode properties are directly
+under "sound"-node the subnode property and subnode names have to be
+prefixed with "simple-audio-card,"-prefix.
 
-Required subnodes:
+Required dai-link subnodes:
 
-- simple-audio-card,dai-link		: container for the CPU and CODEC sub-nodes
-					  This container may be omitted when the
-					  card has only one DAI link.
-					  See the examples.
+- cpu					: CPU   sub-node
+- codec					: CODEC sub-node
 
-- simple-audio-card,cpu			: CPU   sub-node
-- simple-audio-card,codec		: CODEC sub-node
+Optional dai-link subnode properties:
+
+- format				: CPU/CODEC common audio format.
+					  "i2s", "right_j", "left_j" , "dsp_a"
+					  "dsp_b", "ac97", "pdm", "msb", "lsb"
+- frame-master				: Indicates dai-link frame master.
+					  phandle to a cpu or codec subnode.
+- bitclock-master			: Indicates dai-link bit clock master.
+					  phandle to a cpu or codec subnode.
+- bitclock-inversion			: bool property. Add this if the
+					  dai-link uses bit clock inversion.
+- frame-inversion			: bool property. Add this if the
+					  dai-link uses frame clock inversion.
+
+For backward compatibility the frame-master and bitclock-master
+properties can be used as booleans in codec subnode to indicate if the
+codec is the dai-link frame or bit clock master. In this case the same
+properties should not be present at dai-link level and the
+bitclock-inversion and frame-inversion properties should also be placed
+in the codec node if needed.
 
 Required CPU/CODEC subnodes properties:
 
@@ -37,29 +62,21 @@ Required CPU/CODEC subnodes properties:
 
 Optional CPU/CODEC subnodes properties:
 
-- format				: CPU/CODEC specific audio format if needed.
-					  see simple-audio-card,format
-- frame-master				: bool property. add this if subnode is frame master
-- bitclock-master			: bool property. add this if subnode is bitclock master
-- bitclock-inversion			: bool property. add this if subnode has clock inversion
-- frame-inversion			: bool property. add this if subnode has frame inversion
+- dai-tdm-slot-num			: Please refer to tdm-slot.txt.
+- dai-tdm-slot-width			: Please refer to tdm-slot.txt.
 - clocks / system-clock-frequency	: specify subnode's clock if needed.
 					  it can be specified via "clocks" if system has
 					  clock node (= common clock), or "system-clock-frequency"
 					  (if system doens't support common clock)
 
-Note:
- * For 'format', 'frame-master', 'bitclock-master', 'bitclock-inversion' and
-   'frame-inversion', the simple card will use the settings of CODEC for both
-   CPU and CODEC sides as we need to keep the settings identical for both ends
-   of the link.
-
 Example 1 - single DAI link:
 
 sound {
 	compatible = "simple-audio-card";
 	simple-audio-card,name = "VF610-Tower-Sound-Card";
 	simple-audio-card,format = "left_j";
+	simple-audio-card,bitclock-master = <&dailink0_master>;
+	simple-audio-card,frame-master = <&dailink0_master>;
 	simple-audio-card,widgets =
 		"Microphone", "Microphone Jack",
 		"Headphone", "Headphone Jack",
@@ -69,17 +86,12 @@ sound {
 		"Headphone Jack", "HP_OUT",
 		"External Speaker", "LINE_OUT";
 
-	dai-tdm-slot-num = <2>;
-	dai-tdm-slot-width = <8>;
-
 	simple-audio-card,cpu {
 		sound-dai = <&sh_fsi2 0>;
 	};
 
-	simple-audio-card,codec {
+	dailink0_master: simple-audio-card,codec {
 		sound-dai = <&ak4648>;
-		bitclock-master;
-		frame-master;
 		clocks = <&osc>;
 	};
 };
@@ -105,31 +117,31 @@ Example 2 - many DAI links:
 sound {
 	compatible = "simple-audio-card";
 	simple-audio-card,name = "Cubox Audio";
-	simple-audio-card,format = "i2s";
 
 	simple-audio-card,dai-link@0 {		/* I2S - HDMI */
-		simple-audio-card,cpu {
+		format = "i2s";
+		cpu {
 			sound-dai = <&audio1 0>;
 		};
-		simple-audio-card,codec {
+		codec {
 			sound-dai = <&tda998x 0>;
 		};
 	};
 
 	simple-audio-card,dai-link@1 {		/* S/PDIF - HDMI */
-		simple-audio-card,cpu {
+		cpu {
 			sound-dai = <&audio1 1>;
 		};
-		simple-audio-card,codec {
+		codec {
 			sound-dai = <&tda998x 1>;
 		};
 	};
 
 	simple-audio-card,dai-link@2 {		/* S/PDIF - S/PDIF */
-		simple-audio-card,cpu {
+		cpu {
 			sound-dai = <&audio1 1>;
 		};
-		simple-audio-card,codec {
+		codec {
 			sound-dai = <&spdif_codec>;
 		};
 	};
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index e0cc53b..1164366 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -88,7 +88,6 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd)
 
 static int
 asoc_simple_card_sub_parse_of(struct device_node *np,
-			      unsigned int daifmt,
 			      struct asoc_simple_dai *dai,
 			      const struct device_node **p_node,
 			      const char **name)
@@ -117,14 +116,6 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
 		return ret;
 
 	/*
-	 * bitclock-inversion, frame-inversion
-	 * bitclock-master,    frame-master
-	 * and specific "format" if it has
-	 */
-	dai->fmt = snd_soc_of_parse_daifmt(np, NULL, NULL, NULL);
-	dai->fmt |= daifmt;
-
-	/*
 	 * dai->sysclk come from
 	 *  "clocks = <&xxx>" (if system has common clock)
 	 *  or "system-clock-frequency = <xxx>"
@@ -151,37 +142,134 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
 	return 0;
 }
 
-static int simple_card_cpu_codec_of(struct device_node *node,
-				int daifmt,
-				struct snd_soc_dai_link *dai_link,
-				struct simple_dai_props *dai_props)
+static int simple_card_dai_link_of(struct device_node *node,
+				   struct device *dev,
+				   struct snd_soc_dai_link *dai_link,
+				   struct simple_dai_props *dai_props)
 {
-	struct device_node *np;
+	struct device_node *np = NULL;
+	struct device_node *bitclkmaster = NULL;
+	struct device_node *framemaster = NULL;
+	unsigned int daifmt;
+	char *name;
+	char prop[128];
+	char *prefix = "";
 	int ret;
 
-	/* CPU sub-node */
-	ret = -EINVAL;
-	np = of_get_child_by_name(node, "simple-audio-card,cpu");
-	if (np) {
-		ret = asoc_simple_card_sub_parse_of(np, daifmt,
-						&dai_props->cpu_dai,
-						&dai_link->cpu_of_node,
-						&dai_link->cpu_dai_name);
-		of_node_put(np);
+	if (!strcmp("sound", node->name))
+		prefix = "simple-audio-card,";
+
+	daifmt = snd_soc_of_parse_daifmt(node, prefix,
+					 &bitclkmaster, &framemaster);
+	daifmt &= ~SND_SOC_DAIFMT_MASTER_MASK;
+
+	snprintf(prop, sizeof(prop), "%scpu", prefix);
+	np = of_get_child_by_name(node, prop);
+	if (!np) {
+		ret = -EINVAL;
+		dev_err(dev, "%s: Can't find simple-audio-card,cpu DT node\n",
+			__func__);
+		goto dai_link_of_err;
 	}
+
+	ret = asoc_simple_card_sub_parse_of(np, &dai_props->cpu_dai,
+					    &dai_link->cpu_of_node,
+					    &dai_link->cpu_dai_name);
 	if (ret < 0)
-		return ret;
+		goto dai_link_of_err;
+
+	dai_props->cpu_dai.fmt = daifmt;
+	switch (((np == bitclkmaster)<<4)|(np == framemaster)) {
+	case 0x11:
+		dai_props->cpu_dai.fmt |= SND_SOC_DAIFMT_CBS_CFS;
+		break;
+	case 0x10:
+		dai_props->cpu_dai.fmt |= SND_SOC_DAIFMT_CBS_CFM;
+		break;
+	case 0x01:
+		dai_props->cpu_dai.fmt |= SND_SOC_DAIFMT_CBM_CFS;
+		break;
+	default:
+		dai_props->cpu_dai.fmt |= SND_SOC_DAIFMT_CBM_CFM;
+		break;
+	}
 
-	/* CODEC sub-node */
-	ret = -EINVAL;
-	np = of_get_child_by_name(node, "simple-audio-card,codec");
-	if (np) {
-		ret = asoc_simple_card_sub_parse_of(np, daifmt,
-						&dai_props->codec_dai,
-						&dai_link->codec_of_node,
-						&dai_link->codec_dai_name);
-		of_node_put(np);
+	of_node_put(np);
+	snprintf(prop, sizeof(prop), "%scodec", prefix);
+	np = of_get_child_by_name(node, prop);
+	if (!np) {
+		ret = -EINVAL;
+		dev_err(dev, "%s: Can't find simple-audio-card,codec DT node\n",
+			__func__);
+		goto dai_link_of_err;
 	}
+
+	ret = asoc_simple_card_sub_parse_of(np, &dai_props->codec_dai,
+					    &dai_link->codec_of_node,
+					    &dai_link->codec_dai_name);
+	if (ret < 0)
+		goto dai_link_of_err;
+
+	if (!bitclkmaster && !framemaster) {
+		/* Master setting not found from dai_link level, revert back
+		   to legacy DT parsing and take settings from codec node. */
+		dev_dbg(dev, "%s: Revert to legacy daifmt parsing\n",
+			__func__);
+		dai_props->cpu_dai.fmt = dai_props->codec_dai.fmt =
+			snd_soc_of_parse_daifmt(np, NULL, NULL, NULL) |
+			(daifmt & ~SND_SOC_DAIFMT_CLOCK_MASK);
+	} else {
+		dai_props->codec_dai.fmt = daifmt;
+		switch (((np == bitclkmaster)<<4)|(np == framemaster)) {
+		case 0x11:
+			dai_props->codec_dai.fmt |= SND_SOC_DAIFMT_CBM_CFM;
+			break;
+		case 0x10:
+			dai_props->codec_dai.fmt |= SND_SOC_DAIFMT_CBM_CFS;
+			break;
+		case 0x01:
+			dai_props->codec_dai.fmt |= SND_SOC_DAIFMT_CBS_CFM;
+			break;
+		default:
+			dai_props->codec_dai.fmt |= SND_SOC_DAIFMT_CBS_CFS;
+			break;
+		}
+	}
+
+	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;
+
+	/* Link name is created from CPU/CODEC dai name */
+	name = devm_kzalloc(dev,
+			    strlen(dai_link->cpu_dai_name)   +
+			    strlen(dai_link->codec_dai_name) + 2,
+			    GFP_KERNEL);
+	sprintf(name, "%s-%s", dai_link->cpu_dai_name,
+				dai_link->codec_dai_name);
+	dai_link->name = dai_link->stream_name = name;
+
+	dev_dbg(dev, "\tname : %s\n", dai_link->stream_name);
+	dev_dbg(dev, "\tcpu : %s / %04x / %d\n",
+		dai_link->cpu_dai_name,
+		dai_props->cpu_dai.fmt,
+		dai_props->cpu_dai.sysclk);
+	dev_dbg(dev, "\tcodec : %s / %04x / %d\n",
+		dai_link->codec_dai_name,
+		dai_props->codec_dai.fmt,
+		dai_props->codec_dai.sysclk);
+
+dai_link_of_err:
+	if (np)
+		of_node_put(np);
+	if (bitclkmaster)
+		of_node_put(bitclkmaster);
+	if (framemaster)
+		of_node_put(framemaster);
 	return ret;
 }
 
@@ -192,18 +280,11 @@ static int asoc_simple_card_parse_of(struct device_node *node,
 {
 	struct snd_soc_dai_link *dai_link = priv->snd_card.dai_link;
 	struct simple_dai_props *dai_props = priv->dai_props;
-	struct device_node *np;
-	char *name;
-	unsigned int daifmt;
 	int ret;
 
 	/* parsing the card name from DT */
 	snd_soc_of_parse_card_name(&priv->snd_card, "simple-audio-card,name");
 
-	/* get CPU/CODEC common format via simple-audio-card,format */
-	daifmt = snd_soc_of_parse_daifmt(node, "simple-audio-card,", NULL, NULL)&
-		(SND_SOC_DAIFMT_FORMAT_MASK | SND_SOC_DAIFMT_INV_MASK);
-
 	/* off-codec widgets */
 	if (of_property_read_bool(node, "simple-audio-card,widgets")) {
 		ret = snd_soc_of_parse_audio_simple_widgets(&priv->snd_card,
@@ -220,71 +301,30 @@ static int asoc_simple_card_parse_of(struct device_node *node,
 			return ret;
 	}
 
-	/* loop on the DAI links */
-	np = NULL;
-	for (;;) {
-		if (multi) {
-			np = of_get_next_child(node, np);
-			if (!np)
-				break;
-		}
-
-		ret = simple_card_cpu_codec_of(multi ? np : node,
-					daifmt, dai_link, dai_props);
-		if (ret < 0)
-			goto err;
-
-		/*
-		 * overwrite cpu_dai->fmt as its DAIFMT_MASTER bit is based on CODEC
-		 * while the other bits should be identical unless buggy SW/HW design.
-		 */
-		dai_props->cpu_dai.fmt = dai_props->codec_dai.fmt;
+	dev_dbg(dev, "New simple-card: %s\n", priv->snd_card.name ?
+		priv->snd_card.name : "");
 
-		if (!dai_link->cpu_dai_name || !dai_link->codec_dai_name) {
-			ret = -EINVAL;
-			goto err;
+	if (multi) {
+		struct device_node *np = NULL;
+		int i;
+		for (i = 0; (np = of_get_next_child(node, np)); i++) {
+			dev_dbg(dev, "\tlink %d:\n", i);
+			ret = simple_card_dai_link_of(np, dev, dai_link + i,
+						      dai_props + i);
+			of_node_put(np);
+			if (ret < 0)
+				return ret;
 		}
-
-		/* simple-card assumes platform == cpu */
-		dai_link->platform_of_node = dai_link->cpu_of_node;
-
-		name = devm_kzalloc(dev,
-				    strlen(dai_link->cpu_dai_name)   +
-				    strlen(dai_link->codec_dai_name) + 2,
-				    GFP_KERNEL);
-		sprintf(name, "%s-%s", dai_link->cpu_dai_name,
-					dai_link->codec_dai_name);
-		dai_link->name = dai_link->stream_name = name;
-
-		if (!multi)
-			break;
-
-		dai_link++;
-		dai_props++;
+	} else {
+		ret = simple_card_dai_link_of(node, dev, dai_link, dai_props);
+		if (ret < 0)
+			return ret;
 	}
 
-	/* card name is created from CPU/CODEC dai name */
-	dai_link = priv->snd_card.dai_link;
 	if (!priv->snd_card.name)
-		priv->snd_card.name = dai_link->name;
-
-	dev_dbg(dev, "card-name : %s\n", priv->snd_card.name);
-	dev_dbg(dev, "platform : %04x\n", daifmt);
-	dai_props = priv->dai_props;
-	dev_dbg(dev, "cpu : %s / %04x / %d\n",
-		dai_link->cpu_dai_name,
-		dai_props->cpu_dai.fmt,
-		dai_props->cpu_dai.sysclk);
-	dev_dbg(dev, "codec : %s / %04x / %d\n",
-		dai_link->codec_dai_name,
-		dai_props->codec_dai.fmt,
-		dai_props->codec_dai.sysclk);
+		priv->snd_card.name = priv->snd_card.dai_link->name;
 
 	return 0;
-
-err:
-	of_node_put(np);
-	return ret;
 }
 
 /* update the reference count of the devices nodes at end of probe */
-- 
1.7.9.5