On Fri, 22 Mar 2019 at 12:48, Marek Szyprowski <m.szyprowski@xxxxxxxxxxx> wrote:
>
> Add timeout to infinite loops during the CPU powerup procedures. It
> is better to report an error instead of busylooping for infinite time
> in case of failure.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> ---
> arch/arm/mach-exynos/mcpm-exynos.c | 11 ++++++++++-
> arch/arm/mach-exynos/platsmp.c | 9 ++++++++-
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/mcpm-exynos.c b/arch/arm/mach-exynos/mcpm-exynos.c
> index 72bc035bedbe..43d6f7755842 100644
> --- a/arch/arm/mach-exynos/mcpm-exynos.c
> +++ b/arch/arm/mach-exynos/mcpm-exynos.c
> @@ -75,14 +75,23 @@ static int exynos_cpu_powerup(unsigned int cpu, unsigned int cluster)
> */
> if (cluster &&
> cluster == MPIDR_AFFINITY_LEVEL(cpu_logical_map(0), 1)) {
> + unsigned int timeout = 16;
One empty line please.
> /*
> * Before we reset the Little cores, we should wait
> * the SPARE2 register is set to 1 because the init
> * codes of the iROM will set the register after
> * initialization.
> */
> - while (!pmu_raw_readl(S5P_PMU_SPARE2))
> + while (timeout && !pmu_raw_readl(S5P_PMU_SPARE2)) {
> + timeout--;
> udelay(10);
> + }
> +
> + if (timeout == 0) {
> + pr_err("cpu %d cluster %d powerup failed\n",
> + cpu, cluster);
%u - you should have a warning here.
> + return -ETIMEDOUT;
I wander whether reversing is necessary on error path
(exynos_cluster_power_down()). In case of timeout, CPU will be left
with stuck with power supplied. In the same time the next power up try
might not work properly because CPU was not shutdown.
> + }
>
> pmu_raw_writel(EXYNOS5420_KFC_CORE_RESET(cpu),
> EXYNOS_SWRESET);
> diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
> index d5d48fbdab17..c01e2f7ba1ec 100644
> --- a/arch/arm/mach-exynos/platsmp.c
> +++ b/arch/arm/mach-exynos/platsmp.c
> @@ -214,13 +214,20 @@ static inline void __iomem *cpu_boot_reg(int cpu)
> */
> void exynos_core_restart(u32 core_id)
> {
> + unsigned int timeout = 16;
> u32 val;
>
> if (!soc_is_exynos3250())
> return;
>
> - while (!pmu_raw_readl(S5P_PMU_SPARE2))
> + while (timeout && !pmu_raw_readl(S5P_PMU_SPARE2)) {
> + timeout--;
> udelay(10);
> + }
> + if (timeout == 0) {
> + pr_err("cpu core %d restart failed\n", core_id);
%u - you should have a warning here.
Best regards,
Krzysztof
