On Fri, Mar 31, 2017 at 4:53 AM, Dinh Nguyen <dinh.li...@gmail.com> wrote:
> On Thu, Mar 30, 2017 at 8:08 AM, Ley Foon Tan <ley.foon....@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....@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...
Will update commit message for more detail.
>
>> {
>> + 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?
Will check the return value from cm_write_with_phase() and cm_basic_init().
Thanks.
Regards
Ley Foon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
