diff mbox

[3/3] ASoC: simple-card-utils: enable clocks/clock-names/clock-ranges

Message ID 87zikbuezr.wl%kuninori.morimoto.gx@renesas.com (mailing list archive)
State Not Applicable, archived
Delegated to: Stephen Boyd
Headers show

Commit Message

Kuninori Morimoto Dec. 5, 2016, 5:23 a.m. UTC
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current simple-card is supporting this style for clocks

	sound {
		...
		simple-audio-card,cpu {
			sound-dai = <&xxx>;
			clocks = <&cpu_clock>;
		};
		simple-audio-card,codec {
			sound-dai = <&xxx>;
			clocks = <&codec_clock>;
		};
	};

Now, it can support this style too, because we can use
devm_get_clk_from_child() now.

	sound {
		...
		clocks = <&cpu_clock>, <&codec_clock>;
		clock-names = "cpu", "codec";
		clock-ranges;
		...
		simple-audio-card,cpu {
			sound-dai = <&xxx>;
		};
		simple-audio-card,codec {
			sound-dai = <&xxx>;
		};
	};

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 .../devicetree/bindings/sound/simple-card.txt      | 32 ++++++++++++++++++
 sound/soc/generic/simple-card-utils.c              | 39 +++++++++++++++++++++-
 2 files changed, 70 insertions(+), 1 deletion(-)

Comments

Stephen Boyd Dec. 8, 2016, 10:09 p.m. UTC | #1
On 12/05, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Current simple-card is supporting this style for clocks
> 
> 	sound {
> 		...
> 		simple-audio-card,cpu {
> 			sound-dai = <&xxx>;
> 			clocks = <&cpu_clock>;
> 		};
> 		simple-audio-card,codec {
> 			sound-dai = <&xxx>;
> 			clocks = <&codec_clock>;
> 		};
> 	};
> 
> Now, it can support this style too, because we can use
> devm_get_clk_from_child() now.
> 
> 	sound {
> 		...
> 		clocks = <&cpu_clock>, <&codec_clock>;
> 		clock-names = "cpu", "codec";
> 		clock-ranges;
> 		...
> 		simple-audio-card,cpu {
> 			sound-dai = <&xxx>;
> 		};
> 		simple-audio-card,codec {
> 			sound-dai = <&xxx>;
> 		};
> 	};
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

I don't see any reason why we need this patch though. The binding
works as is, so supporting different styles doesn't seem like a
good idea to me. Let's just keep what we have? Even if a sub-node
like cpu or codec gets more than one element in the clocks list
property, we can make that work by passing a clock-name then
based on some sort of other knowledge.
Kuninori Morimoto Dec. 9, 2016, 12:21 a.m. UTC | #2
Hi Stephen

> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > Current simple-card is supporting this style for clocks
> > 
> > 	sound {
> > 		...
> > 		simple-audio-card,cpu {
> > 			sound-dai = <&xxx>;
> > 			clocks = <&cpu_clock>;
> > 		};
> > 		simple-audio-card,codec {
> > 			sound-dai = <&xxx>;
> > 			clocks = <&codec_clock>;
> > 		};
> > 	};
> > 
> > Now, it can support this style too, because we can use
> > devm_get_clk_from_child() now.
> > 
> > 	sound {
> > 		...
> > 		clocks = <&cpu_clock>, <&codec_clock>;
> > 		clock-names = "cpu", "codec";
> > 		clock-ranges;
> > 		...
> > 		simple-audio-card,cpu {
> > 			sound-dai = <&xxx>;
> > 		};
> > 		simple-audio-card,codec {
> > 			sound-dai = <&xxx>;
> > 		};
> > 	};
> > 
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> I don't see any reason why we need this patch though. The binding
> works as is, so supporting different styles doesn't seem like a
> good idea to me. Let's just keep what we have? Even if a sub-node
> like cpu or codec gets more than one element in the clocks list
> property, we can make that work by passing a clock-name then
> based on some sort of other knowledge.

OK, thanks. Let's skip this patch.
But I believe this idea itself is not wrong (?)

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kuninori Morimoto Dec. 9, 2016, 12:22 a.m. UTC | #3
Hi Stephen

> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > Current simple-card is supporting this style for clocks
> > 
> > 	sound {
> > 		...
> > 		simple-audio-card,cpu {
> > 			sound-dai = <&xxx>;
> > 			clocks = <&cpu_clock>;
> > 		};
> > 		simple-audio-card,codec {
> > 			sound-dai = <&xxx>;
> > 			clocks = <&codec_clock>;
> > 		};
> > 	};
> > 
> > Now, it can support this style too, because we can use
> > devm_get_clk_from_child() now.
> > 
> > 	sound {
> > 		...
> > 		clocks = <&cpu_clock>, <&codec_clock>;
> > 		clock-names = "cpu", "codec";
> > 		clock-ranges;
> > 		...
> > 		simple-audio-card,cpu {
> > 			sound-dai = <&xxx>;
> > 		};
> > 		simple-audio-card,codec {
> > 			sound-dai = <&xxx>;
> > 		};
> > 	};
> > 
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> I don't see any reason why we need this patch though. The binding
> works as is, so supporting different styles doesn't seem like a
> good idea to me. Let's just keep what we have? Even if a sub-node
> like cpu or codec gets more than one element in the clocks list
> property, we can make that work by passing a clock-name then
> based on some sort of other knowledge.

OK, thanks. Let's skip this patch.
But I believe this idea/method itself is not wrong (?)

