Re: [PATCH 2/3] cpufreq: qcom-hw: Fix probable nested interrupt handling

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Thu 11 Nov 09:48 CST 2021, Vladimir Zapolskiy wrote:

> Re-enabling an interrupt from its own interrupt handler may cause
> an interrupt storm, if there is a pending interrupt and because its
> handling is disabled due to already done entrance into the handler
> above in the stack.
> 
> Also, apparently it is improper to lock a mutex in an interrupt contex.

There shouldn't be, given that it's a threaded irq...

As a fix for the immediate problem, the change looks good. But I wonder
if we want to make sure the thermal pressure is updated in the irq
handler still? Perhaps it's not worth the resulting complexity of the
implementation...


Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> 
> Fixes: 275157b367f4 ("cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support")
> Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org>
> ---
>  drivers/cpufreq/qcom-cpufreq-hw.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> index 3b5835336658..5d55217caa8b 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -343,9 +343,9 @@ static irqreturn_t qcom_lmh_dcvs_handle_irq(int irq, void *data)
>  
>  	/* Disable interrupt and enable polling */
>  	disable_irq_nosync(c_data->throttle_irq);
> -	qcom_lmh_dcvs_notify(c_data);
> +	schedule_delayed_work(&c_data->throttle_work, 0);
>  
> -	return 0;
> +	return IRQ_HANDLED;
>  }
>  
>  static const struct qcom_cpufreq_soc_data qcom_soc_data = {
> -- 
> 2.32.0
>