Re: [PATCH 2/3] ASoC: add snd_soc_of_get_port_dai_name()

[Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>]

On 02/05/2013 03:11 AM, Kuninori Morimoto wrote:
> This patch adds snd_soc_of_get_port_dai_name() to get
> dai name from device_node and port number.

> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c

> +const char *snd_soc_of_get_port_dai_name(struct device_node *of_node, int port)

OK, that function prototype seems like /almost/ the right kind of thing
to be doing.

"int port" should be "const struct of_phandle_args *portspec", since the
port ID could be encoded into device tree with more than one cell, and
the cells are uint32_t not int.

> +{
> +	struct snd_soc_dai *dai;
> +	int i = 0;
> +
> +	list_for_each_entry(dai, &dai_list, list) {
> +		if (dai->dev->of_node == of_node) {
> +			if (port == i)
> +				return dai->name;

Is this the reason for the previous patch? This is horribly wrong. The
only way this could work is with internal knowledge of how ASoC works,
yet the device tree must be written in a completely OS-/driver-agnostic
fashion, purely describing hardware.

This function should be implemented as follows:

1) Find the driver for dai->dev

2) Call an "of_xlate" function on that driver, passing it the "portspec"
I mentioned above; the driver will interpret the portspec, maps it to
the ASoC port name, and return it.

The DT binding for each audio component must define the list of valid
port IDs, and the number of cells used to encode the value.

See for example the GPIO and PWM subsystem's of_xlate functions.

> +
> +			i++;
> +		}
> +	}
> +
> +	return NULL;
> +}
> +EXPORT_SYMBOL_GPL(snd_soc_of_get_port_dai_name);