[alsa-devel] [PATCH 4/4 v6] ASoC: simple-card: add Device Tree support

[Lars-Peter Clausen]

On 02/20/2013 01:48 AM, Kuninori Morimoto wrote:
> 
> Hi Lars
> 
> Thank you for checking patch
> 
>>> +- simple-audio,dev			: phandle and port for CPU/CODEC
>>> +- simple-audio,frame-master		: frame master
>>> +- simple-audio,bitclock-master		: bitclock master
>>> +#sound-dai-cells integer is required on simple-audio,dev phandle's node
>>
>> Shouldn't the names of '#sound-dai-cells' and 'simple-audio,dev' kind of
>> match? E.g. '#sound-dai-cells' and 'sound-dai'. Maybe drop the sound, since
>> the a in dai kind of implies this.
> 
> Thank you, but I would like to keep "simple-audio" name for it too.
> So, can I use
> simple-audio-dai
> #simple-audio-dai-cells

The requirement to be able to find the DAI via devicetree is not specific to
the simple-audio driver. I'd prefer if we could come up with a common
binding which can be used by multiple drivers. Since you also add the xlate
(or match) function to the sound core it makes sense - in my option - to
also have a generic phandle -> DAI look-up method in the core.

- Lars

> 
>>> +static struct device_node*
>>> +__asoc_simple_card_parse_of(struct device_node *np,
>>> +			    struct asoc_simple_dai *dai)
>>> +{
>>> +	struct device_node *node;
>>> +	char prop[128];
>>> +
>>> +	/* get "simple-audio,dev = <&phandle port>" */
>>> +	snprintf(prop, sizeof(prop), "simple-audio,dev");
>>
>> Why do you need the extra buffer? Can you just pass, "simple-audio,dev"
>> directly to of_parse_phandle?
> 
> Indeed, thank you.
> 
>>> +	node = of_parse_phandle(np, prop, 0);
>>> +	if (!node)
>>> +		return NULL;
>>> +
>>> +	of_node_put(node);
>>
>> You shouldn't drop the reference until you are done processing it. Which in
>> this case is only after the device has been unregistered, since you pass the
>> node on to the ASoC core.
> 
> I see, will fix
> 
> Best regards
> ---
> Kuninori Morimoto
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel