From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755761AbcCXMov (ORCPT ); Thu, 24 Mar 2016 08:44:51 -0400 Received: from mail-wm0-f49.google.com ([74.125.82.49]:37925 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751418AbcCXMon (ORCPT ); Thu, 24 Mar 2016 08:44:43 -0400 Date: Thu, 24 Mar 2016 12:44:38 +0000 From: Lee Jones To: Charles Keepax , Thomas Gleixner Cc: linux-kernel@vger.kernel.org, patches@opensource.wolfsonmicro.com Subject: Re: [PATCH RESEND] mfd: arizona: Fix lockdep recursion warning on set_irq_wake Message-ID: <20160324124438.GA8538@x1> References: <1457605009-7792-1-git-send-email-ckeepax@opensource.wolfsonmicro.com> <20160324123618.GB3496@x1> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20160324123618.GB3496@x1> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > --- > 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