From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linus Walleij Subject: Re: [PATCH v5 1/3] gpio: xgene: Enable X-Gene standby GPIO as interrupt controller Date: Tue, 16 Feb 2016 00:26:37 +0100 Message-ID: References: <1454041735-8434-1-git-send-email-qnguyen@apm.com> <1454041735-8434-2-git-send-email-qnguyen@apm.com> <56BB52B8.4040501@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Return-path: Received: from mail-ob0-f173.google.com ([209.85.214.173]:34412 "EHLO mail-ob0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752017AbcBOX0i (ORCPT ); Mon, 15 Feb 2016 18:26:38 -0500 Received: by mail-ob0-f173.google.com with SMTP id wb13so234146660obb.1 for ; Mon, 15 Feb 2016 15:26:38 -0800 (PST) In-Reply-To: <56BB52B8.4040501@arm.com> Sender: linux-gpio-owner@vger.kernel.org List-Id: linux-gpio@vger.kernel.org To: Marc Zyngier Cc: Quan Nguyen , "linux-gpio@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , Thomas Gleixner , Jason Cooper , Y Vo , Phong Vo , Loc Ho , Feng Kan , Duc Dang , patches On Wed, Feb 10, 2016 at 4:09 PM, Marc Zyngier wrote: > On 29/01/16 04:28, Quan Nguyen wrote: >> +static void xgene_gpio_sb_domain_deactivate(struct irq_domain *d, >> + struct irq_data *irq_data) >> +{ >> + struct xgene_gpio_sb *priv = d->host_data; >> + u32 gpio = HWIRQ_TO_GPIO(priv, irq_data->hwirq); >> + >> + gpiochip_unlock_as_irq(&priv->gc, gpio); > > Is it right to do the unlock both in irq_shutdown and domain_deactivate? > This seems a bit odd to me to have such an inbalance. My hunch is that > you should either implement irq_startup, do the locking there and drop > the unlock drop deactivate, or kill irq_shutdown. > > Linus, what do you think? Those functions should be called in .irq_alloc/release_resources(), that is why Gleixner added those callbacks in the first place. Yours, Linus Walleij From mboxrd@z Thu Jan 1 00:00:00 1970 From: linus.walleij@linaro.org (Linus Walleij) Date: Tue, 16 Feb 2016 00:26:37 +0100 Subject: [PATCH v5 1/3] gpio: xgene: Enable X-Gene standby GPIO as interrupt controller In-Reply-To: <56BB52B8.4040501@arm.com> References: <1454041735-8434-1-git-send-email-qnguyen@apm.com> <1454041735-8434-2-git-send-email-qnguyen@apm.com> <56BB52B8.4040501@arm.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Wed, Feb 10, 2016 at 4:09 PM, Marc Zyngier wrote: > On 29/01/16 04:28, Quan Nguyen wrote: >> +static void xgene_gpio_sb_domain_deactivate(struct irq_domain *d, >> + struct irq_data *irq_data) >> +{ >> + struct xgene_gpio_sb *priv = d->host_data; >> + u32 gpio = HWIRQ_TO_GPIO(priv, irq_data->hwirq); >> + >> + gpiochip_unlock_as_irq(&priv->gc, gpio); > > Is it right to do the unlock both in irq_shutdown and domain_deactivate? > This seems a bit odd to me to have such an inbalance. My hunch is that > you should either implement irq_startup, do the locking there and drop > the unlock drop deactivate, or kill irq_shutdown. > > Linus, what do you think? Those functions should be called in .irq_alloc/release_resources(), that is why Gleixner added those callbacks in the first place. Yours, Linus Walleij