Re: [PATCH 4/4] ASoC: omap-abe-twl6040: Add device tree support

[Peter Ujfalusi <peter.ujfalusi@ti.com>]

On 05/08/2012 04:59 PM, Mark Brown wrote:
> On Tue, May 08, 2012 at 03:15:14PM +0300, Peter Ujfalusi wrote:
> 
>> 	abe-twl6040,jack_detection = <0>;
> 
> Shouldn't this be a boolean property that simply needs to be present to
> flag detection?

It does work like that as well. This property only need to be there (and
to be set to 1) if the board can detect the jack.

> 
>> 	dai-link1,codec = <&twl6040_codec>;
>> 	dai-link1,dai = <&mcpdm>;
> 
> This seems like it ought to be an array of nodes, one per link, instead
> of having an index in the property name.  It should certainly be easier
> to parse.

Very true. We can handle this in a similar way as we handle the
routings. It might be a good idea to add this as a generic ASoC core
feature?

Something like:
abe-twl6040,dai-link =
		<&twl6040_codec>, <&mcpdm>;

Other platforms could use this and we could handle it with the same code
in the core.

> It also seems like it's not at all board specific and should be factored
> out.
> 
>> 	abe-twl6040,audio-routing =
>> 		"Headset Stereophone", "HSOL",
>> 		"Headset Stereophone", "HSOR",
> 
> Binding should document what the board-defined nodes are.

OK, I'll update the documentation (also covering the codec)

> 
>> +Example:
>> +
>> +sound { /* ASoC */
>> +	compatible = "ti,abe-twl6040";
>> +	abe-twl6040,model = "SDP4430";
> 
> Please don't repeat your binding documentation in the changelog, it's
> already in the patch and means it comes after the documentation.

OK, I'll remove the examples from the changelog.

-- 
Péter