[PATCH v2 1/6] ASoC: Allow device tree to specify a card's name

[PATCH v2 1/6] ASoC: Allow device tree to specify a card's name

[Stephen Warren]

If a card's device was instantiated from device tree, and the device tree
has a "user-visible-name" property, use that as the card's name.

Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v2: New patch implementing new functionality

Re: the binding documentation:
* "SoC" here refers to the fact this is a binding oriented at System-on-
  chip audio complexes, rather than having to do with "ASoC"; both names
  were derived from the same root.
* Do we need a compatible property for this "base class" binding at all?
  I think it's a good idea, even though the code doesn't actually rely
  on it.
* Should the vendor field in the compatible property be "generic",
  "linux", or absent? I've tried to make these bindings generic and
  applicable to other OSs, so "linux," seems wrong.
* Should the property "user-visible-name" have a "generic," prefix or
  similar?

 .../bindings/sound/soc-audio-complex.txt           |   14 ++++++++
 sound/soc/soc-core.c                               |   35 ++++++++++++++++++-
 2 files changed, 47 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/soc-audio-complex.txt

diff --git a/Documentation/devicetree/bindings/sound/soc-audio-complex.txt b/Documentation/devicetree/bindings/sound/soc-audio-complex.txt
new file mode 100644
index 0000000..1cc7059
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/soc-audio-complex.txt
@@ -0,0 +1,14 @@
+SoC Audio Complex
+
+Required properties:
+- compatible : "generic,soc-audio-complex". Other compatible values
+  indicating the specific HW model will typically be present too.
+- user-visible-name : The user-visible name of this sound complex.
+
+Example:
+
+sound {
+	compatible = "nvidia,tegra-audio-wm8903",
+		     "generic,soc-audio-complex";
+	user-visible-name = "tegra-wm8903-harmony";
+};
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 5195f06..56d1bc5 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -32,6 +32,7 @@
 #include <linux/platform_device.h>
 #include <linux/ctype.h>
 #include <linux/slab.h>
+#include <linux/of.h>
 #include <sound/ac97_codec.h>
 #include <sound/core.h>
 #include <sound/jack.h>
@@ -2809,6 +2810,25 @@ int snd_soc_dai_digital_mute(struct snd_soc_dai *dai, int mute)
 }
 EXPORT_SYMBOL_GPL(snd_soc_dai_digital_mute);
 
+/* Retrieve a card's name from device tree */
+static int snd_soc_of_parse_card_name(struct snd_soc_card *card)
+{
+	struct device_node *np = card->dev->of_node;
+	int ret;
+
+	ret = of_property_read_string_index(np, "user-visible-name", 0,
+					    &card->name);
+	/*
+	 * EINVAL means the property does not exist. This is fine providing
+	 * card->name was previously set, which is checked later in
+	 * snd_soc_register_card.
+	 */
+	if (ret < 0 && ret != -EINVAL)
+		return ret;
+
+	return 0;
+}
+
 /**
  * snd_soc_register_card - Register a card with the ASoC core
  *
@@ -2817,11 +2837,22 @@ EXPORT_SYMBOL_GPL(snd_soc_dai_digital_mute);
  */
 int snd_soc_register_card(struct snd_soc_card *card)
 {
-	int i;
+	int i, ret;
 
-	if (!card->name || !card->dev)
+	if (!card->dev)
 		return -EINVAL;
 
+	if (card->dev->of_node) {
+		ret = snd_soc_of_parse_card_name(card);
+		if (ret < 0)
+			return ret;
+	}
+
+	if (!card->name) {
+		dev_err(card->dev, "Card name is not set\n");
+		return -EINVAL;
+	}
+
 	dev_set_drvdata(card->dev, card);
 
 	snd_soc_initialize_card_lists(card);
-- 
1.7.0.4

[PATCH v2 2/6] ASoC: Allow the DAPM routes to be stored in device tree

[Stephen Warren]

If a card's device was instantiated from device tree, and card's device
did not specify a DAPM route map, and the device tree has an "audio-
routing" property, parse this to construct the DAPM route array.

Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v2: New patch; separated this code out of the Tegra+WM8903 machine driver
into a generic feature.

Note that this binding does not allow the "control" field of the routes
to be specified via device tree. Should the binding be expanded to allow
this? Representing this while still keeping the binding generic enough
to be useful outside ASoC might be difficult. I'm not sure if this
feature is useful at the card level? I quickly checked, and couldn't see
any in-tree users of this functionality.

 .../bindings/sound/soc-audio-complex.txt           |   15 ++++++
 sound/soc/soc-core.c                               |   48 ++++++++++++++++++++
 2 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/soc-audio-complex.txt b/Documentation/devicetree/bindings/sound/soc-audio-complex.txt
index 1cc7059..f8e73cc 100644
--- a/Documentation/devicetree/bindings/sound/soc-audio-complex.txt
+++ b/Documentation/devicetree/bindings/sound/soc-audio-complex.txt
@@ -4,6 +4,12 @@ Required properties:
 - compatible : "generic,soc-audio-complex". Other compatible values
   indicating the specific HW model will typically be present too.
 - user-visible-name : The user-visible name of this sound complex.
+- 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 determined by the binding for the HW that comprises the
+  audio complex, e.g. the names of an audio codec's pins, the names of
+  the user-visible jacks (plugs) on the board, etc.
 
 Example:
 
@@ -11,4 +17,13 @@ sound {
 	compatible = "nvidia,tegra-audio-wm8903",
 		     "generic,soc-audio-complex";
 	user-visible-name = "tegra-wm8903-harmony";
+	audio-routing =
+		"Headphone Jack", "HPOUTR",
+		"Headphone Jack", "HPOUTL",
+		"Int Spk", "ROP",
+		"Int Spk", "RON",
+		"Int Spk", "LOP",
+		"Int Spk", "LON",
+		"Mic Jack", "MICBIAS",
+		"IN1L", "Mic Jack";
 };
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 56d1bc5..f60e04f 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2829,6 +2829,48 @@ static int snd_soc_of_parse_card_name(struct snd_soc_card *card)
 	return 0;
 }
 
