[alsa-devel] [PATCH v2 1/6] ASoC: Allow device tree to specify a card's name
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@nvidia.com --- 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);
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@nvidia.com --- 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) {
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.
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 = <®-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. */ };
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.
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.
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.
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...
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.
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@nvidia.com --- 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",
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@nvidia.com --- 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);
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.
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@nvidia.com --- 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, };
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.
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@nvidia.com --- 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@nvidia.com"); MODULE_DESCRIPTION("Tegra+WM8903 machine ASoC driver"); MODULE_LICENSE("GPL"); MODULE_ALIAS("platform:" DRV_NAME); +MODULE_DEVICE_TABLE(of, tegra_wm8903_of_match);
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.
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@nvidia.com
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@ti.com
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
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.
participants (3)
-
Liam Girdwood -
Mark Brown -
Stephen Warren