On Tue, Nov 10, 2020 at 08:32:26PM +0100, Sylwester Nawrocki wrote:
> The PLL status polling loops in the set_rate callbacks of some PLLs
> have no timeout detection and may become endless loops when something
> goes wrong with the PLL.
>
> For some PLLs there is already the ktime API based timeout detection,
> but it will not work in all conditions when .set_rate gets called.
> In particular, before the clocksource is initialized or when the
> timekeeping is suspended.
>
> This patch adds a common helper with the PLL status bit polling and
> timeout detection. For conditions where the timekeeping API should not
> be used a simple readl_relaxed/cpu_relax() busy loop is added with the
> iterations limit derived from measurements of readl_relaxed() execution
> time for various PLL types and Exynos SoCs variants.
>
> Actual PLL lock time depends on the P divider value, the VCO frequency
> and a constant PLL type specific LOCK_FACTOR and can be calculated as
>
> lock_time = Pdiv * LOCK_FACTOR / VCO_freq
>
> For the ktime API use cases a common timeout value of 20 ms is applied
> for all the PLLs with an assumption that maximum possible value of Pdiv
> is 64, maximum possible LOCK_FACTOR value is 3000 and minimum VCO
> frequency is 24 MHz.
>
> Signed-off-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
> ---
> I'm not sure whether we actually need to implement precise timeouts,
> likely the simple busy loop case would be enough. AFAIK the PLL
> failures happen very rarely, mostly in early code development stage
> for given platform.
>
> Changes since v3:
> - dropped udelay() from the PLL status bit polling loop as it didn't
> work on arm64 at early boot time, before timekeeping was initialized,
> - use the timekeeping API in cases when it is already initialized and
> not suspended,
> - use samsung_pll_lock_wait() also in samsung_pll3xxx_enable() function,
> now all potential endless loops are removed.
> ---
> drivers/clk/samsung/clk-pll.c | 147 ++++++++++++++++++++----------------------
> 1 file changed, 71 insertions(+), 76 deletions(-)
>
> diff --git a/drivers/clk/samsung/clk-pll.c b/drivers/clk/samsung/clk-pll.c
> index ac70ad7..cefb57e 100644
> --- a/drivers/clk/samsung/clk-pll.c
> +++ b/drivers/clk/samsung/clk-pll.c
> @@ -8,14 +8,17 @@
>
> #include <linux/errno.h>
> #include <linux/hrtimer.h>
> +#include <linux/iopoll.h>
> #include <linux/delay.h>
> #include <linux/slab.h>
> +#include <linux/timekeeping.h>
> #include <linux/clk-provider.h>
> #include <linux/io.h>
> #include "clk.h"
> #include "clk-pll.h"
>
> -#define PLL_TIMEOUT_MS 10
> +#define PLL_TIMEOUT_US 20000U
> +#define PLL_TIMEOUT_LOOPS 1000000U
>
> struct samsung_clk_pll {
> struct clk_hw hw;
> @@ -63,6 +66,53 @@ static long samsung_pll_round_rate(struct clk_hw *hw,
> return rate_table[i - 1].rate;
> }
>
> +static bool __early_timeout = true;
Drop the __ prefix and maybe use "pll_early_timeout".
This looks like __ro_after_init.
Best regards,
Krzysztof
