[alsa-devel] [PATCH v4] ASoC: simple-card: add Device Tree support

[Rob Herring]

On Fri, Nov 15, 2013 at 9:50 AM, Mark Rutland <mark.rutland at arm.com> wrote:
> On Fri, Oct 25, 2013 at 03:14:20AM +0100, Kuninori Morimoto wrote:
>> Support for loading the simple-card module via DeviceTree.
>> It requests CPU/CODEC information.
>>
>> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
>> ---
>> v3 -> v4
>>
>>  - explain detail of each properties on simple-card.txt
>>  - fixup odd examples on simple-card.txt
>>  - remove "simple-card,card-name". create it from cpu/codec name
>>  - use of_get_child_by_name()
>>  - remove odd pointer info from dev_dbg()
>>  - remove subnode format which are no longer needed
>>
>> This is based on asoc/topic/simple branch
>>
>>  .../devicetree/bindings/sound/simple-card.txt      |   73 +++++++++
>>  sound/soc/generic/simple-card.c                    |  156 +++++++++++++++++++-
>>  2 files changed, 223 insertions(+), 6 deletions(-)
>>  create mode 100644 Documentation/devicetree/bindings/sound/simple-card.txt
>>
>> diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt
>> new file mode 100644
>> index 0000000..4871e91
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/simple-card.txt
>> @@ -0,0 +1,73 @@
>> +Simple-Card:
>> +
>> +Simple-Card specifies audio DAI connection of SoC <-> codec.
>> +
>> +Required properties:
>> +
>> +- compatible                         : "simple-audio"

Still not really a fan of this generic name. Can we define in the
description above what simple means.

>> +
>> +Optional properties:
>> +
>> +- simple-audio,format                        : CPU/CODEC common audio format.
>> +                                     "i2s", "right_j", "left_j" , "dsp_a"
>> +                                     "dsp_b", "ac97", "pdm", "msb", "lsb"
>> +Required subnodes:
>> +
>> +- simple-audio,cpu                   : CPU   sub-node
>> +- simple-audio,codec                 : CODEC sub-node
>> +
>> +Required CPU/CODEC subnodes properties:
>> +
>> +- sound-dai                          : phandle and port of CPU/CODEC
>
> Is there a class binding for audio devices this derives from?
>
>> +
>> +Optional CPU/CODEC subnodes properties:
>
> Do these all apply to both sub-nodes?
>
>> +- frame-master                               : bool property. add this if subnode was frame master
>> +- bitclock-master                    : bool property. add this if subnode was bitclock master
>
> s/was/is/
>
>> +- bitclock-inversion                 : bool property. add this if subnode has clock inversion
>> +- frame-inversion                    : bool property. add this if subnode has frame inversion
>> +- clocks / system-clock-frequency    : specify subnode's clock if needed.
>> +                                       it can be specified via "clocks" if system has clock node,
>> +                                          or "system-clock-frequency" if system doesn't have it.
>
> What does "if system doesn't have it" mean? If it doesn't have a clock,
> how does said non-existent clock have a frequency?
>
> It would be possible to use a fixed-clock in place of
> system-clock-frequency, which would make the binding more consistent and
> the driver simpler, at the cost of making the dt marginally more
> complex.

Just plain "clock-frequency" is fairly standard, so please use that
instead. Unless there is need for a fixed-clock to be routed to
several nodes, then I think using the more simple clock-frequency here
is fine.

Can both sub-nodes really have different clocks? Seems like that would
exceed the definition of simple.

Rob