+int snd_soc_of_parse_audio_routing(struct snd_soc_card *card)
+{
+	struct device_node *np = card->dev->of_node;
+	int num_routes;
+	struct snd_soc_dapm_route *routes;
+	int i;
+
+	num_routes = of_property_count_strings(np, "audio-routing");
+	if (num_routes & 1) {
+		dev_err(card->dev,
+			"Property 'audio-routing's length is not even\n");
+		return -EINVAL;
+	}
+	num_routes /= 2;
+	if (!num_routes) {
+		dev_err(card->dev,
+			"Property 'audio-routing's length is zero\n");
+		return -EINVAL;
+	}
+
+	routes = kzalloc(
+		num_routes * sizeof(*routes),
+		GFP_KERNEL);
+	if (!routes) {
+		dev_err(card->dev,
+			"Could not allocate DAPM route table\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < num_routes; i++) {
+		of_property_read_string_index(np, "audio-routing",
+			2 * i, &routes[i].sink);
+		of_property_read_string_index(np, "audio-routing",
+			(2 * i) + 1, &routes[i].source);
+	}
+
+	card->num_dapm_routes = num_routes;
+	card->dapm_routes = routes;
+
+	return 0;
+}
+
 /**
  * snd_soc_register_card - Register a card with the ASoC core
  *
@@ -2846,6 +2888,12 @@ int snd_soc_register_card(struct snd_soc_card *card)
 		ret = snd_soc_of_parse_card_name(card);
 		if (ret < 0)
 			return ret;
+
+		if (!card->num_dapm_routes && !card->dapm_routes) {
+			ret = snd_soc_of_parse_audio_routing(card);
+			if (ret < 0)
+				return ret;
+		}
 	}
 
 	if (!card->name) {
-- 
1.7.0.4

Re: [PATCH v2 2/6] ASoC: Allow the DAPM routes to be stored in device tree

[Mark Brown]

On Wed, Dec 07, 2011 at 01:58:26PM -0700, Stephen Warren wrote:

> +	audio-routing =
> +		"Headphone Jack", "HPOUTR",
> +		"Headphone Jack", "HPOUTL",
> +		"Int Spk", "ROP",
> +		"Int Spk", "RON",
> +		"Int Spk", "LOP",
> +		"Int Spk", "LON",
> +		"Mic Jack", "MICBIAS",
> +		"IN1L", "Mic Jack";

Before we do this we first need to come up with a way of specifying the
DAPM nodes.  For the CODEC pins I'm fairly comfortable with just
documenting the fact that we want to use the pins as labelled on the
CODEC datsheet but for other objects on the board we need to come up
with something.  The tricky bit here is jacks - we need to be able to
express headsets, translate them into the separate input and output
widgets that DAPM uses while coping with multiple biases and custom
detection methods.

RE: [PATCH v2 2/6] ASoC: Allow the DAPM routes to be stored in device tree

[Stephen Warren]

Mark Brown wrote at Wednesday, December 07, 2011 9:32 PM:
> On Wed, Dec 07, 2011 at 01:58:26PM -0700, Stephen Warren wrote:
> 
> > +	audio-routing =
> > +		"Headphone Jack", "HPOUTR",
> > +		"Headphone Jack", "HPOUTL",
> > +		"Int Spk", "ROP",
> > +		"Int Spk", "RON",
> > +		"Int Spk", "LOP",
> > +		"Int Spk", "LON",
> > +		"Mic Jack", "MICBIAS",
> > +		"IN1L", "Mic Jack";
> 
> Before we do this we first need to come up with a way of specifying the
> DAPM nodes.  For the CODEC pins I'm fairly comfortable with just
> documenting the fact that we want to use the pins as labelled on the
> CODEC datsheet but for other objects on the board we need to come up
> with something.  The tricky bit here is jacks - we need to be able to
> express headsets, translate them into the separate input and output
> widgets that DAPM uses while coping with multiple biases and custom
> detection methods.

I'd originally made this property specific to the Tegra+WM8903 machine
driver, and you'd asked me to make it generic. Is there room to use
the binding above for the Tegra+WM8903 machine driver only, in order
to get the driver converted to DT, then define/implement something
completely generic to replace it, i.e. the stuff below?

This would be a fair bit of work to implement, but what are your thoughts
on the binding example below? It's very strongly based on ASoC, but since
that's so strongly based on the HW, I think you can consider it a pure
HW model rather than something derived from the way the driver works.

(The example isn't meant to represent any particular machine, although I
stole a bunch of it from Harmony to start with)

wm8903.dtsi:

/define/ WM8903_PORT_I2S0 0
/define/ WM8903_PIN_HPOUTL 0
/define/ WM8903_PIN_HPOUTR 1
...

tegra-harmony.dts:

wm8903: wm8903@... {
	#embedded-audio-pin-cells = <1>;
	#embedded-audio-port-cells = <1>;
};

i2s1: i2s@... {
	#embedded-audio-port-cells = <0>;
};

sound {
	compatible = "nvidia,tegra-audio-wm8903-harmony",
		     "nvidia,tegra-audio-wm8903"
	model = "NVIDIA Tegra Harmony";

	/*
	 * This is a more device-tree-idiomatic way of representing the
	 * routing table (using phandles and pin IDs) rather than text.
	 * This assumes my /define/ feature in dtc for the symbolic
	 * names
	 */
	audio-routing = <
		&hp                     &wm8903 WM8903_PIN_HPOUTR
		&hp                     &wm8903 WM8903_PIN_HPOUTL
		&spk                    &wm8903 WM8903_PIN_ROP
		&spk                    &wm8903 WM8903_PIN_RON
		&spk                    &wm8903 WM8903_PIN_LOP
		&spk                    &wm8903 WM8903_PIN_LON
		&imic                   &wm8903 WM8903_PIN_MICBIAS
		&wm8903 WM8903_PIN_IN1L &imic
	>;

	/*
	 * A completely generic OF binding would need nodes representing
	 * DAI links, but this binding just for Tegra+WM8903 can assume
	 * just a single link, and hence put the properties right here.
	 */
	nvidia,i2s-controller = <&i2s1>;
	nvidia,audio-codec = <&wm8903 WM8903_PORT_I2S0>;

	spk: endpoint@0 {
		compatible = "embedded-audio-endpoint";
		name = "Int Speakers";
		type = "Speakers";
		#embedded-audio-pin-cells = <0>;
		enable-gpios = <&codec 2 0>;
	};

	hp: endpoint@1 {
		compatible = "embedded-audio-endpoint";
		name = "Headphones";
		type = "Headphone";
		#embedded-audio-pin-cells = <0>;
		/*
		 * GPIO inversion: Could be handled with separate
		 * {en,dis}able-gpios properties as here, or could be
		 * represented in the GPIO controller's binding,
		 * i.e. the extra GPIO cells.
		 */
		/*
		 * This is really used for "hp_mute" on Kaen; perhaps
		 * mute is different to disable...
		 */
		disable-gpios = <&gpio NNN 0>;
	};

	emic: endpoint@2 {
		compatible = "embedded-audio-endpoint";
		name = "Ext Microphone";
		type = "Microphone";
		#embedded-audio-pin-cells = <0>;
		enable-gpios = <&gpio 184 0>; /* gpio PX0 */
	};

	imic: endpoint@3 {
		compatible = "embedded-audio-endpoint";
		name = "Int Microphone";
		type = "Microphone";
		#embedded-audio-pin-cells = <0>;
		supply = <&reg-dmic>;
	};

	lin: endpoint@4 {
		compatible = "embedded-audio-endpoint";
		name = "Line In";
		type = "Line Input";
		#embedded-audio-pin-cells = <0>;
	};

	lout: endpoint@5 {
		compatible = "embedded-audio-endpoint";
		name = "Line Out";
		type = "Line Output";
		#embedded-audio-pin-cells = <0>;
	};

	headsetj: jack@0 {
		compatible = "embedded-audio-jack";
		hosts = <&hp &emic>;
	};

	linj: jack@1 {
		compatible = "embedded-audio-jack";
		hosts = <&lin>;
	};

	loutj: jack@2 {
		compatible = "embedded-audio-jack";
		hosts = <&lout>;
	};

	gpio@0 {
		compatible = "embedded-audio-detect-gpio";
		gpio = <&gpio 178 0>; /* gpio PW2 */
		/*
		 * GPIO inversion: Could use an explicit property here,
		 * or, could be represented in the GPIO controller's
		 * binding, i.e. the extra GPIO cells.
		 */
		debounce-interval = <250>; /* mS */
		/*
		 * Always the whole jack here, or do we need to specify
		 * which endpoints within the jack?
		 */
		affected-jack = <&linj>;
	};

	micbias@0 {
		/*
		 * Is "micbias" the correct name here? Perhaps current-
		 * detect? If all mics need a mic bias, so it's a well
		 * known name, it's OK.
		 */
		compatible = "embedded-audio-detect-micbias";
		/*
		 * Here, I'm assuming that struct snd_soc_codec_driver
		 * would gain a generic "start mic detect" function, so
		 * a generic machine driver could activate that without
		 * having to link against e.g. wm8903_mic_detect().
		 */
		micbias = <&wm8903 WM8903_PIN_MICBIAS>;
		/*
		 * "1" below is index into headsetj's hosted endpoints.
		 * Perhaps this should reference <&imic> instead, and
		 * then map that back to the jack?
		 */
		affected-jack-endpoint = <&headsetj 1>;
	};

	/*
	 * Custom detection methods could be implemented by additional
	 * nodes with their own compatible values and bindings, hence
	 * handled entirely by that custom code.
	 */
};

