diff mbox

[U-Boot,v2,02/20] arm: socfpga: Update clock for Gen5

Message ID 1489019218-4071-3-git-send-email-ley.foon.tan@intel.com
State Superseded
Delegated to: Marek Vasut
Headers show

Commit Message

Ley Foon Tan March 9, 2017, 12:26 a.m. UTC 
Rename some of variables and fixes on clock calculation.

Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
---
 arch/arm/mach-socfpga/clock_manager_gen5.c | 126 +++++++++++++++++------------
 1 file changed, 76 insertions(+), 50 deletions(-)

Comments

Dinh Nguyen March 9, 2017, 4:39 p.m. UTC | #1 
On 03/08/2017 06:26 PM, Ley Foon Tan wrote:
> Rename some of variables and fixes on clock calculation.
> 

Why? Please be more specific with why this change is necessary?

> Signed-off-by: Tien Fong Chee <tien.fong.chee@intel.com>
> Signed-off-by: Ley Foon Tan <ley.foon.tan@intel.com>
> ---
>  arch/arm/mach-socfpga/clock_manager_gen5.c | 126 +++++++++++++++++------------
>  1 file changed, 76 insertions(+), 50 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/clock_manager_gen5.c b/arch/arm/mach-socfpga/clock_manager_gen5.c
> index bca27df..bef0adb 100644
> --- a/arch/arm/mach-socfpga/clock_manager_gen5.c
> +++ b/arch/arm/mach-socfpga/clock_manager_gen5.c
> @@ -1,5 +1,5 @@
>  /*
> - *  Copyright (C) 2013-2017 Altera Corporation <www.altera.com>
> + * Copyright (C) 2013-2017 Altera Corporation <www.altera.com>
>   *
>   * SPDX-License-Identifier:	GPL-2.0+
>   */
> @@ -66,7 +66,6 @@ static void cm_write_with_phase(u32 value,
>   * set source main and peripheral clocks
>   * Ungate clocks
>   */
> -
>  void cm_basic_init(const struct cm_config * const cfg)
>  {
>  	unsigned long end;
> @@ -307,56 +306,73 @@ void cm_basic_init(const struct cm_config * const cfg)
>  
>  static unsigned int cm_get_main_vco_clk_hz(void)
>  {
> -	u32 reg, clock;
> +	u32 src_hz, numer, denom, vco;
>  
>  	/* get the main VCO clock */
> -	reg = readl(&clock_manager_base->main_pll.vco);
> -	clock = cm_get_osc_clk_hz(1);
> -	clock /= ((reg & CLKMGR_MAINPLLGRP_VCO_DENOM_MASK) >>
> -		  CLKMGR_MAINPLLGRP_VCO_DENOM_OFFSET) + 1;
> -	clock *= ((reg & CLKMGR_MAINPLLGRP_VCO_NUMER_MASK) >>
> -		  CLKMGR_MAINPLLGRP_VCO_NUMER_OFFSET) + 1;
> +	vco = readl(&clock_manager_base->main_pll.vco);
>  
> -	return clock;
> +	numer = ((vco & CLKMGR_MAINPLLGRP_VCO_NUMER_MASK) >>
> +		CLKMGR_MAINPLLGRP_VCO_NUMER_OFFSET);
> +	denom = ((vco & CLKMGR_MAINPLLGRP_VCO_DENOM_MASK) >>
> +		 CLKMGR_MAINPLLGRP_VCO_DENOM_OFFSET);
> +
> +	src_hz = cm_get_osc_clk_hz(1);
> +
> +	vco = src_hz;
> +	vco /= (1 + denom);
> +	vco *= (1 + numer);
> +
> +	return vco;

It seems like this change is beyond just "Rename some of variables and
fixes on clock calculation". Could probably be it's own patch?

>  }
>  
>  static unsigned int cm_get_per_vco_clk_hz(void)
>  {
> -	u32 reg, clock = 0;
> +	u32 src_hz = 0;
> +	u32 clk_src = 0;
> +	u32 numer = 0;
> +	u32 denom = 0;
> +	u32 vco = 0;
>  
>  	/* identify PER PLL clock source */
> -	reg = readl(&clock_manager_base->per_pll.vco);
> -	reg = (reg & CLKMGR_PERPLLGRP_VCO_SSRC_MASK) >>
> +	clk_src = readl(&clock_manager_base->per_pll.vco);
> +	clk_src = (clk_src & CLKMGR_PERPLLGRP_VCO_SSRC_MASK) >>
>  	      CLKMGR_PERPLLGRP_VCO_SSRC_OFFSET;
> -	if (reg == CLKMGR_VCO_SSRC_EOSC1)
> -		clock = cm_get_osc_clk_hz(1);
> -	else if (reg == CLKMGR_VCO_SSRC_EOSC2)
> -		clock = cm_get_osc_clk_hz(2);
> -	else if (reg == CLKMGR_VCO_SSRC_F2S)
> -		clock = cm_get_f2s_per_ref_clk_hz();
> +	if (clk_src == CLKMGR_VCO_SSRC_EOSC1)
> +		src_hz = cm_get_osc_clk_hz(1);
> +	else if (clk_src == CLKMGR_VCO_SSRC_EOSC2)
> +		src_hz = cm_get_osc_clk_hz(2);
> +	else if (clk_src == CLKMGR_VCO_SSRC_F2S)
> +		src_hz = cm_get_f2s_per_ref_clk_hz();
>  
>  	/* get the PER VCO clock */
> -	reg = readl(&clock_manager_base->per_pll.vco);
> -	clock /= ((reg & CLKMGR_PERPLLGRP_VCO_DENOM_MASK) >>
> -		  CLKMGR_PERPLLGRP_VCO_DENOM_OFFSET) + 1;
> -	clock *= ((reg & CLKMGR_PERPLLGRP_VCO_NUMER_MASK) >>
> -		  CLKMGR_PERPLLGRP_VCO_NUMER_OFFSET) + 1;
> +	vco = readl(&clock_manager_base->per_pll.vco);
>  
> -	return clock;
> +	numer = (vco & CLKMGR_PERPLLGRP_VCO_NUMER_MASK) >>
> +		CLKMGR_PERPLLGRP_VCO_NUMER_OFFSET;
> +
> +	denom = (vco & CLKMGR_PERPLLGRP_VCO_DENOM_MASK) >>
> +		CLKMGR_PERPLLGRP_VCO_DENOM_OFFSET;
> +
> +	vco = src_hz;
> +	vco /= (1 + denom);
> +	vco *= (1 + numer);
> +
> +	return vco;
>  }

Ditto..

>  
>  unsigned long cm_get_mpu_clk_hz(void)
>  {
> -	u32 reg, clock;
> +	u32 reg, clk_hz;
>  
> -	clock = cm_get_main_vco_clk_hz();
> +	clk_hz = cm_get_main_vco_clk_hz();
>  
>  	/* get the MPU clock */
>  	reg = readl(&clock_manager_base->altera.mpuclk);
> -	clock /= (reg + 1);
> +	clk_hz /= (reg + 1);
>  	reg = readl(&clock_manager_base->main_pll.mpuclk);
> -	clock /= (reg + 1);
> -	return clock;
> +	clk_hz /= (reg + 1);
> +
> +	return clk_hz;
>  }
>  
>  unsigned long cm_get_sdram_clk_hz(void)
> @@ -392,7 +408,8 @@ unsigned long cm_get_sdram_clk_hz(void)
>  
>  unsigned int cm_get_l4_sp_clk_hz(void)
>  {
> -	u32 reg, clock = 0;
> +	u32 clock = 0;
> +	u32 reg;
>  
>  	/* identify the source of L4 SP clock */
>  	reg = readl(&clock_manager_base->main_pll.l4src);
> @@ -426,37 +443,45 @@ unsigned int cm_get_l4_sp_clk_hz(void)
>  
>  unsigned int cm_get_mmc_controller_clk_hz(void)
>  {
> -	u32 reg, clock = 0;
> +	u32 clk_hz = 0;
> +	u32 clk_input = 0;
>  
> -	/* identify the source of MMC clock */
> -	reg = readl(&clock_manager_base->per_pll.src);
> -	reg = (reg & CLKMGR_PERPLLGRP_SRC_SDMMC_MASK) >>
> +	clk_input = readl(&clock_manager_base->per_pll.src);
> +	clk_input = (clk_input & CLKMGR_PERPLLGRP_SRC_SDMMC_MASK) >>
>  	      CLKMGR_PERPLLGRP_SRC_SDMMC_OFFSET;
>  
> -	if (reg == CLKMGR_SDMMC_CLK_SRC_F2S) {
> -		clock = cm_get_f2s_per_ref_clk_hz();
> -	} else if (reg == CLKMGR_SDMMC_CLK_SRC_MAIN) {
> -		clock = cm_get_main_vco_clk_hz();
> +	switch (clk_input) {
> +	case CLKMGR_SDMMC_CLK_SRC_F2S:
> +		clk_hz = cm_get_f2s_per_ref_clk_hz();
> +		break;
> +
> +	case CLKMGR_SDMMC_CLK_SRC_MAIN:
> +		clk_hz = cm_get_main_vco_clk_hz();
>  
>  		/* get the SDMMC clock */
> -		reg = readl(&clock_manager_base->main_pll.mainnandsdmmcclk);
> -		clock /= (reg + 1);
> -	} else if (reg == CLKMGR_SDMMC_CLK_SRC_PER) {
> -		clock = cm_get_per_vco_clk_hz();
> +		clk_input =
> +			readl(&clock_manager_base->main_pll.mainnandsdmmcclk);
> +		clk_hz /= (clk_input + 1);
> +		break;
> +
> +	case CLKMGR_SDMMC_CLK_SRC_PER:
> +		clk_hz = cm_get_per_vco_clk_hz();
>  
>  		/* get the SDMMC clock */
> -		reg = readl(&clock_manager_base->per_pll.pernandsdmmcclk);
> -		clock /= (reg + 1);
> +		clk_input =
> +			readl(&clock_manager_base->per_pll.pernandsdmmcclk);
> +		clk_hz /= (clk_input + 1);
> +		break;
>  	}
>  
> -	/* further divide by 4 as we have fixed divider at wrapper */
> -	clock /= 4;
> -	return clock;
> +	return clk_hz / 4;
>  }
>  
> +
>  unsigned int cm_get_qspi_controller_clk_hz(void)
>  {
> -	u32 reg, clock = 0;
> +	u32 clock = 0;
> +	u32 reg;
>  

Why is this necessary?

>  	/* identify the source of QSPI clock */
>  	reg = readl(&clock_manager_base->per_pll.src);
> @@ -484,7 +509,8 @@ unsigned int cm_get_qspi_controller_clk_hz(void)
>  
>  unsigned int cm_get_spi_controller_clk_hz(void)
>  {
> -	u32 reg, clock = 0;
> +	u32 clock = 0;
> +	u32 reg;
>  

Ditto..

Dinh
diff mbox

Patch

diff --git a/arch/arm/mach-socfpga/clock_manager_gen5.c b/arch/arm/mach-socfpga/clock_manager_gen5.c
index bca27df..bef0adb 100644
--- a/arch/arm/mach-socfpga/clock_manager_gen5.c
+++ b/arch/arm/mach-socfpga/clock_manager_gen5.c
@@ -1,5 +1,5 @@ 
 /*
- *  Copyright (C) 2013-2017 Altera Corporation <www.altera.com>
+ * Copyright (C) 2013-2017 Altera Corporation <www.altera.com>
  *
  * SPDX-License-Identifier:	GPL-2.0+
  */
@@ -66,7 +66,6 @@  static void cm_write_with_phase(u32 value,
  * set source main and peripheral clocks
  * Ungate clocks
  */
-
 void cm_basic_init(const struct cm_config * const cfg)
 {
 	unsigned long end;
@@ -307,56 +306,73 @@  void cm_basic_init(const struct cm_config * const cfg)
 
 static unsigned int cm_get_main_vco_clk_hz(void)
 {
-	u32 reg, clock;
+	u32 src_hz, numer, denom, vco;
 
 	/* get the main VCO clock */
-	reg = readl(&clock_manager_base->main_pll.vco);
-	clock = cm_get_osc_clk_hz(1);
-	clock /= ((reg & CLKMGR_MAINPLLGRP_VCO_DENOM_MASK) >>
-		  CLKMGR_MAINPLLGRP_VCO_DENOM_OFFSET) + 1;
-	clock *= ((reg & CLKMGR_MAINPLLGRP_VCO_NUMER_MASK) >>
-		  CLKMGR_MAINPLLGRP_VCO_NUMER_OFFSET) + 1;
+	vco = readl(&clock_manager_base->main_pll.vco);
 
-	return clock;
+	numer = ((vco & CLKMGR_MAINPLLGRP_VCO_NUMER_MASK) >>
+		CLKMGR_MAINPLLGRP_VCO_NUMER_OFFSET);
+	denom = ((vco & CLKMGR_MAINPLLGRP_VCO_DENOM_MASK) >>
+		 CLKMGR_MAINPLLGRP_VCO_DENOM_OFFSET);
+
+	src_hz = cm_get_osc_clk_hz(1);
+
+	vco = src_hz;
+	vco /= (1 + denom);
+	vco *= (1 + numer);
+
+	return vco;
 }
 
 static unsigned int cm_get_per_vco_clk_hz(void)
 {
-	u32 reg, clock = 0;
+	u32 src_hz = 0;
+	u32 clk_src = 0;
+	u32 numer = 0;
+	u32 denom = 0;
+	u32 vco = 0;
 
 	/* identify PER PLL clock source */
-	reg = readl(&clock_manager_base->per_pll.vco);
-	reg = (reg & CLKMGR_PERPLLGRP_VCO_SSRC_MASK) >>
+	clk_src = readl(&clock_manager_base->per_pll.vco);
+	clk_src = (clk_src & CLKMGR_PERPLLGRP_VCO_SSRC_MASK) >>
 	      CLKMGR_PERPLLGRP_VCO_SSRC_OFFSET;
-	if (reg == CLKMGR_VCO_SSRC_EOSC1)
-		clock = cm_get_osc_clk_hz(1);
-	else if (reg == CLKMGR_VCO_SSRC_EOSC2)
-		clock = cm_get_osc_clk_hz(2);
-	else if (reg == CLKMGR_VCO_SSRC_F2S)
-		clock = cm_get_f2s_per_ref_clk_hz();
+	if (clk_src == CLKMGR_VCO_SSRC_EOSC1)
+		src_hz = cm_get_osc_clk_hz(1);
+	else if (clk_src == CLKMGR_VCO_SSRC_EOSC2)
+		src_hz = cm_get_osc_clk_hz(2);
+	else if (clk_src == CLKMGR_VCO_SSRC_F2S)
+		src_hz = cm_get_f2s_per_ref_clk_hz();
 
 	/* get the PER VCO clock */
-	reg = readl(&clock_manager_base->per_pll.vco);
-	clock /= ((reg & CLKMGR_PERPLLGRP_VCO_DENOM_MASK) >>
-		  CLKMGR_PERPLLGRP_VCO_DENOM_OFFSET) + 1;
-	clock *= ((reg & CLKMGR_PERPLLGRP_VCO_NUMER_MASK) >>
-		  CLKMGR_PERPLLGRP_VCO_NUMER_OFFSET) + 1;
+	vco = readl(&clock_manager_base->per_pll.vco);
 
-	return clock;
+	numer = (vco & CLKMGR_PERPLLGRP_VCO_NUMER_MASK) >>
+		CLKMGR_PERPLLGRP_VCO_NUMER_OFFSET;
+
+	denom = (vco & CLKMGR_PERPLLGRP_VCO_DENOM_MASK) >>
+		CLKMGR_PERPLLGRP_VCO_DENOM_OFFSET;
+
+	vco = src_hz;
+	vco /= (1 + denom);
+	vco *= (1 + numer);
+
+	return vco;
 }
 
 unsigned long cm_get_mpu_clk_hz(void)
 {
-	u32 reg, clock;
+	u32 reg, clk_hz;
 
-	clock = cm_get_main_vco_clk_hz();
+	clk_hz = cm_get_main_vco_clk_hz();
 
 	/* get the MPU clock */
 	reg = readl(&clock_manager_base->altera.mpuclk);
-	clock /= (reg + 1);
+	clk_hz /= (reg + 1);
 	reg = readl(&clock_manager_base->main_pll.mpuclk);
-	clock /= (reg + 1);
-	return clock;
+	clk_hz /= (reg + 1);
+
+	return clk_hz;
 }
 
 unsigned long cm_get_sdram_clk_hz(void)
@@ -392,7 +408,8 @@  unsigned long cm_get_sdram_clk_hz(void)
 
 unsigned int cm_get_l4_sp_clk_hz(void)
 {
-	u32 reg, clock = 0;
+	u32 clock = 0;
+	u32 reg;
 
 	/* identify the source of L4 SP clock */
 	reg = readl(&clock_manager_base->main_pll.l4src);
@@ -426,37 +443,45 @@  unsigned int cm_get_l4_sp_clk_hz(void)
 
 unsigned int cm_get_mmc_controller_clk_hz(void)
 {
-	u32 reg, clock = 0;
+	u32 clk_hz = 0;
+	u32 clk_input = 0;
 
-	/* identify the source of MMC clock */
-	reg = readl(&clock_manager_base->per_pll.src);
-	reg = (reg & CLKMGR_PERPLLGRP_SRC_SDMMC_MASK) >>
+	clk_input = readl(&clock_manager_base->per_pll.src);
+	clk_input = (clk_input & CLKMGR_PERPLLGRP_SRC_SDMMC_MASK) >>
 	      CLKMGR_PERPLLGRP_SRC_SDMMC_OFFSET;
 
-	if (reg == CLKMGR_SDMMC_CLK_SRC_F2S) {
-		clock = cm_get_f2s_per_ref_clk_hz();
-	} else if (reg == CLKMGR_SDMMC_CLK_SRC_MAIN) {
-		clock = cm_get_main_vco_clk_hz();
+	switch (clk_input) {
+	case CLKMGR_SDMMC_CLK_SRC_F2S:
+		clk_hz = cm_get_f2s_per_ref_clk_hz();
+		break;
+
+	case CLKMGR_SDMMC_CLK_SRC_MAIN:
+		clk_hz = cm_get_main_vco_clk_hz();
 
 		/* get the SDMMC clock */
-		reg = readl(&clock_manager_base->main_pll.mainnandsdmmcclk);
-		clock /= (reg + 1);
-	} else if (reg == CLKMGR_SDMMC_CLK_SRC_PER) {
-		clock = cm_get_per_vco_clk_hz();
+		clk_input =
+			readl(&clock_manager_base->main_pll.mainnandsdmmcclk);
+		clk_hz /= (clk_input + 1);
+		break;
+
+	case CLKMGR_SDMMC_CLK_SRC_PER:
+		clk_hz = cm_get_per_vco_clk_hz();
 
 		/* get the SDMMC clock */
-		reg = readl(&clock_manager_base->per_pll.pernandsdmmcclk);
-		clock /= (reg + 1);
+		clk_input =
+			readl(&clock_manager_base->per_pll.pernandsdmmcclk);
+		clk_hz /= (clk_input + 1);
+		break;
 	}
 
-	/* further divide by 4 as we have fixed divider at wrapper */
-	clock /= 4;
-	return clock;
+	return clk_hz / 4;
 }
 
+
 unsigned int cm_get_qspi_controller_clk_hz(void)
 {
-	u32 reg, clock = 0;
+	u32 clock = 0;
+	u32 reg;
 
 	/* identify the source of QSPI clock */
 	reg = readl(&clock_manager_base->per_pll.src);
@@ -484,7 +509,8 @@  unsigned int cm_get_qspi_controller_clk_hz(void)
 
 unsigned int cm_get_spi_controller_clk_hz(void)
 {
-	u32 reg, clock = 0;
+	u32 clock = 0;
+	u32 reg;
 
 	clock = cm_get_per_vco_clk_hz();