Patchwork [2/2] ARM: imx: set CKO1 parent clock source in imx6q sabresd

login
register
mail settings
Submitter Gary Zhang
Date Feb. 1, 2013, 6:46 a.m.
Message ID <1359701217-23931-1-git-send-email-b13634@freescale.com>
Download mbox | patch
Permalink /patch/217353/
State New
Headers show

Comments

Gary Zhang - Feb. 1, 2013, 6:46 a.m.
in imx6q sabresd board, set ahb as CKO1 parent clock sourrce

Signed-off-by: Gary Zhang <b13634@freescale.com>
---
 arch/arm/mach-imx/mach-imx6q.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)
Shawn Guo - Feb. 3, 2013, 10:46 a.m.
On Fri, Feb 01, 2013 at 02:46:57PM +0800, Gary Zhang wrote:
> in imx6q sabresd board, set ahb as CKO1 parent clock sourrce
> 
> Signed-off-by: Gary Zhang <b13634@freescale.com>
> ---
>  arch/arm/mach-imx/mach-imx6q.c |   15 ++++++++++++---
>  1 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
> index 4eb1b3a..860e272 100644
> --- a/arch/arm/mach-imx/mach-imx6q.c
> +++ b/arch/arm/mach-imx/mach-imx6q.c
> @@ -119,7 +119,7 @@ static int ksz9021rn_phy_fixup(struct phy_device *phydev)
>  	return 0;
>  }
>  
> -static void __init imx6q_sabrelite_cko1_setup(void)
> +static void __init cko1_setup(unsigned long freq)
>  {
>  	struct clk *cko1_sel, *ahb, *cko1;
>  	unsigned long rate;
> @@ -132,8 +132,10 @@ static void __init imx6q_sabrelite_cko1_setup(void)
>  		goto put_clk;
>  	}
>  	clk_set_parent(cko1_sel, ahb);
> -	rate = clk_round_rate(cko1, 16000000);
> +	rate = clk_round_rate(cko1, freq);
>  	clk_set_rate(cko1, rate);
> +
> +	return;

Why does this "return" needs to be added here?

Shawn

>  put_clk:
>  	if (!IS_ERR(cko1_sel))
>  		clk_put(cko1_sel);
> @@ -148,7 +150,12 @@ static void __init imx6q_sabrelite_init(void)
>  	if (IS_BUILTIN(CONFIG_PHYLIB))
>  		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
>  				ksz9021rn_phy_fixup);
> -	imx6q_sabrelite_cko1_setup();
> +	cko1_setup(16000000);
> +}
> +
> +static void __init imx6q_sabresd_init(void)
> +{
> +	cko1_setup(24000000);
>  }
>  
>  static void __init imx6q_1588_init(void)
> @@ -193,6 +200,8 @@ static void __init imx6q_init_machine(void)
>  {
>  	if (of_machine_is_compatible("fsl,imx6q-sabrelite"))
>  		imx6q_sabrelite_init();
> +	else if (of_machine_is_compatible("fsl,imx6q-sabresd"))
> +		imx6q_sabresd_init();
>  
>  	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
>  
> -- 
> 1.7.0.4
> 
>
Gary Zhang - Feb. 4, 2013, 3:13 a.m.
Hi Shawn,
Please see the feedback inline


Best Regards
Gary


>  -----Original Message-----
>  From: Shawn Guo [mailto:shawn.guo@linaro.org]
>  Sent: Sunday, February 03, 2013 18:47
>  To: Zhang Quan-B13634
>  Cc: kernel@pengutronix.de; linux@arm.linux.org.uk; linux-arm-
>  kernel@lists.infradead.org
>  Subject: Re: [PATCH 2/2] ARM: imx: set CKO1 parent clock source in imx6q
>  sabresd
>  
>  On Fri, Feb 01, 2013 at 02:46:57PM +0800, Gary Zhang wrote:
>  > in imx6q sabresd board, set ahb as CKO1 parent clock sourrce
>  >
>  > Signed-off-by: Gary Zhang <b13634@freescale.com>
>  > ---
>  >  arch/arm/mach-imx/mach-imx6q.c |   15 ++++++++++++---
>  >  1 files changed, 12 insertions(+), 3 deletions(-)
>  >
>  > diff --git a/arch/arm/mach-imx/mach-imx6q.c
>  > b/arch/arm/mach-imx/mach-imx6q.c index 4eb1b3a..860e272 100644
>  > --- a/arch/arm/mach-imx/mach-imx6q.c
>  > +++ b/arch/arm/mach-imx/mach-imx6q.c
>  > @@ -119,7 +119,7 @@ static int ksz9021rn_phy_fixup(struct phy_device *phydev)
>  >  	return 0;
>  >  }
>  >
>  > -static void __init imx6q_sabrelite_cko1_setup(void)
>  > +static void __init cko1_setup(unsigned long freq)
>  >  {
>  >  	struct clk *cko1_sel, *ahb, *cko1;
>  >  	unsigned long rate;
>  > @@ -132,8 +132,10 @@ static void __init imx6q_sabrelite_cko1_setup(void)
>  >  		goto put_clk;
>  >  	}
>  >  	clk_set_parent(cko1_sel, ahb);
>  > -	rate = clk_round_rate(cko1, 16000000);
>  > +	rate = clk_round_rate(cko1, freq);
>  >  	clk_set_rate(cko1, rate);
>  > +
>  > +	return;
>  
>  Why does this "return" needs to be added here?
[Gary-b13634] because the codes below may be ignored when execute 'return' ,
Saving cpu consumption.
>  
>  Shawn
>  
>  >  put_clk:
>  >  	if (!IS_ERR(cko1_sel))
>  >  		clk_put(cko1_sel);
>  > @@ -148,7 +150,12 @@ static void __init imx6q_sabrelite_init(void)
>  >  	if (IS_BUILTIN(CONFIG_PHYLIB))
>  >  		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
>  >  				ksz9021rn_phy_fixup);
>  > -	imx6q_sabrelite_cko1_setup();
>  > +	cko1_setup(16000000);
>  > +}
>  > +
>  > +static void __init imx6q_sabresd_init(void) {
>  > +	cko1_setup(24000000);
>  >  }
>  >
>  >  static void __init imx6q_1588_init(void) @@ -193,6 +200,8 @@ static
>  > void __init imx6q_init_machine(void)  {
>  >  	if (of_machine_is_compatible("fsl,imx6q-sabrelite"))
>  >  		imx6q_sabrelite_init();
>  > +	else if (of_machine_is_compatible("fsl,imx6q-sabresd"))
>  > +		imx6q_sabresd_init();
>  >
>  >  	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);
>  >
>  > --
>  > 1.7.0.4
>  >
>  >
Shawn Guo - Feb. 4, 2013, 5:12 a.m.
On Mon, Feb 04, 2013 at 03:13:40AM +0000, Zhang Quan-B13634 wrote:
> >  > -static void __init imx6q_sabrelite_cko1_setup(void)
> >  > +static void __init cko1_setup(unsigned long freq)
> >  >  {
> >  >  	struct clk *cko1_sel, *ahb, *cko1;
> >  >  	unsigned long rate;
> >  > @@ -132,8 +132,10 @@ static void __init imx6q_sabrelite_cko1_setup(void)
> >  >  		goto put_clk;
> >  >  	}
> >  >  	clk_set_parent(cko1_sel, ahb);
> >  > -	rate = clk_round_rate(cko1, 16000000);
> >  > +	rate = clk_round_rate(cko1, freq);
> >  >  	clk_set_rate(cko1, rate);
> >  > +
> >  > +	return;
> >  
> >  Why does this "return" needs to be added here?
> [Gary-b13634] because the codes below may be ignored when execute 'return' ,
> Saving cpu consumption.

