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 <[email protected]>
> > Signed-off-by: Ley Foon Tan <[email protected]>
> > ---
> > 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 <common.h>
> > #include <asm/io.h>
> > #include <asm/arch/clock_manager.h>
> > +#include <fdtdec.h>
> >
> > 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 <common.h>
> > +#include <asm/io.h>
> > +#include <asm/arch/clock_manager.h>
> > +#include <fdtdec.h>
> > +
> > +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 <www.altera.com>
> > + * 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 <common.h>
> > @@ -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
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/listinfo/u-boot
