[PATCHv1 0/7] simple-card: simplify the code.

[PATCHv1 0/7] simple-card: simplify the code.

[Xiubo Li]

Change in v1:
- Add simple-card dts node patches.
- Fix format parsing bug from Jean-Francois's comment.
- Rebase to Kuninori-san's newest changes in next branch.


Xiubo Li (7):
  ASoC: simple-card: Merge single and muti DAI link code.
  ASoC: simple-card: Adjust the comments of simple card.
  ASoC: dts: vf610-twr: To support simple card newest style.
  ASoC: dts: kirkwood-t5325: To support simple card newest style.
  ASoC: dts: r8a7740-armadillo800eva-reference: To support simple card
    newest style.
  ASoC: dts: sh73a0-kzm9g-reference: To support simple card newest
    style.
  ASoC: dts: kirkwood-openrd-client: To support simple card newest
    style.

 .../devicetree/bindings/sound/simple-card.txt      | 101 ++++++------
 arch/arm/boot/dts/kirkwood-openrd-client.dts       |  15 +-
 arch/arm/boot/dts/kirkwood-t5325.dts               |  15 +-
 .../boot/dts/r8a7740-armadillo800eva-reference.dts |  21 +--
 arch/arm/boot/dts/sh73a0-kzm9g-reference.dts       |  18 ++-
 arch/arm/boot/dts/vf610-twr.dts                    |  17 +-
 sound/soc/generic/simple-card.c                    | 176 ++++++++-------------
 7 files changed, 160 insertions(+), 203 deletions(-)

-- 
1.8.4

[PATCHv1 1/7] ASoC: simple-card: Merge single and muti DAI link code.

[Xiubo Li]

This patch merge single DAI link and muti-DAI links code together,
and simply the simple-card driver code.

And also do some other improvement:

Since from the DAI format micro SND_SOC_DAIFMT_CBx_CFx, the 'CBx'
mean Codec's bit clock is as master/slave and the 'CFx' mean Codec's
frame clock is as master/slave.

So these same DAI formats should be informed to CPU and CODE DAIs at
the same time. For the Codec driver will set the bit clock and frame
clock as the DAI formats said, but for the CPU driver, if the the
bit clock or frame clock is as Codec master, so it should be set CPU
DAI device as bit clock or frame clock as slave, and vice versa.

The old code will cause confusion, and we should be clear that the
letter 'C' here mean to Codec.

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Cc: Jean-Francois Moine <moinejf@free.fr>
Cc: Jyri Sarha <jsarha@ti.com>
Cc: Nicolin Chen <nicoleotsuka@gmail.com>
---
 .../devicetree/bindings/sound/simple-card.txt      | 101 +++++++-------
 sound/soc/generic/simple-card.c                    | 152 +++++++--------------
 2 files changed, 102 insertions(+), 151 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
index c2e9841..6cc44a6a 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.txt
+++ b/Documentation/devicetree/bindings/sound/simple-card.txt
@@ -1,13 +1,12 @@
 Simple-Card:
 
+====
 Simple-Card specifies audio DAI connections of SoC <-> codec.
 
 Required properties:
-
 - compatible				: "simple-audio-card"
 
 Optional properties:
-
 - simple-audio-card,name		: User specified audio sound card name, one string
 					  property.
 - simple-audio-card,widgets		: Please refer to widgets.txt.
@@ -18,68 +17,57 @@ Optional properties:
 - simple-audio-card,mclk-fs             : Multiplication factor between stream rate and codec
   					  mclk.
 
-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.
+====
+DAI link node(s) and its or their subnodes:
 
-Required dai-link subnodes:
+There must be one DAI link node exsit at least, and each DAI link must
+contain one CPU subnode and one CODEC subnode.
 
-- cpu					: CPU   sub-node
-- codec					: CODEC sub-node
-
-Optional dai-link subnode properties:
+Required DAI link node(s):
+- simple-audio-card,dai-link		: Container for DAI link level
+					  properties and the CPU and CODEC
+					  sub-nodes.
 
+Optional DAI link node 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 there
-should be no dai-link node, the same properties should not be present
-at sound-node level, and the bitclock-inversion and frame-inversion
-properties should also be placed in the codec node if needed.
-
-Required CPU/CODEC subnodes properties:
-
-- sound-dai				: phandle and port of CPU/CODEC
+- frame-master				: Boolean property. If present, for this
+					  DAI link the Codec DAI's frame will be
+					  as master and the CPU DAI's frame will
+					  be as slave, or vice versa.
+- bitclock-master			: Boolean property. If present, for this
+					  DAI link the Codec DAI's bit clock will
+					  be as master and the CPU DAI's bit clock
+					  will be as slave, or vice versa.
+====
+DAI link's subnodes and their properties:
+
+Required DAI link subnodes:
+- cpu					: CPU   sub-node
+- codec					: CODEC sub-node
 
-Optional CPU/CODEC subnodes properties:
+Required DAI link subnodes' properties:
+- sound-dai				: Phandle and port of CPU/CODEC
 
+Optional DAI link subnodes' properties:
+- bitclock-inversion			: Boolean property. Add this if the DAI
+					  device needs bit clock to be inversed.
+- frame-inversion			: Boolean property. Add this if the DAI
+					  device needs frame clock to be inversed.
 - 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)
+====
 
 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",
