From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ley Foon Tan Date: Mon, 06 Mar 2017 15:10:48 +0800 Subject: [U-Boot] [PATCH 07/20] arm: socfpga: add clock driver for Arria 10 In-Reply-To: References: <1487756858-16730-1-git-send-email-ley.foon.tan@intel.com> <1487756858-16730-8-git-send-email-ley.foon.tan@intel.com> Message-ID: <1488784248.2433.10.camel@intel.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de On Sab, 2017-02-25 at 22:35 +0100, Marek Vasut wrote: > On 02/22/2017 10:47 AM, Ley Foon Tan wrote: > > > > Add clock driver support for Arria 10 and update Gen5 clock driver. > > > > Signed-off-by: Tien Fong Chee > > Signed-off-by: Ley Foon Tan > > --- > >  arch/arm/mach-socfpga/Makefile                     |    3 +- > >  arch/arm/mach-socfpga/clock_manager.c              |   36 +- > >  arch/arm/mach-socfpga/clock_manager_arria10.c      | 1104 > > ++++++++++++++++++++ > >  arch/arm/mach-socfpga/clock_manager_gen5.c         |  132 ++- > >  arch/arm/mach-socfpga/include/mach/clock_manager.h |    8 +- > >  .../include/mach/clock_manager_arria10.h           |  231 ++++ > >  6 files changed, 1455 insertions(+), 59 deletions(-) > >  create mode 100644 arch/arm/mach-socfpga/clock_manager_arria10.c > >  mode change 100755 => 100644 arch/arm/mach- > > socfpga/clock_manager_gen5.c > >  create mode 100644 arch/arm/mach- > > socfpga/include/mach/clock_manager_arria10.h > > > > diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach- > > socfpga/Makefile > > index d81f003..c494930 100644 > > --- a/arch/arm/mach-socfpga/Makefile > > +++ b/arch/arm/mach-socfpga/Makefile > > @@ -10,7 +10,8 @@ > >  obj-y += misc.o timer.o reset_manager.o clock_manager.o \ > >      fpga_manager.o board.o > >   > > -obj-$(CONFIG_TARGET_SOCFPGA_ARRIA10) += reset_manager_arria10.o > > +obj-$(CONFIG_TARGET_SOCFPGA_ARRIA10) += clock_manager_arria10.o \ > > + reset_manager_arria10.o > >   > >  obj-$(CONFIG_SPL_BUILD) += spl.o freeze_controller.o > >   > > diff --git a/arch/arm/mach-socfpga/clock_manager.c b/arch/arm/mach- > > socfpga/clock_manager.c > > index d675973..7f48193 100644 > > --- a/arch/arm/mach-socfpga/clock_manager.c > > +++ b/arch/arm/mach-socfpga/clock_manager.c > > @@ -7,6 +7,7 @@ > >  #include > >  #include > >  #include > > +#include > >   > >  DECLARE_GLOBAL_DATA_PTR; > >   > > @@ -18,7 +19,12 @@ void cm_wait_for_lock(u32 mask) > >   register u32 inter_val; > >   u32 retry = 0; > >   do { > > +#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10) > A10 should be below Gen5 to keep this sorted chronologically IMO. Okay. > > > > > + inter_val = readl(&clock_manager_base->stat) & > > mask; > > +#elif defined(CONFIG_TARGET_SOCFPGA_GEN5) > >   inter_val = readl(&clock_manager_base->inter) & > > mask; > > +#endif > > + /* Wait for stable lock */ > >   if (inter_val == mask) > >   retry++; > >   else > > @@ -31,22 +37,39 @@ void cm_wait_for_lock(u32 mask) > >  /* function to poll in the fsm busy bit */ > >  void cm_wait_for_fsm(void) > >  { > > - while (readl(&clock_manager_base->stat) & > > CLKMGR_STAT_BUSY) > > + while (readl(&clock_manager_base->stat) & > > +        CLKMGR_CLKMGR_STAT_BUSY_SET_MSK) > >   ; > >  } > >   > >  static void cm_print_clock_quick_summary(void) > >  { > >   printf("MPU       %10ld kHz\n", cm_get_mpu_clk_hz() / > > 1000); > > + printf("MMC         %8d kHz\n", > > cm_get_mmc_controller_clk_hz() / 1000); > > + printf("QSPI        %8d kHz\n", > > +        cm_get_qspi_controller_clk_hz() / 1000); > > + printf("SPI         %8d kHz\n", > > cm_get_spi_controller_clk_hz() / 1000); > > +#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10) > > + printf("EOSC1       %8d kHz\n", eosc1_hz / 1000); > > + printf("cb_intosc   %8d kHz\n", cb_intosc_hz / 1000); > > + printf("f2s_free    %8d kHz\n", f2s_free_hz / 1000); > > + printf("Main VCO    %8d kHz\n", > > cm_get_main_vco_clk_hz()/1000); > > + printf("NOC         %8d kHz\n", cm_get_noc_clk_hz()/1000); > > + printf("L4 Main     %8d kHz\n", > > +        cm_get_l4_noc_hz(CLKMGR_MAINPLL_NOCDIV_L4MAINCLK_LS > > B)/1000); > > + printf("L4 MP       %8d kHz\n", > > +        cm_get_l4_noc_hz(CLKMGR_MAINPLL_NOCDIV_L4MPCLK_LSB) > > /1000); > > + printf("L4 SP       %8d kHz\n", > > +        cm_get_l4_sp_clk_hz()/1000); > > + printf("L4 sys free %8d kHz\n", > > cm_l4_sys_free_clk_hz/1000); > > +#elif defined(CONFIG_TARGET_SOCFPGA_GEN5) > >   printf("DDR       %10ld kHz\n", cm_get_sdram_clk_hz() / > > 1000); > >   printf("EOSC1       %8d kHz\n", cm_get_osc_clk_hz(1) / > > 1000); > >   printf("EOSC2       %8d kHz\n", cm_get_osc_clk_hz(2) / > > 1000); > >   printf("F2S_SDR_REF %8d kHz\n", > > cm_get_f2s_sdr_ref_clk_hz() / 1000); > >   printf("F2S_PER_REF %8d kHz\n", > > cm_get_f2s_per_ref_clk_hz() / 1000); > > - printf("MMC         %8d kHz\n", > > cm_get_mmc_controller_clk_hz() / 1000); > > - printf("QSPI        %8d kHz\n", > > cm_get_qspi_controller_clk_hz() / 1000); > >   printf("UART        %8d kHz\n", cm_get_l4_sp_clk_hz() / > > 1000); > > - printf("SPI         %8d kHz\n", > > cm_get_spi_controller_clk_hz() / 1000); > > +#endif > >  } > >   > >  int set_cpu_clk_info(void) > > @@ -57,7 +80,12 @@ int set_cpu_clk_info(void) > >   > >   gd->bd->bi_arm_freq = cm_get_mpu_clk_hz() / 1000000; > >   gd->bd->bi_dsp_freq = 0; > > + > > +#if defined(CONFIG_TARGET_SOCFPGA_ARRIA10) > > + gd->bd->bi_ddr_freq = 0; > > +#elif defined(CONFIG_TARGET_SOCFPGA_GEN5) > >   gd->bd->bi_ddr_freq = cm_get_sdram_clk_hz() / 1000000; > > +#endif > >   > >   return 0; > >  } > > diff --git a/arch/arm/mach-socfpga/clock_manager_arria10.c > > b/arch/arm/mach-socfpga/clock_manager_arria10.c > > new file mode 100644 > > index 0000000..4fa841f > > --- /dev/null > > +++ b/arch/arm/mach-socfpga/clock_manager_arria10.c > > @@ -0,0 +1,1104 @@ > > +/* > > + * Copyright (C) 2016-2017 Intel Corporation > > + * > > + * SPDX-License-Identifier:    GPL-2.0 > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > + > > +DECLARE_GLOBAL_DATA_PTR; > > + > > +u32 eosc1_hz; > > +u32 cb_intosc_hz; > > +u32 f2s_free_hz; > > +u32 cm_l4_main_clk_hz; > > +u32 cm_l4_sp_clk_hz; > > +u32 cm_l4_mp_clk_hz; > > +u32 cm_l4_sys_free_clk_hz; > Shouldn't these values be static ? (if needed at all) ? They are used in clock_manager.c clock info printing. But, I can restructure that part and keep all these static. > > > > > +struct strtopu32 { > > + const char *str; > > + u32 *p; > > +}; > What is this ^ and is this needed ? Probably not ... Will remove. > [...] > > +struct alteragrp_cfg { > > + u32 nocclk; > > + u32 mpuclk; > > +}; > > + > > +static const struct socfpga_clock_manager *clock_manager_base = > > + (struct socfpga_clock_manager *)SOCFPGA_CLKMGR_ADDRESS; > > + > > +static int of_to_struct(const void *blob, int node, int cfg_len, > > void *cfg) > > +{ > > + if (fdtdec_get_int_array(blob, node, "altr,of_reg_value", > > +  (u32 *)cfg, cfg_len)) { > > + /* could not find required property */ > > + return 100; > 100 ? Why ? Will change to -EINVAL. > > > > > + } > > + > > + return 0; > > +} > > + > > +static int of_get_input_clks(const void *blob, int node, void > > *cfg) > > +{ > > + if (fdtdec_get_int_array(blob, node, "clock-frequency", > > +  (u32 *)cfg, 1)) { > > + /* could not find required property */ > > + return 100; > DTTO Same here. > > > > > + } > > + > > + return 0; > > +} > > + > > +static int of_get_clk_cfg(const void *blob, struct mainpll_cfg > > *main_cfg, > > +   struct perpll_cfg *per_cfg, > > +   struct alteragrp_cfg *altrgrp_cfg) > > +{ > > + int node, child, len; > > + const char *node_name; > > + > > + node = fdtdec_next_compatible(blob, 0, > > COMPAT_ALTERA_SOCFPGA_CLK); > > + > > + if (node < 0) > > + return 1; > > + > > + child = fdt_first_subnode(blob, node); > > + > Drop this extra space. Noted. > > > > > + if (child < 0) > > + return 2; > Use errno.h retvals ? Will change to -EINVAL. Same for error below. > > > > > + child = fdt_first_subnode(blob, child); > > + > > + if (child < 0) > > + return 3; > > + > > + node_name = fdt_get_name(blob, child, &len); > > + > > + while (node_name) { > > + if (!strcmp(node_name, "osc1")) { > > + if (of_get_input_clks(blob, child, > > &eosc1_hz)) > > + return 101; > > + } else if (!strcmp(node_name, "cb_intosc_ls_clk")) > > { > > + if (of_get_input_clks(blob, child, > > &cb_intosc_hz)) > > + return 102; > > + } else if (!strcmp(node_name, "f2s_free_clk")) { > > + if (of_get_input_clks(blob, child, > > &f2s_free_hz)) > > + return 103; > > + } else if (!strcmp(node_name, "main_pll")) { > > + if (of_to_struct(blob, child, > > +  sizeof(*main_cfg)/sizeof( > > u32), > > +  main_cfg)) > > + return 104; > > + } else if (!strcmp(node_name, "periph_pll")) { > > + if (of_to_struct(blob, child, > > +  sizeof(*per_cfg)/sizeof(u > > 32), > > +  per_cfg)) > > + return 105; > > + } else if (!strcmp(node_name, "altera")) { > > + if (of_to_struct(blob, child, > > +  sizeof(*altrgrp_cfg)/size > > of(u32), > > +  altrgrp_cfg)) > > + return 106; > > + > > + main_cfg->mpuclk = altrgrp_cfg->mpuclk; > > + main_cfg->nocclk = altrgrp_cfg->nocclk; > > + } > > + child = fdt_next_subnode(blob, child); > > + > > + if (child < 0) > > + break; > > + > > + node_name = fdt_get_name(blob, child, &len); > > + } > > + > > + return 0; > > +} > [...] > > > > > +/* ramping the main PLL to final value */ > > +static void cm_pll_ramp_main(struct mainpll_cfg *main_cfg, > > +     struct perpll_cfg *per_cfg, > > +     unsigned int pll_ramp_main_hz) > > +{ > > + unsigned int clk_hz = 0, clk_incr_hz = 0, clk_final_hz = > > 0; > > + > > + /* find out the increment value */ > > + if (main_cfg->mpuclk_src == > > CLKMGR_MAINPLL_MPUCLK_SRC_MAIN) { > > + clk_incr_hz = CLKMGR_PLL_RAMP_MPUCLK_INCREMENT_HZ; > > + clk_final_hz = > > cm_calc_handoff_mpu_clk_hz(main_cfg, per_cfg); > > + } else if (main_cfg->nocclk_src == > > CLKMGR_MAINPLL_NOCCLK_SRC_MAIN) { > > + clk_incr_hz = CLKMGR_PLL_RAMP_NOCCLK_INCREMENT_HZ; > > + clk_final_hz = > > cm_calc_handoff_noc_clk_hz(main_cfg, per_cfg); > > + } > > + /* execute the ramping here */ > > + for (clk_hz = pll_ramp_main_hz + clk_incr_hz; > > +      clk_hz < clk_final_hz; clk_hz += clk_incr_hz) { > > + writel((main_cfg->vco1_denom << > > + CLKMGR_MAINPLL_VCO1_DENOM_LSB) | > > + cm_calc_safe_pll_numer(0, main_cfg, > > per_cfg, clk_hz), > > + &clock_manager_base->main_pll.vco1); > > + udelay(1000); > mdelay(1); Okay. > > > > > + cm_wait_for_lock(LOCKED_MASK); > > + } > > + writel((main_cfg->vco1_denom << > > CLKMGR_MAINPLL_VCO1_DENOM_LSB) | > > + main_cfg->vco1_numer, > > + &clock_manager_base->main_pll.vco1); > > + udelay(1000); > > + cm_wait_for_lock(LOCKED_MASK); > > +} > > [...] > > > > > diff --git a/arch/arm/mach-socfpga/clock_manager_gen5.c > > b/arch/arm/mach-socfpga/clock_manager_gen5.c > > old mode 100755 > > new mode 100644 > > index 1df2ed4..5632843 > > --- a/arch/arm/mach-socfpga/clock_manager_gen5.c > > +++ b/arch/arm/mach-socfpga/clock_manager_gen5.c > > @@ -1,7 +1,7 @@ > >  /* > > - *  Copyright (C) 2013-2017 Altera Corporation > > + * Copyright (C) 2016-2017 Intel Corporation > >   * > > - * SPDX-License-Identifier: GPL-2.0+ > > + * SPDX-License-Identifier:    GPL-2.0 > Keep the license, GPL-2.0+ ... Okay. > > > > >   */ > >   > >  #include > > @@ -65,7 +65,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; > > @@ -304,58 +303,75 @@ void cm_basic_init(const struct cm_config * > > const cfg) > >          &clock_manager_base->inter); > >  } > >   > > -static unsigned int cm_get_main_vco_clk_hz(void) > > +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); > How many of the changes here are relevant ? > > The changes to this file are total chaos and this really does need > some > splitting, it's unreviewable. This file mainly to rename of variables and minor fixes on some clock calculation. I will separate these changes from this patch and move to after "[PATCH 01/20] arm: socfpga: restructure clock manager driver". > > > > > - 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; > >  } > [...] > Thanks Regards Ley Foon