Re: [alsa-devel] [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
On Wed, Aug 14, 2013 at 04:48:02PM +0800, Nicolin Chen wrote:
Hi Sascha,
On Wed, Aug 14, 2013 at 09:50:17AM +0200, Sascha Hauer wrote:
+ - tx-clksrc-names : The names for all available clock sources for tx,
which
+ is also being listed in SoC reference manual, ClkSrc_Sel bit of
SPDIF_SRPC.
+ And the name list would be different between different SoC. Use 'null'
for
+ those unlisted names, and the max number of tx-clksrc-names should be
8.
+
+ - rx-clksrc-names : The names for all available clock sources for rx,
which
+ is also being listed in SoC reference manual, TxClk_Source bit of
SPDIF_STC.
+ And the name list would be different between different SoC. Use 'null'
for
+ those unlisted names, and the max number of rx-clksrc-names should be
16.
+
+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.
+
+Example1:
+
+spdif: spdif@02004000 {
+ clocks = clks 197;
+ clock-names = core;
+ rx-clksrc-lock;
+ rx-clksrc-names =
+ lock.ext, lock.spdif, lock.asrc,
+ lock.spdif_ext, lock.esai, ext,
+ spdif, asrc, spdif_ext, esai,
+ lock.mlb, lock.mlb_phy, mlb,
+ mlb_phy;
+ tx-clksrc-names =
+ xtal, spdif, asrc, spdif_ext,
+ esai, ipg, mlb, mlb_phy;
I had a hard time understanding what you are doing here.
With this the clk names in arch/arm/mach-imx/clk-imx6q.c become an API
between the Kernel and the devicetree. Don't do that.
There is a standardized devicetree binding for clocks. Use it.
I think I should first explain to you what this part is doing:
The driver needs to set Clk_source bit for TX/RX to select the
clock from a clock mux. The names listed above are those of the
clocks connecting to the mux, while they are not only internal
clocks which're included in clk-imx6q.c but also external ones,
an on-board external osc for example.
The driver does get the clock by using the standard DT binding,
see the 'clocks = clks 197' above, and then compare this
obtained clock-name with the name list to decide which value
should be set to the Clk_source bit.
==
ClkSrc_Sel from i.MX6Q reference manual:
Clock source selection, all other settings not shown are reserved:
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
==
So the name list here basically is not being used to obtain a
clock like what standardized DT binding does but to provide the
driver a full list to look up which value should be exactly used
according to the obtained clock.
I think I should revise the binding doc for these two lists. It
might be hard to explain within that kinda short paragraph.
Surely, if I misunderstand your point, please correct me. And
if you have any sage idea, please guide me.
Something like this:
clocks = clks 197, clks 3, clks 197, clks 107, clks SPDIF_EXT,
clks 118, clks 62, clks 139, clks MLB_PHY
clock-names = core, rxtx0, rxtx1, rxtx2, rxtx3, rxtx4, rxtx5,
rxtx6, rxtx7
This describes the different input clocks to the spdif core and also
gives a hint to the array index (rxtx_n_) to use.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [alsa-devel] [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
Hi, On Wed, Aug 14, 2013 at 11:56:52AM +0200, Sascha Hauer wrote: I think I should first explain to you what this part is doing: The driver needs to set Clk_source bit for TX/RX to select the clock from a clock mux. The names listed above are those of the clocks connecting to the mux, while they are not only internal clocks which're included in clk-imx6q.c but also external ones, an on-board external osc for example. The driver does get the clock by using the standard DT binding, see the 'clocks = clks 197' above, and then compare this obtained clock-name with the name list to decide which value should be set to the Clk_source bit. == ClkSrc_Sel from i.MX6Q reference manual: Clock source selection, all other settings not shown are reserved: 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 == So the name list here basically is not being used to obtain a clock like what standardized DT binding does but to provide the driver a full list to look up which value should be exactly used according to the obtained clock. I think I should revise the binding doc for these two lists. It might be hard to explain within that kinda short paragraph. Surely, if I misunderstand your point, please correct me. And if you have any sage idea, please guide me. Something like this: clocks = clks 197, clks 3, clks 197, clks 107, clks SPDIF_EXT, clks 118, clks 62, clks 139, clks MLB_PHY clock-names = core, rxtx0, rxtx1, rxtx2, rxtx3, rxtx4, rxtx5, rxtx6, rxtx7 This describes the different input clocks to the spdif core and also gives a hint to the array index (rxtx_n_) to use. Thank you for the idea, and..hmm..I'm a bit confused.. Is this really a nicer way? Actually the rx clock list and tx clock list are totally different. So doing this I have to list, in the maximum case, 24 (8 + 16) clock phandles for these two lists. And plussing another 6 I've listed in this binding doc -- thus there are totally 30 clock phanldes. But the 24 of 30 are only used to get two indexes. I think I need a little help here to understand why this is better. It looks more complicated to me. Creating the two name lists just because I can describe them in the dtsi of SoC, since they are totally fixed and identical to the SoC reference manual, while the clocks area can be remained for users to select the actual clocks (core/rx/tx/tx-32000/tx-44100/tx-48000), so they can configure these clocks in dts file of a specific board, and they don't need to touch the name lists any more. Thank you, Nicolin Chen ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [alsa-devel] [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
On Wed, Aug 14, 2013 at 06:23:46PM +0800, Nicolin Chen wrote: Hi, Surely, if I misunderstand your point, please correct me. And if you have any sage idea, please guide me. Something like this: clocks = clks 197, clks 3, clks 197, clks 107, clks SPDIF_EXT, clks 118, clks 62, clks 139, clks MLB_PHY clock-names = core, rxtx0, rxtx1, rxtx2, rxtx3, rxtx4, rxtx5, rxtx6, rxtx7 This describes the different input clocks to the spdif core and also gives a hint to the array index (rxtx_n_) to use. Thank you for the idea, and..hmm..I'm a bit confused.. Is this really a nicer way? Yes, since the clk names are not an API. Exposing them to the devicetree is not an option. The fact that the names are defined in arch/arm/mach-imx/clk-imx6q.c and are used in the spdif driver makes this really clear. Actually the rx clock list and tx clock list are totally different. So doing this I have to list, in the maximum case, 24 (8 + 16) clock phandles for these two lists. And plussing another 6 I've listed in this binding doc -- thus there are totally 30 clock phanldes. But the 24 of 30 are only used to get two indexes. The spdif core has 8 input clocks which have to be described in the devicetree. Nobody says the mapping which clock name corresponds to which bit combination has to be in the devicetree. Look at the possible clocks: 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 Only half of them actually are clocks. if (DPLL Locked) SPDIF_RxClk else ... is not a clock. Every sane hardware developer would have introduced a mux with 8 entries and an additional Use DPLL if possible bit. Now this is not the case here so we have to live with it and maintain the above table in the driver. And another one for the i.MX35 and still another one for i.MX53. I think I need a little help here to understand why this is better. As said, the clock names are not an API. Nobody changing clk-imx6q.c expects to break devicetree bindings. Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [alsa-devel] [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver
On Wed, Aug 14, 2013 at 02:19:37PM +0200, Sascha Hauer wrote: Yes, since the clk names are not an API. Exposing them to the devicetree is not an option. The fact that the names are defined in arch/arm/mach-imx/clk-imx6q.c and are used in the spdif driver makes this really clear. The spdif core has 8 input clocks which have to be described in the devicetree. Nobody says the mapping which clock name corresponds to which bit combination has to be in the devicetree. Thank you for the explain. I get your point and really appreciate it. Look at the possible clocks: 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 Only half of them actually are clocks. if (DPLL Locked) SPDIF_RxClk else ... is not a clock. Every sane hardware developer would have introduced a mux with 8 entries and an additional Use DPLL if possible bit. Now this is not the case here so we have to live with it and maintain the above table in the driver. And another one for the i.MX35 and still another one for i.MX53. I think I just have an idea for the table. I'll put them into the next version. Please take a look after I send it. Thank you, Nicolin Chen ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
