diff mbox

[RFC,1/2] ARM: imx: add BYPASS support for PLL clocks

Message ID 20140829084125.GB3135@dragon (mailing list archive)
State New, archived
Headers show

Commit Message

Shawn Guo Aug. 29, 2014, 8:41 a.m. UTC
On Fri, Aug 29, 2014 at 11:33:47AM +0800, Shengjiu Wang wrote:
> Otherwise I still need to do some step in below to source the clock to ESAI.
> 
>         clk_set_parent(pll4_bypass_src, lvds2_in);
>         clk_set_parent(pll4_bypass, pll4_bypass_src);
>         clk_set_rate(pll4_audio_div, 24576000);
>         clk_set_rate(esai_extal, 24576000);
> 
> I think it is what we can expect, right? if yes, the patch is ok for ESAI.

Yes, we still need to set up parent and rate specifically for
sabreauto ESAI use case.

But these setup can be done in device tree now with commit 86be408bfbd8
(clk: Support for clock parents and rates assigned from device tree).
Here is change that I tested with.  (Not sure if the info in esai node
is all correct)



With above dts changes, I can get the following clock setup for ESAI.

 anaclk2                                  0            0    24576000          0
    lvds2_in                              0            0    24576000          0
       pll4_bypass_src                    0            0    24576000          0
          pll4_bypass                     0            0    24576000          0
             pll4_audio                   0            0    24576000          0
                pll4_post_div             0            0    24576000          0
                   pll4_audio_div           0            0    24576000          0
                      esai_sel            0            0    24576000          0
                         esai_pred           0            0    24576000          0
                            esai_podf           0            0    24576000          0
                               esai_extal           0            0    24576000          0

Shawn

Comments

Shengjiu Wang Aug. 29, 2014, 9:32 a.m. UTC | #1
On Fri, Aug 29, 2014 at 04:41:28PM +0800, Shawn Guo wrote:
> On Fri, Aug 29, 2014 at 11:33:47AM +0800, Shengjiu Wang wrote:
> > Otherwise I still need to do some step in below to source the clock to ESAI.
> > 
> >         clk_set_parent(pll4_bypass_src, lvds2_in);
> >         clk_set_parent(pll4_bypass, pll4_bypass_src);
> >         clk_set_rate(pll4_audio_div, 24576000);
> >         clk_set_rate(esai_extal, 24576000);
> > 
> > I think it is what we can expect, right? if yes, the patch is ok for ESAI.
> 
> Yes, we still need to set up parent and rate specifically for
> sabreauto ESAI use case.
> 
> But these setup can be done in device tree now with commit 86be408bfbd8
> (clk: Support for clock parents and rates assigned from device tree).
> Here is change that I tested with.  (Not sure if the info in esai node
> is all correct)
> 
Great. Thanks.

> diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> index 6692115b0138..9fe4169c79dd 100644
> --- a/arch/arm/boot/dts/imx6qdl.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> @@ -268,8 +268,18 @@
>                                 };
> 
>                                 esai: esai@02024000 {
> +                                       compatible = "fsl,imx6q-esai", "fsl,imx35-esai";
>                                         reg = <0x02024000 0x4000>;
>                                         interrupts = <0 51 IRQ_TYPE_LEVEL_HIGH>;
> +                                       clocks = <&clks IMX6QDL_CLK_ESAI_IPG>,
> +                                                <&clks IMX6QDL_CLK_ESAI_EXTAL>,
> +                                                <&clks IMX6QDL_CLK_ESAI_IPG>;
> +                                       clock-names = "core", "extal", "fsys";
> +                                       dmas = <&sdma 23 21 0>, <&sdma 24 21 0>;
> +                                       dma-names = "rx", "tx";
> +                                       fsl,fifo-depth = <128>;
> +                                       fsl,esai-synchronous;
we alway don't set fsl,fifo-depth and fsl,esai-synchronous, just use the
default value in driver.
> +                                       status = "disabled";
>                                 };
> 
>                                 ssi1: ssi@02028000 {
> 
> diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> index 009abd69385d..1084394197ad 100644
> --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> @@ -17,6 +17,14 @@
>                 reg = <0x10000000 0x80000000>;
>         };
> 
> +       clocks {
> +               anaclk2 { /* on-board 24.576MHz audio oscillator */
> +                       compatible = "fixed-clock";
> +                       #clock-cells = <0>;
> +                       clock-frequency = <24576000>;
> +               };
> +       };
> +
>         leds {
>                 compatible = "gpio-leds";
>                 pinctrl-names = "default";
> @@ -45,6 +53,17 @@
>         };
>  };
> 
> +&clks {
> +       assigned-clocks = <&clks IMX6QDL_PLL4_BYPASS_SRC>,
> +                         <&clks IMX6QDL_PLL4_BYPASS>,
> +                         <&clks IMX6QDL_CLK_ESAI_SEL>,
> +                         <&clks IMX6QDL_CLK_PLL4_POST_DIV>;
> +       assigned-clock-parents = <&clks IMX6QDL_CLK_LVDS2_IN>,
> +                                <&clks IMX6QDL_PLL4_BYPASS_SRC>,
> +                                <&clks IMX6QDL_CLK_PLL4_AUDIO_DIV>;
> +       assigned-clock-rates = <0>, <0>, <0>, <24576000>;
> +};
> +
>  &ecspi1 {
>         fsl,spi-num-chipselects = <1>;
>         cs-gpios = <&gpio3 19 0>;
> @@ -61,6 +80,12 @@
>         };
>  };
> 
> +&esai {
> +       assigned-clocks = <&clks IMX6QDL_CLK_ESAI_EXTAL>;
> +       assigned-clock-rates = <24576000>;
> +       status = "okay";
> +};
> +