-- 
nvpublic

Re: [PATCH v2 2/6] ASoC: Allow the DAPM routes to be stored in device tree

[Mark Brown]

On Fri, Dec 09, 2011 at 01:52:00PM -0800, Stephen Warren wrote:

> I'd originally made this property specific to the Tegra+WM8903 machine
> driver, and you'd asked me to make it generic. Is there room to use
> the binding above for the Tegra+WM8903 machine driver only, in order
> to get the driver converted to DT, then define/implement something
> completely generic to replace it, i.e. the stuff below?

I was asking for the code to be generic, not the binding itself.  If the 
code could for example take a property name as an argument that'd allow 
other bindings to use the same code without having to have a generic 
binding which has bits which depend strongly on some Linux specific
machine driver.

> This would be a fair bit of work to implement, but what are your thoughts
> on the binding example below? It's very strongly based on ASoC, but since
> that's so strongly based on the HW, I think you can consider it a pure
> HW model rather than something derived from the way the driver works.

It's starting to get an idiomatic way of representing the external
nodes, HDA is probably a good source of inspiration :)  We do need to
have a think about the jacks, though - simple ones are fine but
multifinction is more fun.

Re: [PATCH v2 2/6] ASoC: Allow the DAPM routes to be stored in device tree

[Stephen Warren]

On 12/10/2011 05:49 AM, Mark Brown wrote:
> On Fri, Dec 09, 2011 at 01:52:00PM -0800, Stephen Warren wrote:
> 
>> I'd originally made this property specific to the Tegra+WM8903 machine
>> driver, and you'd asked me to make it generic. Is there room to use
>> the binding above for the Tegra+WM8903 machine driver only, in order
>> to get the driver converted to DT, then define/implement something
>> completely generic to replace it, i.e. the stuff below?
> 
> I was asking for the code to be generic, not the binding itself.  If the 
> code could for example take a property name as an argument that'd allow 
> other bindings to use the same code without having to have a generic 
> binding which has bits which depend strongly on some Linux specific
> machine driver.

Ah right, I misunderstood then.

So, does the following sound reasonable:

Add:

snd_soc_of_parse_card_name(struct snd_soc_card *card,
			   const char *prop);

.. and have the machine driver call that whenever it decides, rather
than having e.g. snd_soc_register_card() call it.

Add:

int snd_soc_of_parse_audio_routing(struct snd_soc_card *card,
				   const char *prop);

.. and have the machine driver call that whenever it decides, rather
than having e.g. snd_soc_register_card() call it.

Remove
Documentation/devicetree/bindings/sound/embedded-audio-complex.txt from
the patches completely, and document the properties solely in the
Tegra+WM8903 machine driver binding; we can write
embedded-audio-complex.txt if we create a truly generic ASoC driver binding.

Does that sound reasonable?

Thanks.

-- 
nvpublic

Re: [PATCH v2 2/6] ASoC: Allow the DAPM routes to be stored in device tree

[Mark Brown]

On Mon, Dec 12, 2011 at 12:08:24PM -0700, Stephen Warren wrote:

> So, does the following sound reasonable:

> Add:

> snd_soc_of_parse_card_name(struct snd_soc_card *card,
> 			   const char *prop);

> .. and have the machine driver call that whenever it decides, rather
> than having e.g. snd_soc_register_card() call it.

Yes, all this sounds good.

RE: [PATCH v2 2/6] ASoC: Allow the DAPM routes to be stored in device tree

[Stephen Warren]

Mark Brown wrote at Saturday, December 10, 2011 5:49 AM:
> On Fri, Dec 09, 2011 at 01:52:00PM -0800, Stephen Warren wrote:
...
> > This would be a fair bit of work to implement, but what are your thoughts
> > on the binding example below? It's very strongly based on ASoC, but since
> > that's so strongly based on the HW, I think you can consider it a pure
> > HW model rather than something derived from the way the driver works.
> 
> It's starting to get an idiomatic way of representing the external
> nodes, HDA is probably a good source of inspiration :)  We do need to
> have a think about the jacks, though - simple ones are fine but
> multifinction is more fun.

