devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/2] ASoC: core: add multi-codec support in DT
       [not found] ` <cover.1415096705.git.moinejf-GANU6spQydw@public.gmane.org>
@ 2014-11-03 17:42   ` Jean-Francois Moine
       [not found]     ` <6b04d690576a0036b401d00784299ee576051c0d.1415096705.git.moinejf-GANU6spQydw@public.gmane.org>
  2014-11-04 10:21   ` [PATCH v3 2/2] ASoC: simple-card: add multi-CODECs " Jean-Francois Moine
  1 sibling, 1 reply; 9+ messages in thread
From: Jean-Francois Moine @ 2014-11-03 17:42 UTC (permalink / raw)
  To: Mark Brown
  Cc: Benoit Cousson, Jyri Sarha, Kuninori Morimoto, Xiubo Li,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA

SoC audio cards may have many CODECs per audio link.
This patch exports a core function to handle such links
when they are described in the DT.

Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>
---
 include/sound/soc.h  |  3 +++
 sound/soc/soc-core.c | 75 +++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 63 insertions(+), 15 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index 7ba7130..a255f8b 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1451,6 +1451,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_node *of_node,
+				   const char *list_name,
+				   struct snd_soc_dai_link *dai_link);
 
 #include <sound/soc-dai.h>
 
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 3818cf3..e585d6d 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -3724,36 +3724,30 @@ unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
 }
 EXPORT_SYMBOL_GPL(snd_soc_of_parse_daifmt);
 
-int snd_soc_of_get_dai_name(struct device_node *of_node,
-			    const char **dai_name)
+static int snd_soc_get_dai_name(struct of_phandle_args *args,
+				const char **dai_name)
 {
 	struct snd_soc_component *pos;
-	struct of_phandle_args args;
-	int ret;
-
-	ret = of_parse_phandle_with_args(of_node, "sound-dai",
-					 "#sound-dai-cells", 0, &args);
-	if (ret)
-		return ret;
-
-	ret = -EPROBE_DEFER;
+	int ret = -EPROBE_DEFER;
 
 	mutex_lock(&client_mutex);
 	list_for_each_entry(pos, &component_list, list) {
-		if (pos->dev->of_node != args.np)
+		if (pos->dev->of_node != args->np)
 			continue;
 
 		if (pos->driver->of_xlate_dai_name) {
-			ret = pos->driver->of_xlate_dai_name(pos, &args, dai_name);
+			ret = pos->driver->of_xlate_dai_name(pos,
+							     args,
+							     dai_name);
 		} else {
 			int id = -1;
 
-			switch (args.args_count) {
+			switch (args->args_count) {
 			case 0:
 				id = 0; /* same as dai_drv[0] */
 				break;
 			case 1:
-				id = args.args[0];
+				id = args->args[0];
 				break;
 			default:
 				/* not supported */
@@ -3775,6 +3769,21 @@ int snd_soc_of_get_dai_name(struct device_node *of_node,
 		break;
 	}
 	mutex_unlock(&client_mutex);
+	return ret;
+}
+
+int snd_soc_of_get_dai_name(struct device_node *of_node,
+			    const char **dai_name)
+{
+	struct of_phandle_args args;
+	int ret;
+
+	ret = of_parse_phandle_with_args(of_node, "sound-dai",
+					 "#sound-dai-cells", 0, &args);
+	if (ret)
+		return ret;
+
+	ret = snd_soc_get_dai_name(&args, dai_name);
 
 	of_node_put(args.np);
 
@@ -3782,6 +3791,42 @@ int snd_soc_of_get_dai_name(struct device_node *of_node,
 }
 EXPORT_SYMBOL_GPL(snd_soc_of_get_dai_name);
 
+int snd_soc_of_get_dai_link_codecs(struct device_node *of_node,
+				   const char *list_name,
+				   struct snd_soc_dai_link *dai_link)
+{
+	struct of_phandle_args args;
+	struct snd_soc_dai_link_component *component;
+	int index, ret;
+
+	for (index = 0, component = dai_link->codecs;
+	     index < dai_link->num_codecs;
+	     index++, component++) {
+		ret = of_parse_phandle_with_args(of_node, list_name,
+						 "#sound-dai-cells",
+						  index, &args);
+		if (ret)
+			goto err;
+		component->of_node = args.np;
+		ret = snd_soc_get_dai_name(&args, &component->dai_name);
+		if (ret < 0)
+			goto err;
+	}
+	return 0;
+
+err:
+	for (index = 0, component = dai_link->codecs;
+	     index < dai_link->num_codecs;
+	     index++, component++) {
+		if (!component->of_node)
+			break;
+		of_node_put((struct device_node *) component->of_node);
+		component->of_node = NULL;
+	}
+	return ret;
+}
+EXPORT_SYMBOL_GPL(snd_soc_of_get_dai_link_codecs);
+
 static int __init snd_soc_init(void)
 {
 #ifdef CONFIG_DEBUG_FS
-- 
2.1.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 2/2] ASoC: simple-card: add multi-CODECs in DT
       [not found] ` <cover.1415096705.git.moinejf-GANU6spQydw@public.gmane.org>
  2014-11-03 17:42   ` [PATCH v3 1/2] ASoC: core: add multi-codec support in DT Jean-Francois Moine
@ 2014-11-04 10:21   ` Jean-Francois Moine
       [not found]     ` <53ef077d2174f8eb5b861d6cebe7c9e732f06d52.1415096705.git.moinejf-GANU6spQydw@public.gmane.org>
  1 sibling, 1 reply; 9+ messages in thread
From: Jean-Francois Moine @ 2014-11-04 10:21 UTC (permalink / raw)
  To: Mark Brown
  Cc: Benoit Cousson, Jyri Sarha, Kuninori Morimoto, Xiubo Li,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

Signed-off-by: Jean-Francois Moine <moinejf-GANU6spQydw@public.gmane.org>
---
 .../devicetree/bindings/sound/simple-card.txt      |  28 ++---
 sound/soc/generic/simple-card.c                    | 135 +++++++++++++--------
 2 files changed, 94 insertions(+), 69 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
index c3cba60..6cb1816 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.txt
+++ b/Documentation/devicetree/bindings/sound/simple-card.txt
@@ -65,7 +65,11 @@ properties should also be placed in the codec node if needed.
 
 Required CPU/CODEC subnodes properties:
 
-- sound-dai				: phandle and port of CPU/CODEC
+either
+  - sound-dai				: phandle and port of CPU/CODEC
+or
+  - sound-dais				: list of phandle and port of CODECs
+					  
 
 Optional CPU/CODEC subnodes properties:
 
@@ -119,37 +123,29 @@ 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";
 	simple-audio-card,name = "Cubox Audio";
 
-	simple-audio-card,dai-link@0 {		/* I2S - HDMI */
+	simple-audio-card,dai-link@0 {		/* S/PDIF - HDMI & S/PDIF */
 		format = "i2s";
 		cpu {
-			sound-dai = <&audio1 0>;
-		};
-		codec {
-			sound-dai = <&tda998x 0>;
-		};
-	};
-
-	simple-audio-card,dai-link@1 {		/* S/PDIF - HDMI */
-		cpu {
 			sound-dai = <&audio1 1>;
 		};
 		codec {
-			sound-dai = <&tda998x 1>;
+			sound-dais = <&hdmi 0>, <&spdif_codec>;
 		};
 	};
 
-	simple-audio-card,dai-link@2 {		/* S/PDIF - S/PDIF */
+	simple-audio-card,dai-link@1 {		/* I2S - HDMI */
+		format = "i2s";
 		cpu {
-			sound-dai = <&audio1 1>;
+			sound-dai = <&audio1 0>;
 		};
 		codec {
-			sound-dai = <&spdif_codec>;
+			sound-dai = <&hdmi 1>;
 		};
 	};
 };
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index cd49d50..ed3b125 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -172,34 +172,12 @@ 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,
 			      struct asoc_simple_dai *dai,
-			      struct device_node **p_node,
-			      const char **name,
-			      int *args_count)
+			      const struct device_node *dai_np)
 {
-	struct of_phandle_args args;
 	struct clk *clk;
 	u32 val;
 	int ret;
 
-	/*
-	 * Get node via "sound-dai = <&phandle port>"
-	 * it will be used as xxx_of_node on soc_bind_dai_link()
-	 */
-	ret = of_parse_phandle_with_args(np, "sound-dai",
-					 "#sound-dai-cells", 0, &args);
-	if (ret)
-		return ret;
-
-	*p_node = args.np;
-
-	if (args_count)
-		*args_count = args.args_count;
-
-	/* Get dai->name */
-	ret = snd_soc_of_get_dai_name(np, name);
-	if (ret < 0)
-		return ret;
-
 	/* Parse TDM slot */
 	ret = snd_soc_of_parse_tdm_slot(np, &dai->slots, &dai->slot_width);
 	if (ret)
@@ -222,7 +200,7 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
 	} else if (!of_property_read_u32(np, "system-clock-frequency", &val)) {
 		dai->sysclk = val;
 	} else {
-		clk = of_clk_get(args.np, 0);
+		clk = of_clk_get((struct device_node *) dai_np, 0);
 		if (!IS_ERR(clk))
 			dai->sysclk = clk_get_rate(clk);
 	}
@@ -232,7 +210,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)
 {
@@ -285,62 +262,99 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
 	struct device *dev = simple_priv_to_dev(priv);
 	struct snd_soc_dai_link *dai_link = simple_priv_to_link(priv, idx);
 	struct simple_dai_props *dai_props = simple_priv_to_props(priv, idx);
-	struct device_node *cpu = NULL;
-	struct device_node *codec = NULL;
+	struct device_node *np = NULL;
+	struct snd_soc_dai_link_component *component;
+	struct of_phandle_args args;
 	char *name;
 	char prop[128];
 	char *prefix = "";
-	int ret, cpu_args;
+	int ret, cpu_args, i, num_codecs;
 
 	/* For single DAI link & old style of DT node */
 	if (is_top_level_node)
 		prefix = "simple-audio-card,";
 
 	snprintf(prop, sizeof(prop), "%scpu", prefix);
-	cpu = of_get_child_by_name(node, prop);
-
-	snprintf(prop, sizeof(prop), "%scodec", prefix);
-	codec = of_get_child_by_name(node, prop);
-
-	if (!cpu || !codec) {
+	np = of_get_child_by_name(node, prop);
+	if (!np) {
 		ret = -EINVAL;
 		dev_err(dev, "%s: Can't find %s DT node\n", __func__, prop);
 		goto dai_link_of_err;
 	}
 
-	ret = asoc_simple_card_parse_daifmt(node, priv,
-					    cpu, codec, prefix, idx);
-	if (ret < 0)
+	/* Get the CPU node and name */
+	ret = of_parse_phandle_with_args(np, "sound-dai",
+					 "#sound-dai-cells", 0, &args);
+	if (ret)
 		goto dai_link_of_err;
+	dai_link->cpu_of_node = args.np;
+	cpu_args = args.args_count;
 
-	ret = asoc_simple_card_sub_parse_of(cpu, &dai_props->cpu_dai,
-					    &dai_link->cpu_of_node,
-					    &dai_link->cpu_dai_name,
-					    &cpu_args);
+	ret = snd_soc_of_get_dai_name(np, &dai_link->cpu_dai_name);
 	if (ret < 0)
 		goto dai_link_of_err;
 
-	ret = asoc_simple_card_sub_parse_of(codec, &dai_props->codec_dai,
-					    &dai_link->codec_of_node,
-					    &dai_link->codec_dai_name, NULL);
+	ret = asoc_simple_card_sub_parse_of(np, &dai_props->cpu_dai,
+					    dai_link->cpu_of_node);
 	if (ret < 0)
 		goto dai_link_of_err;
+	of_node_put(np);
 
-	if (!dai_link->cpu_dai_name || !dai_link->codec_dai_name) {
+	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 %s DT node\n", __func__, prop);
+		goto dai_link_of_err;
+	}
+
+	/* Get the node and name of the CODEC(s) */
+	name = "sound-dai";
+	num_codecs = of_count_phandle_with_args(np, name,
+						"#sound-dai-cells");
+	if (num_codecs <= 0) {
+		name = "sound-dais";
+		num_codecs = of_count_phandle_with_args(np, name,
+							"#sound-dai-cells");
+		if (num_codecs <= 0) {
+			ret = -EINVAL;
+			dev_err(dev, "No CODEC DAI phandle\n");
+			goto dai_link_of_err;
+		}
+	}
+	component = devm_kzalloc(dev,
+				 sizeof *component * num_codecs,
+				 GFP_KERNEL);
+	if (!component) {
+		ret = -ENOMEM;
 		goto dai_link_of_err;
 	}
+	dai_link->codecs = component;
+	dai_link->num_codecs = num_codecs;
+	ret = snd_soc_of_get_dai_link_codecs(np, name, dai_link);
+	if (ret < 0)
+		goto dai_link_of_err;
+
+	ret = asoc_simple_card_sub_parse_of(np, &dai_props->codec_dai,
+					    dai_link->codecs[0].of_node);
+	if (ret < 0)
+		goto dai_link_of_err;
 
 	/* Simple Card assumes platform == cpu */
 	dai_link->platform_of_node = dai_link->cpu_of_node;
 
+	ret = asoc_simple_card_parse_daifmt(node, priv,
+					    np, prefix, idx);
+	if (ret < 0)
+		goto dai_link_of_err;
+
 	/* DAI 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,
+			    strlen(dai_link->codecs[0].dai_name) + 2,
 			    GFP_KERNEL);
 	sprintf(name, "%s-%s", dai_link->cpu_dai_name,
-				dai_link->codec_dai_name);
+				dai_link->codecs[0].dai_name);
 	dai_link->name = dai_link->stream_name = name;
 	dai_link->ops = &asoc_simple_card_ops;
 	dai_link->init = asoc_simple_card_dai_init;
@@ -351,7 +365,7 @@ static int asoc_simple_card_dai_link_of(struct device_node *node,
 		dai_props->cpu_dai.fmt,
 		dai_props->cpu_dai.sysclk);
 	dev_dbg(dev, "\tcodec : %s / %04x / %d\n",
-		dai_link->codec_dai_name,
+		dai_link->codecs[0].dai_name,
 		dai_props->codec_dai.fmt,
 		dai_props->codec_dai.sysclk);
 
@@ -366,11 +380,20 @@ 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:
-	of_node_put(cpu);
-	of_node_put(codec);
+	for (i = 0, component = dai_link->codecs;
+	     i < dai_link->num_codecs;
+	     i++, component++) {
+		if (!component->of_node)
+			break;
+		of_node_put((struct device_node *) component->of_node);
+		component->of_node = NULL;
+	}
 
+out:
+	of_node_put(np);
 	return ret;
 }
 
@@ -457,16 +480,22 @@ static int asoc_simple_card_unref(struct platform_device *pdev)
 {
 	struct snd_soc_card *card = platform_get_drvdata(pdev);
 	struct snd_soc_dai_link *dai_link;
+	struct snd_soc_dai_link_component *component;
 	struct device_node *np;
-	int num_links;
+	int num_links, i;
 
 	for (num_links = 0, dai_link = card->dai_link;
 	     num_links < card->num_links;
 	     num_links++, dai_link++) {
 		np = (struct device_node *) dai_link->cpu_of_node;
 		of_node_put(np);
-		np = (struct device_node *) dai_link->codec_of_node;
-		of_node_put(np);
+		for (i = 0, component = dai_link->codecs;
+		     i < dai_link->num_codecs;
+		     i++, component++) {
+			if (!component->of_node)
+				break;
+			of_node_put((struct device_node *) component->of_node);
+		}
 	}
 	return 0;
 }
-- 
2.1.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3 0/2] ASoC: simple-card: Add multi-CODEC support
@ 2014-11-04 10:25 Jean-Francois Moine
       [not found] ` <cover.1415096705.git.moinejf-GANU6spQydw@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Jean-Francois Moine @ 2014-11-04 10:25 UTC (permalink / raw)
  To: Mark Brown
  Cc: devicetree, alsa-devel, Jyri Sarha, Xiubo Li, Benoit Cousson,
	Kuninori Morimoto

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

v3: rebase on broonie git 2014/11/03
v2: accept list of CODECs in 'sound-dais' (Benoit Cousson)

Jean-Francois Moine (2):
  ASoC: core: add multi-codec support in DT
  ASoC: simple-card: add multi-CODECs in DT

 .../devicetree/bindings/sound/simple-card.txt      |  28 ++---
 include/sound/soc.h                                |   3 +
 sound/soc/generic/simple-card.c                    | 135 +++++++++++++--------
 sound/soc/soc-core.c                               |  75 +++++++++---
 4 files changed, 157 insertions(+), 84 deletions(-)

-- 
2.1.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] ASoC: simple-card: add multi-CODECs in DT
       [not found]     ` <53ef077d2174f8eb5b861d6cebe7c9e732f06d52.1415096705.git.moinejf-GANU6spQydw@public.gmane.org>
@ 2014-11-07 12:30       ` Mark Brown
       [not found]         ` <20141107123035.GO8509-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Mark Brown @ 2014-11-07 12:30 UTC (permalink / raw)
  To: Jean-Francois Moine
  Cc: Benoit Cousson, Jyri Sarha, Kuninori Morimoto, Xiubo Li,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Tue, Nov 04, 2014 at 11:21:46AM +0100, Jean-Francois Moine wrote:

This looks like it combines some refactoring to split out some of the
DAI node parsing code with some new code.  It's quite hard to read
what's going on here, separating the code motion and the new code would
make life a lot easier, I've stared at it for a bit and I'm still not
100% sure it's doing everything it's supposed to be.

> -Example 2 - many DAI links:
> +Example 2 - many DAI links and multi-CODECs:

This would be a lot easier to read if the change didn't completely
rewrite the example but just added the thing it wants to add an example
of; the example may be drawn from a real system but that's not the
point.

>  	} else {
> -		clk = of_clk_get(args.np, 0);
> +		clk = of_clk_get((struct device_node *) dai_np, 0);

Adding this cast looks suspicous - why?  As far as I can tell the
original code didn't need one.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 1/2] ASoC: core: add multi-codec support in DT
       [not found]     ` <6b04d690576a0036b401d00784299ee576051c0d.1415096705.git.moinejf-GANU6spQydw@public.gmane.org>
@ 2014-11-07 14:01       ` Mark Brown
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2014-11-07 14:01 UTC (permalink / raw)
  To: Jean-Francois Moine
  Cc: Benoit Cousson, Jyri Sarha, Kuninori Morimoto, Xiubo Li,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Mon, Nov 03, 2014 at 06:42:22PM +0100, Jean-Francois Moine wrote:
> SoC audio cards may have many CODECs per audio link.
> This patch exports a core function to handle such links
> when they are described in the DT.

It would be a lot easier to review this change if the changelog said
what the description this was intended to parse looks like...

> +int snd_soc_of_get_dai_link_codecs(struct device_node *of_node,
> +				   const char *list_name,
> +				   struct snd_soc_dai_link *dai_link)

This is a newly exported function, please add kerneldoc for it.

> +{
> +	struct of_phandle_args args;
> +	struct snd_soc_dai_link_component *component;
> +	int index, ret;
> +
> +	for (index = 0, component = dai_link->codecs;
> +	     index < dai_link->num_codecs;
> +	     index++, component++) {
> +		ret = of_parse_phandle_with_args(of_node, list_name,
> +						 "#sound-dai-cells",
> +						  index, &args);
> +		if (ret)
> +			goto err;
> +		component->of_node = args.np;
> +		ret = snd_soc_get_dai_name(&args, &component->dai_name);
> +		if (ret < 0)
> +			goto err;
> +	}

We're relying on the caller to fill in the number of CODECs and not
checking that there aren't extra things in the list in the DT that we're
ignoring.  I'd expect us to either get the number of CODECs as part of
this function or check that there isn't anything extra in the list.

I'd also expect some sort of error message on parse failures to help
people writing DTs.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] ASoC: simple-card: add multi-CODECs in DT
       [not found]         ` <20141107123035.GO8509-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
@ 2014-11-07 19:05           ` Jean-Francois Moine
  2014-11-07 20:07             ` [alsa-devel] " Lars-Peter Clausen
  2014-11-08  9:48             ` Mark Brown
  0 siblings, 2 replies; 9+ messages in thread
From: Jean-Francois Moine @ 2014-11-07 19:05 UTC (permalink / raw)
  To: Mark Brown
  Cc: Benoit Cousson, Jyri Sarha, Kuninori Morimoto, Xiubo Li,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On Fri, 7 Nov 2014 12:30:35 +0000
Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:

> >  	} else {
> > -		clk = of_clk_get(args.np, 0);
> > +		clk = of_clk_get((struct device_node *) dai_np, 0);  
> 
> Adding this cast looks suspicous - why?  As far as I can tell the
> original code didn't need one.

Right, 'args.np' was (struct device_node *), but, now, 'dai_np' is
(const struct device_node *).

Changing 'dai_np' to (struct device_node *) in the
asoc_simple_card_sub_parse_of() argument asks for a cast in calling
this function because the field 'of_node' of the struct
snd_soc_dai_link_component is (const struct device_node *).

Do you better like this last cast?

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [alsa-devel] [PATCH v3 2/2] ASoC: simple-card: add multi-CODECs in DT
  2014-11-07 19:05           ` Jean-Francois Moine
@ 2014-11-07 20:07             ` Lars-Peter Clausen
       [not found]               ` <545D2667.4070707-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
  2014-11-08  9:48             ` Mark Brown
  1 sibling, 1 reply; 9+ messages in thread
From: Lars-Peter Clausen @ 2014-11-07 20:07 UTC (permalink / raw)
  To: Jean-Francois Moine
  Cc: Mark Brown, devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Jyri Sarha, Xiubo Li,
	Benoit Cousson, Kuninori Morimoto

On 11/07/2014 08:05 PM, Jean-Francois Moine wrote:
> On Fri, 7 Nov 2014 12:30:35 +0000
> Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>
>>>   	} else {
>>> -		clk = of_clk_get(args.np, 0);
>>> +		clk = of_clk_get((struct device_node *) dai_np, 0);
>>
>> Adding this cast looks suspicous - why?  As far as I can tell the
>> original code didn't need one.
>
> Right, 'args.np' was (struct device_node *), but, now, 'dai_np' is
> (const struct device_node *).
>
> Changing 'dai_np' to (struct device_node *) in the
> asoc_simple_card_sub_parse_of() argument asks for a cast in calling
> this function because the field 'of_node' of the struct
> snd_soc_dai_link_component is (const struct device_node *).
>
> Do you better like this last cast?

Casting const away is usually a sign that something is wrong and has the 
potential to result in undefined behavior. The correct fix in this case is 
probably to make the np argument for of_clk_get() const.

- Lars

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/2] ASoC: simple-card: add multi-CODECs in DT
  2014-11-07 19:05           ` Jean-Francois Moine
  2014-11-07 20:07             ` [alsa-devel] " Lars-Peter Clausen
@ 2014-11-08  9:48             ` Mark Brown
  1 sibling, 0 replies; 9+ messages in thread
From: Mark Brown @ 2014-11-08  9:48 UTC (permalink / raw)
  To: Jean-Francois Moine
  Cc: Benoit Cousson, Jyri Sarha, Kuninori Morimoto, Xiubo Li,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

On Fri, Nov 07, 2014 at 08:05:57PM +0100, Jean-Francois Moine wrote:
> Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:

> > >  	} else {
> > > -		clk = of_clk_get(args.np, 0);
> > > +		clk = of_clk_get((struct device_node *) dai_np, 0);  

> > Adding this cast looks suspicous - why?  As far as I can tell the
> > original code didn't need one.

> Right, 'args.np' was (struct device_node *), but, now, 'dai_np' is
> (const struct device_node *).

> Changing 'dai_np' to (struct device_node *) in the
> asoc_simple_card_sub_parse_of() argument asks for a cast in calling
> this function because the field 'of_node' of the struct
> snd_soc_dai_link_component is (const struct device_node *).

> Do you better like this last cast?

No.  As Lars says having a cast in the first place is almost always a
sign that you're doing something wrong outside of passing things through
some of the generic callback interfaces.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [alsa-devel] [PATCH v3 2/2] ASoC: simple-card: add multi-CODECs in DT
       [not found]               ` <545D2667.4070707-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
@ 2014-11-09 18:42                 ` Jean-Francois Moine
  0 siblings, 0 replies; 9+ messages in thread
From: Jean-Francois Moine @ 2014-11-09 18:42 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Xiubo Li, Jyri Sarha,
	Mark Brown, Benoit Cousson, Kuninori Morimoto

On Fri, 07 Nov 2014 21:07:03 +0100
Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> wrote:

> Casting const away is usually a sign that something is wrong and has the 
> potential to result in undefined behavior. The correct fix in this case is 
> probably to make the np argument for of_clk_get() const.

Thanks. I am sending a patch for that.

-- 
Ken ar c'hentañ	|	      ** Breizh ha Linux atav! **
Jef		|		http://moinejf.free.fr/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-11-09 18:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-04 10:25 [PATCH v3 0/2] ASoC: simple-card: Add multi-CODEC support Jean-Francois Moine
     [not found] ` <cover.1415096705.git.moinejf-GANU6spQydw@public.gmane.org>
2014-11-03 17:42   ` [PATCH v3 1/2] ASoC: core: add multi-codec support in DT Jean-Francois Moine
     [not found]     ` <6b04d690576a0036b401d00784299ee576051c0d.1415096705.git.moinejf-GANU6spQydw@public.gmane.org>
2014-11-07 14:01       ` Mark Brown
2014-11-04 10:21   ` [PATCH v3 2/2] ASoC: simple-card: add multi-CODECs " Jean-Francois Moine
     [not found]     ` <53ef077d2174f8eb5b861d6cebe7c9e732f06d52.1415096705.git.moinejf-GANU6spQydw@public.gmane.org>
2014-11-07 12:30       ` Mark Brown
     [not found]         ` <20141107123035.GO8509-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2014-11-07 19:05           ` Jean-Francois Moine
2014-11-07 20:07             ` [alsa-devel] " Lars-Peter Clausen
     [not found]               ` <545D2667.4070707-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
2014-11-09 18:42                 ` Jean-Francois Moine
2014-11-08  9:48             ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).