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

2013-08-14 Thread Nicolin Chen
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


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

2013-08-14 Thread Nicolin Chen
Hi Philipp,

   Thank again for the comments.

On Wed, Aug 14, 2013 at 02:06:24PM +0200, Philipp Zabel wrote:
> ==
> From i.MX53 reference manual:
> 
>  if (DPLL Locked) SPDIF_RxClk else extal
> 0001 if (DPLL Locked) SPDIF_RxClk else spdif_clk
> 0011 if (DPLL Locked) SPDIF_RxClk else asrc_clk
> 0100 if (DPLL Locked) SPDIF_Rxclk else esai_hckt
> 0101 extal_clk
> 0110 spdif_clk
> 1000 asrc_clk
> 1001 esai_hckt
> 1010 if (DPLL Locked) SPDIF_RxClk else mlb_clk
> 1011 if (DPLL Locked) SPDIF_RxClk else camp_clk
> 1100 mkb_clk
> 1101 camp_clk
> ==
> 
> To me this looks like the device tree should just contain the list of
> unique clock inputs using phandles.
>   /* for i.MX6Q: */
>   clocks = <&...>;
>   clock-names = "xtal", "spdif", "asrc",
> "spdif_ext", "esai", "mlb";
> 
>   /* for i.MX53: */
>   clocks = <&...>;
>   clock-names = "xtal", "spdif", "asrc",
> "esai", "mlb", "camp";
> 
> The driver could contain this list of named inputs to the multiplexer
> and the DPLL locking information for each SoC version. The per-clock
> DPLL locking bit shouldn't be in the device tree at all.

I understand your point. I'll put the DPLL-locking info to the driver.
And I just found that the DPLL-lock condition seems to be fixed within
the range -- {0x0 ~ 0x4, 0xa ~ 0xb} (I checked i.MX6Q/6SL/53/35, all
of them are in such pattern.) So I don't need to put them into DT.

I'll revise it in next ver.

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