But we haven't finished our job.  We need to "put" those clocks before
returning to balance the "get" call.

Shawn

Patch

diff --git a/arch/arm/mach-imx/mach-imx6q.c b/arch/arm/mach-imx/mach-imx6q.c
index 4eb1b3a..860e272 100644
--- a/arch/arm/mach-imx/mach-imx6q.c
+++ b/arch/arm/mach-imx/mach-imx6q.c
@@ -119,7 +119,7 @@  static int ksz9021rn_phy_fixup(struct phy_device *phydev)
 	return 0;
 }
 
-static void __init imx6q_sabrelite_cko1_setup(void)
+static void __init cko1_setup(unsigned long freq)
 {
 	struct clk *cko1_sel, *ahb, *cko1;
 	unsigned long rate;
@@ -132,8 +132,10 @@  static void __init imx6q_sabrelite_cko1_setup(void)
 		goto put_clk;
 	}
 	clk_set_parent(cko1_sel, ahb);
-	rate = clk_round_rate(cko1, 16000000);
+	rate = clk_round_rate(cko1, freq);
 	clk_set_rate(cko1, rate);
+
+	return;
 put_clk:
 	if (!IS_ERR(cko1_sel))
 		clk_put(cko1_sel);
@@ -148,7 +150,12 @@  static void __init imx6q_sabrelite_init(void)
 	if (IS_BUILTIN(CONFIG_PHYLIB))
 		phy_register_fixup_for_uid(PHY_ID_KSZ9021, MICREL_PHY_ID_MASK,
 				ksz9021rn_phy_fixup);
-	imx6q_sabrelite_cko1_setup();
+	cko1_setup(16000000);
+}
+
+static void __init imx6q_sabresd_init(void)
+{
+	cko1_setup(24000000);
 }
 
 static void __init imx6q_1588_init(void)
@@ -193,6 +200,8 @@  static void __init imx6q_init_machine(void)
 {
 	if (of_machine_is_compatible("fsl,imx6q-sabrelite"))
 		imx6q_sabrelite_init();
+	else if (of_machine_is_compatible("fsl,imx6q-sabresd"))
+		imx6q_sabresd_init();
 
 	of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL);