Re: [PATCH v8 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver

[Stephen Warren <swarren@wwwdotorg.org>]

On 08/19/2013 06:08 AM, Nicolin Chen wrote:
> This patch implements a device-tree-only CPU DAI driver for Freescale
> S/PDIF controller that supports stereo playback and record feature.

> diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.txt b/Documentation/devicetree/bindings/sound/fsl,spdif.txt

> +Freescale Sony/Philips Digital Interface Format (S/PDIF) Controller
> +
> +The Freescale S/PDIF audio block is a stereo transceiver that allows the
> +processor to receive and transmit digital audio via an coaxial cable or
> +a fibre cable.

Well, the cable type is more a property of the components that are
hooked to the SoC signals that this module drives, so are somewhat out
of scope of this binding document. However, this is nit-picky comment,
and I'm not too worried about it.

> +  - clock-names : Includes the following entries:
> +	"core"		The core clock of spdif controller
> +	"rxtx<0-7>"	Clock source list for tx and rx clock.
> +			This clock list should be identical to
> +			the source list connecting to the spdif
> +			clock mux in "SPDIF Transceiver Clock
> +			Diagram" of SoC reference manual. It
> +			can also be referred to TxClk_Source
> +			bit of register SPDIF_STC.

So, the HW block has 1 clock input, yet there's a mux somewhere else in
the SoC which has 8 inputs?

If so, I'm not completely sure it's correct to reference anything other
than the "core" clock in this binding. I think the other clocks would be
more suitably represented in the system-level "sound card" binding that
I guess patch 2/2 (which I haven't read yet) adds, since I assume those
clock are more to do with system-level clock tree setup decisions, and
might not even exist in some other SoC that included this IP block.

What do others think, assuming I'm correct about my HW design assumptions?