On Sun, Apr 9, 2017 at 7:45 PM, Kuninori Morimoto kuninori.morimoto.gx@renesas.com wrote:
Hi Rob, again
+{
struct device_node *node;
struct device_node *endpoint;
int i, id;
node = of_graph_get_port_parent(ep);
i = 0;
id = -1;
for_each_endpoint_of_node(node, endpoint) {
if (endpoint == ep)
id = i;
I don't see how this works when you have 1 DAI controller with multiple endpoints versus multiple DAI controllers with a single endpoint each. All the IDs will be 0 in the latter case.
It support 1:1 endpoint pattern only.
Then the endpoint id is always 0 and this function is pointless.
Sorry, I checked my patch-list, and I noticed that this function will be expand to use callback function in next patch-set (= HDMI support). Thus, inded current function is pointless at this point. I will merge this expansion patch in v5
1 correction.
sound graph might have multi ports (= not multi endpoint), like below. Each endpoints are 1:1.
ports { port { endpoint }; /* ID = 0 */
port@0
port { endpoint }; /* ID = 1 */
port@1, etc.
port { endpoint }; /* ID = 2 */
These ports are audio channels? If these are 3 separate data paths, then this is correct. If you have a single data path with multiple connections (e.g. a mux), then that should be a single port with multiple endpoints. For example, a design that routes the same I2S interface to HDMI and a codec and their use is mutually exclusive. I imagine you will need to support both.
The pattern I prefer to see calling graph functions is that drivers are specific about which port and endpoint number for a parent node they want. Not just searching the graph for any match.
};
1 question
It will support HDMI sound feature, thus I separated it into OF-graph (= this patch-set) and HDMI (= next patch-set). Should I merge it ?
I think so if it affects the functions here. It seems better to let the driver controlling the DAI determine the id mapping than trying to do it in the core.
Below is the expansion patch for HDMI support
Subject: [PATCH 14/63] ASoC: add .of_xlate_dai_id() callback
ALSA SoC needs to know connected DAI ID for probing. It is not a big problem if device/driver was only for sound, but getting DAI ID will be difficult if device includes both Video/Sound, like HDMI. To solve this issue, this patch adds new .of_xlate_dai_id callback on component driver, and use it from snd_soc_get_dai_id()
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
include/sound/soc.h | 3 +++ sound/soc/soc-core.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 95abbcb..0055fa0 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -14,6 +14,7 @@ #define __LINUX_SND_SOC_H
#include <linux/of.h> +#include <linux/of_graph.h> #include <linux/platform_device.h> #include <linux/types.h> #include <linux/notifier.h> @@ -793,6 +794,8 @@ struct snd_soc_component_driver { int (*of_xlate_dai_name)(struct snd_soc_component *component, struct of_phandle_args *args, const char **dai_name);
int (*of_xlate_dai_id)(struct snd_soc_component *comment,
struct device_node *endpoint); void (*seq_notifier)(struct snd_soc_component *, enum snd_soc_dapm_type, int subseq); int (*stream_event)(struct snd_soc_component *, int event);
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 7a10522..07e4eec 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -4042,12 +4042,45 @@ unsigned int snd_soc_of_parse_daifmt(struct device_node *np,
int snd_soc_get_dai_id(struct device_node *ep) {
struct snd_soc_component *pos; struct device_node *node; struct device_node *endpoint; int i, id;
int ret; node = of_graph_get_port_parent(ep);
/*
* For example HDMI case, HDMI has video/sound port,
* but ALSA SoC needs sound port number only.
* Thus counting HDMI DT port/endpoint doesn't work.
* Then, it should have .of_xlate_dai_id
Perhaps you should always require this function and provide a default implementation (or several).
*/
ret = -ENOTSUPP;
mutex_lock(&client_mutex);
list_for_each_entry(pos, &component_list, list) {
struct device_node *component_of_node = pos->dev->of_node;
if (!component_of_node && pos->dev->parent)
component_of_node = pos->dev->parent->of_node;
if (component_of_node != node)
continue;
if (pos->driver->of_xlate_dai_id)
ret = pos->driver->of_xlate_dai_id(pos, ep);
break;
}
mutex_unlock(&client_mutex);
if (ret != -ENOTSUPP)
return ret;
/*
* Non HDMI sound case, counting port/endpoint on its DT
* is enough. Let's count it.
*/ i = 0; id = -1; for_each_endpoint_of_node(node, endpoint) {
-- 1.9.1
Best regards
Kuninori Morimoto