Can we move the clocks for &esai to &clks ? I just think that will look better.
Anyway, this is just my view. you can do it follow the formal principle.

>  &fec {
>         pinctrl-names = "default";
>         pinctrl-0 = <&pinctrl_enet>;
> 
> 
> With above dts changes, I can get the following clock setup for ESAI.
> 
>  anaclk2                                  0            0    24576000          0
>     lvds2_in                              0            0    24576000          0
>        pll4_bypass_src                    0            0    24576000          0
>           pll4_bypass                     0            0    24576000          0
>              pll4_audio                   0            0    24576000          0
>                 pll4_post_div             0            0    24576000          0
>                    pll4_audio_div           0            0    24576000          0
>                       esai_sel            0            0    24576000          0
>                          esai_pred           0            0    24576000          0
>                             esai_podf           0            0    24576000          0
>                                esai_extal           0            0    24576000          0
> 
> Shawn
Shawn Guo Aug. 29, 2014, 10:33 a.m. UTC | #2
On Fri, Aug 29, 2014 at 05:32:40PM +0800, Shengjiu Wang wrote:
> > diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
> > index 6692115b0138..9fe4169c79dd 100644
> > --- a/arch/arm/boot/dts/imx6qdl.dtsi
> > +++ b/arch/arm/boot/dts/imx6qdl.dtsi
> > @@ -268,8 +268,18 @@
> >                                 };
> > 
> >                                 esai: esai@02024000 {
> > +                                       compatible = "fsl,imx6q-esai", "fsl,imx35-esai";
> >                                         reg = <0x02024000 0x4000>;
> >                                         interrupts = <0 51 IRQ_TYPE_LEVEL_HIGH>;
> > +                                       clocks = <&clks IMX6QDL_CLK_ESAI_IPG>,
> > +                                                <&clks IMX6QDL_CLK_ESAI_EXTAL>,
> > +                                                <&clks IMX6QDL_CLK_ESAI_IPG>;
> > +                                       clock-names = "core", "extal", "fsys";
> > +                                       dmas = <&sdma 23 21 0>, <&sdma 24 21 0>;
> > +                                       dma-names = "rx", "tx";
> > +                                       fsl,fifo-depth = <128>;
> > +                                       fsl,esai-synchronous;
> we alway don't set fsl,fifo-depth and fsl,esai-synchronous, just use the
> default value in driver.

Okay.  The change here is just to get ESAI driver probed, so that the
assigned-clocks in ESAI node can be handled.  I suppose you will send me
a patch to get ESAI node ready for working.

> > +                                       status = "disabled";
> >                                 };
> > 
> >                                 ssi1: ssi@02028000 {
> > 
> > diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> > index 009abd69385d..1084394197ad 100644
> > --- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> > +++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
> > @@ -17,6 +17,14 @@
> >                 reg = <0x10000000 0x80000000>;
> >         };
> > 
> > +       clocks {
> > +               anaclk2 { /* on-board 24.576MHz audio oscillator */
> > +                       compatible = "fixed-clock";
> > +                       #clock-cells = <0>;
> > +                       clock-frequency = <24576000>;
> > +               };
> > +       };
> > +
> >         leds {
> >                 compatible = "gpio-leds";
> >                 pinctrl-names = "default";
> > @@ -45,6 +53,17 @@
> >         };
> >  };
> > 
> > +&clks {
> > +       assigned-clocks = <&clks IMX6QDL_PLL4_BYPASS_SRC>,
> > +                         <&clks IMX6QDL_PLL4_BYPASS>,
> > +                         <&clks IMX6QDL_CLK_ESAI_SEL>,
> > +                         <&clks IMX6QDL_CLK_PLL4_POST_DIV>;
> > +       assigned-clock-parents = <&clks IMX6QDL_CLK_LVDS2_IN>,
> > +                                <&clks IMX6QDL_PLL4_BYPASS_SRC>,
> > +                                <&clks IMX6QDL_CLK_PLL4_AUDIO_DIV>;
> > +       assigned-clock-rates = <0>, <0>, <0>, <24576000>;
> > +};
> > +
> >  &ecspi1 {
> >         fsl,spi-num-chipselects = <1>;
> >         cs-gpios = <&gpio3 19 0>;
> > @@ -61,6 +80,12 @@
> >         };
> >  };
> > 
> > +&esai {
> > +       assigned-clocks = <&clks IMX6QDL_CLK_ESAI_EXTAL>;
> > +       assigned-clock-rates = <24576000>;
> > +       status = "okay";
> > +};
> > +
> 
> Can we move the clocks for &esai to &clks ? I just think that will look better.
> Anyway, this is just my view. you can do it follow the formal principle.

The general principle is that clocks used by multiple clients like bus
clock and pll are configured in &clk node, and leaf clocks only used by
particular client device should be configured in that device node.

Shawn
diff mbox

Patch

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 6692115b0138..9fe4169c79dd 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -268,8 +268,18 @@ 
                                };

                                esai: esai@02024000 {
+                                       compatible = "fsl,imx6q-esai", "fsl,imx35-esai";
                                        reg = <0x02024000 0x4000>;
                                        interrupts = <0 51 IRQ_TYPE_LEVEL_HIGH>;
+                                       clocks = <&clks IMX6QDL_CLK_ESAI_IPG>,
+                                                <&clks IMX6QDL_CLK_ESAI_EXTAL>,
+                                                <&clks IMX6QDL_CLK_ESAI_IPG>;
+                                       clock-names = "core", "extal", "fsys";
+                                       dmas = <&sdma 23 21 0>, <&sdma 24 21 0>;
+                                       dma-names = "rx", "tx";
+                                       fsl,fifo-depth = <128>;
+                                       fsl,esai-synchronous;
+                                       status = "disabled";
                                };

                                ssi1: ssi@02028000 {

diff --git a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
index 009abd69385d..1084394197ad 100644
--- a/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
+++ b/arch/arm/boot/dts/imx6qdl-sabreauto.dtsi
@@ -17,6 +17,14 @@ 
                reg = <0x10000000 0x80000000>;
        };

+       clocks {
+               anaclk2 { /* on-board 24.576MHz audio oscillator */
+                       compatible = "fixed-clock";
+                       #clock-cells = <0>;
+                       clock-frequency = <24576000>;
+               };
+       };
+
        leds {
                compatible = "gpio-leds";
                pinctrl-names = "default";
@@ -45,6 +53,17 @@ 
        };
 };

+&clks {
+       assigned-clocks = <&clks IMX6QDL_PLL4_BYPASS_SRC>,
+                         <&clks IMX6QDL_PLL4_BYPASS>,
+                         <&clks IMX6QDL_CLK_ESAI_SEL>,
+                         <&clks IMX6QDL_CLK_PLL4_POST_DIV>;
+       assigned-clock-parents = <&clks IMX6QDL_CLK_LVDS2_IN>,
+                                <&clks IMX6QDL_PLL4_BYPASS_SRC>,
+                                <&clks IMX6QDL_CLK_PLL4_AUDIO_DIV>;
+       assigned-clock-rates = <0>, <0>, <0>, <24576000>;
+};
+
 &ecspi1 {
        fsl,spi-num-chipselects = <1>;
        cs-gpios = <&gpio3 19 0>;
@@ -61,6 +80,12 @@ 
        };
 };

+&esai {
+       assigned-clocks = <&clks IMX6QDL_CLK_ESAI_EXTAL>;
+       assigned-clock-rates = <24576000>;
+       status = "okay";
+};
+
 &fec {
        pinctrl-names = "default";
        pinctrl-0 = <&pinctrl_enet>;