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

[Mark Rutland]

On Mon, Aug 19, 2013 at 11:13:04AM +0100, Nicolin Chen wrote:
> On Mon, Aug 19, 2013 at 10:54:33AM +0100, Mark Rutland wrote:
> > > I see, so 'Compatible list, must contains "fsl,imx35-spdif"' would be okay?
> > 
> > While that needs to be mentioned, other values which might be present
> > (e.g. "fsl,imx6q-spdif") must be mentioned, or we can't rely on them
> > if we want to use them in future from drivers to provide additional
> > information (and thus they're useless).
> 
> Well, imx6q-spdif is identical to the imx35-spdif. So I think the
> 'must contains 35' would be enough for the current case. If any
> future modification happens to the list, I can update this doc later.
> 
> > > 
> > > > > +  - interrupts : Contains spdif interrupt.
> > > > 
> > > > Is that the only interrupt the device generates?
> > > 
> > > Yes, how could I improve this description?
> > 
> > It's probably not possible to make it much clearar to be honest,
> > "Contains the sole interrupt generated by the device" might be a little
> > overkill.
> 
> Then I keep it no-change :)

Ok :)

> 
> 
> > > > > +       "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.
> 
> > > The list is also identical to the TxClk_Source bit value list of
> > > register SPDIF_STC.
> > 
> > What I don't understand is how the value of the SPDIF_STC register
> > within the spdif IP block can describe the necessary details of clocks
> > coming from an external clock provider.
> >
> > What do the TxClk_Source bits represent? The configuration of the clock
> > inputs on the spdif IP block, or the outputs on some clock provider? Are
> > they writeable, or configured at integration time? If the clock provider
> > were replaced with another arbitrary clock provider, what would they
> > represent?
> 
> 
> Actually there's a clock mux for TxClk and it's connecting with 8 clock 
> sources. So the TxClk_Source bit show the connection between its value
> with the correspond source on the clock mux:
> 
> TxClk_Source	000 XTAL clk input
> 		001 CCM spdif0_clk_root input
> 		010 asrc_clk input
> 		011 spdif_extclk input, from pads
> 		100 esai_hckt input
> 		101 frequency divided ipg_clk input
> 		110 mlb_clk input
> 		111 mlb phy clk input

Ah. So are these the actual input names on the mux, or the names of the
outputs that are wired up to the mux? If the former, these may be better
clock-names than rxtx<0-7>.

Is there a similar Rx mux?

Given the "rxtx<0-7>" names you've given these clocks, are there 8 clock
inputs that get duplicated within the spdif block and fed into both
muxes, or are there 16 external inputs that happen to be two groups of 8
identical sets of clocks in systems so far?

> 
> 
> > > I guess it's better to drop the 'imx6q-spdif' here?
> > 
> > That depends:
> > 
> > * If the two IP blocks are identical, only the "imx35-spdif" name is
> >   necessary, and we can forget about "fsl,imx6q-spdif".
> > 
> > * If "fsl,imx6q-spdif" is a strict superset of "fsl,imx35-spdif", having
> >   both names documented and in a compatible list for a "fsl,imx6q-spdif"
> >   device makes sense.
> > 
> > * If "fsl,imx6q-spdif" is a variation of "fsl,imx35-spdif", and the
> >   "fsl,imx6q-spdif" cannot always be treated identically to a
> >   "fsl,imx35-spdif", then it makes sense to have separate compatible
> >   strings, with a device being listed as either "fsl,imx6q-spdif" or
> >   "fsl,imx35-spdif".
> > 
> > I don't know enough about the hardware to make that judgement call.
> 
> Thank you for explaining! I will choose A, because they are internally
> identical except their external clock sources.

Ok, that sounds like the right course of action to me.

Thanks,
Mark.