[V2] ARM: imx: correct usecount of IPG, ARM and MMDC clk on i.mx6sl

Submitted by Anson Huang on Jan. 21, 2014, 10:50 a.m.

Details

Message ID 1390301452-10143-1-git-send-email-b20788@freescale.com
State New
Headers show

Commit Message

Anson Huang Jan. 21, 2014, 10:50 a.m.
IPG, ARM and MMDC's clock should be enabled during kernel boot up,
so we need to maintain their use count, otherwise, they may be
disabled unexpectedly if their children's clock are turned off,
which is not allowed.

Signed-off-by: Anson Huang <b20788@freescale.com>
---
 arch/arm/mach-imx/clk-imx6sl.c |   16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Comments

Fabio Estevam Jan. 21, 2014, 10:56 a.m.
On Tue, Jan 21, 2014 at 8:50 AM, Anson Huang <b20788@freescale.com> wrote:
> IPG, ARM and MMDC's clock should be enabled during kernel boot up,
> so we need to maintain their use count, otherwise, they may be
> disabled unexpectedly if their children's clock are turned off,
> which is not allowed.
>
> Signed-off-by: Anson Huang <b20788@freescale.com>
> ---
>  arch/arm/mach-imx/clk-imx6sl.c |   16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/arch/arm/mach-imx/clk-imx6sl.c b/arch/arm/mach-imx/clk-imx6sl.c
> index 78f3bd6..047f4ff 100644
> --- a/arch/arm/mach-imx/clk-imx6sl.c
> +++ b/arch/arm/mach-imx/clk-imx6sl.c
> @@ -291,6 +291,22 @@ static void __init imx6sl_clocks_init(struct device_node *ccm_node)
>                 pr_warn("%s: failed to set AHB clock rate %d!\n",
>                         __func__, ret);
>
> +       /*
> +        * Make sure those always on clocks are enabled to maintain the correct
> +        * usecount and enabling/disabling of parent PLLs.
> +        */
> +       ret = clk_prepare_enable(clks[IMX6SL_CLK_IPG]);
> +       if (ret)
> +               pr_warn("%s: failed to enable IPG clock %d\n", __func__, ret);

What about adding WARN_ON(!ret ) ?

Regards,

Fabio Estevam
Sascha Hauer Jan. 22, 2014, 6:47 a.m.
On Tue, Jan 21, 2014 at 06:50:52PM +0800, Anson Huang wrote:
> IPG, ARM and MMDC's clock should be enabled during kernel boot up,
> so we need to maintain their use count, otherwise, they may be
> disabled unexpectedly if their children's clock are turned off,
> which is not allowed.
> 
> Signed-off-by: Anson Huang <b20788@freescale.com>
> ---
>  arch/arm/mach-imx/clk-imx6sl.c |   16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/arm/mach-imx/clk-imx6sl.c b/arch/arm/mach-imx/clk-imx6sl.c
> index 78f3bd6..047f4ff 100644
> --- a/arch/arm/mach-imx/clk-imx6sl.c
> +++ b/arch/arm/mach-imx/clk-imx6sl.c
> @@ -291,6 +291,22 @@ static void __init imx6sl_clocks_init(struct device_node *ccm_node)
>  		pr_warn("%s: failed to set AHB clock rate %d!\n",
>  			__func__, ret);
>  
> +	/*
> +	 * Make sure those always on clocks are enabled to maintain the correct
> +	 * usecount and enabling/disabling of parent PLLs.
> +	 */
> +	ret = clk_prepare_enable(clks[IMX6SL_CLK_IPG]);
> +	if (ret)
> +		pr_warn("%s: failed to enable IPG clock %d\n", __func__, ret);
> +
> +	ret = clk_prepare_enable(clks[IMX6SL_CLK_ARM]);
> +	if (ret)
> +		pr_warn("%s: failed to enable ARM clock %d\n", __func__, ret);
> +
> +	ret = clk_prepare_enable(clks[IMX6SL_CLK_MMDC_ROOT]);
> +	if (ret)
> +		pr_warn("%s: failed to enable MMDC clock %d\n", __func__, ret);
> +

Consider using a clk_init_on array like we have in clk-imx6q.c

Sascha
Anson Huang Jan. 22, 2014, 7:16 a.m.
On Wed, Jan 22, 2014 at 07:47:40AM +0100, Sascha Hauer wrote:
> On Tue, Jan 21, 2014 at 06:50:52PM +0800, Anson Huang wrote:
> > IPG, ARM and MMDC's clock should be enabled during kernel boot up,
> > so we need to maintain their use count, otherwise, they may be
> > disabled unexpectedly if their children's clock are turned off,
> > which is not allowed.
> > 
> > Signed-off-by: Anson Huang <b20788@freescale.com>
> > ---
> >  arch/arm/mach-imx/clk-imx6sl.c |   16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/arch/arm/mach-imx/clk-imx6sl.c b/arch/arm/mach-imx/clk-imx6sl.c
> > index 78f3bd6..047f4ff 100644
> > --- a/arch/arm/mach-imx/clk-imx6sl.c
> > +++ b/arch/arm/mach-imx/clk-imx6sl.c
> > @@ -291,6 +291,22 @@ static void __init imx6sl_clocks_init(struct device_node *ccm_node)
> >  		pr_warn("%s: failed to set AHB clock rate %d!\n",
> >  			__func__, ret);
> >  
> > +	/*
> > +	 * Make sure those always on clocks are enabled to maintain the correct
> > +	 * usecount and enabling/disabling of parent PLLs.
> > +	 */
> > +	ret = clk_prepare_enable(clks[IMX6SL_CLK_IPG]);
> > +	if (ret)
> > +		pr_warn("%s: failed to enable IPG clock %d\n", __func__, ret);
> > +
> > +	ret = clk_prepare_enable(clks[IMX6SL_CLK_ARM]);
> > +	if (ret)
> > +		pr_warn("%s: failed to enable ARM clock %d\n", __func__, ret);
> > +
> > +	ret = clk_prepare_enable(clks[IMX6SL_CLK_MMDC_ROOT]);
> > +	if (ret)
> > +		pr_warn("%s: failed to enable MMDC clock %d\n", __func__, ret);
> > +
> 
> Consider using a clk_init_on array like we have in clk-imx6q.c
> 
> Sascha
> 

Right, that can make code more clean, thanks for the comment, please help review V3 patch.

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

Patch hide | download patch | download mbox

diff --git a/arch/arm/mach-imx/clk-imx6sl.c b/arch/arm/mach-imx/clk-imx6sl.c
index 78f3bd6..047f4ff 100644
--- a/arch/arm/mach-imx/clk-imx6sl.c
+++ b/arch/arm/mach-imx/clk-imx6sl.c
@@ -291,6 +291,22 @@  static void __init imx6sl_clocks_init(struct device_node *ccm_node)
 		pr_warn("%s: failed to set AHB clock rate %d!\n",
 			__func__, ret);
 
+	/*
+	 * Make sure those always on clocks are enabled to maintain the correct
+	 * usecount and enabling/disabling of parent PLLs.
+	 */
+	ret = clk_prepare_enable(clks[IMX6SL_CLK_IPG]);
+	if (ret)
+		pr_warn("%s: failed to enable IPG clock %d\n", __func__, ret);
+
+	ret = clk_prepare_enable(clks[IMX6SL_CLK_ARM]);
+	if (ret)
+		pr_warn("%s: failed to enable ARM clock %d\n", __func__, ret);
+
+	ret = clk_prepare_enable(clks[IMX6SL_CLK_MMDC_ROOT]);
+	if (ret)
+		pr_warn("%s: failed to enable MMDC clock %d\n", __func__, ret);
+
 	if (IS_ENABLED(CONFIG_USB_MXS_PHY)) {
 		clk_prepare_enable(clks[IMX6SL_CLK_USBPHY1_GATE]);
 		clk_prepare_enable(clks[IMX6SL_CLK_USBPHY2_GATE]);