Messages in this thread | | | Date | Thu, 24 Mar 2016 12:44:38 +0000 | From | Lee Jones <> | Subject | Re: [PATCH RESEND] mfd: arizona: Fix lockdep recursion warning on set_irq_wake |
| |
FAO Thomas
> Lockdep explicitly sets all the irq_desc locks as a single lock-class, > which causes a "possible recursive locking detected" warning when we > attempt to propagate the IRQ wake to our parent IRQ in > arizona_irq_set_wake. Although this appears to be a false positive > because an IRQ is unlikely to be its own parent, this was clearly > intentionally prohibited. > > To avoid this lockdep warning, take a cue from the regmap-irq system, > and add bus lock callbacks on the IRQ chip and propagate the wake in > the bus unlock which will happen after the desc lock has been released > and thus avoid the issue.
This looks like a hack to me. I'd like Thomas (Cc'ed) to look it over.
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com> > --- > drivers/mfd/arizona-irq.c | 35 ++++++++++++++++++++++++++++++++++- > include/linux/mfd/arizona/core.h | 3 +++ > 2 files changed, 37 insertions(+), 1 deletion(-) > > diff --git a/drivers/mfd/arizona-irq.c b/drivers/mfd/arizona-irq.c > index 5fef014..6497a65 100644 > --- a/drivers/mfd/arizona-irq.c > +++ b/drivers/mfd/arizona-irq.c > @@ -154,17 +154,48 @@ static void arizona_irq_disable(struct irq_data *data) > { > } > > +static void arizona_irq_lock(struct irq_data *data) > +{ > + struct arizona *arizona = irq_data_get_irq_chip_data(data); > + > + mutex_lock(&arizona->irq_lock); > +} > + > +static void arizona_irq_sync_unlock(struct irq_data *data) > +{ > + struct arizona *arizona = irq_data_get_irq_chip_data(data); > + int i; > + > + if (arizona->pending_wake_count < 0) > + for (i = arizona->pending_wake_count; i < 0; i++) > + irq_set_irq_wake(arizona->irq, 0); > + else if (arizona->pending_wake_count > 0) > + for (i = 0; i < arizona->pending_wake_count; i++) > + irq_set_irq_wake(arizona->irq, 1); > + > + arizona->pending_wake_count = 0; > + > + mutex_unlock(&arizona->irq_lock); > +} > + > static int arizona_irq_set_wake(struct irq_data *data, unsigned int on) > { > struct arizona *arizona = irq_data_get_irq_chip_data(data); > > - return irq_set_irq_wake(arizona->irq, on); > + if (on) > + arizona->pending_wake_count++; > + else > + arizona->pending_wake_count--; > + > + return 0; > } > > static struct irq_chip arizona_irq_chip = { > .name = "arizona", > .irq_disable = arizona_irq_disable, > .irq_enable = arizona_irq_enable, > + .irq_bus_lock = arizona_irq_lock, > + .irq_bus_sync_unlock = arizona_irq_sync_unlock, > .irq_set_wake = arizona_irq_set_wake, > }; > > @@ -193,6 +224,8 @@ int arizona_irq_init(struct arizona *arizona) > const struct regmap_irq_chip *aod, *irq; > struct irq_data *irq_data; > > + mutex_init(&arizona->irq_lock); > + > arizona->ctrlif_error = true; > > switch (arizona->type) { > diff --git a/include/linux/mfd/arizona/core.h b/include/linux/mfd/arizona/core.h > index d55a422..a0374ea 100644 > --- a/include/linux/mfd/arizona/core.h > +++ b/include/linux/mfd/arizona/core.h > @@ -148,6 +148,9 @@ struct arizona { > uint16_t dac_comp_coeff; > uint8_t dac_comp_enabled; > struct mutex dac_comp_lock; > + > + int pending_wake_count; > + struct mutex irq_lock; > }; > > int arizona_clk32k_enable(struct arizona *arizona);
-- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
| |