[alsa-devel] [PATCH v2 2/2] dt: binding: sound cs42l52 driver

[Brian Austin]

>> +  - compatible : "cirrus,cs42l52"
>> +
>> +  - reg : the I2C address of the device for I2C
>> +
>> +Optional properties:
>> +
>> +  - reset-gpio : GPIO controller's phandle and the number
>> +		 of the GPIO used to reset the codec.
>
> As Kumar has already mentioned, all device-specific properties should be
> prefixed with vendor prefix, "cirrus," in this case.
>

I can do that, Thanks


>> +  - chgfreq    : Charge Pump Frequency values. Allowable values of
>> +		 0x00 through 0x0F.
>> +		 Frequency = (64xFs)/(N+2)
>
> I suppose N means the value of chgfreq property here, but this should be
> clearly stated. Same for Fs - I suppose it is sampling frequency, but
> is it default, minimum, maximum or maybe something else?
>
Frequency = (64xFs)/(N+2)
N = chgfreq value
Fs = sample rate


> Personally I don't like this kind of raw values being passed using Device
> Tree, but this one can't be really represented reasonably in a generic
> way (such as in Hz units) without too much effort to calculate original
> value in the driver, then it's fine.
>
>> +  - mica-cfg   : MIC A single-ended or differential select.
>> +		 0x00 = Single-Ended
>> +		 0x01 = Differential
>
> I'd rather make it boolean and call it cirrus,mic-a-differential.
>
>> +  - micb-cfg   : MIC B single-ended or differential select.
>> +		 0x00 = Single-Ended
>> +		 0x01 = Differential
>
> Ditto.
>

I can do that as well. Thanks

>> +  - mica-sel   : MIC A single ended input select.  For Single-Ended
>> +		 configuration, select which MIC to use.
>> +		 0x00 = MIC1
>> +		 0x01 = MIC2
>> +  - micb-sel   : MIC B single ended input select.  For Single-Ended
>> +		 configuration, select which MIC to use.
>> +		 0x00 = MIC1
>> +		 0x01 = MIC2
>
> Could you explain what are MIC A, MIC B, MIC1 and MIC2?
>
I can add a little more but it is best explained in the datasheet. I can 
add a little more explaination and reference the datasheet section. I feel 
this file might be the wrong place to go into too much depth of the pieces 
of the device when the datasheet could be referenced. Would a reference be 
OK?


>> +  - micbias-lvl: Set the output voltage level on the MICBIAS Pin
>> +		 0x00 = 0.5 x VA
>> +		 0x01 = 0.6 x VA
>> +		 0x02 = 0.7 x VA
>> +		 0x03 = 0.8 x VA
>> +		 0x04 = 0.83 x VA
>> +		 0x05 = 0.91 x VA
>
> For such small range of values, decimal representation is preferred from
> readability reasons.
>
OK, Thanks
>> +
>> +Example:
>> +
>> +codec: cs42l52 at 4a {
>
> coding style: Node name should be generic, i.e. codec at 4a.

I can change this as well.  While we are talking about coding style, is 
there a new format document somewhere with the style that was agreed to at 
the conference just recently?

Thanks,
Brian