I just wanted to confirm that you're talking about multiple audio
functions here (e.g. mic+headphones), rather than mixing audio with
other features (e.g. s-video + line out). I don't think the latter has
any kind of representation in the kernel right now though? That said,
it might be a good idea to represent jacks outside any audio complex
DT node, so that they could host functionality of different types if
the HW does so...

-- 
nvpublic

Re: [PATCH v2 2/6] ASoC: Allow the DAPM routes to be stored in device tree

[Mark Brown]

On Mon, Dec 12, 2011 at 11:34:13AM -0800, Stephen Warren wrote:
> Mark Brown wrote at Saturday, December 10, 2011 5:49 AM:

> > It's starting to get an idiomatic way of representing the external
> > nodes, HDA is probably a good source of inspiration :)  We do need to
> > have a think about the jacks, though - simple ones are fine but
> > multifinction is more fun.

> I just wanted to confirm that you're talking about multiple audio
> functions here (e.g. mic+headphones), rather than mixing audio with
> other features (e.g. s-video + line out). I don't think the latter has

Both.

> any kind of representation in the kernel right now though? That said,

Depends; if you listen to Lennart and Kay it's totally there already and
there's nothing to worry about.

> it might be a good idea to represent jacks outside any audio complex
> DT node, so that they could host functionality of different types if
> the HW does so...

Clearly, and even audio jacks have non-audio functionality like buttons.

[PATCH v2 3/6] ASoC: Refactor some conditions and loop in soc_bind_dai_link()

[Stephen Warren]

Transform some loops from:

for_each(x) {
    if (f(x)) {
        work_on(x);
    }
}

to new structure:

for_each(x) {
    if (!f(x))
        continue;

    work_on(x);
}

This will allow future modification of f(x) with less impact to the code.

Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v2: New patch

 sound/soc/soc-core.c |   52 ++++++++++++++++++++++++++++---------------------
 1 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index f60e04f..d2f9df5 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -764,10 +764,11 @@ static int soc_bind_dai_link(struct snd_soc_card *card, int num)
 	}
 	/* no, then find CPU DAI from registered DAIs*/
 	list_for_each_entry(cpu_dai, &dai_list, list) {
-		if (!strcmp(cpu_dai->name, dai_link->cpu_dai_name)) {
-			rtd->cpu_dai = cpu_dai;
-			goto find_codec;
-		}
+		if (strcmp(cpu_dai->name, dai_link->cpu_dai_name))
+			continue;
+
+		rtd->cpu_dai = cpu_dai;
+		goto find_codec;
 	}
 	dev_dbg(card->dev, "CPU DAI %s not registered\n",
 			dai_link->cpu_dai_name);
