Re: [PATCH v2 3/3] ASoC: da7213: Add bindings documentation for codec driver

[Rob Herring]

On Thu, Oct 8, 2015 at 3:42 AM, Opensource [Adam Thomson]
<Adam.Thomson.Opensource@xxxxxxxxxxx> wrote:
> On October 07, 2015 17:22, Rob Herring wrote:
>
>> > +- dlg,micbias1-lvl : Voltage (mV) for Mic Bias 1
>> > +       [<1600>, <2200>, <2500>, <3000>]
>> > +- dlg,micbias2-lvl : Voltage (mV) for Mic Bias 2
>> > +       [<1600>, <2200>, <2500>, <3000>]
>>
>> Please append the units (-microvolt).
>
> Given that a user needs to read the bindings document to understand what is
> available, and what they're for, this seems a little unnecessary.

You may think so, but it is standard practice.

>>
>> > +- dlg,dmic-data-sel : DMIC channel select based on clock edge.
>> > +       ["lrise_rfall", "lfall_rrise"]
>> > +- dlg,dmic-samplephase : When to sample audio from DMIC.
>> > +       ["on_clkedge", "between_clkedge"]
>>
>> How about boolean for these two.
>
> Wanted these to be explicit, hence not choosing boolean. Would prefer to keep
> them as is.
>
>>
>> > +- dlg,dmic-clkrate : DMIC clock frequency (MHz).
>> > +       [<1500000>, <3000000>]
>>
>> So 1.5GHz or 3GHz?
>>
>> Add units (-hz).
>
> Agreed, the description of MHz is misleading so will update to Hz. Thanks.
> Again though, I don't see what the suffix gives you, and would prefer to leave
> the binding as is.

It tells someone reading the dts what the units are without having to
find the documentation and helps prevent people using properties with
differing units.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/