[PATCH 0/3] ASoC: generic simple sound card DT bindings

[PATCH 0/3] ASoC: generic simple sound card DT bindings

[Markus Pargmann]

Hi,

The patches were already in the fsl-ssi ac97 series. I moved the seperated
simple sound card DT driver into the existing simple card driver.

Regards,

Markus


Markus Pargmann (3):
      ASoC: generic: simple card, use private data
      ASoC: generic simple sound card DT bindings
      ASoC: fsl: Kconfig, visible config items for simple card

 .../bindings/sound/generic,simple-dt-card.txt      |  28 +++
 include/sound/simple_card.h                        |   4 -
 sound/soc/fsl/Kconfig                              |  23 ++-
 sound/soc/generic/Kconfig                          |   1 +
 sound/soc/generic/simple-card.c                    | 225 ++++++++++++++++++---
 5 files changed, 246 insertions(+), 35 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/generic,simple-dt-card.txt

[PATCH 1/3] ASoC: generic: simple card, use private data

[Markus Pargmann]

Move snd_link and snd_card from platform data to private data.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 include/sound/simple_card.h     |  4 ----
 sound/soc/generic/simple-card.c | 44 ++++++++++++++++++++++++++---------------
 2 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/include/sound/simple_card.h b/include/sound/simple_card.h
index 6c74527..e1ac996 100644
--- a/include/sound/simple_card.h
+++ b/include/sound/simple_card.h
@@ -29,10 +29,6 @@ struct asoc_simple_card_info {
 	unsigned int daifmt;
 	struct asoc_simple_dai cpu_dai;
 	struct asoc_simple_dai codec_dai;
-
-	/* used in simple-card.c */
-	struct snd_soc_dai_link snd_link;
-	struct snd_soc_card snd_card;
 };
 
 #endif /* __SIMPLE_CARD_H */
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 6cf8355..63cbf3e 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -15,6 +15,10 @@
 
 #define asoc_simple_get_card_info(p) \
 	container_of(p->dai_link, struct asoc_simple_card_info, snd_link)
+struct asoc_simple_card_data {
+	struct snd_soc_dai_link snd_link;
+	struct snd_soc_card snd_card;
+};
 
 static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
 				       struct asoc_simple_dai *set,
@@ -35,7 +39,8 @@ static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
 
 static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd)
 {
-	struct asoc_simple_card_info *info = asoc_simple_get_card_info(rtd);
+	struct device *dev = rtd->card->dev;
+	struct asoc_simple_card_info *info = dev->platform_data;
 	struct snd_soc_dai *codec = rtd->codec_dai;
 	struct snd_soc_dai *cpu = rtd->cpu_dai;
 	unsigned int daifmt = info->daifmt;
@@ -55,6 +60,7 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd)
 static int asoc_simple_card_probe(struct platform_device *pdev)
 {
 	struct asoc_simple_card_info *cinfo = pdev->dev.platform_data;
+	struct asoc_simple_card_data *priv;
 	struct device *dev = &pdev->dev;
 
 	if (!cinfo) {
@@ -62,6 +68,10 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
 	if (!cinfo->name	||
 	    !cinfo->card	||
 	    !cinfo->codec	||
@@ -75,31 +85,33 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
 	/*
 	 * init snd_soc_dai_link
 	 */
-	cinfo->snd_link.name		= cinfo->name;
-	cinfo->snd_link.stream_name	= cinfo->name;
-	cinfo->snd_link.cpu_dai_name	= cinfo->cpu_dai.name;
-	cinfo->snd_link.platform_name	= cinfo->platform;
-	cinfo->snd_link.codec_name	= cinfo->codec;
-	cinfo->snd_link.codec_dai_name	= cinfo->codec_dai.name;
-	cinfo->snd_link.init		= asoc_simple_card_dai_init;
+	priv->snd_link.name		= cinfo->name;
+	priv->snd_link.stream_name	= cinfo->name;
+	priv->snd_link.cpu_dai_name	= cinfo->cpu_dai.name;
+	priv->snd_link.platform_name	= cinfo->platform;
+	priv->snd_link.codec_name	= cinfo->codec;
+	priv->snd_link.codec_dai_name	= cinfo->codec_dai.name;
+	priv->snd_link.init		= asoc_simple_card_dai_init;
 
 	/*
 	 * init snd_soc_card
 	 */
-	cinfo->snd_card.name		= cinfo->card;
-	cinfo->snd_card.owner		= THIS_MODULE;
-	cinfo->snd_card.dai_link	= &cinfo->snd_link;
-	cinfo->snd_card.num_links	= 1;
-	cinfo->snd_card.dev		= &pdev->dev;
+	priv->snd_card.name		= cinfo->card;
+	priv->snd_card.owner		= THIS_MODULE;
+	priv->snd_card.dai_link		= &priv->snd_link;
+	priv->snd_card.num_links	= 1;
+	priv->snd_card.dev		= &pdev->dev;
 
-	return snd_soc_register_card(&cinfo->snd_card);
+	dev_set_drvdata(&pdev->dev, priv);
+
+	return snd_soc_register_card(&priv->snd_card);
 }
 
 static int asoc_simple_card_remove(struct platform_device *pdev)
 {
-	struct asoc_simple_card_info *cinfo = pdev->dev.platform_data;
+	struct asoc_simple_card_data *priv = dev_get_drvdata(&pdev->dev);
 
-	return snd_soc_unregister_card(&cinfo->snd_card);
+	return snd_soc_unregister_card(&priv->snd_card);
 }
 
 static struct platform_driver asoc_simple_card = {
-- 
1.8.4.rc3

[PATCH 2/3] ASoC: generic simple sound card DT bindings

[Markus Pargmann]

Simple sound card initialized using DT. When used with AC97, ac97-codec
is used to automatically discover the used codec.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 .../bindings/sound/generic,simple-dt-card.txt      |  28 +++
 sound/soc/generic/Kconfig                          |   1 +
 sound/soc/generic/simple-card.c                    | 195 +++++++++++++++++++--
 3 files changed, 206 insertions(+), 18 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/generic,simple-dt-card.txt

diff --git a/Documentation/devicetree/bindings/sound/generic,simple-dt-card.txt b/Documentation/devicetree/bindings/sound/generic,simple-dt-card.txt
new file mode 100644
index 0000000..6b41dbe
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/generic,simple-dt-card.txt
@@ -0,0 +1,28 @@
+ASoC Simple Sound Card
+
+A simple generic driver that connects a CPU DAI with a CODEC.
+
+Required properties:
+ - compatible: "asoc-simple-card" used for standard ssi, codec
+   combinations, or "asoc-simple-ac97-card" to use ac97 to discover the
+   codec.
+ - cpu-dai: CPU DAI connected to the codec.
+
+Required properties for "asoc-simple-card":
+ - audio-codec: Codec phandle.
+ - codec-dai-name: DAI name within the codec.
+
+Examples:
+
+sound {
+	compatible = "asoc-simple-card";
+	cpu-dai = <&ssi1>;
+	audio-codec = <&wm9712>;
+};
+
+For AC97:
+
+sound {
+	compatible = "asoc-simple-ac97-card";
+	cpu-dai = <&ssi1>;
+};
diff --git a/sound/soc/generic/Kconfig b/sound/soc/generic/Kconfig
index 610f612..8718ac6 100644
--- a/sound/soc/generic/Kconfig
+++ b/sound/soc/generic/Kconfig
@@ -1,4 +1,5 @@
 config SND_SIMPLE_CARD
 	tristate "ASoC Simple sound card support"
+	select SND_SOC_AC97_BUS
 	help
 	  This option enables generic simple sound card support
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 63cbf3e..9c9c080 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -11,15 +11,141 @@
 
 #include <linux/platform_device.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
 #include <sound/simple_card.h>
 
-#define asoc_simple_get_card_info(p) \
-	container_of(p->dai_link, struct asoc_simple_card_info, snd_link)
 struct asoc_simple_card_data {
 	struct snd_soc_dai_link snd_link;
 	struct snd_soc_card snd_card;
+
+	struct device_node *cpu_np;
+	struct device_node *codec_np;
+	struct platform_device *ac97_codec;
+};
+
+static struct snd_soc_dai_link simple_ac97_dt_card_dai = {
+	.name		= "HiFi",
+	.stream_name	= "HiFi",
+	.codec_dai_name	= "ac97-hifi",
+	.codec_name	= "ac97-codec",
+};
+
+static const struct snd_soc_card simple_ac97_dt_card = {
+	.name		= "ASoC simple AC97 sound card",
+	.owner		= THIS_MODULE,
+};
+
+static struct snd_soc_dai_link simple_dt_card_dai = {
+	.name		= "simple-link",
+	.stream_name	= "simple-link",
+};
+
+static const struct snd_soc_card simple_dt_card = {
+	.name		= "ASoC simple sound card",
+	.owner		= THIS_MODULE,
 };
 
+enum asoc_simple_card_type {
+	SIMPLE_CARD,
+	SIMPLE_CARD_AC97,
+};
+
+static const struct of_device_id asoc_simple_card_of_dev_ids[] = {
+	{
+		.compatible = "asoc-simple-card",
+		.data = (void *)SIMPLE_CARD,
+	}, {
+		.compatible = "asoc-simple-ac97-card",
+		.data = (void *)SIMPLE_CARD_AC97,
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(of, asoc_simple_card_of_dev_ids);
+
+static int asoc_simple_card_dt_probe(struct platform_device *pdev,
+		struct asoc_simple_card_data *priv)
+{
+	const struct of_device_id *of_id = of_match_device(
+			asoc_simple_card_of_dev_ids, &pdev->dev);
+	enum asoc_simple_card_type type =
+			(enum asoc_simple_card_type)of_id->data;
+	int ret;
+
+	priv->cpu_np = of_parse_phandle(pdev->dev.of_node, "cpu-dai", 0);
+	if (!priv->cpu_np) {
+		dev_err(&pdev->dev, "No valid CPU DAI phandle found\n");
+		return -EINVAL;
+	}
+
+	if (type == SIMPLE_CARD_AC97) {
+		struct platform_device *ac97_codec;
+		memcpy(&priv->snd_card, &simple_ac97_dt_card,
+				sizeof(simple_ac97_dt_card));
+		memcpy(&priv->snd_link, &simple_ac97_dt_card_dai,
+				sizeof(simple_ac97_dt_card_dai));
+
+		ac97_codec = platform_device_register_simple("ac97-codec", -1,
+				NULL, 0);
+		if (IS_ERR(ac97_codec)) {
+			dev_err(&pdev->dev, "Failed to register ac97-codec device\n");
+			ret = PTR_ERR(priv->ac97_codec);
+			goto err;
+		}
+		priv->ac97_codec = ac97_codec;
+	} else {
+		priv->codec_np = of_parse_phandle(pdev->dev.of_node,
+				"audio-codec", 0);
+		if (!priv->codec_np) {
+			dev_err(&pdev->dev, "No valid codec phandle found\n");
+			ret = -EINVAL;
+			goto err;
+		}
+
+		memcpy(&priv->snd_link, &simple_dt_card_dai,
+				sizeof(simple_dt_card_dai));
+		memcpy(&priv->snd_card, &simple_dt_card,
+				sizeof(simple_dt_card));
+		priv->snd_link.codec_of_node = priv->codec_np;
+
+		ret = of_property_read_string(pdev->dev.of_node,
+				"codec-dai-name", &priv->snd_link.stream_name);
+		if (ret) {
+			dev_err(&pdev->dev, "Failed to parse codec-dai-name string\n");
+			ret = -EINVAL;
+			goto err;
+		}
+	}
+
+	priv->snd_link.cpu_of_node = priv->cpu_np;
+	priv->snd_link.platform_of_node = priv->cpu_np;
+
+	return 0;
+
+err:
+	if (priv->cpu_np)
+		of_node_put(priv->cpu_np);
+	if (priv->codec_np)
+		of_node_put(priv->codec_np);
+	if (priv->ac97_codec)
+		platform_device_unregister(priv->ac97_codec);
+	return ret;
+}
+
+static int asoc_simple_card_dt_remove(struct platform_device *pdev,
+		struct asoc_simple_card_data *priv)
+{
+	if (priv->cpu_np)
+		of_node_put(priv->cpu_np);
+	if (priv->codec_np)
+		of_node_put(priv->codec_np);
+	if (priv->ac97_codec)
+		platform_device_unregister(priv->ac97_codec);
+
+	return 0;
+}
+
 static int __asoc_simple_card_dai_init(struct snd_soc_dai *dai,
 				       struct asoc_simple_dai *set,
 				       unsigned int daifmt)
@@ -57,21 +183,10 @@ static int asoc_simple_card_dai_init(struct snd_soc_pcm_runtime *rtd)
 	return 0;
 }
 
-static int asoc_simple_card_probe(struct platform_device *pdev)
+static int asoc_simple_card_pdata_probe(struct device *dev,
+		struct asoc_simple_card_info *cinfo,
+		struct asoc_simple_card_data *priv)
 {
-	struct asoc_simple_card_info *cinfo = pdev->dev.platform_data;
-	struct asoc_simple_card_data *priv;
-	struct device *dev = &pdev->dev;
-
-	if (!cinfo) {
-		dev_err(dev, "no info for asoc-simple-card\n");
-		return -EINVAL;
-	}
-
-	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
-		return -ENOMEM;
-
 	if (!cinfo->name	||
 	    !cinfo->card	||
 	    !cinfo->codec	||
@@ -97,6 +212,28 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
 	 * init snd_soc_card
 	 */
 	priv->snd_card.name		= cinfo->card;
+
+	return 0;
+}
+
+static int asoc_simple_card_probe(struct platform_device *pdev)
+{
+	struct asoc_simple_card_info *cinfo = pdev->dev.platform_data;
+	struct asoc_simple_card_data *priv;
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	if (cinfo)
+		ret = asoc_simple_card_pdata_probe(&pdev->dev, cinfo, priv);
+	else
+		ret = asoc_simple_card_dt_probe(pdev, priv);
+
+	if (ret)
+		return ret;
+
 	priv->snd_card.owner		= THIS_MODULE;
 	priv->snd_card.dai_link		= &priv->snd_link;
 	priv->snd_card.num_links	= 1;
@@ -104,19 +241,40 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
 
 	dev_set_drvdata(&pdev->dev, priv);
 
-	return snd_soc_register_card(&priv->snd_card);
+	ret = snd_soc_register_card(&priv->snd_card);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to register soc card\n");
+		goto err;
+	}
+
+	return 0;
+err:
+	if (priv->cpu_np)
+		of_node_put(priv->cpu_np);
+	if (priv->codec_np)
+		of_node_put(priv->codec_np);
+	if (priv->ac97_codec)
+		platform_device_unregister(priv->ac97_codec);
+	return ret;
 }
 
 static int asoc_simple_card_remove(struct platform_device *pdev)
 {
 	struct asoc_simple_card_data *priv = dev_get_drvdata(&pdev->dev);
 
-	return snd_soc_unregister_card(&priv->snd_card);
+	snd_soc_unregister_card(&priv->snd_card);
+
+	if (!pdev->dev.platform_data)
+		asoc_simple_card_dt_remove(pdev, priv);
+
+	return 0;
 }
 
 static struct platform_driver asoc_simple_card = {
 	.driver = {
 		.name	= "asoc-simple-card",
+		.owner	= THIS_MODULE,
+		.of_match_table = asoc_simple_card_of_dev_ids,
 	},
 	.probe		= asoc_simple_card_probe,
 	.remove		= asoc_simple_card_remove,
@@ -127,3 +285,4 @@ module_platform_driver(asoc_simple_card);
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("ASoC Simple Sound Card");
 MODULE_AUTHOR("Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>");
+MODULE_AUTHOR("Markus Pargmann <mpa@pengutronix.de>");
-- 
1.8.4.rc3

Re: [PATCH 2/3] ASoC: generic simple sound card DT bindings

[Lars-Peter Clausen]

On 08/31/2013 12:44 PM, Markus Pargmann wrote:
> Simple sound card initialized using DT. When used with AC97, ac97-codec
> is used to automatically discover the used codec.
> 
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> ---
>  .../bindings/sound/generic,simple-dt-card.txt      |  28 +++
>  sound/soc/generic/Kconfig                          |   1 +
>  sound/soc/generic/simple-card.c                    | 195 +++++++++++++++++++--
>  3 files changed, 206 insertions(+), 18 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/sound/generic,simple-dt-card.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/generic,simple-dt-card.txt b/Documentation/devicetree/bindings/sound/generic,simple-dt-card.txt
> new file mode 100644
> index 0000000..6b41dbe
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/generic,simple-dt-card.txt
> @@ -0,0 +1,28 @@
> +ASoC Simple Sound Card
> +
> +A simple generic driver that connects a CPU DAI with a CODEC.
> +
> +Required properties:
> + - compatible: "asoc-simple-card" used for standard ssi, codec
> +   combinations, or "asoc-simple-ac97-card" to use ac97 to discover the
> +   codec.
> + - cpu-dai: CPU DAI connected to the codec.
> +
> +Required properties for "asoc-simple-card":
> + - audio-codec: Codec phandle.
> + - codec-dai-name: DAI name within the codec.

In my opinion this binding exposes way to much of the ASoC internal data
structures. E.g. CODECs are referenced by phandle, but the DAI of the CODEC
is reference by a string. This string is completely ASoC internal. The
binding also assumes that a CPU controller may have one DAI at most. In my
opinion this binding ought to use the upcoming of_xlate stuff for ASoC
components.

- Lars

Re: [PATCH 2/3] ASoC: generic simple sound card DT bindings

[Markus Pargmann]

On Sat, Aug 31, 2013 at 05:10:30PM +0200, Lars-Peter Clausen wrote:
> On 08/31/2013 12:44 PM, Markus Pargmann wrote:
> > Simple sound card initialized using DT. When used with AC97, ac97-codec
> > is used to automatically discover the used codec.
> > 
> > Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
> > ---
> >  .../bindings/sound/generic,simple-dt-card.txt      |  28 +++
> >  sound/soc/generic/Kconfig                          |   1 +
> >  sound/soc/generic/simple-card.c                    | 195 +++++++++++++++++++--
> >  3 files changed, 206 insertions(+), 18 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/sound/generic,simple-dt-card.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/generic,simple-dt-card.txt b/Documentation/devicetree/bindings/sound/generic,simple-dt-card.txt
> > new file mode 100644
> > index 0000000..6b41dbe
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/generic,simple-dt-card.txt
> > @@ -0,0 +1,28 @@
> > +ASoC Simple Sound Card
> > +
> > +A simple generic driver that connects a CPU DAI with a CODEC.
> > +
> > +Required properties:
> > + - compatible: "asoc-simple-card" used for standard ssi, codec
> > +   combinations, or "asoc-simple-ac97-card" to use ac97 to discover the
> > +   codec.
> > + - cpu-dai: CPU DAI connected to the codec.
> > +
> > +Required properties for "asoc-simple-card":
> > + - audio-codec: Codec phandle.
> > + - codec-dai-name: DAI name within the codec.
> 
> In my opinion this binding exposes way to much of the ASoC internal data
> structures. E.g. CODECs are referenced by phandle, but the DAI of the CODEC
> is reference by a string. This string is completely ASoC internal. The
> binding also assumes that a CPU controller may have one DAI at most. In my
> opinion this binding ought to use the upcoming of_xlate stuff for ASoC
> components.

Okay I will wait for the of_xlate stuff.

Regards,

Markus

> 
> - Lars
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 2/3] ASoC: generic simple sound card DT bindings

[Stephen Warren]

On 08/31/2013 09:10 AM, Lars-Peter Clausen wrote:
> On 08/31/2013 12:44 PM, Markus Pargmann wrote:
>> Simple sound card initialized using DT. When used with AC97, ac97-codec
>> is used to automatically discover the used codec.

>> diff --git a/Documentation/devicetree/bindings/sound/generic,simple-dt-card.txt b/Documentation/devicetree/bindings/sound/generic,simple-dt-card.txt

>> +ASoC Simple Sound Card

As Lars mentions below, we shouldn't have bindings for ASoC; we should
have bindings that describe the HW. Bindings are supposed to be
OS-agnostic, whereas "ASoC" is Linux-specific.

>> +
>> +A simple generic driver that connects a CPU DAI with a CODEC.
>> +
>> +Required properties:
>> + - compatible: "asoc-simple-card" used for standard ssi, codec
>> +   combinations, or "asoc-simple-ac97-card" to use ac97 to discover the
>> +   codec.
>> + - cpu-dai: CPU DAI connected to the codec.
>> +
>> +Required properties for "asoc-simple-card":
>> + - audio-codec: Codec phandle.
>> + - codec-dai-name: DAI name within the codec.
> 
> In my opinion this binding exposes way to much of the ASoC internal data
> structures. E.g. CODECs are referenced by phandle, but the DAI of the CODEC
> is reference by a string. This string is completely ASoC internal.

Mark has made the argument that (at least for CODEC analog pins) we can
simply put those strings into the binding document, and make them as
much a part of the binding as anything else. After all, (at least for
CODEC analog pins) the values are simply the names of the pins on the
package, as listed by the HW documentation itself.

We could presumably do the same thing for DAIs; in DT, use a
string-based DAI name derived directly from the HW documentation, rather
than the current intra-ASoC DAI name strings.

That said, I will admit that I personally don't really like the idea of
using strings in bindings. That opinion certainly isn't universal though.

> The
> binding also assumes that a CPU controller may have one DAI at most. In my
> opinion this binding ought to use the upcoming of_xlate stuff for ASoC
> components.

That restriction seems reasonable for a *simple* DT sound binding. For
more complex cards, one could presumably create more complex bindings,
be they generic bindings that cover arbitrary more complex cases, or
bindings for specific configurations that happen to include multiple DAIs.

Re: [PATCH 2/3] ASoC: generic simple sound card DT bindings

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1988 bytes --]

On Tue, Sep 03, 2013 at 01:25:09PM -0600, Stephen Warren wrote:

> Mark has made the argument that (at least for CODEC analog pins) we can
> simply put those strings into the binding document, and make them as
> much a part of the binding as anything else. After all, (at least for
> CODEC analog pins) the values are simply the names of the pins on the
> package, as listed by the HW documentation itself.

> We could presumably do the same thing for DAIs; in DT, use a
> string-based DAI name derived directly from the HW documentation, rather
> than the current intra-ASoC DAI name strings.

> That said, I will admit that I personally don't really like the idea of
> using strings in bindings. That opinion certainly isn't universal though.

I think either works - with DAIs there is a tendency (though not
universal) for devices to just have numbered interfaces which makes the
numbers more natural.  I'm more concerned with the binding being legible
than with what ends up physically written in there, the original reason
for strings (apart from the fact that they're in the drivers already)
was that there was a lot of resistance in the DT community to symbolic
constants.  That would have lead to bindings which looked like line
noise.

> > The
> > binding also assumes that a CPU controller may have one DAI at most. In my
> > opinion this binding ought to use the upcoming of_xlate stuff for ASoC
> > components.

> That restriction seems reasonable for a *simple* DT sound binding. For
> more complex cards, one could presumably create more complex bindings,
> be they generic bindings that cover arbitrary more complex cases, or
> bindings for specific configurations that happen to include multiple DAIs.

The complexity there comes from the device that's being used rather than
the design of the sound card though - the fact that a SoC has an audio
block with many DAIs shouldn't prevent it using the simple bindings if
someone just hung a simple CODEC off one of the DAIs.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/3] ASoC: generic simple sound card DT bindings

[Stephen Warren]

On 09/03/2013 05:19 PM, Mark Brown wrote:
> On Tue, Sep 03, 2013 at 01:25:09PM -0600, Stephen Warren wrote:
> 
>> Mark has made the argument that (at least for CODEC analog pins)
>> we can simply put those strings into the binding document, and
>> make them as much a part of the binding as anything else. After
>> all, (at least for CODEC analog pins) the values are simply the
>> names of the pins on the package, as listed by the HW
>> documentation itself.
> 
>> We could presumably do the same thing for DAIs; in DT, use a 
>> string-based DAI name derived directly from the HW documentation,
>> rather than the current intra-ASoC DAI name strings.
> 
>> That said, I will admit that I personally don't really like the
>> idea of using strings in bindings. That opinion certainly isn't
>> universal though.
> 
> I think either works - with DAIs there is a tendency (though not 
> universal) for devices to just have numbered interfaces which makes
> the numbers more natural.  I'm more concerned with the binding
> being legible than with what ends up physically written in there,
> the original reason for strings (apart from the fact that they're
> in the drivers already) was that there was a lot of resistance in
> the DT community to symbolic constants.  That would have lead to
> bindings which looked like line noise.

True.

>>> The binding also assumes that a CPU controller may have one DAI
>>> at most. In my opinion this binding ought to use the upcoming
>>> of_xlate stuff for ASoC components.
> 
>> That restriction seems reasonable for a *simple* DT sound
>> binding. For more complex cards, one could presumably create more
>> complex bindings, be they generic bindings that cover arbitrary
>> more complex cases, or bindings for specific configurations that
>> happen to include multiple DAIs.
> 
> The complexity there comes from the device that's being used rather
> than the design of the sound card though - the fact that a SoC has
> an audio block with many DAIs shouldn't prevent it using the simple
> bindings if someone just hung a simple CODEC off one of the DAIs.

Yes, exactly.

[PATCH 3/3] ASoC: fsl: Kconfig, visible config items for simple card

[Markus Pargmann]

The simple sound card does not automatically select the used
chipdrivers. This patch makes them user-selectable so simple sound cards
can be used without a problem.

Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
 sound/soc/fsl/Kconfig | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index 704e246..3205230 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -1,5 +1,11 @@
 config SND_SOC_FSL_SSI
-	tristate
+	tristate "Freescale SSI unit"
+	help
+	  Say Y here if you want tou use a Freescale SSI unit. This option is
+	  automatically selected if your board has a seperated sound card
+	  driver. If you are using one of the generic drivers, please select
+	  this manually.
+
 
 config SND_SOC_FSL_SPDIF
 	tristate
@@ -112,15 +118,24 @@ config SND_SOC_IMX_SSI
 	tristate
 
 config SND_SOC_IMX_PCM_FIQ
-	tristate
+	tristate "i.MX PCM FIQ"
 	select FIQ
+	help
+	  Say Y here if you are using a generic soundcard and you have a chip
+	  that with this audio muxer. Otherwise select N.
 
 config SND_SOC_IMX_PCM_DMA
-	tristate
+	tristate "i.MX PCM DMA"
 	select SND_SOC_GENERIC_DMAENGINE_PCM
+	help
+	  Say Y here if you are using a generic soundcard and you have a DMA
+	  capable SSI unit. Otherwise select N.
 
 config SND_SOC_IMX_AUDMUX
-	tristate
+	tristate "i.MX Audio muxer"
+	help
+	  Say Y here if you are using a generic soundcard and you have chip
+	  with this audio muxer. Otherwise select N.
 
 config SND_MXC_SOC_WM1133_EV1
 	tristate "Audio on the i.MX31ADS with WM1133-EV1 fitted"
-- 
1.8.4.rc3