| Message ID | f2c4c88d90bf7473e1b84b8a99b7b33d7a081764.1612249657.git.matti.vaittinen@fi.rohmeurope.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | [1/2] regulator: qcom-labibb: avoid unbalanced IRQ enable | expand |
Il 02/02/21 08:37, Matti Vaittinen ha scritto: > Calling the disable_irq() from irq handler might be a bad idea as > disable_irq() should wait for handlers to run. I don't see why > this wouldn't deadlock in wait_event waiting for the threaded > handler to complete. > > Use disable_irq_nosync() instead. > It didn't deadlock, but looking at it again -- oh my, I agree with you. Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@somainline.org> > Fixes: 390af53e04114 ("regulator: qcom-labibb: Implement short-circuit and over-current IRQs") > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> > --- > This fix is done purely based on code reading. No testing is done. > > I don't have the HW (and even if I did I might have hard time producing > these errors) I have not tested this and I am unsure if my code-reading > is correct => I would _really_ appreciate second opinion and/or testing > > drivers/regulator/qcom-labibb-regulator.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/regulator/qcom-labibb-regulator.c b/drivers/regulator/qcom-labibb-regulator.c > index 5ac4566f9b7f..40e92670e307 100644 > --- a/drivers/regulator/qcom-labibb-regulator.c > +++ b/drivers/regulator/qcom-labibb-regulator.c > @@ -283,7 +283,7 @@ static irqreturn_t qcom_labibb_ocp_isr(int irq, void *chip) > * Disable the interrupt temporarily, or it will fire continuously; > * we will re-enable it in the recovery worker function. > */ > - disable_irq(irq); > + disable_irq_nosync(irq); > > /* Warn the user for overcurrent */ > dev_warn(vreg->dev, "Over-Current interrupt fired!\n"); > @@ -536,7 +536,7 @@ static irqreturn_t qcom_labibb_sc_isr(int irq, void *chip) > * Disable the interrupt temporarily, or it will fire continuously; > * we will re-enable it in the recovery worker function. > */ > - disable_irq(irq); > + disable_irq_nosync(irq); > > /* Signal out of regulation event to drivers */ > regulator_notifier_call_chain(vreg->rdev, >
diff --git a/drivers/regulator/qcom-labibb-regulator.c b/drivers/regulator/qcom-labibb-regulator.c index 5ac4566f9b7f..40e92670e307 100644 --- a/drivers/regulator/qcom-labibb-regulator.c +++ b/drivers/regulator/qcom-labibb-regulator.c @@ -283,7 +283,7 @@ static irqreturn_t qcom_labibb_ocp_isr(int irq, void *chip) * Disable the interrupt temporarily, or it will fire continuously; * we will re-enable it in the recovery worker function. */ - disable_irq(irq); + disable_irq_nosync(irq); /* Warn the user for overcurrent */ dev_warn(vreg->dev, "Over-Current interrupt fired!\n"); @@ -536,7 +536,7 @@ static irqreturn_t qcom_labibb_sc_isr(int irq, void *chip) * Disable the interrupt temporarily, or it will fire continuously; * we will re-enable it in the recovery worker function. */ - disable_irq(irq); + disable_irq_nosync(irq); /* Signal out of regulation event to drivers */ regulator_notifier_call_chain(vreg->rdev,
Calling the disable_irq() from irq handler might be a bad idea as disable_irq() should wait for handlers to run. I don't see why this wouldn't deadlock in wait_event waiting for the threaded handler to complete. Use disable_irq_nosync() instead. Fixes: 390af53e04114 ("regulator: qcom-labibb: Implement short-circuit and over-current IRQs") Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> --- This fix is done purely based on code reading. No testing is done. I don't have the HW (and even if I did I might have hard time producing these errors) I have not tested this and I am unsure if my code-reading is correct => I would _really_ appreciate second opinion and/or testing drivers/regulator/qcom-labibb-regulator.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)