Re: [PATCH RFC 2/2] ASoC: simple-card: Move dai-link level properties away from dai subnodes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 03/23/2014 11:54 AM, Jean-Francois Moine wrote:
On Fri, 21 Mar 2014 18:47:23 +0200
Jyri Sarha <jsarha@xxxxxx> wrote:

The properties like format, bitclock-master, frame-master,
bitclock-inversion, and frame-inversion should be common to the dais
connected with a dai-link. For bitclock-master and frame-master
properties to be unambiguous they need to indicate the mastering dai
node with a phandle.

Signed-off-by: Jyri Sarha <jsarha@xxxxxx>
---
[...]
-Required subnodes:
+Requred dai-link subnodes:

Typo: 'Required'


Fixed. Thanks!

[...]
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -8,6 +8,7 @@
   * it under the terms of the GNU General Public License version 2 as
   * published by the Free Software Foundation.
   */
+#define DEBUG

Should be removed.


Removed. Thanks!

[...]
+	if (!bitclkmaster && !framemaster) {
+		/* Master setting not found from dai_link level, revert back
+		   to legacy DT parsing and take settings from codec node. */
+		dev_dbg(dev, "%s: Revert to legacy daifmt parsing\n",
+			__func__);
+		dai_props->cpu_dai.fmt = dai_props->codec_dai.fmt =
+			snd_soc_of_parse_daifmt(np, NULL, NULL, NULL) |
+			(daifmt & ~SND_SOC_DAIFMT_CLOCK_MASK);

We are here each time there is no bitclock-master and no frame-master,
i.e. each time for me. It could be simpler to keep the first 'daifmt'
instead of parsing again.


Sorry, I missed this and comments bellow first time around. I'll make another patch set shortly.

Adding another check to test if we are parsing top level sound node would save you from the legacy mode parsing. I'll do that and update the
DT binding document accordingly.

[...]
+	if (multi) {
+		struct device_node *np = NULL;
+		int i;
+		for (i = 0; (np = of_get_next_child(node, np)); i++) {
+			dev_dbg(dev, "\tlink %d:\n", i);
+			ret = simple_card_dai_link_of(np, dev, dai_link + i,
+						      dai_props + i);

I feel that my loop was quicker:


I was targeting for readability rather than speed. I doubt the difference is significant since most string processing is anyway taking most of the time anyway.

		struct device_node *np = NULL;

		for (;;) {
			np = of_get_next_child(node, np);
			if (!np)
				break;
			dev_dbg(dev, "\tlink %d:\n", dai_link - priv->snd_card.dai_link);
			ret = simple_card_dai_link_of(np, dev, dai_link++,
						      dai_props++);


+			of_node_put(np);
+			if (ret < 0)
+				return ret;

The np reference count is updated in of_get_next_child(), so:

			if (ret < 0) {
				of_node_put(np);
				return ret;
			}


Oops, need to fix that. Thanks !

Otherwise, it works for me.

Tested-by: Jean-Francois Moine <moinejf@xxxxxxx>


Best regards,
Jyri

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




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux