| Message ID | 20211111154808.2024808-3-vladimir.zapolskiy@linaro.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | cpufreq: qcom-hw: a few fixes in qcom cpufreq driver | expand |
On Thu, Nov 11, 2021 at 05:48:07PM +0200, 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. > > Fixes: 275157b367f4 ("cpufreq: qcom-cpufreq-hw: Add dcvs interrupt support") > Signed-off-by: Vladimir Zapolskiy <vladimir.zapolskiy@linaro.org> Reviewed-by: Matthias Kaehlcke <mka@chromium.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 >
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 = {
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. 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(-)