2013-08-14 Thread Sascha Hauer
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: [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver

2013-08-14 Thread Philipp Zabel
Am Mittwoch, den 14.08.2013, 16:48 +0800 schrieb Nicolin Chen:
> 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
> ==

==
>From i.MX53 reference manual:

 if (DPLL Locked) SPDIF_RxClk else extal
0001 if (DPLL Locked) SPDIF_RxClk else spdif_clk
0011 if (DPLL Locked) SPDIF_RxClk else asrc_clk
0100 if (DPLL Locked) SPDIF_Rxclk else esai_hckt
0101 extal_clk
0110 spdif_clk
1000 asrc_clk
1001 esai_hckt
1010 if (DPLL Locked) SPDIF_RxClk else mlb_clk
1011 if (DPLL Locked) SPDIF_RxClk else camp_clk
1100 mkb_clk
1101 camp_clk
==

To me this looks like the device tree should just contain the list of
unique clock inputs using phandles.
/* for i.MX6Q: */
clocks = <&...>;
clock-names = "xtal", "spdif", "asrc",
  "spdif_ext", "esai", "mlb";

/* for i.MX53: */
clocks = <&...>;
clock-names = "xtal", "spdif", "asrc",
  "esai", "mlb", "camp";

The driver could contain this list of named inputs to the multiplexer
and the DPLL locking information for each SoC version. The per-clock
DPLL locking bit shouldn't be in the device tree at all.

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

regards
Philipp

___
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

2013-08-14 Thread Nicolin Chen
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

2013-08-14 Thread Sascha Hauer
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: [PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver

2013-08-14 Thread Nicolin Chen
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.

Thank you,
Nicolin Chen



___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev


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

2013-08-14 Thread Sascha Hauer
On Mon, Aug 12, 2013 at 08:05:27PM +0800, Nicolin Chen wrote:
> This patch add S/PDIF controller driver for Freescale SoC.
> 
> Signed-off-by: Nicolin Chen 
> +
> +Required properties:
> +
> +  - compatible : Compatible list, contains "fsl,-spdif". Using general
> +  "fsl,fsl-spdif" will get the default SoC type -- imx6q-spdif.
> +
> +  - reg : Offset and length of the register set for the device.
> +
> +  - interrupts : Contains spdif interrupt.
> +
> +  - dmas : Generic dma devicetree binding as described in
> +  Documentation/devicetree/bindings/dma/dma.txt.
> +
> +  - dma-names : Two dmas have to be defined, "tx" and "rx".
> +
> +  - clocks : Contains an entry for each entry in clock-names.
> +
> +  - clock-names : Includes the following entries:
> + nametypecomments
> + "core"  RequiredThe core clock of spdif controller
> + "rx"OptionalRx clock source for spdif record.
> + If absent, will use core clock.
> + "tx"OptionalTx clock source for spdif playback.
> + If absent, will use core clock.
> + "tx-32000"  OptionalTx clock source for 32000Hz sample rate
> + playback. If absent, will use tx clock.
> + "tx-44100"  OptionalTx clock source for 44100Hz sample rate
> + playback. If absent, will use tx clock.
> + "tx-48000"  OptionalTx clock source for 48000Hz sample rate
> + playback. If absent, will use tx clock.
> +
> +  - 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 {
> + compatible = "fsl,imx6q-spdif";
> + reg = <0x02004000 0x4000>;
> + interrupts = <0 52 0x04>;
> + dmas = <&sdma 14 18 0>,
> +<&sdma 15 18 0>;
> + dma-names = "rx", "tx";
> +
> + 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.

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


[PATCH v4 resent 1/2] ASoC: fsl: Add S/PDIF CPU DAI driver

2013-08-12 Thread Nicolin Chen
This patch add S/PDIF controller driver for Freescale SoC.

Signed-off-by: Nicolin Chen 
---
 .../devicetree/bindings/sound/fsl,spdif.txt|  100 ++
 sound/soc/fsl/Kconfig  |3 +
 sound/soc/fsl/Makefile |2 +
 sound/soc/fsl/fsl_spdif.c  | 1350 
 sound/soc/fsl/fsl_spdif.h  |  223 
 5 files changed, 1678 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/fsl,spdif.txt
 create mode 100644 sound/soc/fsl/fsl_spdif.c
 create mode 100644 sound/soc/fsl/fsl_spdif.h

diff --git a/Documentation/devicetree/bindings/sound/fsl,spdif.txt 
b/Documentation/devicetree/bindings/sound/fsl,spdif.txt
new file mode 100644
index 000..e95318e
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/fsl,spdif.txt
@@ -0,0 +1,100 @@
+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.
+
+Required properties:
+
+  - compatible : Compatible list, contains "fsl,-spdif". Using general
+  "fsl,fsl-spdif" will get the default SoC type -- imx6q-spdif.
+
+  - reg : Offset and length of the register set for the device.
+
+  - interrupts : Contains spdif interrupt.
+
+  - dmas : Generic dma devicetree binding as described in
+  Documentation/devicetree/bindings/dma/dma.txt.
+
+  - dma-names : Two dmas have to be defined, "tx" and "rx".
+
+  - clocks : Contains an entry for each entry in clock-names.
+
+  - clock-names : Includes the following entries:
+   nametypecomments
+   "core"  RequiredThe core clock of spdif controller
+   "rx"OptionalRx clock source for spdif record.
+   If absent, will use core clock.
+   "tx"OptionalTx clock source for spdif playback.
+   If absent, will use core clock.
+   "tx-32000"  OptionalTx clock source for 32000Hz sample rate
+   playback. If absent, will use tx clock.
+   "tx-44100"  OptionalTx clock source for 44100Hz sample rate
+   playback. If absent, will use tx clock.
+   "tx-48000"  OptionalTx clock source for 48000Hz sample rate
+   playback. If absent, will use tx clock.
+
+  - 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 {
+   compatible = "fsl,imx6q-spdif";
+   reg = <0x02004000 0x4000>;
+   interrupts = <0 52 0x04>;
+   dmas = <&sdma 14 18 0>,
+  <&sdma 15 18 0>;
+   dma-names = "rx", "tx";
+
+   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";
+
+   status = "okay";
+};
+
+Example2:
+
+spdif: spdif@02004000 {
+   compatible = "fsl,imx6sl-spdif";
+   reg = <0x02004000 0x4000>;
+   interrupts = <0 52 0x04>;
+   dmas = <&sdma 14 18 0>,
+  <&sdma 15 18 0>;
+   dma-names = "rx", "tx";
+
+   clocks = <&clks 122>;
+   clock-names = "core";
+   rx-clksrc-lock;
+   rx-clksrc-names =
+   "lock.xtal", "lock.spdif", "null", "lock.spdif_ext",
+   "null", "xtal", "spdif", "null", "spdif_ext", "null",
+   "null", "null", "mlb";
+   tx-clksrc-names =
+   "xtal", "spdif", "null", "spdif_ext",
+   "null", "ipg";
+
+   status = "okay";
+};
diff --git a/sound/soc/fsl/Kconfig b/sound/soc/fsl/Kconfig
index e15f771..2c518db 100644
--- a/sound/soc/fsl/Kconfig
+++ b/sound/soc/fsl/Kconfig
@@ -1,