Hi Tomasz, Thank you for the comments. I'll revise them in v6. And below is my reply for you comments. On Thu, Aug 15, 2013 at 02:18:22PM +0200, Tomasz Figa wrote: > > + - clock-names : Includes the following entries: > > + name type comments > > + "core" Required The core clock of spdif controller > > + > > + "rx" Optional Rx clock source for spdif record. > > + If absent, will use core clock. > > + > > + "tx" Optional Tx clock source for spdif playback. > > + If absent, will use core clock. > > + > > + "tx-32000" Optional Tx clock source for 32000Hz sample rate > > + playback. If absent, will use tx clock. > > + > > + "tx-44100" Optional Tx clock source for 44100Hz sample rate > > + playback. If absent, will use tx clock. > > + > > + "tx-48000" Optional Tx clock source for 48000Hz sample rate > > + playback. If absent, will use tx clock. > > + > > + "src<0-7>" Optional Clock source list for tx and rx clock > > + to look up their clock source indexes. > > + This clock list should be identical to > > + the list of TxClk_Source bit value of > > + register SPDIF_STC. If absent or failed > > + to look up, tx and rx clock would then > > + ignore the "rx", "tx" "tx-32000", > > + "tx-44100", "tx-48000" clock phandles > > + and select the core clock as default > > + tx and rx clock. > > I suspect a little abuse of clocks property here. From the description of > "core" and "src<0-7>" clocks I assume that the IP can have up to 9 clock > inputs - core clock and up to 8 extra source clocks. Is it correct? > > If yes, this makes the "tx", "rx" and "tx-*" clocks describe > configuration, not hardware. IMHO it should be up to the driver which > source clocks to use for tx and rx channels and for each sampling rate. First, you are right that all the properties you just commented are software configurations. And I got the point that device tree now can't allow any software configuration even if the actual hardware connection will depend on it. If so, I would like to remove those abused clocks and also drop the unused clocks in src<0-7>, then just remain those needed clocks src. I think that can be plausible because there'll be no more clock abuse and the driver will be able to get the source index from the name 'src<num>'. And you are right about the 9 clock inputs, just there're not only 9 inputs but also an extra external clock from S/PDIF transmitter via coaxial cable or optical fiber -- RxCLK. Please check the following list: 0000 if (DPLL Locked) SPDIF_RxClk else extal 0001 if (DPLL Locked) SPDIF_RxClk else spdif_clk 0010 if (DPLL Locked) SPDIF_RxClk else asrc_clk 0011 if (DPLL Locked) SPDIF_RxClk else spdif_extclk 0100 if (DPLL Locked) SPDIF_Rxclk else esai_hckt 0101 extal_clk 0110 spdif_clk 0111 asrc_clk 1000 spdif_extclk 1001 esai_hckt 1010 if (DPLL Locked) SPDIF_RxClk else mlb_clk 1011 if (DPLL Locked) SPDIF_RxClk else mlb_phy_clk 1100 mkb_clk 1101 mlb_phy_clk When (DPLL Locked) condition matches, the rx clock can ignore the 8 input clocks from clock mux then use the external one from a S/PDIF transmitter. So for the below part: > > +Optional properties: > > + > > + - rx-clksrc-lock: This is a boolean property. If present, ClkSrc_Sel > > bit + of SPDIF_SRPC would be set a clock source that cares DPLL locked > > condition. + > > This again looks like software configuration, not hardware description. > Could you elaborate a bit more on meaning of this property? I think the rx-clksrc-lock property should be included in DT as well, since it's exactly a available clock source for rx. But I guess I just need to figure out a better way or a more elaborated description. Thank you, Nicolin Chen -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