--
To unsubscribe from this list: send the line "unsubscribe linux-clk" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd Dec. 9, 2016, 12:26 a.m. UTC | #4
On 12/09, Kuninori Morimoto wrote:
> 
> Hi Stephen
> 
> > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > > 
> > > Current simple-card is supporting this style for clocks
> > > 
> > > 	sound {
> > > 		...
> > > 		simple-audio-card,cpu {
> > > 			sound-dai = <&xxx>;
> > > 			clocks = <&cpu_clock>;
> > > 		};
> > > 		simple-audio-card,codec {
> > > 			sound-dai = <&xxx>;
> > > 			clocks = <&codec_clock>;
> > > 		};
> > > 	};
> > > 
> > > Now, it can support this style too, because we can use
> > > devm_get_clk_from_child() now.
> > > 
> > > 	sound {
> > > 		...
> > > 		clocks = <&cpu_clock>, <&codec_clock>;
> > > 		clock-names = "cpu", "codec";
> > > 		clock-ranges;
> > > 		...
> > > 		simple-audio-card,cpu {
> > > 			sound-dai = <&xxx>;
> > > 		};
> > > 		simple-audio-card,codec {
> > > 			sound-dai = <&xxx>;
> > > 		};
> > > 	};
> > > 
> > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > I don't see any reason why we need this patch though. The binding
> > works as is, so supporting different styles doesn't seem like a
> > good idea to me. Let's just keep what we have? Even if a sub-node
> > like cpu or codec gets more than one element in the clocks list
> > property, we can make that work by passing a clock-name then
> > based on some sort of other knowledge.
> 
> OK, thanks. Let's skip this patch.
> But I believe this idea/method itself is not wrong (?)
> 

Right it's not wrong, just seems confusing to have two methods.
Kuninori Morimoto Dec. 9, 2016, 12:55 a.m. UTC | #5
Hi Stephen

> > > I don't see any reason why we need this patch though. The binding
> > > works as is, so supporting different styles doesn't seem like a
> > > good idea to me. Let's just keep what we have? Even if a sub-node
> > > like cpu or codec gets more than one element in the clocks list
> > > property, we can make that work by passing a clock-name then
> > > based on some sort of other knowledge.
> > 
> > OK, thanks. Let's skip this patch.
> > But I believe this idea/method itself is not wrong (?)
> > 
> Right it's not wrong, just seems confusing to have two methods.

Thanks.
Very clear for me :)
--
To unsubscribe from this list: send the line "unsubscribe linux-clk" 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/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
index c7a9393..43a710b 100644
--- a/Documentation/devicetree/bindings/sound/simple-card.txt
+++ b/Documentation/devicetree/bindings/sound/simple-card.txt
@@ -86,6 +86,7 @@  Optional CPU/CODEC subnodes properties:
 					  in dai startup() and disabled with
 					  clk_disable_unprepare() in dai
 					  shutdown().
+					  see Clock Example.
 
 Example 1 - single DAI link:
 
@@ -199,3 +200,34 @@  sound {
 		clocks = ...
 	};
 };
+
+
+Clock Example 1 - clock settings on each subnode
+
+sound {
+	...
+	simple-audio-card,cpu {
+		sound-dai = <&xxx>;
+		clocks = <&cpu_clock>;
+	};
+	simple-audio-card,codec {
+		sound-dai = <&xxx>;
+		clocks = <&codec_clock>;
+	};
+};
+
+Clock Example 2 - clock settings by clocks
+
+sound {
+	...
+	clocks = <&cpu_clock>, <&codec_clock>;
+	clock-names = "cpu", "codec";
+	clock-ranges;
+	...
+	simple-audio-card,cpu {
+		sound-dai = <&xxx>;
+	};
+	simple-audio-card,codec {
+		sound-dai = <&xxx>;
+	};
+};
diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index 4924575..c3031a5 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -104,15 +104,52 @@  int asoc_simple_card_parse_clk(struct device *dev,
 			       struct asoc_simple_dai *simple_dai)
 {
 	struct clk *clk;
+	const char *con_id = NULL;
+	const char *port_name[] = {
+		"cpu", "codec"
+	};
 	u32 val;
 
 	/*
+	 * We can use this style if "con_id" is not NULL
+	 *
+	 * sound {
+	 *	...
+	 *	clocks = <&xxx>, <&xxx>;
+	 *	clock-names = "cpu", "codec";
+	 *	clock-ranges;
+	 *
+	 *	simple-audio-card,cpu {
+	 *		sound-dai = <&xxx>;
+	 *	};
+	 *	simple-audio-card,codec {
+	 *		sound-dai = <&xxx>;
+	 *	};
+	 * };
+	 */
+	if (of_find_property(dev->of_node, "clock-names", NULL)) {
+		int i;
+		int port_len, node_len;
+
+		for (i = 0; i < ARRAY_SIZE(port_name); i++) {
+			node_len = strlen(node->name);
+			port_len = strlen(port_name[i]);
+
+			if (0 == strncmp(node->name + node_len - port_len,
+					 port_name[i], port_len)) {
+				con_id = port_name[i];
+				break;
+			}
+		}
+	}
+
+	/*
 	 * Parse dai->sysclk come from "clocks = <&xxx>"
 	 * (if system has common clock)
 	 *  or "system-clock-frequency = <xxx>"
 	 *  or device's module clock.
 	 */
-	clk = devm_get_clk_from_child(dev, node, NULL);
+	clk = devm_get_clk_from_child(dev, node, con_id);
 	if (!IS_ERR(clk)) {
 		simple_dai->sysclk = clk_get_rate(clk);
 	} else if (!of_property_read_u32(node, "system-clock-frequency", &val)) {