Subject: Re: [PATCH] clk: exynos: use irqsave version of spin_lock to avoid
 deadlock with irqs
To: Mike Turquette <mturquette@baylibre.com>,
        Stephen Boyd <sboyd@codeaurora.org>
References: <1449844798-27384-1-git-send-email-m.szyprowski@samsung.com>
Cc: Marek Szyprowski <m.szyprowski@samsung.com>,
        linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-pm@vger.kernel.org, Tomasz Figa <tomasz.figa@gmail.com>,
        Krzysztof Kozlowski <k.kozlowski@samsung.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
        Viresh Kumar <viresh.kumar@linaro.org>,
        Thomas Abraham <thomas.ab@samsung.com>,
        Ben Gamari <ben@smart-cactus.org>,
        Lukasz Majewski <l.majewski@samsung.com>,
        Chanwoo Choi <cw00.choi@samsung.com>,
        Kevin Hilman <khilman@linaro.org>, stable@vger.kernel.org
From: Sylwester Nawrocki <s.nawrocki@samsung.com>
Message-id: <566EF10C.2000705@samsung.com>
Date: Mon, 14 Dec 2015 17:40:44 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.4.0
MIME-version: 1.0
In-reply-to: <1449844798-27384-1-git-send-email-m.szyprowski@samsung.com>
Content-type: text/plain; charset=windows-1252
Content-transfer-encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3267
Lines: 86

On 11/12/15 15:38, Marek Szyprowski wrote:
> It is allowed to enable/disable clocks from interrupts, so common Exynos
> ARM clock management code for CPUfreq should use 'irqsave' version of
> spin_lock calls to avoid potential deadlock caused by spin_lock recursion.
> The same spin_lock is used by gate/mux clocks during enable/disable calls.
> 
> This deadlock, can be reproduced by enabling CPUfreq (ondemand or
> userspace) and decoding video with s5p-mfc driver.

> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> CC: stable@vger.kernel.org  # v4.2+

Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

Mike, Stephen, could you apply this patch directly?
It would be nice to have it in 4.4 as the bug fixed here causes
some of exynos boards in mainline to fail booting with default
config. I could resend the patch directly to you if needed.

Thanks,
Sylwester

> ---
>  drivers/clk/samsung/clk-cpu.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c
> index 2fe37f708dc7..813003d6ce09 100644
> --- a/drivers/clk/samsung/clk-cpu.c
> +++ b/drivers/clk/samsung/clk-cpu.c
> @@ -148,6 +148,7 @@ static int exynos_cpuclk_pre_rate_change(struct clk_notifier_data *ndata,
>  	unsigned long alt_prate = clk_get_rate(cpuclk->alt_parent);
>  	unsigned long alt_div = 0, alt_div_mask = DIV_MASK;
>  	unsigned long div0, div1 = 0, mux_reg;
> +	unsigned long flags;
>  
>  	/* find out the divider values to use for clock data */
>  	while ((cfg_data->prate * 1000) != ndata->new_rate) {
> @@ -156,7 +157,7 @@ static int exynos_cpuclk_pre_rate_change(struct clk_notifier_data *ndata,
>  		cfg_data++;
>  	}
>  
> -	spin_lock(cpuclk->lock);
> +	spin_lock_irqsave(cpuclk->lock, flags);
>  
>  	/*
>  	 * For the selected PLL clock frequency, get the pre-defined divider
> @@ -212,7 +213,7 @@ static int exynos_cpuclk_pre_rate_change(struct clk_notifier_data *ndata,
>  				DIV_MASK_ALL);
>  	}
>  
> -	spin_unlock(cpuclk->lock);
> +	spin_unlock_irqrestore(cpuclk->lock, flags);
>  	return 0;
>  }
>  
> @@ -223,6 +224,7 @@ static int exynos_cpuclk_post_rate_change(struct clk_notifier_data *ndata,
>  	const struct exynos_cpuclk_cfg_data *cfg_data = cpuclk->cfg;
>  	unsigned long div = 0, div_mask = DIV_MASK;
>  	unsigned long mux_reg;
> +	unsigned long flags;
>  
>  	/* find out the divider values to use for clock data */
>  	if (cpuclk->flags & CLK_CPU_NEEDS_DEBUG_ALT_DIV) {
> @@ -233,7 +235,7 @@ static int exynos_cpuclk_post_rate_change(struct clk_notifier_data *ndata,
>  		}
>  	}
>  
> -	spin_lock(cpuclk->lock);
> +	spin_lock_irqsave(cpuclk->lock, flags);
>  
>  	/* select mout_apll as the alternate parent */
>  	mux_reg = readl(base + E4210_SRC_CPU);
> @@ -246,7 +248,7 @@ static int exynos_cpuclk_post_rate_change(struct clk_notifier_data *ndata,
>  	}
>  
>  	exynos_set_safe_div(base, div, div_mask);
> -	spin_unlock(cpuclk->lock);
> +	spin_unlock_irqrestore(cpuclk->lock, flags);
>  	return 0;
>  }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/