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

Hi Mark,

   Thank you for the commenst. I'll Fix them in v8.

   Here are some remaining question:

On Mon, Aug 19, 2013 at 10:18:09AM +0100, Mark Rutland wrote:

quoted

+Required properties:
+
+  - compatible : Compatible list, contains "fsl,<chip>-spdif".

What are valid values for <chip>? The binding should mention this. There
are bindings that don't, but they need to be fixed. Undocumented ABIs
are a bad idea.

I see, so 'Compatible list, must contains "fsl,imx35-spdif"' would be okay?

quoted

+  - interrupts : Contains spdif interrupt.

Is that the only interrupt the device generates?

Yes, how could I improve this description?

quoted

+       "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.

Could you elaborate on the last sentence? I'm not sure exactly what you
meant.

The list is also identical to the TxClk_Source bit value list of
register SPDIF_STC.

quoted

+
+Example:
+
+spdif: spdif@02004000 {
+       compatible = "fsl,imx6q-spdif",
+               "fsl,imx35-spdif";

Is "fsl,imx35-spdif" necessary in the list, or is it not the case all
"fsl,<chip>-spdif" variants are compatible with it?

That should be mentioned along with the list of valid compatible
strings.

I guess it's better to drop the 'imx6q-spdif' here?


Thank you,
Nicolin