@@ -780,22 +781,28 @@ find_codec:
 
 	/* no, then find CODEC from registered CODECs*/
 	list_for_each_entry(codec, &codec_list, list) {
-		if (!strcmp(codec->name, dai_link->codec_name)) {
-			rtd->codec = codec;
-
-			/* CODEC found, so find CODEC DAI from registered DAIs from this CODEC*/
-			list_for_each_entry(codec_dai, &dai_list, list) {
-				if (codec->dev == codec_dai->dev &&
-						!strcmp(codec_dai->name, dai_link->codec_dai_name)) {
-					rtd->codec_dai = codec_dai;
-					goto find_platform;
-				}
-			}
-			dev_dbg(card->dev, "CODEC DAI %s not registered\n",
-					dai_link->codec_dai_name);
+		if (strcmp(codec->name, dai_link->codec_name))
+			continue;
+
+		rtd->codec = codec;
 
-			goto find_platform;
+		/*
+		 * CODEC found, so find CODEC DAI from registered DAIs from
+		 * this CODEC
+		 */
+		list_for_each_entry(codec_dai, &dai_list, list) {
+			if (codec->dev == codec_dai->dev &&
+				!strcmp(codec_dai->name,
+					dai_link->codec_dai_name)) {
+
+				rtd->codec_dai = codec_dai;
+				goto find_platform;
+			}
 		}
+		dev_dbg(card->dev, "CODEC DAI %s not registered\n",
+				dai_link->codec_dai_name);
+
+		goto find_platform;
 	}
 	dev_dbg(card->dev, "CODEC %s not registered\n",
 			dai_link->codec_name);
@@ -812,10 +819,11 @@ find_platform:
 
 	/* no, then find one from the set of registered platforms */
 	list_for_each_entry(platform, &platform_list, list) {
-		if (!strcmp(platform->name, platform_name)) {
-			rtd->platform = platform;
-			goto out;
-		}
+		if (strcmp(platform->name, platform_name))
+			continue;
+
+		rtd->platform = platform;
+		goto out;
 	}
 
 	dev_dbg(card->dev, "platform %s not registered\n",
-- 
1.7.0.4

Re: [PATCH v2 3/6] ASoC: Refactor some conditions and loop in soc_bind_dai_link()

[Mark Brown]

On Wed, Dec 07, 2011 at 01:58:27PM -0700, Stephen Warren wrote:
> Transform some loops from:

Applied, thanks.

[PATCH v2 4/6] ASoC: Allow DAI links to be specified using device tree nodes

[Stephen Warren]

DAI link endpoints and platform (DMA) devices are currently specified
by name. When instantiating sound cards from device tree, it may be more
convenient to refer to these devices by phandle in the device tree, and
for code to describe DAI links using the "struct device_node *"
("of_node") those phandles map to.

This change adds new fields to snd_soc_dai_link which can "name" devices
using of_node, enhances soc_bind_dai_link() to allow binding based on
of_node, and enhances snd_soc_register_card() to ensure that illegal
combinations of name and of_node are not used.

Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v2: New patch implementing a new feature

 include/sound/soc.h  |    4 +++
 sound/soc/soc-core.c |   64 ++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 61 insertions(+), 7 deletions(-)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index d9aa66b..40c256e 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -231,6 +231,7 @@ enum snd_soc_bias_level {
 	SND_SOC_BIAS_ON = 3,
 };
 
+struct device_node;
 struct snd_jack;
 struct snd_soc_card;
 struct snd_soc_pcm_stream;
@@ -704,8 +705,11 @@ struct snd_soc_dai_link {
 	const char *name;			/* Codec name */
 	const char *stream_name;		/* Stream name */
 	const char *codec_name;		/* for multi-codec */
+	const struct device_node *codec_of_node;
 	const char *platform_name;	/* for multi-platform */
+	const struct device_node *platform_of_node;
 	const char *cpu_dai_name;
+	const struct device_node *cpu_dai_of_node;
 	const char *codec_dai_name;
 
 	unsigned int dai_fmt;           /* format to set on init */
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index d2f9df5..07e536f 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -764,8 +764,13 @@ static int soc_bind_dai_link(struct snd_soc_card *card, int num)
 	}
 	/* no, then find CPU DAI from registered DAIs*/
 	list_for_each_entry(cpu_dai, &dai_list, list) {
-		if (strcmp(cpu_dai->name, dai_link->cpu_dai_name))
-			continue;
+		if (dai_link->cpu_dai_of_node) {
+			if (cpu_dai->dev->of_node != dai_link->cpu_dai_of_node)
+				continue;
+		} else {
+			if (strcmp(cpu_dai->name, dai_link->cpu_dai_name))
+				continue;
+		}
 
 		rtd->cpu_dai = cpu_dai;
 		goto find_codec;
@@ -781,8 +786,13 @@ find_codec:
 
 	/* no, then find CODEC from registered CODECs*/
 	list_for_each_entry(codec, &codec_list, list) {
-		if (strcmp(codec->name, dai_link->codec_name))
-			continue;
+		if (dai_link->codec_of_node) {
+			if (codec->dev->of_node != dai_link->codec_of_node)
+				continue;
+		} else {
+			if (strcmp(codec->name, dai_link->codec_name))
+				continue;
+		}
 
 		rtd->codec = codec;
 
@@ -814,13 +824,19 @@ find_platform:
 
 	/* if there's no platform we match on the empty platform */
 	platform_name = dai_link->platform_name;
-	if (!platform_name)
+	if (!platform_name && !dai_link->platform_of_node)
 		platform_name = "snd-soc-dummy";
 
 	/* no, then find one from the set of registered platforms */
 	list_for_each_entry(platform, &platform_list, list) {
-		if (strcmp(platform->name, platform_name))
-			continue;
+		if (dai_link->platform_of_node) {
+			if (platform->dev->of_node !=
+			    dai_link->platform_of_node)
+				continue;
+		} else {
+			if (strcmp(platform->name, platform_name))
+				continue;
+		}
 
 		rtd->platform = platform;
 		goto out;
@@ -2909,6 +2925,40 @@ int snd_soc_register_card(struct snd_soc_card *card)
 		return -EINVAL;
 	}
 
+	for (i = 0; i < card->num_links; i++) {
+		struct snd_soc_dai_link *link = &card->dai_link[i];
+
+		/*
+		 * Codec must be specified by 1 of name or OF node,
+		 * not both or neither.
+		 */
+		if (!!link->codec_name == !!link->codec_of_node) {
+			dev_err(card->dev,
+				"Neither/both codec name/of_node are set\n");
+			return -EINVAL;
+		}
+
+		/*
+		 * Platform may be specified by either name or OF node, but
+		 * can be left unspecified, and a dummy platform will be used.
+		 */
+		if (link->platform_name && link->platform_of_node) {
+			dev_err(card->dev,
+				"Both platform name/of_node are set\n");
+			return -EINVAL;
+		}
+
+		/*
+		 * CPU DAI must be specified by 1 of name or OF node,
+		 * not both or neither.
+		 */
+		if (!!link->cpu_dai_name == !!link->cpu_dai_of_node) {
+			dev_err(card->dev,
+				"Neither/both cpu_dai name/of_node are set\n");
+			return -EINVAL;
+		}
+	}
+
 	dev_set_drvdata(card->dev, card);
 
 	snd_soc_initialize_card_lists(card);
-- 
1.7.0.4

Re: [PATCH v2 4/6] ASoC: Allow DAI links to be specified using device tree nodes

[Mark Brown]

On Wed, Dec 07, 2011 at 01:58:28PM -0700, Stephen Warren wrote:
> DAI link endpoints and platform (DMA) devices are currently specified
> by name. When instantiating sound cards from device tree, it may be more
> convenient to refer to these devices by phandle in the device tree, and
> for code to describe DAI links using the "struct device_node *"
> ("of_node") those phandles map to.

I would've applied this but it depends on a previous patch so it won't
apply as-is.

[PATCH v2 5/6] ASoC: Tegra: Move DAS configuration into DAS driver

[Stephen Warren]

Move DAS routing setup into the DAS driver itself. This removes the need
to duplicate this in each machine driver, of which we'll soon have three.

An added advantage is that the machine drivers no longer call the Tegra20-
specific DAS functions by name, so the machine driver no longer needs to
be split up into Tegra20 and Tegra30 versions.

If individual machine drivers need a different routing setup to this
default, they can still call the DAS functions to set that up.

Long-term, DAS will be a codec driver, and user-space will be able to
control its routing, possibly within constraints that the machine driver
sets up. Configuring the DAS routing from the DAS driver is a very slight
move in that direction.

Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v2: New patch. Sorry for the churn here. This should avoid the Paz00
(Toshiba AC100) machine driver from having to add the calls to the DAS
driver.

 sound/soc/tegra/tegra_das.c    |   13 +++++++++++++
 sound/soc/tegra/tegra_wm8903.c |   13 -------------
 sound/soc/tegra/trimslice.c    |   23 -----------------------
 3 files changed, 13 insertions(+), 36 deletions(-)

diff --git a/sound/soc/tegra/tegra_das.c b/sound/soc/tegra/tegra_das.c
index 5b82b4e..3b3c1ba 100644
--- a/sound/soc/tegra/tegra_das.c
+++ b/sound/soc/tegra/tegra_das.c
@@ -202,6 +202,19 @@ static int __devinit tegra_das_probe(struct platform_device *pdev)
 		goto err;
 	}
 
+	ret = tegra_das_connect_dap_to_dac(TEGRA_DAS_DAP_ID_1,
+					   TEGRA_DAS_DAP_SEL_DAC1);
+	if (ret) {
+		dev_err(&pdev->dev, "Can't set up DAS DAP connection\n");
+		goto err;
+	}
+	ret = tegra_das_connect_dac_to_dap(TEGRA_DAS_DAC_ID_1,
+					   TEGRA_DAS_DAC_SEL_DAP1);
+	if (ret) {
+		dev_err(&pdev->dev, "Can't set up DAS DAC connection\n");
+		goto err;
+	}
+
 	tegra_das_debug_add(das);
 
 	platform_set_drvdata(pdev, das);
diff --git a/sound/soc/tegra/tegra_wm8903.c b/sound/soc/tegra/tegra_wm8903.c
index 2f5b107..ba2d23e 100644
--- a/sound/soc/tegra/tegra_wm8903.c
+++ b/sound/soc/tegra/tegra_wm8903.c
@@ -249,19 +249,6 @@ static int tegra_wm8903_init(struct snd_soc_pcm_runtime *rtd)
 	struct tegra_wm8903_platform_data *pdata = machine->pdata;
 	int ret;
 
-	ret = tegra_das_connect_dap_to_dac(TEGRA_DAS_DAP_ID_1,
-					   TEGRA_DAS_DAP_SEL_DAC1);
-	if (ret) {
-		dev_err(card->dev, "Can't set up DAS DAP connection\n");
-		return ret;
-	}
-	ret = tegra_das_connect_dac_to_dap(TEGRA_DAS_DAC_ID_1,
-					   TEGRA_DAS_DAC_SEL_DAP1);
-	if (ret) {
-		dev_err(card->dev, "Can't set up DAS DAC connection\n");
-		return ret;
-	}
-
 	if (gpio_is_valid(pdata->gpio_spkr_en)) {
 		ret = gpio_request(pdata->gpio_spkr_en, "spkr_en");
 		if (ret) {
diff --git a/sound/soc/tegra/trimslice.c b/sound/soc/tegra/trimslice.c
index 043eb7c..7d95b76 100644
--- a/sound/soc/tegra/trimslice.c
+++ b/sound/soc/tegra/trimslice.c
@@ -115,28 +115,6 @@ static const struct snd_soc_dapm_route trimslice_audio_map[] = {
 	{"RLINEIN", NULL, "Line In"},
 };
 
-static int trimslice_asoc_init(struct snd_soc_pcm_runtime *rtd)
-{
-	struct snd_soc_codec *codec = rtd->codec;
-	struct snd_soc_card *card = codec->card;
-	int ret;
-
-	ret = tegra_das_connect_dap_to_dac(TEGRA_DAS_DAP_ID_1,
-					   TEGRA_DAS_DAP_SEL_DAC1);
-	if (ret) {
-		dev_err(card->dev, "Can't set up DAS DAP connection\n");
-		return ret;
-	}
-	ret = tegra_das_connect_dac_to_dap(TEGRA_DAS_DAC_ID_1,
-					   TEGRA_DAS_DAC_SEL_DAP1);
-	if (ret) {
-		dev_err(card->dev, "Can't set up DAS DAC connection\n");
-		return ret;
-	}
-
-	return 0;
-}
-
 static struct snd_soc_dai_link trimslice_tlv320aic23_dai = {
 	.name = "TLV320AIC23",
 	.stream_name = "AIC23",
@@ -144,7 +122,6 @@ static struct snd_soc_dai_link trimslice_tlv320aic23_dai = {
 	.platform_name = "tegra-pcm-audio",
 	.cpu_dai_name = "tegra-i2s.0",
 	.codec_dai_name = "tlv320aic23-hifi",
-	.init = trimslice_asoc_init,
 	.ops = &trimslice_asoc_ops,
 };
 
-- 
1.7.0.4

Re: [PATCH v2 5/6] ASoC: Tegra: Move DAS configuration into DAS driver

[Mark Brown]

On Wed, Dec 07, 2011 at 01:58:29PM -0700, Stephen Warren wrote:
> Move DAS routing setup into the DAS driver itself. This removes the need
> to duplicate this in each machine driver, of which we'll soon have three.

Applied, thanks.

[PATCH v2 6/6] ASoC: Tegra+WM8903 machine: Add device tree binding

[Stephen Warren]

This driver is parameterized in two ways:

a) Platform data, which supplies the set of GPIOs used by the driver.
   These GPIOs can now be parsed out of device tree.

b) Machine-specific DAPM route arrays embedded into the ASoC machine
   driver itself. Historically, the driver picks the appropriate array
   to use using machine_is_*(). The driver now requires this array to
   be parsed from device tree when instantiated through device tree,
   using the core ASoC support for this parsing.

Based on work by John Bonesio, but significantly reworked since then.

Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
v2:
* Split DAPM map parsing into a core ASoC feature.
* Updating binding to require codec and I2S controller phandles, use new
  ASoC core feature to bind DAI links using OF nodes rather than device
  names.
* Rebase on top of WM8903 MICBIAS supply conversion.
* Other minor cleanups.

 .../bindings/sound/tegra-audio-wm8903.txt          |   72 +++++++++++++
 sound/soc/tegra/tegra_wm8903.c                     |  106 ++++++++++++++++----
 2 files changed, 156 insertions(+), 22 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/tegra-audio-wm8903.txt

diff --git a/Documentation/devicetree/bindings/sound/tegra-audio-wm8903.txt b/Documentation/devicetree/bindings/sound/tegra-audio-wm8903.txt
new file mode 100644
index 0000000..b96639d
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/tegra-audio-wm8903.txt
@@ -0,0 +1,72 @@
+NVIDIA Tegra audio complex
+
+Required properties:
+- compatible : "nvidia,tegra-audio-wm8903" (specific HW model)
+               "generic,soc-audio-complex" (generic audio complex)
+- user-visible-name : As defined by soc-audio-complex.txt.
+- audio-routing : As defined by soc-audio-complex.txt.
+  Valid names for sources and sinks are the WM8903's pins, and the jacks
+  on the board:
+
+  WM8903 pins:
+
+  * IN1L
+  * IN1R
+  * IN2L
+  * IN2R
+  * IN3L
+  * IN3R
+  * DMICDAT
+  * HPOUTL
+  * HPOUTR
+  * LINEOUTL
+  * LINEOUTR
+  * LOP
+  * LON
+  * ROP
+  * RON
+  * MICBIAS
+
+  Board connectors:
+
+  * Headphone Jack
+  * Int Spk
+  * Mic Jack
+
+- nvidia,i2s-controller : The phandle of the Tegra I2S1 controller
+- nvidia,audio-codec : The phandle of the WM8903 audio codec
+
+Optional properties:
+- nvidia,spkr-en-gpios : The GPIO that enables the speakers
+- nvidia,hp-mute-gpios : The GPIO that mutes the headphones
+- nvidia,hp-det-gpios : The GPIO that detect headphones are plugged in
+- nvidia,int-mic-en-gpios : The GPIO that enables the internal microphone
+- nvidia,ext-mic-en-gpios : The GPIO that enables the external microphone
+
+Example:
+
+sound {
+	compatible = "nvidia,tegra-audio-wm8903-harmony",
+		     "nvidia,tegra-audio-wm8903",
+		     "generic,soc-audio-complex";
+	user-visible-name = "tegra-wm8903-harmony";
+
+	audio-routing =
+		"Headphone Jack", "HPOUTR",
+		"Headphone Jack", "HPOUTL",
+		"Int Spk", "ROP",
+		"Int Spk", "RON",
+		"Int Spk", "LOP",
+		"Int Spk", "LON",
+		"Mic Jack", "MICBIAS",
+		"IN1L", "Mic Jack";
+
+	nvidia,i2s-controller = <&i2s1>;
+	nvidia,audio-codec = <&wm8903>;
+
+	nvidia,spkr-en-gpios = <&codec 2 0>;
+	nvidia,hp-det-gpios = <&gpio 178 0>; /* gpio PW2 */
+	nvidia,int-mic-en-gpios = <&gpio 184 0>; /*gpio PX0 */
+	nvidia,ext-mic-en-gpios = <&gpio 185 0>; /* gpio PX1 */
+};
+
diff --git a/sound/soc/tegra/tegra_wm8903.c b/sound/soc/tegra/tegra_wm8903.c
index ba2d23e..e1534f2 100644
--- a/sound/soc/tegra/tegra_wm8903.c
+++ b/sound/soc/tegra/tegra_wm8903.c
@@ -34,6 +34,7 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/gpio.h>
+#include <linux/of_gpio.h>
 
 #include <mach/tegra_wm8903_pdata.h>
 
@@ -59,8 +60,9 @@
 #define GPIO_HP_DET     BIT(4)
 
 struct tegra_wm8903 {
+	struct tegra_wm8903_platform_data pdata;
+	struct platform_device *pcm_dev;
 	struct tegra_asoc_utils_data util_data;
-	struct tegra_wm8903_platform_data *pdata;
 	int gpio_requested;
 };
 
@@ -160,7 +162,7 @@ static int tegra_wm8903_event_int_spk(struct snd_soc_dapm_widget *w,
 	struct snd_soc_dapm_context *dapm = w->dapm;
 	struct snd_soc_card *card = dapm->card;
 	struct tegra_wm8903 *machine = snd_soc_card_get_drvdata(card);
-	struct tegra_wm8903_platform_data *pdata = machine->pdata;
+	struct tegra_wm8903_platform_data *pdata = &machine->pdata;
 
 	if (!(machine->gpio_requested & GPIO_SPKR_EN))
 		return 0;
@@ -177,7 +179,7 @@ static int tegra_wm8903_event_hp(struct snd_soc_dapm_widget *w,
 	struct snd_soc_dapm_context *dapm = w->dapm;
 	struct snd_soc_card *card = dapm->card;
 	struct tegra_wm8903 *machine = snd_soc_card_get_drvdata(card);
-	struct tegra_wm8903_platform_data *pdata = machine->pdata;
+	struct tegra_wm8903_platform_data *pdata = &machine->pdata;
 
 	if (!(machine->gpio_requested & GPIO_HP_MUTE))
 		return 0;
@@ -246,9 +248,36 @@ static int tegra_wm8903_init(struct snd_soc_pcm_runtime *rtd)
 	struct snd_soc_dapm_context *dapm = &codec->dapm;
 	struct snd_soc_card *card = codec->card;
 	struct tegra_wm8903 *machine = snd_soc_card_get_drvdata(card);
-	struct tegra_wm8903_platform_data *pdata = machine->pdata;
+	struct tegra_wm8903_platform_data *pdata = &machine->pdata;
+	struct device_node *np = card->dev->of_node;
 	int ret;
 
+	if (card->dev->platform_data) {
+		memcpy(pdata, card->dev->platform_data, sizeof(*pdata));
+	} else if (np) {
+		/*
+		 * This part must be in init() rather than probe() in order to
+		 * guarantee that the WM8903 has been probed, and hence its
+		 * GPIO controller registered, which is a pre-condition for
+		 * of_get_named_gpio() to be able to map the phandles in the
+		 * properties to the controller node. Given this, all
+		 * pdata handling is in init() for consistency.
+		 */
+		pdata->gpio_spkr_en = of_get_named_gpio(np,
+						"nvidia,spkr-en-gpios", 0);
+		pdata->gpio_hp_mute = of_get_named_gpio(np,
+						"nvidia,hp-mute-gpios", 0);
+		pdata->gpio_hp_det = of_get_named_gpio(np,
+						"nvidia,hp-det-gpios", 0);
+		pdata->gpio_int_mic_en = of_get_named_gpio(np,
+						"nvidia,int-mic-en-gpios", 0);
+		pdata->gpio_ext_mic_en = of_get_named_gpio(np,
+						"nvidia,ext-mic-en-gpios", 0);
+	} else {
+		dev_err(card->dev, "No platform data supplied\n");
+		return -EINVAL;
+	}
+
 	if (gpio_is_valid(pdata->gpio_spkr_en)) {
 		ret = gpio_request(pdata->gpio_spkr_en, "spkr_en");
 		if (ret) {
@@ -348,11 +377,9 @@ static __devinit int tegra_wm8903_driver_probe(struct platform_device *pdev)
 {
 	struct snd_soc_card *card = &snd_soc_tegra_wm8903;
 	struct tegra_wm8903 *machine;
-	struct tegra_wm8903_platform_data *pdata;
 	int ret;
 
-	pdata = pdev->dev.platform_data;
-	if (!pdata) {
+	if (!pdev->dev.platform_data && !pdev->dev.of_node) {
 		dev_err(&pdev->dev, "No platform data supplied\n");
 		return -EINVAL;
 	}
@@ -364,29 +391,52 @@ static __devinit int tegra_wm8903_driver_probe(struct platform_device *pdev)
 		ret = -ENOMEM;
 		goto err;
 	}
+	machine->pcm_dev = ERR_PTR(-EINVAL);
+
+	if (pdev->dev.of_node) {
+		machine->pcm_dev = platform_device_register_simple(
+					"tegra-pcm-audio", -1, NULL, 0);
+		if (IS_ERR(machine->pcm_dev)) {
+			dev_err(&pdev->dev, "Can't instantiate ASoC DMA\n");
+			ret = PTR_ERR(machine->pcm_dev);
+			goto err;
+		}
+
+		tegra_wm8903_dai.codec_name = NULL;
+		tegra_wm8903_dai.codec_of_node = of_parse_phandle(
+				pdev->dev.of_node, "nvidia,audio-codec", 0);
 
-	machine->pdata = pdata;
+		tegra_wm8903_dai.cpu_dai_name = NULL;
+		tegra_wm8903_dai.cpu_dai_of_node = of_parse_phandle(
+				pdev->dev.of_node, "nvidia,i2s-controller", 0);
+	}
 
 	ret = tegra_asoc_utils_init(&machine->util_data, &pdev->dev);
 	if (ret)
-		goto err;
+		goto err_unregister;
 
 	card->dev = &pdev->dev;
 	platform_set_drvdata(pdev, card);
 	snd_soc_card_set_drvdata(card, machine);
 
-	if (machine_is_harmony()) {
-		card->dapm_routes = harmony_audio_map;
-		card->num_dapm_routes = ARRAY_SIZE(harmony_audio_map);
-	} else if (machine_is_seaboard()) {
-		card->dapm_routes = seaboard_audio_map;
-		card->num_dapm_routes = ARRAY_SIZE(seaboard_audio_map);
-	} else if (machine_is_kaen()) {
-		card->dapm_routes = kaen_audio_map;
-		card->num_dapm_routes = ARRAY_SIZE(kaen_audio_map);
-	} else {
-		card->dapm_routes = aebl_audio_map;
-		card->num_dapm_routes = ARRAY_SIZE(aebl_audio_map);
+	/*
+	 * With device tree, we require the route table to be parsed from the
+	 * device tree by the ASoC core.
+	 */
+	if (!pdev->dev.of_node) {
+		if (machine_is_harmony()) {
+			card->dapm_routes = harmony_audio_map;
+			card->num_dapm_routes = ARRAY_SIZE(harmony_audio_map);
+		} else if (machine_is_seaboard()) {
+			card->dapm_routes = seaboard_audio_map;
+			card->num_dapm_routes = ARRAY_SIZE(seaboard_audio_map);
+		} else if (machine_is_kaen()) {
+			card->dapm_routes = kaen_audio_map;
+			card->num_dapm_routes = ARRAY_SIZE(kaen_audio_map);
+		} else {
+			card->dapm_routes = aebl_audio_map;
+			card->num_dapm_routes = ARRAY_SIZE(aebl_audio_map);
+		}
 	}
 
 	ret = snd_soc_register_card(card);
@@ -400,6 +450,9 @@ static __devinit int tegra_wm8903_driver_probe(struct platform_device *pdev)
 
 err_fini_utils:
 	tegra_asoc_utils_fini(&machine->util_data);
+err_unregister:
+	if (!IS_ERR(machine->pcm_dev))
+		platform_device_unregister(machine->pcm_dev);
 err:
 	return ret;
 }
@@ -408,7 +461,7 @@ static int __devexit tegra_wm8903_driver_remove(struct platform_device *pdev)
 {
 	struct snd_soc_card *card = platform_get_drvdata(pdev);
 	struct tegra_wm8903 *machine = snd_soc_card_get_drvdata(card);
-	struct tegra_wm8903_platform_data *pdata = machine->pdata;
+	struct tegra_wm8903_platform_data *pdata = &machine->pdata;
 
 	if (machine->gpio_requested & GPIO_HP_DET)
 		snd_soc_jack_free_gpios(&tegra_wm8903_hp_jack,
@@ -427,15 +480,23 @@ static int __devexit tegra_wm8903_driver_remove(struct platform_device *pdev)
 	snd_soc_unregister_card(card);
 
 	tegra_asoc_utils_fini(&machine->util_data);
+	if (!IS_ERR(machine->pcm_dev))
+		platform_device_unregister(machine->pcm_dev);
 
 	return 0;
 }
 
+static const struct of_device_id tegra_wm8903_of_match[] __devinitconst = {
+	{ .compatible = "nvidia,tegra-audio-wm8903", },
+	{},
+};
+
 static struct platform_driver tegra_wm8903_driver = {
 	.driver = {
 		.name = DRV_NAME,
 		.owner = THIS_MODULE,
 		.pm = &snd_soc_pm_ops,
+		.of_match_table = tegra_wm8903_of_match,
 	},
 	.probe = tegra_wm8903_driver_probe,
 	.remove = __devexit_p(tegra_wm8903_driver_remove),
@@ -446,3 +507,4 @@ MODULE_AUTHOR("Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>");
 MODULE_DESCRIPTION("Tegra+WM8903 machine ASoC driver");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:" DRV_NAME);
+MODULE_DEVICE_TABLE(of, tegra_wm8903_of_match);
-- 
1.7.0.4

Re: [PATCH v2 1/6] ASoC: Allow device tree to specify a card's name

[Mark Brown]

On Wed, Dec 07, 2011 at 01:58:25PM -0700, Stephen Warren wrote:

> +sound {
> +	compatible = "nvidia,tegra-audio-wm8903",
> +		     "generic,soc-audio-complex";
> +	user-visible-name = "tegra-wm8903-harmony";

That user visible name is idiomatic for device tree but it's not
idiomatic for humans I fear :)

I'd be tempted to use embedded or something indicating that it's for the
use of discrete components rather than SoC.  Not had a chance to look at
the actual code yet, sorry.

Re: [PATCH v2 1/6] ASoC: Allow device tree to specify a card's name

[Liam Girdwood]

On Wed, 2011-12-07 at 13:58 -0700, Stephen Warren wrote:
> If a card's device was instantiated from device tree, and the device tree
> has a "user-visible-name" property, use that as the card's name.
> 
> Signed-off-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
> v2: New patch implementing new functionality
> 
> Re: the binding documentation:
> * "SoC" here refers to the fact this is a binding oriented at System-on-
>   chip audio complexes, rather than having to do with "ASoC"; both names
>   were derived from the same root.
> * Do we need a compatible property for this "base class" binding at all?
>   I think it's a good idea, even though the code doesn't actually rely
>   on it.
> * Should the vendor field in the compatible property be "generic",
>   "linux", or absent? I've tried to make these bindings generic and
>   applicable to other OSs, so "linux," seems wrong.
> * Should the property "user-visible-name" have a "generic," prefix or
>   similar?
> 

Just had a quick look and 3 & 4 look mostly fine to me.

3 & 4

Acked-by: Liam Girdwood <lrg-l0cyMroinI0@public.gmane.org>

It also seems that once 1 & 2 are applied, we would almost be able to
just have a generic "device tree" machine driver for some simple ASoC
machines that have DAPM and no other external logic atm. We would just
be missing some runtime configuration for the the DAIs though.

Liam

Re: [PATCH v2 1/6] ASoC: Allow device tree to specify a card's name

[Mark Brown]

On Thu, Dec 08, 2011 at 07:17:05PM +0000, Liam Girdwood wrote:

> It also seems that once 1 & 2 are applied, we would almost be able to
> just have a generic "device tree" machine driver for some simple ASoC
> machines that have DAPM and no other external logic atm. We would just
> be missing some runtime configuration for the the DAIs though.

The big problem being sidestepped here is clocks.