[U-Boot] [PATCH v3 01/19] arm: socfpga: Restructure clock manager driver

[Dinh Nguyen]

On Thu, Mar 30, 2017 at 8:08 AM, Ley Foon Tan <ley.foon.tan at intel.com> wrote:
> Restructure clock manager driver in the preparation to support A10.
> Move the Gen5 specific code to _gen5 files. No functional change.
>
> Change all uint32_t to u32 and change to use macro BIT(n) for bit shift.
>
> Signed-off-by: Ley Foon Tan <ley.foon.tan at intel.com>
> ---
>  arch/arm/mach-socfpga/Makefile                     |   3 +-
>  arch/arm/mach-socfpga/clock_manager.c              | 515 +--------------------
>  .../{clock_manager.c => clock_manager_gen5.c}      | 104 ++---
>  arch/arm/mach-socfpga/include/mach/clock_manager.h | 316 +------------
>  .../mach/{clock_manager.h => clock_manager_gen5.h} | 151 +++---
>  5 files changed, 125 insertions(+), 964 deletions(-)
>  copy arch/arm/mach-socfpga/{clock_manager.c => clock_manager_gen5.c} (88%)
>  copy arch/arm/mach-socfpga/include/mach/{clock_manager.h => clock_manager_gen5.h} (80%)
>
> diff --git a/arch/arm/mach-socfpga/Makefile b/arch/arm/mach-socfpga/Makefile
> index 809cd47..b76de4c 100644

[snip]

>
>  /* function to write a clock register that has phase information */
> -static void cm_write_with_phase(uint32_t value,
> -                               uint32_t reg_address, uint32_t mask)
> +static int cm_write_with_phase(u32 value, u32 reg_address, u32 mask)

You changed the return type of this function here...

>  {
> +       int ret;
> +
>         /* poll until phase is zero */
> -       while (readl(reg_address) & mask)
> -               ;
> +       ret = wait_for_bit(__func__, (const u32 *)reg_address, mask,
> +                          false, 20000, false);
> +       if (ret)
> +               return ret;
>
>         writel(value, reg_address);
>
> -       while (readl(reg_address) & mask)
> -               ;
> +       return wait_for_bit(__func__, (const u32 *)reg_address, mask,
> +                           false, 20000, false);
>  }

because now, you got a 20 seconds timeout and a return value if there
is a timeout,

>
>  /*
> @@ -233,11 +215,6 @@ void cm_basic_init(const struct cm_config * const cfg)
>
>         writel(cfg->gpiodiv, &clock_manager_base->per_pll.gpiodiv);
>
> -#define LOCKED_MASK \
> -       (CLKMGR_INTER_SDRPLLLOCKED_MASK  | \
> -       CLKMGR_INTER_PERPLLLOCKED_MASK  | \
> -       CLKMGR_INTER_MAINPLLLOCKED_MASK)
> -
>         cm_wait_for_lock(LOCKED_MASK);
>
>         /* write the sdram clock counters before toggling outreset all */
> @@ -257,13 +234,13 @@ void cm_basic_init(const struct cm_config * const cfg)
>          * after locking, but before taking out of bypass
>          * assert/deassert outresetall
>          */
> -       uint32_t mainvco = readl(&clock_manager_base->main_pll.vco);
> +       u32 mainvco = readl(&clock_manager_base->main_pll.vco);
>
>         /* assert main outresetall */
>         writel(mainvco | CLKMGR_MAINPLLGRP_VCO_OUTRESETALL_MASK,
>                &clock_manager_base->main_pll.vco);
>
> -       uint32_t periphvco = readl(&clock_manager_base->per_pll.vco);
> +       u32 periphvco = readl(&clock_manager_base->per_pll.vco);
>
>         /* assert pheriph outresetall */
>         writel(periphvco | CLKMGR_PERPLLGRP_VCO_OUTRESETALL_MASK,
> @@ -291,20 +268,20 @@ void cm_basic_init(const struct cm_config * const cfg)
>          * are aligned nicely; so we can change any phase.
>          */
>         cm_write_with_phase(cfg->ddrdqsclk,
> -                           (uint32_t)&clock_manager_base->sdr_pll.ddrdqsclk,
> +                           (u32)&clock_manager_base->sdr_pll.ddrdqsclk,
>                             CLKMGR_SDRPLLGRP_DDRDQSCLK_PHASE_MASK);
>
>         /* SDRAM DDR2XDQSCLK */
>         cm_write_with_phase(cfg->ddr2xdqsclk,
> -                           (uint32_t)&clock_manager_base->sdr_pll.ddr2xdqsclk,
> +                           (u32)&clock_manager_base->sdr_pll.ddr2xdqsclk,
>                             CLKMGR_SDRPLLGRP_DDR2XDQSCLK_PHASE_MASK);
>
>         cm_write_with_phase(cfg->ddrdqclk,
> -                           (uint32_t)&clock_manager_base->sdr_pll.ddrdqclk,
> +                           (u32)&clock_manager_base->sdr_pll.ddrdqclk,
>                             CLKMGR_SDRPLLGRP_DDRDQCLK_PHASE_MASK);
>
>         cm_write_with_phase(cfg->s2fuser2clk,
> -                           (uint32_t)&clock_manager_base->sdr_pll.s2fuser2clk,
> +                           (u32)&clock_manager_base->sdr_pll.s2fuser2clk,
>                             CLKMGR_SDRPLLGRP_S2FUSER2CLK_PHASE_MASK);
>

but you're not checking the return value at all?

Dinh