[U-Boot] [PATCH v2 02/20] arm: socfpga: Update clock for Gen5
Dinh Nguyen
dinguyen at kernel.org
Thu Mar 9 16:39:06 UTC 2017
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 at intel.com>
> Signed-off-by: Ley Foon Tan <ley.foon.tan at 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
More information about the U-Boot
mailing list