diff mbox

[v5,02/12] ASoC: davinci-evm: Add device tree binding

Message ID 66c21e66320e30453e6deded0f37ef582b4fba7c.1382110089.git.jsarha@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jyri Sarha Oct. 18, 2013, 3:37 p.m. UTC
From: "Hebbar, Gururaja" <gururaja.hebbar@ti.com>

Device tree support for Davinci Machine driver

When the board boots with device tree, the driver will receive card,
codec, dai interface details (like the card name, DAPM routing map,
phandle for the audio components described in the dts file, codec mclk
speed). The card will be set up based on this information. Since the
routing is provided via DT we can mark the card fully routed so core
can take care of disconnecting the unused pins.

Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>
Signed-off-by: Darren Etheridge <detheridge@ti.com>
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 .../bindings/sound/davinci-evm-audio.txt           |   58 ++++++++++
 sound/soc/davinci/davinci-evm.c                    |  120 +++++++++++++++++++-
 2 files changed, 176 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/davinci-evm-audio.txt

Comments

Mark Brown Oct. 22, 2013, 11:01 a.m. UTC | #1
On Fri, Oct 18, 2013 at 06:37:41PM +0300, Jyri Sarha wrote:

> +  TLV320AIC3X pins:
> +
> +  * LLOUT
> +  * RLOUT
> +  * MONO_LOUT
> +  * HPLOUT
> +  * HPROUT
> +  * HPLCOM
> +  * HPRCOM
> +  * MIC3L
> +  * MIC3R
> +  * LINE1L
> +  * LINE2L
> +  * LINE1R
> +  * LINE2R

These should be documented in the CODEC binding and not in the card
driver, that way this doesn't need to be replicated in every machine and
any future software compatible devices can just work with the existing
binding.

> +	ret = snd_soc_register_card(&evm_soc_card);

This should be using devm_snd_soc_register_card().

> +	/*
> +	 * If dtb is there, the devices will be created dynamically.
> +	 * Only register platfrom driver structure.
> +	 */
> +	if (of_have_populated_dt())
> +		return platform_driver_register(&davinci_evm_driver);
> +

Why not register the driver unconditionally?
Jyri Sarha Oct. 22, 2013, 12:26 p.m. UTC | #2
On 10/22/2013 02:01 PM, Mark Brown wrote:
> On Fri, Oct 18, 2013 at 06:37:41PM +0300, Jyri Sarha wrote:
>
>> +  TLV320AIC3X pins:
...
>
> These should be documented in the CODEC binding and not in the card
> driver, that way this doesn't need to be replicated in every machine and
> any future software compatible devices can just work with the existing
> binding.
>

Ok, I'll remove the codec pins.

>> +	ret = snd_soc_register_card(&evm_soc_card);
>
> This should be using devm_snd_soc_register_card().
>

Ok, I'll move on top of your for-next branch and change it.

>> +	/*
>> +	 * If dtb is there, the devices will be created dynamically.
>> +	 * Only register platfrom driver structure.
>> +	 */
>> +	if (of_have_populated_dt())
>> +		return platform_driver_register(&davinci_evm_driver);
>> +
>
> Why not register the driver unconditionally?
>

I am just trying touch the legacy davinci audio support as little as 
possible. Would registering the driver in non DT mode do some good?

Best regards,
Jyri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown Oct. 22, 2013, 12:39 p.m. UTC | #3
On Tue, Oct 22, 2013 at 03:26:11PM +0300, Jyri Sarha wrote:
> On 10/22/2013 02:01 PM, Mark Brown wrote:

> >>+	/*
> >>+	 * If dtb is there, the devices will be created dynamically.
> >>+	 * Only register platfrom driver structure.
> >>+	 */
> >>+	if (of_have_populated_dt())
> >>+		return platform_driver_register(&davinci_evm_driver);

> >Why not register the driver unconditionally?

> I am just trying touch the legacy davinci audio support as little as
> possible. Would registering the driver in non DT mode do some good?

It would look a good deal less odd which is useful in itself; the best
thing would be if they could both use the same driver which uses both
platform and DT data.
Jyri Sarha Oct. 22, 2013, 4:41 p.m. UTC | #4
On 10/22/2013 03:39 PM, Mark Brown wrote:
> It would look a good deal less odd which is useful in itself; the best
> thing would be if they could both use the same driver which uses both
> platform and DT data.

I see. To be honest the whole non DT part looks pretty hairy to me too, 
but I can not clean it up because I do not have access to the relevant HW.

Would it look better to you if I would use module_platform_driver() 
macro inside #ifdef CONFIG_OF and put the module_init() and 
module_exit() into #else branch?

Best regards,
Jyri
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt b/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt
new file mode 100644
index 0000000..e6b61ff
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/davinci-evm-audio.txt
@@ -0,0 +1,58 @@ 
+* Texas Instruments SoC audio setups with TLV320AIC3X Codec
+
+Required properties:
+- compatible : "ti,da830-evm-audio" : forDM365/DA8xx/OMAPL1x/AM33xx
+- ti,model : The user-visible name of this sound complex.
+- ti,audio-codec : The phandle of the TLV320AIC3x audio codec
+- ti,mcasp-controller : The phandle of the McASP controller
+- ti,codec-clock-rate : The Codec Clock rate (in Hz) applied to the Codec
+- ti,audio-routing : A list of the connections between audio components.
+  Each entry is a pair of strings, the first being the connection's sink,
+  the second being the connection's source. Valid names for sources and
+  sinks are the codec's pins, and the jacks on the board:
+
+  TLV320AIC3X pins:
+
+  * LLOUT
+  * RLOUT
+  * MONO_LOUT
+  * HPLOUT
+  * HPROUT
+  * HPLCOM
+  * HPRCOM
+  * MIC3L
+  * MIC3R
+  * LINE1L
+  * LINE2L
+  * LINE1R
+  * LINE2R
+
+  Board connectors:
+
+  * Headphone Jack
+  * Line Out
+  * Mic Jack
+  * Line In
+
+
+Example:
+
+sound {
+	compatible = "ti,da830-evm-audio";
+	ti,model = "DA830 EVM";
+	ti,audio-codec = <&tlv320aic3x>;
+	ti,mcasp-controller = <&mcasp1>;
+	ti,codec-clock-rate = <12000000>;
+	ti,audio-routing =
+		"Headphone Jack",       "HPLOUT",
+		"Headphone Jack",       "HPROUT",
+		"Line Out",             "LLOUT",
+		"Line Out",             "RLOUT",
+		"MIC3L",                "Mic Bias 2V",
+		"MIC3R",                "Mic Bias 2V",
+		"Mic Bias 2V",          "Mic Jack",
+		"LINE1L",               "Line In",
+		"LINE2L",               "Line In",
+		"LINE1R",               "Line In",
+		"LINE2R",               "Line In";
+};
diff --git a/sound/soc/davinci/davinci-evm.c b/sound/soc/davinci/davinci-evm.c
index 2f8161c..340a68d 100644
--- a/sound/soc/davinci/davinci-evm.c
+++ b/sound/soc/davinci/davinci-evm.c
@@ -16,6 +16,7 @@ 
 #include <linux/platform_device.h>
 #include <linux/platform_data/edma.h>
 #include <linux/i2c.h>
+#include <linux/of_platform.h>
 #include <sound/core.h>
 #include <sound/pcm.h>
 #include <sound/soc.h>
@@ -23,6 +24,8 @@ 
 #include <asm/dma.h>
 #include <asm/mach-types.h>
 
+#include <linux/edma.h>
+
 #include "davinci-pcm.h"
 #include "davinci-i2s.h"
 #include "davinci-mcasp.h"