@@ -89,13 +77,23 @@ sound {
 		"Headphone Jack", "HP_OUT",
 		"External Speaker", "LINE_OUT";
 
-	simple-audio-card,cpu {
-		sound-dai = <&sh_fsi2 0>;
-	};
+	simple-audio-card,dai-link {
+		format = "left_j";
+		bitclock-master;
+		frame-master;
+
+		cpu {
+			sound-dai = <&sh_fsi2 0>;
 
-	dailink0_master: simple-audio-card,codec {
-		sound-dai = <&ak4648>;
-		clocks = <&osc>;
+			bitclock-inversion;
+			frame-inversion;
+		};
+		codec {
+			sound-dai = <&ak4648>;
+			clocks = <&osc>;
+
+			bitclock-inversion;
+		};
 	};
 };
 
@@ -123,6 +121,9 @@ sound {
 
 	simple-audio-card,dai-link@0 {		/* I2S - HDMI */
 		format = "i2s";
+		bitclock-master;
+		frame-master;
+
 		cpu {
 			sound-dai = <&audio1 0>;
 		};
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 08c5d7d..d45c8dd 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -112,13 +112,24 @@ static int
 asoc_simple_card_sub_parse_of(struct device_node *np,
 			      struct asoc_simple_dai *dai,
 			      struct device_node **p_node,
-			      const char **name)
+			      const char **name,
+			      unsigned int *daifmt)
 {
 	struct device_node *node;
 	struct clk *clk;
 	u32 val;
 	int ret;
 
+	if (!daifmt)
+		return -EINVAL;
+
+	/*
+	 * Parse format, bitclock-inversion and frame-inversion
+	 * for DAI each device.
+	 */
+	*daifmt = snd_soc_of_parse_daifmt(np, NULL, NULL, NULL);
+	*daifmt &= ~(SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK);
+
 	/*
 	 * get node via "sound-dai = <&phandle port>"
 	 * it will be used as xxx_of_node on soc_bind_dai_link()
@@ -166,102 +177,58 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
 static int asoc_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,
-					bool is_top_level_node)
+					struct simple_dai_props *dai_props)
 {
 	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;
 
-	if (is_top_level_node)
-		prefix = "simple-audio-card,";
-
-	daifmt = snd_soc_of_parse_daifmt(node, prefix,
-					 &bitclkmaster, &framemaster);
-	daifmt &= ~SND_SOC_DAIFMT_MASTER_MASK;
+	/* Parse bitclock-master and frame-master for DAI link */
+	daifmt = snd_soc_of_parse_daifmt(node, NULL, NULL, NULL);
+	daifmt &= (SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK);
+	dai_props->cpu_dai.fmt = dai_props->codec_dai.fmt = daifmt;
 
-	snprintf(prop, sizeof(prop), "%scpu", prefix);
-	np = of_get_child_by_name(node, prop);
+	/* Parse CPU DAI */
+	np = of_get_child_by_name(node, "cpu");
 	if (!np) {
 		ret = -EINVAL;
-		dev_err(dev, "%s: Can't find %s DT node\n", __func__, prop);
+		dev_err(dev, "%s: Can't find 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);
+					    &dai_link->cpu_dai_name,
+					    &daifmt);
 	if (ret < 0)
 		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;
-	}
-
+	dai_props->cpu_dai.fmt |= daifmt;
 	of_node_put(np);
-	snprintf(prop, sizeof(prop), "%scodec", prefix);
-	np = of_get_child_by_name(node, prop);
+
+	/* Parse CODEC DAI */
+	np = of_get_child_by_name(node, "codec");
 	if (!np) {
 		ret = -EINVAL;
-		dev_err(dev, "%s: Can't find %s DT node\n", __func__, prop);
+		dev_err(dev, "%s: Can't find 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);
+					    &dai_link->codec_dai_name,
+					    &daifmt);
 	if (ret < 0)
 		goto dai_link_of_err;
 
-	if (strlen(prefix) && !bitclkmaster && !framemaster) {
-		/* No dai-link level and master setting was not found from
-		   sound node level, revert back to legacy DT parsing and
-		   take the 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;
 	}
 
+	dai_props->codec_dai.fmt |= daifmt;
+
 	/* simple-card assumes platform == cpu */
 	dai_link->platform_of_node = dai_link->cpu_of_node;
 
@@ -300,10 +267,6 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
 dai_link_of_err:
 	if (np)
 		of_node_put(np);
-	if (bitclkmaster)
-		of_node_put(bitclkmaster);
-	if (framemaster)
-		of_node_put(framemaster);
 	return ret;
 }
 
@@ -325,13 +288,13 @@ asoc_simple_card_unref(const struct snd_soc_dai_link *dai_link,
 
 static int asoc_simple_card_parse_of(struct device_node *node,
 				     struct simple_card_data *priv,
-				     struct device *dev,
-				     int multi)
+				     struct device *dev)
 {
 	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 = NULL;
 	u32 val;
-	int ret;
+	int ret, i = 0;
 
 	/* parsing the card name from DT */
 	snd_soc_of_parse_card_name(&priv->snd_card, "simple-audio-card,name");
@@ -360,30 +323,20 @@ static int asoc_simple_card_parse_of(struct device_node *node,
 	dev_dbg(dev, "New simple-card: %s\n", priv->snd_card.name ?
 		priv->snd_card.name : "");
 
-	if (multi) {
-		struct device_node *np = NULL;
-		int i = 0;
-
-		for_each_child_of_node(node, np) {
-			dev_dbg(dev, "\tlink %d:\n", i);
-			ret = asoc_simple_card_dai_link_of(np, dev,
-							   dai_link + i,
-							   dai_props + i,
-							   false);
-			if (ret < 0) {
-				asoc_simple_card_unref(dai_link, i + 1);
-				of_node_put(np);
-				return ret;
-			}
-			i++;
-		}
-		of_node_put(np);
-	} else {
-		ret = asoc_simple_card_dai_link_of(node, dev,
-						   dai_link, dai_props, true);
-		if (ret < 0)
+	/* Parse DAI link(s) */
+	for_each_child_of_node(node, np) {
+		dev_dbg(dev, "\tlink %d:\n", i);
+		ret = asoc_simple_card_dai_link_of(np, dev,
+				dai_link + i,
+				dai_props + i);
+		if (ret < 0) {
+			asoc_simple_card_unref(dai_link, i + 1);
+			of_node_put(np);
 			return ret;
+		}
+		i++;
 	}
+	of_node_put(np);
 
 	if (!priv->snd_card.name)
 		priv->snd_card.name = priv->snd_card.dai_link->name;
@@ -397,16 +350,13 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
 	struct snd_soc_dai_link *dai_link;
 	struct device_node *np = pdev->dev.of_node;
 	struct device *dev = &pdev->dev;
-	int num_links, multi, ret;
+	int num_links, ret;
 
 	/* get the number of DAI links */
-	if (np && of_get_child_by_name(np, "simple-audio-card,dai-link")) {
+	if (np && of_get_child_by_name(np, "simple-audio-card,dai-link"))
 		num_links = of_get_child_count(np);
-		multi = 1;
-	} else {
-		num_links = 1;
-		multi = 0;
-	}
+	else
+		return -EINVAL;
 
 	/* allocate the private data and the DAI link array */
 	priv = devm_kzalloc(dev,
@@ -433,7 +383,7 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
 
 	if (np && of_device_is_available(np)) {
 
-		ret = asoc_simple_card_parse_of(np, priv, dev, multi);
+		ret = asoc_simple_card_parse_of(np, priv, dev);
 		if (ret < 0) {
 			if (ret != -EPROBE_DEFER)
 				dev_err(dev, "parse error %d\n", ret);
-- 
1.8.4

Re: [alsa-devel] [PATCHv1 1/7] ASoC: simple-card: Merge single and muti DAI link code.

[Kuninori Morimoto]

Hi Xiubo

Thank you for your patch.
This clean-up is very nice for me.
But, I have 1 small comment

>  asoc_simple_card_sub_parse_of(struct device_node *np,
>  			      struct asoc_simple_dai *dai,
>  			      struct device_node **p_node,
> -			      const char **name)
> +			      const char **name,
> +			      unsigned int *daifmt)
>  {
>  	struct device_node *node;
>  	struct clk *clk;
>  	u32 val;
>  	int ret;
>  
> +	if (!daifmt)
> +		return -EINVAL;
> +
> +	/*
> +	 * Parse format, bitclock-inversion and frame-inversion
> +	 * for DAI each device.
> +	 */
> +	*daifmt = snd_soc_of_parse_daifmt(np, NULL, NULL, NULL);
> +	*daifmt &= ~(SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK);
(snip)
>  	ret = asoc_simple_card_sub_parse_of(np, &dai_props->cpu_dai,
>  					    &dai_link->cpu_of_node,
> -					    &dai_link->cpu_dai_name);
> +					    &dai_link->cpu_dai_name,
> +					    &daifmt);
(snip)
> +	dai_props->cpu_dai.fmt |= daifmt;
(snip)
>  	ret = asoc_simple_card_sub_parse_of(np, &dai_props->codec_dai,
>  					    &dai_link->codec_of_node,
> -					    &dai_link->codec_dai_name);
> +					    &dai_link->codec_dai_name,
> +					    &daifmt);
(snip)
> +	dai_props->codec_dai.fmt |= daifmt;

These are using

      asoc_simple_card_sub_parse_of(np, &dai_props->cpu_dai, xxx)
      asoc_simple_card_sub_parse_of(np, &dai_props->codec_dai, xxx)

I guess, asoc_simple_card_sub_parse_of() can update
below inside function

	dai_props->cpu_dai.fmt |= daifmt;
	dai_props->codec_dai.fmt |= daifmt;


Best regards
---
Kuninori Morimoto

RE: [alsa-devel] [PATCHv1 1/7] ASoC: simple-card: Merge single and muti DAI link code.

[Li.Xiubo]

> Hi Xiubo
> 
> Thank you for your patch.
> This clean-up is very nice for me.
> But, I have 1 small comment
> 
> >  asoc_simple_card_sub_parse_of(struct device_node *np,
> >  			      struct asoc_simple_dai *dai,
> >  			      struct device_node **p_node,
> > -			      const char **name)
> > +			      const char **name,
> > +			      unsigned int *daifmt)
> >  {
> >  	struct device_node *node;
> >  	struct clk *clk;
> >  	u32 val;
> >  	int ret;
> >
> > +	if (!daifmt)
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * Parse format, bitclock-inversion and frame-inversion
> > +	 * for DAI each device.
> > +	 */
> > +	*daifmt = snd_soc_of_parse_daifmt(np, NULL, NULL, NULL);
> > +	*daifmt &= ~(SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK);
> (snip)
> >  	ret = asoc_simple_card_sub_parse_of(np, &dai_props->cpu_dai,
> >  					    &dai_link->cpu_of_node,
> > -					    &dai_link->cpu_dai_name);
> > +					    &dai_link->cpu_dai_name,
> > +					    &daifmt);
> (snip)
> > +	dai_props->cpu_dai.fmt |= daifmt;
> (snip)
> >  	ret = asoc_simple_card_sub_parse_of(np, &dai_props->codec_dai,
> >  					    &dai_link->codec_of_node,
> > -					    &dai_link->codec_dai_name);
> > +					    &dai_link->codec_dai_name,
> > +					    &daifmt);
> (snip)
> > +	dai_props->codec_dai.fmt |= daifmt;
> 
> These are using
> 
>       asoc_simple_card_sub_parse_of(np, &dai_props->cpu_dai, xxx)
>       asoc_simple_card_sub_parse_of(np, &dai_props->codec_dai, xxx)
> 
> I guess, asoc_simple_card_sub_parse_of() can update
> below inside function
> 
> 	dai_props->cpu_dai.fmt |= daifmt;
> 	dai_props->codec_dai.fmt |= daifmt;
> 
> 

Yes, yes, that's very well.

Thanks very much for you advice.

For some reason I must resent this patch series, and then I will
fix this together.


BRs
Xiubo

RE: [alsa-devel] [PATCHv1 1/7] ASoC: simple-card: Merge single and muti DAI link code.

[Li.Xiubo]

Hi Kuninori-san

This patch series will break the old DTs, and I will follow Mark's
Advice to maintain compatibility with the old DTs.

I will send another version, please help me to review it. And I will also
split the update patch series to many small ones.

Thanks very much,

BRs
Xiubo

> -----Original Message-----
> From: Kuninori Morimoto [mailto:kuninori.morimoto.gx@gmail.com]
> Sent: Monday, September 01, 2014 3:43 PM
> To: Xiubo Li-B47053
> Cc: broonie@kernel.org; lgirdwood@gmail.com; perex@perex.cz; tiwai@suse.de;
> kuninori.morimoto.gx@renesas.com; moinejf@free.fr; andrew@lunn.ch;
> jsarha@ti.com; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-sh@vger.kernel.org; alsa-devel@alsa-
> project.org; Guo Shawn-R65073; Nicolin Chen; linux-kernel@vger.kernel.org
> Subject: Re: [alsa-devel] [PATCHv1 1/7] ASoC: simple-card: Merge single and
> muti DAI link code.
> 
> 
> Hi Xiubo
> 
> Thank you for your patch.
> This clean-up is very nice for me.
> But, I have 1 small comment
> 
> >  asoc_simple_card_sub_parse_of(struct device_node *np,
> >  			      struct asoc_simple_dai *dai,
> >  			      struct device_node **p_node,
> > -			      const char **name)
> > +			      const char **name,
> > +			      unsigned int *daifmt)
> >  {
> >  	struct device_node *node;
> >  	struct clk *clk;
> >  	u32 val;
> >  	int ret;
> >
> > +	if (!daifmt)
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * Parse format, bitclock-inversion and frame-inversion
> > +	 * for DAI each device.
> > +	 */
> > +	*daifmt = snd_soc_of_parse_daifmt(np, NULL, NULL, NULL);
> > +	*daifmt &= ~(SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK);
> (snip)
> >  	ret = asoc_simple_card_sub_parse_of(np, &dai_props->cpu_dai,
> >  					    &dai_link->cpu_of_node,
> > -					    &dai_link->cpu_dai_name);
> > +					    &dai_link->cpu_dai_name,
> > +					    &daifmt);
> (snip)
> > +	dai_props->cpu_dai.fmt |= daifmt;
> (snip)
> >  	ret = asoc_simple_card_sub_parse_of(np, &dai_props->codec_dai,
> >  					    &dai_link->codec_of_node,
> > -					    &dai_link->codec_dai_name);
> > +					    &dai_link->codec_dai_name,
> > +					    &daifmt);
> (snip)
> > +	dai_props->codec_dai.fmt |= daifmt;
> 
> These are using
> 
>       asoc_simple_card_sub_parse_of(np, &dai_props->cpu_dai, xxx)
>       asoc_simple_card_sub_parse_of(np, &dai_props->codec_dai, xxx)
> 
> I guess, asoc_simple_card_sub_parse_of() can update
> below inside function
> 
> 	dai_props->cpu_dai.fmt |= daifmt;
> 	dai_props->codec_dai.fmt |= daifmt;
> 
> 
> Best regards
> ---
> Kuninori Morimoto

Re: [PATCHv1 1/7] ASoC: simple-card: Merge single and muti DAI link code.

[Andrew Lunn]

On Mon, Sep 01, 2014 at 12:29:35PM +0800, Xiubo Li wrote:
> This patch merge single DAI link and muti-DAI links code together,
> and simply the simple-card driver code.

Hi Xiubo

It would be good to note that this breaks the Binary API with DT
blobs. Old blobs will not work with this new C code.

> And also do some other improvement:

It would be good to put these improvements into separate patches,
making them easier to review.

> +====
> +DAI link node(s) and its or their subnodes:
>  
> -Required dai-link subnodes:
> +There must be one DAI link node exsit at least, and each DAI link must

typ0. It would also be better to say:

There must be at least one DAI link, and each DAI link must

> +contain one CPU subnode and one CODEC subnode.

  Andrew

Re: [PATCHv1 1/7] ASoC: simple-card: Merge single and muti DAI link code.

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

On Mon, Sep 01, 2014 at 03:34:12PM +0200, Andrew Lunn wrote:
> On Mon, Sep 01, 2014 at 12:29:35PM +0800, Xiubo Li wrote:
> > This patch merge single DAI link and muti-DAI links code together,
> > and simply the simple-card driver code.

> It would be good to note that this breaks the Binary API with DT
> blobs. Old blobs will not work with this new C code.

That's not on, we need to maintian compatibility with the old DTs unless
there's a very strong reason not to.  Revising the binding is one option
for doing this, keeping the old binding in place and making a v2 binding
with the new functionality.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCHv1 1/7] ASoC: simple-card: Merge single and muti DAI link code.

[Li.Xiubo]

> 
> On Mon, Sep 01, 2014 at 03:34:12PM +0200, Andrew Lunn wrote:
> > On Mon, Sep 01, 2014 at 12:29:35PM +0800, Xiubo Li wrote:
> > > This patch merge single DAI link and muti-DAI links code together,
> > > and simply the simple-card driver code.
> 
> > It would be good to note that this breaks the Binary API with DT
> > blobs. Old blobs will not work with this new C code.
> 
> That's not on, we need to maintian compatibility with the old DTs unless
> there's a very strong reason not to.  Revising the binding is one option
> for doing this, keeping the old binding in place and making a v2 binding
> with the new functionality.

Yes, agree.

Actually, the current code is a bit more complicated that we should keep in mind
of two styles of single DAI link and muti-DAI links. And the binding document and
code are a little unreadable.

For others must read the simple card code carefully to add one DT node.

Thanks,

BRs

Re: [PATCHv1 1/7] ASoC: simple-card: Merge single and muti DAI link code.

[Jason Cooper]

As Andrew Lunn is on vacation until the 12th, please keep me in the Cc
on future revisions of this patch.

I'm not at all familiar with the audio subsystem, so I'll refrain from
commenting on the details.  However, with respect to DT, this is a
no-go.  This series breaks all boards in the field as soon as they
upgrade the kernel.  Please don't assume that the DT is tied to the
kernel version.  Any changes to the DT binding must remain backward
compatible with existing dtbs already in the wild.

Mark, please don't merge this until Andrew has had a chance to test.

thx,

Jason.

On Mon, Sep 01, 2014 at 12:29:35PM +0800, Xiubo Li wrote:
> This patch merge single DAI link and muti-DAI links code together,
> and simply the simple-card driver code.
> 
> And also do some other improvement:
> 
> Since from the DAI format micro SND_SOC_DAIFMT_CBx_CFx, the 'CBx'
> mean Codec's bit clock is as master/slave and the 'CFx' mean Codec's
> frame clock is as master/slave.
> 
> So these same DAI formats should be informed to CPU and CODE DAIs at
> the same time. For the Codec driver will set the bit clock and frame
> clock as the DAI formats said, but for the CPU driver, if the the
> bit clock or frame clock is as Codec master, so it should be set CPU
> DAI device as bit clock or frame clock as slave, and vice versa.
> 
> The old code will cause confusion, and we should be clear that the
> letter 'C' here mean to Codec.
> 
> Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> Cc: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Cc: Jean-Francois Moine <moinejf@free.fr>
> Cc: Jyri Sarha <jsarha@ti.com>
> Cc: Nicolin Chen <nicoleotsuka@gmail.com>
> ---
>  .../devicetree/bindings/sound/simple-card.txt      | 101 +++++++-------
>  sound/soc/generic/simple-card.c                    | 152 +++++++--------------
>  2 files changed, 102 insertions(+), 151 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
> index c2e9841..6cc44a6a 100644
> --- a/Documentation/devicetree/bindings/sound/simple-card.txt
> +++ b/Documentation/devicetree/bindings/sound/simple-card.txt
> @@ -1,13 +1,12 @@
>  Simple-Card:
>  
> +====
>  Simple-Card specifies audio DAI connections of SoC <-> codec.
>  
>  Required properties:
> -
>  - compatible				: "simple-audio-card"
>  
>  Optional properties:
> -
>  - simple-audio-card,name		: User specified audio sound card name, one string
>  					  property.
>  - simple-audio-card,widgets		: Please refer to widgets.txt.
> @@ -18,68 +17,57 @@ Optional properties:
>  - simple-audio-card,mclk-fs             : Multiplication factor between stream rate and codec
>    					  mclk.
>  
> -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.
> +====
> +DAI link node(s) and its or their subnodes:
>  
> -Required dai-link subnodes:
> +There must be one DAI link node exsit at least, and each DAI link must
> +contain one CPU subnode and one CODEC subnode.
>  
> -- cpu					: CPU   sub-node
> -- codec					: CODEC sub-node
> -
> -Optional dai-link subnode properties:
> +Required DAI link node(s):
> +- simple-audio-card,dai-link		: Container for DAI link level
> +					  properties and the CPU and CODEC
> +					  sub-nodes.
>  
> +Optional DAI link node 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 there
> -should be no dai-link node, the same properties should not be present
> -at sound-node level, and the bitclock-inversion and frame-inversion
> -properties should also be placed in the codec node if needed.
> -
> -Required CPU/CODEC subnodes properties:
> -
> -- sound-dai				: phandle and port of CPU/CODEC
> +- frame-master				: Boolean property. If present, for this
> +					  DAI link the Codec DAI's frame will be
> +					  as master and the CPU DAI's frame will
> +					  be as slave, or vice versa.
> +- bitclock-master			: Boolean property. If present, for this
> +					  DAI link the Codec DAI's bit clock will
> +					  be as master and the CPU DAI's bit clock
> +					  will be as slave, or vice versa.
> +====
> +DAI link's subnodes and their properties:
> +
> +Required DAI link subnodes:
> +- cpu					: CPU   sub-node
> +- codec					: CODEC sub-node
>  
> -Optional CPU/CODEC subnodes properties:
> +Required DAI link subnodes' properties:
> +- sound-dai				: Phandle and port of CPU/CODEC
>  
> +Optional DAI link subnodes' properties:
> +- bitclock-inversion			: Boolean property. Add this if the DAI
> +					  device needs bit clock to be inversed.
> +- frame-inversion			: Boolean property. Add this if the DAI
> +					  device needs frame clock to be inversed.
>  - 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)
> +====
>  
>  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",
> @@ -89,13 +77,23 @@ sound {
>  		"Headphone Jack", "HP_OUT",
>  		"External Speaker", "LINE_OUT";
>  
> -	simple-audio-card,cpu {
> -		sound-dai = <&sh_fsi2 0>;
> -	};
> +	simple-audio-card,dai-link {
> +		format = "left_j";
> +		bitclock-master;
> +		frame-master;
> +
> +		cpu {
> +			sound-dai = <&sh_fsi2 0>;
>  
> -	dailink0_master: simple-audio-card,codec {
> -		sound-dai = <&ak4648>;
> -		clocks = <&osc>;
> +			bitclock-inversion;
> +			frame-inversion;
> +		};
> +		codec {
> +			sound-dai = <&ak4648>;
> +			clocks = <&osc>;
> +
> +			bitclock-inversion;
> +		};
>  	};
>  };
>  
> @@ -123,6 +121,9 @@ sound {
>  
>  	simple-audio-card,dai-link@0 {		/* I2S - HDMI */
>  		format = "i2s";
> +		bitclock-master;
> +		frame-master;
> +
>  		cpu {
>  			sound-dai = <&audio1 0>;
>  		};
> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index 08c5d7d..d45c8dd 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -112,13 +112,24 @@ static int
>  asoc_simple_card_sub_parse_of(struct device_node *np,
>  			      struct asoc_simple_dai *dai,
>  			      struct device_node **p_node,
> -			      const char **name)
> +			      const char **name,
> +			      unsigned int *daifmt)
>  {
>  	struct device_node *node;
>  	struct clk *clk;
>  	u32 val;
>  	int ret;
>  
> +	if (!daifmt)
> +		return -EINVAL;
> +
> +	/*
> +	 * Parse format, bitclock-inversion and frame-inversion
> +	 * for DAI each device.
> +	 */
> +	*daifmt = snd_soc_of_parse_daifmt(np, NULL, NULL, NULL);
> +	*daifmt &= ~(SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK);
> +
>  	/*
>  	 * get node via "sound-dai = <&phandle port>"
>  	 * it will be used as xxx_of_node on soc_bind_dai_link()
> @@ -166,102 +177,58 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
>  static int asoc_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,
> -					bool is_top_level_node)
> +					struct simple_dai_props *dai_props)
>  {
>  	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;
>  
> -	if (is_top_level_node)
> -		prefix = "simple-audio-card,";
> -
> -	daifmt = snd_soc_of_parse_daifmt(node, prefix,
> -					 &bitclkmaster, &framemaster);
> -	daifmt &= ~SND_SOC_DAIFMT_MASTER_MASK;
> +	/* Parse bitclock-master and frame-master for DAI link */
> +	daifmt = snd_soc_of_parse_daifmt(node, NULL, NULL, NULL);
> +	daifmt &= (SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK);
> +	dai_props->cpu_dai.fmt = dai_props->codec_dai.fmt = daifmt;
>  
> -	snprintf(prop, sizeof(prop), "%scpu", prefix);
> -	np = of_get_child_by_name(node, prop);
> +	/* Parse CPU DAI */
> +	np = of_get_child_by_name(node, "cpu");
>  	if (!np) {
>  		ret = -EINVAL;
> -		dev_err(dev, "%s: Can't find %s DT node\n", __func__, prop);
> +		dev_err(dev, "%s: Can't find 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);
> +					    &dai_link->cpu_dai_name,
> +					    &daifmt);
>  	if (ret < 0)
>  		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;
> -	}
> -
> +	dai_props->cpu_dai.fmt |= daifmt;
>  	of_node_put(np);
> -	snprintf(prop, sizeof(prop), "%scodec", prefix);
> -	np = of_get_child_by_name(node, prop);
> +
> +	/* Parse CODEC DAI */
> +	np = of_get_child_by_name(node, "codec");
>  	if (!np) {
>  		ret = -EINVAL;
> -		dev_err(dev, "%s: Can't find %s DT node\n", __func__, prop);
> +		dev_err(dev, "%s: Can't find 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);
> +					    &dai_link->codec_dai_name,
> +					    &daifmt);
>  	if (ret < 0)
>  		goto dai_link_of_err;
>  
> -	if (strlen(prefix) && !bitclkmaster && !framemaster) {
> -		/* No dai-link level and master setting was not found from
> -		   sound node level, revert back to legacy DT parsing and
> -		   take the 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;
>  	}
>  
> +	dai_props->codec_dai.fmt |= daifmt;
> +
>  	/* simple-card assumes platform == cpu */
>  	dai_link->platform_of_node = dai_link->cpu_of_node;
>  
> @@ -300,10 +267,6 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
>  dai_link_of_err:
>  	if (np)
>  		of_node_put(np);
> -	if (bitclkmaster)
> -		of_node_put(bitclkmaster);
> -	if (framemaster)
> -		of_node_put(framemaster);
>  	return ret;
>  }
>  
> @@ -325,13 +288,13 @@ asoc_simple_card_unref(const struct snd_soc_dai_link *dai_link,
>  
>  static int asoc_simple_card_parse_of(struct device_node *node,
>  				     struct simple_card_data *priv,
> -				     struct device *dev,
> -				     int multi)
> +				     struct device *dev)
>  {
>  	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 = NULL;
>  	u32 val;
> -	int ret;
> +	int ret, i = 0;
>  
>  	/* parsing the card name from DT */
>  	snd_soc_of_parse_card_name(&priv->snd_card, "simple-audio-card,name");
> @@ -360,30 +323,20 @@ static int asoc_simple_card_parse_of(struct device_node *node,
>  	dev_dbg(dev, "New simple-card: %s\n", priv->snd_card.name ?
>  		priv->snd_card.name : "");
>  
> -	if (multi) {
> -		struct device_node *np = NULL;
> -		int i = 0;
> -
> -		for_each_child_of_node(node, np) {
> -			dev_dbg(dev, "\tlink %d:\n", i);
> -			ret = asoc_simple_card_dai_link_of(np, dev,
> -							   dai_link + i,
> -							   dai_props + i,
> -							   false);
> -			if (ret < 0) {
> -				asoc_simple_card_unref(dai_link, i + 1);
> -				of_node_put(np);
> -				return ret;
> -			}
> -			i++;
> -		}
> -		of_node_put(np);
> -	} else {
> -		ret = asoc_simple_card_dai_link_of(node, dev,
> -						   dai_link, dai_props, true);
> -		if (ret < 0)
> +	/* Parse DAI link(s) */
> +	for_each_child_of_node(node, np) {
> +		dev_dbg(dev, "\tlink %d:\n", i);
> +		ret = asoc_simple_card_dai_link_of(np, dev,
> +				dai_link + i,
> +				dai_props + i);
> +		if (ret < 0) {
> +			asoc_simple_card_unref(dai_link, i + 1);
> +			of_node_put(np);
>  			return ret;
> +		}
> +		i++;
>  	}
> +	of_node_put(np);
>  
>  	if (!priv->snd_card.name)
>  		priv->snd_card.name = priv->snd_card.dai_link->name;
> @@ -397,16 +350,13 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
>  	struct snd_soc_dai_link *dai_link;
>  	struct device_node *np = pdev->dev.of_node;
>  	struct device *dev = &pdev->dev;
> -	int num_links, multi, ret;
> +	int num_links, ret;
>  
>  	/* get the number of DAI links */
> -	if (np && of_get_child_by_name(np, "simple-audio-card,dai-link")) {
> +	if (np && of_get_child_by_name(np, "simple-audio-card,dai-link"))
>  		num_links = of_get_child_count(np);
> -		multi = 1;
> -	} else {
> -		num_links = 1;
> -		multi = 0;
> -	}
> +	else
> +		return -EINVAL;
>  
>  	/* allocate the private data and the DAI link array */
>  	priv = devm_kzalloc(dev,
> @@ -433,7 +383,7 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
>  
>  	if (np && of_device_is_available(np)) {
>  
> -		ret = asoc_simple_card_parse_of(np, priv, dev, multi);
> +		ret = asoc_simple_card_parse_of(np, priv, dev);
>  		if (ret < 0) {
>  			if (ret != -EPROBE_DEFER)
>  				dev_err(dev, "parse error %d\n", ret);
> -- 
> 1.8.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv1 2/7] ASoC: simple-card: Adjust the comments of simple card.

[Xiubo Li]

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 sound/soc/generic/simple-card.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index d45c8dd..e9d5a69 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -131,7 +131,7 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
 	*daifmt &= ~(SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK);
 
 	/*
-	 * get node via "sound-dai = <&phandle port>"
+	 * Get node via "sound-dai = <&phandle port>"
 	 * it will be used as xxx_of_node on soc_bind_dai_link()
 	 */
 	node = of_parse_phandle(np, "sound-dai", 0);
@@ -139,12 +139,12 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
 		return -ENODEV;
 	*p_node = node;
 
-	/* get dai->name */
+	/* Get dai->name */
 	ret = snd_soc_of_get_dai_name(np, name);
 	if (ret < 0)
 		return ret;
 
-	/* parse TDM slot */
+	/* Parse TDM slot */
 	ret = snd_soc_of_parse_tdm_slot(np, &dai->slots, &dai->slot_width);
 	if (ret)
 		return ret;
@@ -189,7 +189,7 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
 	daifmt &= (SND_SOC_DAIFMT_MASTER_MASK | SND_SOC_DAIFMT_FORMAT_MASK);
 	dai_props->cpu_dai.fmt = dai_props->codec_dai.fmt = daifmt;
 
-	/* Parse CPU DAI */
+	/* Parse CPU DAI node */
 	np = of_get_child_by_name(node, "cpu");
 	if (!np) {
 		ret = -EINVAL;
@@ -207,7 +207,7 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
 	dai_props->cpu_dai.fmt |= daifmt;
 	of_node_put(np);
 
-	/* Parse CODEC DAI */
+	/* Parse CODEC DAI node */
 	np = of_get_child_by_name(node, "codec");
 	if (!np) {
 		ret = -EINVAL;
@@ -229,10 +229,10 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
 
 	dai_props->codec_dai.fmt |= daifmt;
 
-	/* simple-card assumes platform == cpu */
+	/* Simple Card assumes platform == cpu */
 	dai_link->platform_of_node = dai_link->cpu_of_node;
 
-	/* Link name is created from CPU/CODEC dai name */
+	/* DAI Link's 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,
@@ -254,7 +254,7 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
 		dai_props->codec_dai.sysclk);
 
 	/*
-	 * soc_bind_dai_link() will check cpu name
+	 * asoc_bind_dai_link() will check cpu name
 	 * after of_node matching if dai_link has cpu_dai_name.
 	 * but, it will never match if name was created by fmt_single_name()
 	 * remove cpu_dai_name to escape name matching.
@@ -296,10 +296,10 @@ static int asoc_simple_card_parse_of(struct device_node *node,
 	u32 val;
 	int ret, i = 0;
 
-	/* parsing the card name from DT */
+	/* Parse the card name from DT */
 	snd_soc_of_parse_card_name(&priv->snd_card, "simple-audio-card,name");
 
-	/* off-codec widgets */
+	/* Parse off-codec widgets */
 	if (of_property_read_bool(node, "simple-audio-card,widgets")) {
 		ret = snd_soc_of_parse_audio_simple_widgets(&priv->snd_card,
 					"simple-audio-card,widgets");
@@ -352,13 +352,13 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	int num_links, ret;
 
-	/* get the number of DAI links */
+	/* Get the number of DAI links */
 	if (np && of_get_child_by_name(np, "simple-audio-card,dai-link"))
 		num_links = of_get_child_count(np);
 	else
 		return -EINVAL;
 
-	/* allocate the private data and the DAI link array */
+	/* Allocate the private data and the DAI link array */
 	priv = devm_kzalloc(dev,
 			sizeof(*priv) + sizeof(*dai_link) * num_links,
 			GFP_KERNEL);
@@ -366,7 +366,7 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	/*
-	 * init snd_soc_card
+	 * Init snd_soc_card
 	 */
 	priv->snd_card.owner = THIS_MODULE;
 	priv->snd_card.dev = dev;
@@ -374,7 +374,7 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
 	priv->snd_card.dai_link = dai_link;
 	priv->snd_card.num_links = num_links;
 
-	/* get room for the other properties */
+	/* Get room for the other properties */
 	priv->dai_props = devm_kzalloc(dev,
 			sizeof(*priv->dai_props) * num_links,
 			GFP_KERNEL);
-- 
1.8.4

[PATCHv1 3/7] ASoC: dts: vf610-twr: To support simple card newest style.

[Xiubo Li]

This patch depends on the following simple card patch:
===
ASoC: simple-card: Merge single and muti DAI link code.

This patch merge single DAI link and muti-DAI links code together,
and simply the simple-card driver code.

And also do some other improvement:

Since from the DAI format micro SND_SOC_DAIFMT_CBx_CFx, the 'CBx'
mean Codec's bit clock is as master/slave and the 'CFx' mean Codec's
frame clock is as master/slave.

So these same DAI formats should be informed to CPU and CODE DAIs at
the same time. For the Codec driver will set the bit clock and frame
clock as the DAI formats said, but for the CPU driver, if the the
bit clock or frame clock is as Codec master, so it should be set CPU
DAI device as bit clock or frame clock as slave, and vice versa.

The old code will cause confusion, and we should be clear that the
letter 'C' here mean to Codec.
===

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 arch/arm/boot/dts/vf610-twr.dts | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/arm/boot/dts/vf610-twr.dts b/arch/arm/boot/dts/vf610-twr.dts
index b8a5e8c..841a45a 100644
--- a/arch/arm/boot/dts/vf610-twr.dts
+++ b/arch/arm/boot/dts/vf610-twr.dts
@@ -61,7 +61,6 @@
 
 	sound {
 		compatible = "simple-audio-card";
-		simple-audio-card,format = "i2s";
 		simple-audio-card,widgets =
 			"Microphone", "Microphone Jack",
 			"Headphone", "Headphone Jack",
@@ -74,17 +73,17 @@
 			"Headphone Jack", "HP_OUT",
 			"Speaker Ext", "LINE_OUT";
 
-		simple-audio-card,cpu {
-			sound-dai = <&sai2>;
-			master-clkdir-out;
+		simple-audio-card,dai-link {
+			format = "i2s";
 			frame-master;
 			bitclock-master;
-		};
 
-		simple-audio-card,codec {
-			sound-dai = <&codec>;
-			frame-master;
-			bitclock-master;
+			cpu {
+				sound-dai = <&sai2>;
+			};
+			codec {
+				sound-dai = <&codec>;
+			};
 		};
 	};
 };
-- 
1.8.4

[PATCHv1 4/7] ASoC: dts: kirkwood-t5325: To support simple card newest style.

[Xiubo Li]

This patch depends on the following simple card patch:
===
ASoC: simple-card: Merge single and muti DAI link code.

This patch merge single DAI link and muti-DAI links code together,
and simply the simple-card driver code.

And also do some other improvement:

Since from the DAI format micro SND_SOC_DAIFMT_CBx_CFx, the 'CBx'
mean Codec's bit clock is as master/slave and the 'CFx' mean Codec's
frame clock is as master/slave.

So these same DAI formats should be informed to CPU and CODE DAIs at
the same time. For the Codec driver will set the bit clock and frame
clock as the DAI formats said, but for the CPU driver, if the the
bit clock or frame clock is as Codec master, so it should be set CPU
DAI device as bit clock or frame clock as slave, and vice versa.

The old code will cause confusion, and we should be clear that the
letter 'C' here mean to Codec.
===

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 arch/arm/boot/dts/kirkwood-t5325.dts | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/kirkwood-t5325.dts b/arch/arm/boot/dts/kirkwood-t5325.dts
index 610ec0f..25d1223 100644
--- a/arch/arm/boot/dts/kirkwood-t5325.dts
+++ b/arch/arm/boot/dts/kirkwood-t5325.dts
@@ -189,7 +189,6 @@
 
 	sound {
 		compatible = "simple-audio-card";
-		simple-audio-card,format = "i2s";
 		simple-audio-card,routing =
 			"Headphone Jack", "HPL",
 			"Headphone Jack", "HPR",
@@ -204,12 +203,14 @@
 
 		simple-audio-card,mclk-fs = <256>;
 
-		simple-audio-card,cpu {
-			sound-dai = <&audio>;
-		};
-
-		simple-audio-card,codec {
-			sound-dai = <&alc5621>;
+		simple-audio-card,dai-link {
+			format = "i2s";
+			cpu {
+				sound-dai = <&audio>;
+			};
+			codec {
+				sound-dai = <&alc5621>;
+			};
 		};
 	};
 };
-- 
1.8.4

Re: [PATCHv1 4/7] ASoC: dts: kirkwood-t5325: To support simple card newest style.

[Andrew Lunn]

On Mon, Sep 01, 2014 at 12:29:38PM +0800, Xiubo Li wrote:
> This patch depends on the following simple card patch:
> ===
> ASoC: simple-card: Merge single and muti DAI link code.

Saying what a patch depends on, is not the best of ChangeLog.

Say something like:

The simple-card binding has been changed, so that a dai-link subnode
is now required, and the properties directly under the sound node are
no longer allowed. Modify the DT to fit this new binding.

   Andrew


> 
> This patch merge single DAI link and muti-DAI links code together,
> and simply the simple-card driver code.
> 
> And also do some other improvement:
> 
> Since from the DAI format micro SND_SOC_DAIFMT_CBx_CFx, the 'CBx'
> mean Codec's bit clock is as master/slave and the 'CFx' mean Codec's
> frame clock is as master/slave.
> 
> So these same DAI formats should be informed to CPU and CODE DAIs at
> the same time. For the Codec driver will set the bit clock and frame
> clock as the DAI formats said, but for the CPU driver, if the the
> bit clock or frame clock is as Codec master, so it should be set CPU
> DAI device as bit clock or frame clock as slave, and vice versa.
> 
> The old code will cause confusion, and we should be clear that the
> letter 'C' here mean to Codec.
> ===
> 
> Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> ---
>  arch/arm/boot/dts/kirkwood-t5325.dts | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/kirkwood-t5325.dts b/arch/arm/boot/dts/kirkwood-t5325.dts
> index 610ec0f..25d1223 100644
> --- a/arch/arm/boot/dts/kirkwood-t5325.dts
> +++ b/arch/arm/boot/dts/kirkwood-t5325.dts
> @@ -189,7 +189,6 @@
>  
>  	sound {
>  		compatible = "simple-audio-card";
> -		simple-audio-card,format = "i2s";
>  		simple-audio-card,routing =
>  			"Headphone Jack", "HPL",
>  			"Headphone Jack", "HPR",
> @@ -204,12 +203,14 @@
>  
>  		simple-audio-card,mclk-fs = <256>;
>  
> -		simple-audio-card,cpu {
> -			sound-dai = <&audio>;
> -		};
> -
> -		simple-audio-card,codec {
> -			sound-dai = <&alc5621>;
> +		simple-audio-card,dai-link {
> +			format = "i2s";
> +			cpu {
> +				sound-dai = <&audio>;
> +			};
> +			codec {
> +				sound-dai = <&alc5621>;
> +			};
>  		};
>  	};
>  };
> -- 
> 1.8.4
> 

RE: [PATCHv1 4/7] ASoC: dts: kirkwood-t5325: To support simple card newest style.

[Li.Xiubo]

Hi Andrew,

Thanks very much for you comment and advice.

I will resend this patch series to compatibility with the old DTs, and it will
Up to the owners to update the DTs to support the new style of DTs.

BRs
Xiubo




> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Monday, September 01, 2014 9:42 PM
> To: Xiubo Li-B47053
> Cc: broonie@kernel.org; lgirdwood@gmail.com; perex@perex.cz; tiwai@suse.de;
> kuninori.morimoto.gx@renesas.com; moinejf@free.fr; andrew@lunn.ch;
> jsarha@ti.com; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-sh@vger.kernel.org; alsa-devel@alsa-
> project.org; Guo Shawn-R65073; linux-kernel@vger.kernel.org; Jason Cooper
> Subject: Re: [PATCHv1 4/7] ASoC: dts: kirkwood-t5325: To support simple card
> newest style.
> 
> On Mon, Sep 01, 2014 at 12:29:38PM +0800, Xiubo Li wrote:
> > This patch depends on the following simple card patch:
> > ===
> > ASoC: simple-card: Merge single and muti DAI link code.
> 
> Saying what a patch depends on, is not the best of ChangeLog.
> 
> Say something like:
> 
> The simple-card binding has been changed, so that a dai-link subnode
> is now required, and the properties directly under the sound node are
> no longer allowed. Modify the DT to fit this new binding.
> 
>    Andrew
> 
> 
> >
> > This patch merge single DAI link and muti-DAI links code together,
> > and simply the simple-card driver code.
> >
> > And also do some other improvement:
> >
> > Since from the DAI format micro SND_SOC_DAIFMT_CBx_CFx, the 'CBx'
> > mean Codec's bit clock is as master/slave and the 'CFx' mean Codec's
> > frame clock is as master/slave.
> >
> > So these same DAI formats should be informed to CPU and CODE DAIs at
> > the same time. For the Codec driver will set the bit clock and frame
> > clock as the DAI formats said, but for the CPU driver, if the the
> > bit clock or frame clock is as Codec master, so it should be set CPU
> > DAI device as bit clock or frame clock as slave, and vice versa.
> >
> > The old code will cause confusion, and we should be clear that the
> > letter 'C' here mean to Codec.
> > ===
> >
> > Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
> > ---
> >  arch/arm/boot/dts/kirkwood-t5325.dts | 15 ++++++++-------
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/arm/boot/dts/kirkwood-t5325.dts
> b/arch/arm/boot/dts/kirkwood-t5325.dts
> > index 610ec0f..25d1223 100644
> > --- a/arch/arm/boot/dts/kirkwood-t5325.dts
> > +++ b/arch/arm/boot/dts/kirkwood-t5325.dts
> > @@ -189,7 +189,6 @@
> >
> >  	sound {
> >  		compatible = "simple-audio-card";
> > -		simple-audio-card,format = "i2s";
> >  		simple-audio-card,routing =
> >  			"Headphone Jack", "HPL",
> >  			"Headphone Jack", "HPR",
> > @@ -204,12 +203,14 @@
> >
> >  		simple-audio-card,mclk-fs = <256>;
> >
> > -		simple-audio-card,cpu {
> > -			sound-dai = <&audio>;
> > -		};
> > -
> > -		simple-audio-card,codec {
> > -			sound-dai = <&alc5621>;
> > +		simple-audio-card,dai-link {
> > +			format = "i2s";
> > +			cpu {
> > +				sound-dai = <&audio>;
> > +			};
> > +			codec {
> > +				sound-dai = <&alc5621>;
> > +			};
> >  		};
> >  	};
> >  };
> > --
> > 1.8.4
> >

[PATCHv1 5/7] ASoC: dts: r8a7740-armadillo800eva-reference: To support simple card newest style.

[Xiubo Li]

This patch depends on the following simple card patch:
===
ASoC: simple-card: Merge single and muti DAI link code.

This patch merge single DAI link and muti-DAI links code together,
and simply the simple-card driver code.

And also do some other improvement:

Since from the DAI format micro SND_SOC_DAIFMT_CBx_CFx, the 'CBx'
mean Codec's bit clock is as master/slave and the 'CFx' mean Codec's
frame clock is as master/slave.

So these same DAI formats should be informed to CPU and CODE DAIs at
the same time. For the Codec driver will set the bit clock and frame
clock as the DAI formats said, but for the CPU driver, if the the
bit clock or frame clock is as Codec master, so it should be set CPU
DAI device as bit clock or frame clock as slave, and vice versa.

The old code will cause confusion, and we should be clear that the
letter 'C' here mean to Codec.
===

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 .../boot/dts/r8a7740-armadillo800eva-reference.dts  | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts b/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts
index ee9e7d5..b83d95a 100644
--- a/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts
+++ b/arch/arm/boot/dts/r8a7740-armadillo800eva-reference.dts
@@ -150,18 +150,19 @@
 	sound {
 		compatible = "simple-audio-card";
 
-		simple-audio-card,format = "i2s";
-
-		simple-audio-card,cpu {
-			sound-dai = <&sh_fsi2 0>;
-			bitclock-inversion;
-		};
-
-		simple-audio-card,codec {
-			sound-dai = <&wm8978>;
+		simple-audio-card,dai-link {
+			format = "i2s";
 			bitclock-master;
 			frame-master;
-			system-clock-frequency = <12288000>;
+
+			cpu {
+				sound-dai = <&sh_fsi2 0>;
+				bitclock-inversion;
+			};
+			codec {
+				sound-dai = <&wm8978>;
+				system-clock-frequency = <12288000>;
+			};
 		};
 	};
 };
-- 
1.8.4

[PATCHv1 6/7] ASoC: dts: sh73a0-kzm9g-reference: To support simple card newest style.

[Xiubo Li]

This patch depends on the following simple card patch:
===
ASoC: simple-card: Merge single and muti DAI link code.

This patch merge single DAI link and muti-DAI links code together,
and simply the simple-card driver code.

And also do some other improvement:

Since from the DAI format micro SND_SOC_DAIFMT_CBx_CFx, the 'CBx'
mean Codec's bit clock is as master/slave and the 'CFx' mean Codec's
frame clock is as master/slave.

So these same DAI formats should be informed to CPU and CODE DAIs at
the same time. For the Codec driver will set the bit clock and frame
clock as the DAI formats said, but for the CPU driver, if the the
bit clock or frame clock is as Codec master, so it should be set CPU
DAI device as bit clock or frame clock as slave, and vice versa.

The old code will cause confusion, and we should be clear that the
letter 'C' here mean to Codec.
===

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 arch/arm/boot/dts/sh73a0-kzm9g-reference.dts | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
index 18662ae..e689ae3 100644
--- a/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
+++ b/arch/arm/boot/dts/sh73a0-kzm9g-reference.dts
@@ -160,15 +160,19 @@
 
 	sound {
 		compatible = "simple-audio-card";
-		simple-audio-card,format = "left_j";
-		simple-audio-card,cpu {
-			sound-dai = <&sh_fsi2 0>;
-		};
-		simple-audio-card,codec {
-			sound-dai = <&ak4648>;
+
+		simple-audio-card,dai-link {
+			format = "left_j";
 			bitclock-master;
 			frame-master;
-			system-clock-frequency = <11289600>;
+
+			cpu {
+				sound-dai = <&sh_fsi2 0>;
+			};
+			codec {
+				sound-dai = <&ak4648>;
+				system-clock-frequency = <11289600>;
+			};
 		};
 	};
 };
-- 
1.8.4

[PATCHv1 7/7] ASoC: dts: kirkwood-openrd-client: To support simple card newest style.

[Xiubo Li]

This patch depends on the following simple card patch:
===
ASoC: simple-card: Merge single and muti DAI link code.

This patch merge single DAI link and muti-DAI links code together,
and simply the simple-card driver code.

And also do some other improvement:

Since from the DAI format micro SND_SOC_DAIFMT_CBx_CFx, the 'CBx'
mean Codec's bit clock is as master/slave and the 'CFx' mean Codec's
frame clock is as master/slave.

So these same DAI formats should be informed to CPU and CODE DAIs at
the same time. For the Codec driver will set the bit clock and frame
clock as the DAI formats said, but for the CPU driver, if the the
bit clock or frame clock is as Codec master, so it should be set CPU
DAI device as bit clock or frame clock as slave, and vice versa.

The old code will cause confusion, and we should be clear that the
letter 'C' here mean to Codec.
===

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 arch/arm/boot/dts/kirkwood-openrd-client.dts | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/dts/kirkwood-openrd-client.dts b/arch/arm/boot/dts/kirkwood-openrd-client.dts
index 887b9c1..3b6f169 100644
--- a/arch/arm/boot/dts/kirkwood-openrd-client.dts
+++ b/arch/arm/boot/dts/kirkwood-openrd-client.dts
@@ -33,15 +33,16 @@
 
 	sound {
 		compatible = "simple-audio-card";
-		simple-audio-card,format = "i2s";
 		simple-audio-card,mclk-fs = <256>;
 
-		simple-audio-card,cpu {
-			sound-dai = <&audio0>;
-		};
-
-		simple-audio-card,codec {
-			sound-dai = <&cs42l51>;
+		simple-audio-card,dai-link {
+			format = "i2s";
+			cpu {
+				sound-dai = <&audio0>;
+			};
+			codec {
+				sound-dai = <&cs42l51>;
+			};
 		};
 	};
 };
-- 
1.8.4