RE: [PATCH v2 08/11] socfpga: arria10: Replace delays with busy waiting in cm_full_cfg

2022-06-16 Thread Chee, Tien Fong 
Hi Pawel,

> -Original Message-
> From: Paweł Anikiel 
> Sent: Thursday, 26 May, 2022 10:37 PM
> To: Vasut, Marek ; simon.k.r.goldschm...@gmail.com;
> Chee, Tien Fong ; michal.si...@xilinx.com
> Cc: u-boot@lists.denx.de; s...@chromium.org; feste...@denx.de;
> ja...@amarulasolutions.com; andre.przyw...@arm.com; Armstrong, Neil
> ; pbrobin...@gmail.com;
> thar...@gateworks.com; paul@linaro.org; christianshew...@gmail.com;
> adrian.fiergol...@fastree3d.com; marek.be...@nic.cz; Denk, Wolfgang
> ; Lim, Elly Siew Chin ;
> m...@semihalf.com; Paweł Anikiel 
> Subject: [PATCH v2 08/11] socfpga: arria10: Replace delays with busy waiting
> in cm_full_cfg
> 
> Using udelay while the clocks aren't fully configured causes the timer system
> to save the wrong clock rate. Use sdelay and wait_on_value instead (the
> values used in these functions were found experimentally).
> 
> Signed-off-by: Paweł Anikiel 
> ---
>  arch/arm/mach-socfpga/clock_manager.c  |  7 ---
>  arch/arm/mach-socfpga/clock_manager_arria10.c  | 12 ++--
>  arch/arm/mach-socfpga/include/mach/clock_manager.h |  4 
>  3 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-socfpga/clock_manager.c b/arch/arm/mach-
> socfpga/clock_manager.c
> index 9e645a4253..c9bd4859f7 100644
> --- a/arch/arm/mach-socfpga/clock_manager.c
> +++ b/arch/arm/mach-socfpga/clock_manager.c
> @@ -39,9 +39,10 @@ void cm_wait_for_lock(u32 mask)
>  /* function to poll in the fsm busy bit */  int cm_wait_for_fsm(void)  {
> - return wait_for_bit_le32((const void *)(socfpga_get_clkmgr_addr()
> +
> -  CLKMGR_STAT), CLKMGR_STAT_BUSY, false,
> 2,
> -  false);
> + void *reg = (void *)(socfpga_get_clkmgr_addr() + CLKMGR_STAT);
> +
> + /* 20s timeout */
> + return wait_on_value(CLKMGR_STAT_BUSY, 0, reg, 1);

This change breaking the socfpga_stratix10_atf_defconfig build(all aarch64 
socfpga are impacted), can you fix it ?

aarch64-none-linux-gnu-ld.bfd: arch/arm/mach-socfpga/clock_manager.o: in 
function `cm_wait_for_fsm':
/nfs/png/disks/swuser_work_tfchee/uboot_mainline/denx/u-boot/arch/arm/mach-socfpga/clock_manager.c:45:
 undefined reference to `wait_on_value'
/nfs/png/disks/swuser_work_tfchee/uboot_mainline/denx/u-boot/arch/arm/mach-socfpga/clock_manager.c:45:(.text.cm_wait_for_fsm+0x24):
 relocation truncated to fit: R_AARCH64_JUMP26 against undefined symbol 
`wait_on_value'
make[1]: *** [scripts/Makefile.spl:525: spl/u-boot-spl] Error 1
make: *** [Makefile:2106: spl/u-boot-spl] Error 2
aarch64-none-linux-gnu-objcopy: 'spl/u-boot-spl-dtb.bin': No such file

Thanks.

Best regards,
Tien Fong.

Re: [PATCH v2 08/11] socfpga: arria10: Replace delays with busy waiting in cm_full_cfg

2022-05-27 Thread Simon Glass 
On Thu, 26 May 2022 at 07:38, Paweł Anikiel  wrote:
>
> Using udelay while the clocks aren't fully configured causes the timer
> system to save the wrong clock rate. Use sdelay and wait_on_value
> instead (the values used in these functions were found experimentally).
>
> Signed-off-by: Paweł Anikiel 
> ---
>  arch/arm/mach-socfpga/clock_manager.c  |  7 ---
>  arch/arm/mach-socfpga/clock_manager_arria10.c  | 12 ++--
>  arch/arm/mach-socfpga/include/mach/clock_manager.h |  4 
>  3 files changed, 14 insertions(+), 9 deletions(-)

Reviewed-by: Simon Glass

[PATCH v2 08/11] socfpga: arria10: Replace delays with busy waiting in cm_full_cfg

2022-05-26 Thread Paweł Anikiel 
Using udelay while the clocks aren't fully configured causes the timer
system to save the wrong clock rate. Use sdelay and wait_on_value
instead (the values used in these functions were found experimentally).

Signed-off-by: Paweł Anikiel 
---
 arch/arm/mach-socfpga/clock_manager.c  |  7 ---
 arch/arm/mach-socfpga/clock_manager_arria10.c  | 12 ++--
 arch/arm/mach-socfpga/include/mach/clock_manager.h |  4 
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-socfpga/clock_manager.c 
b/arch/arm/mach-socfpga/clock_manager.c
index 9e645a4253..c9bd4859f7 100644
--- a/arch/arm/mach-socfpga/clock_manager.c
+++ b/arch/arm/mach-socfpga/clock_manager.c
@@ -39,9 +39,10 @@ void cm_wait_for_lock(u32 mask)
 /* function to poll in the fsm busy bit */
 int cm_wait_for_fsm(void)
 {
-   return wait_for_bit_le32((const void *)(socfpga_get_clkmgr_addr() +
-CLKMGR_STAT), CLKMGR_STAT_BUSY, false, 2,
-false);
+   void *reg = (void *)(socfpga_get_clkmgr_addr() + CLKMGR_STAT);
+
+   /* 20s timeout */
+   return wait_on_value(CLKMGR_STAT_BUSY, 0, reg, 1);
 }
 
 int set_cpu_clk_info(void)
diff --git a/arch/arm/mach-socfpga/clock_manager_arria10.c 
b/arch/arm/mach-socfpga/clock_manager_arria10.c
index 58d5d3fd8a..daa06b9d03 100644
--- a/arch/arm/mach-socfpga/clock_manager_arria10.c
+++ b/arch/arm/mach-socfpga/clock_manager_arria10.c
@@ -551,13 +551,13 @@ static void cm_pll_ramp_main(struct mainpll_cfg *main_cfg,
CLKMGR_MAINPLL_VCO1_DENOM_LSB) |
cm_calc_safe_pll_numer(0, main_cfg, per_cfg, clk_hz),
socfpga_get_clkmgr_addr() + CLKMGR_A10_MAINPLL_VCO1);
-   mdelay(1);
+   sdelay(100); /* 1ms */
cm_wait_for_lock(LOCKED_MASK);
}
writel((main_cfg->vco1_denom << CLKMGR_MAINPLL_VCO1_DENOM_LSB) |
main_cfg->vco1_numer,
socfpga_get_clkmgr_addr() + CLKMGR_A10_MAINPLL_VCO1);
-   mdelay(1);
+   sdelay(100); /* 1ms */
cm_wait_for_lock(LOCKED_MASK);
 }
 
@@ -585,13 +585,13 @@ static void cm_pll_ramp_periph(struct mainpll_cfg 
*main_cfg,
 clk_hz),
  socfpga_get_clkmgr_addr() +
  CLKMGR_A10_PERPLL_VCO1);
-   mdelay(1);
+   sdelay(100); /* 1ms */
cm_wait_for_lock(LOCKED_MASK);
}
writel((per_cfg->vco1_denom << CLKMGR_PERPLL_VCO1_DENOM_LSB) |
  per_cfg->vco1_numer,
  socfpga_get_clkmgr_addr() + CLKMGR_A10_PERPLL_VCO1);
-   mdelay(1);
+   sdelay(100); /* 1ms */
cm_wait_for_lock(LOCKED_MASK);
 }
 
@@ -727,7 +727,7 @@ static int cm_full_cfg(struct mainpll_cfg *main_cfg, struct 
perpll_cfg *per_cfg)
socfpga_get_clkmgr_addr() + CLKMGR_A10_PERPLL_VCO1);
 
/* Wait for at least 5 us */
-   udelay(5);
+   sdelay(5000);
 
/* Now deassert BGPWRDN and PWRDN */
clrbits_le32(socfpga_get_clkmgr_addr() + CLKMGR_A10_MAINPLL_VCO0,
@@ -738,7 +738,7 @@ static int cm_full_cfg(struct mainpll_cfg *main_cfg, struct 
perpll_cfg *per_cfg)
 CLKMGR_PERPLL_VCO0_PWRDN_SET_MSK);
 
/* Wait for at least 7 us */
-   udelay(7);
+   sdelay(7000);
 
/* enable the VCO and disable the external regulator to PLL */
writel((readl(socfpga_get_clkmgr_addr() + CLKMGR_A10_MAINPLL_VCO0) &
diff --git a/arch/arm/mach-socfpga/include/mach/clock_manager.h 
b/arch/arm/mach-socfpga/include/mach/clock_manager.h
index a8cb07a1c4..78013f0527 100644
--- a/arch/arm/mach-socfpga/include/mach/clock_manager.h
+++ b/arch/arm/mach-socfpga/include/mach/clock_manager.h
@@ -20,6 +20,10 @@ int cm_set_qspi_controller_clk_hz(u32 clk_hz);
 #endif
 #endif
 
+void sdelay(unsigned long loops);
+u32 wait_on_value(u32 read_bit_mask, u32 match_value, void *read_addr,
+ u32 bound);
+
 #if defined(CONFIG_TARGET_SOCFPGA_GEN5)
 #include 
 #elif defined(CONFIG_TARGET_SOCFPGA_ARRIA10)
-- 
2.36.1.124.g0e6072fb45-goog