@@ -121,13 +124,22 @@  static int evm_aic3x_init(struct snd_soc_pcm_runtime *rtd)
 {
 	struct snd_soc_codec *codec = rtd->codec;
 	struct snd_soc_dapm_context *dapm = &codec->dapm;
+	struct device_node *np = codec->card->dev->of_node;
+	int ret;
 
 	/* Add davinci-evm specific widgets */
 	snd_soc_dapm_new_controls(dapm, aic3x_dapm_widgets,
 				  ARRAY_SIZE(aic3x_dapm_widgets));
 
-	/* Set up davinci-evm specific audio path audio_map */
-	snd_soc_dapm_add_routes(dapm, audio_map, ARRAY_SIZE(audio_map));
+	if (np) {
+		ret = snd_soc_of_parse_audio_routing(codec->card,
+							"ti,audio-routing");
+		if (ret)
+			return ret;
+	} else {
+		/* Set up davinci-evm specific audio path audio_map */
+		snd_soc_dapm_add_routes(dapm, audio_map, ARRAY_SIZE(audio_map));
+	}
 
 	/* not connected */
 	snd_soc_dapm_disable_pin(dapm, "MONO_LOUT");
@@ -312,6 +324,98 @@  static struct snd_soc_card da850_snd_soc_card = {
 	.drvdata = &da850_snd_soc_card_drvdata,
 };
 
+#if defined(CONFIG_OF)
+
+/*
+ * The struct is used as place holder. It will be completely
+ * filled with data from dt node.
+ */
+static struct snd_soc_dai_link evm_dai_tlv320aic3x = {
+	.name		= "TLV320AIC3X",
+	.stream_name	= "AIC3X",
+	.codec_dai_name	= "tlv320aic3x-hifi",
+	.ops            = &evm_ops,
+	.init           = evm_aic3x_init,
+};
+
+static const struct of_device_id davinci_evm_dt_ids[] = {
+	{
+		.compatible = "ti,da830-evm-audio",
+		.data = (void *) &evm_dai_tlv320aic3x,
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, davinci_evm_dt_ids);
+
+/* davinci evm audio machine driver */
+static struct snd_soc_card evm_soc_card = {
+	.owner = THIS_MODULE,
+	.num_links = 1,
+};
+
+static int davinci_evm_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	const struct of_device_id *match =
+		of_match_device(of_match_ptr(davinci_evm_dt_ids), &pdev->dev);
+	struct snd_soc_dai_link *dai = (struct snd_soc_dai_link *) match->data;
+	struct snd_soc_card_drvdata_davinci *drvdata = NULL;
+	int ret = 0;
+
+	evm_soc_card.dai_link = dai;
+
+	dai->codec_of_node = of_parse_phandle(np, "ti,audio-codec", 0);
+	if (!dai->codec_of_node)
+		return -EINVAL;
+
+	dai->cpu_of_node = of_parse_phandle(np, "ti,mcasp-controller", 0);
+	if (!dai->cpu_of_node)
+		return -EINVAL;
+
+	dai->platform_of_node = dai->cpu_of_node;
+
+	evm_soc_card.dev = &pdev->dev;
+	ret = snd_soc_of_parse_card_name(&evm_soc_card, "ti,model");
+	if (ret)
+		return ret;
+
+	drvdata = devm_kzalloc(&pdev->dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+
+	ret = of_property_read_u32(np, "ti,codec-clock-rate", &drvdata->sysclk);
+	if (ret < 0)
+		return -EINVAL;
+
+	snd_soc_card_set_drvdata(&evm_soc_card, drvdata);
+	ret = snd_soc_register_card(&evm_soc_card);
+
+	if (ret)
+		dev_err(&pdev->dev, "snd_soc_register_card failed (%d)\n", ret);
+
+	return ret;
+}
+
+static int davinci_evm_remove(struct platform_device *pdev)
+{
+	struct snd_soc_card *card = platform_get_drvdata(pdev);
+
+	snd_soc_unregister_card(card);
+
+	return 0;
+}
+
+static struct platform_driver davinci_evm_driver = {
+	.probe		= davinci_evm_probe,
+	.remove		= davinci_evm_remove,
+	.driver		= {
+		.name	= "davinci_evm",
+		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(davinci_evm_dt_ids),
+	},
+};
+#endif
+
 static struct platform_device *evm_snd_device;
 
 static int __init evm_init(void)
@@ -320,6 +424,13 @@  static int __init evm_init(void)
 	int index;
 	int ret;
 
+	/*
+	 * If dtb is there, the devices will be created dynamically.
+	 * Only register platfrom driver structure.
+	 */
+	if (of_have_populated_dt())
+		return platform_driver_register(&davinci_evm_driver);
+
 	if (machine_is_davinci_evm()) {
 		evm_snd_dev_data = &dm6446_snd_soc_card_evm;
 		index = 0;
@@ -355,6 +466,11 @@  static int __init evm_init(void)
 
 static void __exit evm_exit(void)
 {
+	if (of_have_populated_dt()) {
+		platform_driver_unregister(&davinci_evm_driver);
+		return;
+	}
+
 	platform_device_unregister(evm_snd_device);
 }