On 11/29/2012 05:35 PM, Kuninori Morimoto wrote:
Hi Stephen
I think it'd be more typical to represent as a single integer property, where the value is an enumeration indicating the type.
Sorry, I couldn't understand correctly this. Do you mean like this ?
snd.soc.daifmt.format = <3> /* SND_SOC_DAIFMT_LEFT_J */ snd.soc.daifmt.clock_gate = <1> /* SND_SOC_DAIFMT_CONT */ snd.soc.daifmt.inversion = <3> /* SND_SOC_DAIFMT_IB_NF */ snd.soc.daifmt.hw_clock = <2> /* SND_SOC_DAIFMT_CBS_CFM */
Sorry for the slow response.
What I meant was that rather than:
- ret |= __snd_soc_of_parse_daifmt(np, prefix, fmt,
"i2s", SND_SOC_DAIFMT_I2S);- ret |= __snd_soc_of_parse_daifmt(np, prefix, fmt,
"right_j", SND_SOC_DAIFMT_RIGHT_J);- ret |= __snd_soc_of_parse_daifmt(np, prefix, fmt,
"left_j", SND_SOC_DAIFMT_LEFT_J);- ret |= __snd_soc_of_parse_daifmt(np, prefix, fmt,
"dsp_a", SND_SOC_DAIFMT_DSP_A);- ret |= __snd_soc_of_parse_daifmt(np, prefix, fmt,
"dsp_b", SND_SOC_DAIFMT_DSP_B);
I'd expect to see something more like:
fmt = of_property_read_u32_array(np, "bit-format");
But perhaps your binding is representing the set of possible legal formats? I had assumed it was attempting to represent the single specific format to choose.
Related, I also wasn't suggesting combining format, clock-gate, inversion, ... into a single property.
I added SND_SOC_DAIFMT_xxx here, but this style is not readable/understandable for me. And it is difficult to keep compatible if the number was changed. (never happen ? I'm not sure)
Well, once a DT binding is created, you can't change the numbers, or you would break the ability for an old DT to work with a newer kernel.
How about to use string ?
snd.soc.daifmt.format = "left_j" snd.soc.daifmt.clock_gate = "cont" snd.soc.daifmt.inversion = "ib_nf" snd.soc.daifmt.hw_clock = "cbs_cfm"
That's probably the best we can do for now. Using a pre-processor would be best:
#define SND_SOC_DAIFMT_LEFT_J 3
snd.soc.daifmt.format = <SND_SOC_DAIFMT_LEFT_J>;
... but we can't do that yet...