From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Stuebner Subject: Re: [PATCH] pinctrl: rockchip: add irq_enable & irq_disable ops Date: Wed, 15 Mar 2017 18:46:38 +0100 Message-ID: <2305921.lIYBC71m00@phil> References: <1488434212-6567-1-git-send-email-jeffy.chen@rock-chips.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <1488434212-6567-1-git-send-email-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+glpar-linux-rockchip=m.gmane.org-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org To: Jeffy Chen , John Keeping Cc: linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Linus Walleij , Brian Norris , Douglas Anderson , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Tomasz Figa , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org List-Id: linux-gpio@vger.kernel.org Am Donnerstag, 2. M=E4rz 2017, 13:56:52 CET schrieb Jeffy Chen: > Currently we are trying to enable/disable the clk of irq's gpio bank when > unmask/mask irq. But the kernel's "lazy disable approach" will skip maski= ng > irq when the irq chip doesn't support irq_disable ops. > = > So we may hit this case: > irq_enable-> enable clk > irq_disable-> noop > irq_enable-> enable clk again > irq_disable-> noop > = > Signed-off-by: Jeffy Chen While I'm not that versed in irqchip details, reading through some other = irqchips makes this look correct handling, so also makes sense in the conte= xt = of the pinctrl irqs here Reviewed-by: Heiko Stuebner > --- > = > drivers/pinctrl/pinctrl-rockchip.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > = > diff --git a/drivers/pinctrl/pinctrl-rockchip.c > b/drivers/pinctrl/pinctrl-rockchip.c index 7813599..a9a7dfa 100644 > --- a/drivers/pinctrl/pinctrl-rockchip.c > +++ b/drivers/pinctrl/pinctrl-rockchip.c > @@ -2061,7 +2061,7 @@ static void rockchip_irq_resume(struct irq_data *d) > clk_disable(bank->clk); > } > = > -static void rockchip_irq_gc_mask_clr_bit(struct irq_data *d) > +static void rockchip_irq_enable(struct irq_data *d) > { > struct irq_chip_generic *gc =3D irq_data_get_irq_chip_data(d); > struct rockchip_pin_bank *bank =3D gc->private; > @@ -2070,7 +2070,7 @@ static void rockchip_irq_gc_mask_clr_bit(struct > irq_data *d) irq_gc_mask_clr_bit(d); > } > = > -static void rockchip_irq_gc_mask_set_bit(struct irq_data *d) > +static void rockchip_irq_disable(struct irq_data *d) > { > struct irq_chip_generic *gc =3D irq_data_get_irq_chip_data(d); > struct rockchip_pin_bank *bank =3D gc->private; > @@ -2137,9 +2137,10 @@ static int rockchip_interrupts_register(struct > platform_device *pdev, gc->chip_types[0].regs.mask =3D GPIO_INTMASK; > gc->chip_types[0].regs.ack =3D GPIO_PORTS_EOI; > gc->chip_types[0].chip.irq_ack =3D irq_gc_ack_set_bit; > - gc->chip_types[0].chip.irq_mask =3D rockchip_irq_gc_mask_set_bit; > - gc->chip_types[0].chip.irq_unmask =3D > - rockchip_irq_gc_mask_clr_bit; > + gc->chip_types[0].chip.irq_mask =3D irq_gc_mask_set_bit; > + gc->chip_types[0].chip.irq_unmask =3D irq_gc_mask_clr_bit; > + gc->chip_types[0].chip.irq_enable =3D rockchip_irq_enable; > + gc->chip_types[0].chip.irq_disable =3D rockchip_irq_disable; > gc->chip_types[0].chip.irq_set_wake =3D irq_gc_set_wake; > gc->chip_types[0].chip.irq_suspend =3D rockchip_irq_suspend; > gc->chip_types[0].chip.irq_resume =3D rockchip_irq_resume; From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753312AbdCOSa0 convert rfc822-to-8bit (ORCPT ); Wed, 15 Mar 2017 14:30:26 -0400 Received: from gloria.sntech.de ([95.129.55.99]:51514 "EHLO gloria.sntech.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751487AbdCOS37 (ORCPT ); Wed, 15 Mar 2017 14:29:59 -0400 From: Heiko Stuebner To: Jeffy Chen , John Keeping Cc: linux-gpio@vger.kernel.org, Brian Norris , Douglas Anderson , Tomasz Figa , linux-rockchip@lists.infradead.org, Linus Walleij , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH] pinctrl: rockchip: add irq_enable & irq_disable ops Date: Wed, 15 Mar 2017 18:46:38 +0100 Message-ID: <2305921.lIYBC71m00@phil> User-Agent: KMail/5.2.3 (Linux/4.9.0-1-amd64; KDE/5.27.0; x86_64; ; ) In-Reply-To: <1488434212-6567-1-git-send-email-jeffy.chen@rock-chips.com> References: <1488434212-6567-1-git-send-email-jeffy.chen@rock-chips.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="iso-8859-1" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am Donnerstag, 2. März 2017, 13:56:52 CET schrieb Jeffy Chen: > Currently we are trying to enable/disable the clk of irq's gpio bank when > unmask/mask irq. But the kernel's "lazy disable approach" will skip masking > irq when the irq chip doesn't support irq_disable ops. > > So we may hit this case: > irq_enable-> enable clk > irq_disable-> noop > irq_enable-> enable clk again > irq_disable-> noop > > Signed-off-by: Jeffy Chen While I'm not that versed in irqchip details, reading through some other irqchips makes this look correct handling, so also makes sense in the context of the pinctrl irqs here Reviewed-by: Heiko Stuebner > --- > > drivers/pinctrl/pinctrl-rockchip.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-rockchip.c > b/drivers/pinctrl/pinctrl-rockchip.c index 7813599..a9a7dfa 100644 > --- a/drivers/pinctrl/pinctrl-rockchip.c > +++ b/drivers/pinctrl/pinctrl-rockchip.c > @@ -2061,7 +2061,7 @@ static void rockchip_irq_resume(struct irq_data *d) > clk_disable(bank->clk); > } > > -static void rockchip_irq_gc_mask_clr_bit(struct irq_data *d) > +static void rockchip_irq_enable(struct irq_data *d) > { > struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); > struct rockchip_pin_bank *bank = gc->private; > @@ -2070,7 +2070,7 @@ static void rockchip_irq_gc_mask_clr_bit(struct > irq_data *d) irq_gc_mask_clr_bit(d); > } > > -static void rockchip_irq_gc_mask_set_bit(struct irq_data *d) > +static void rockchip_irq_disable(struct irq_data *d) > { > struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); > struct rockchip_pin_bank *bank = gc->private; > @@ -2137,9 +2137,10 @@ static int rockchip_interrupts_register(struct > platform_device *pdev, gc->chip_types[0].regs.mask = GPIO_INTMASK; > gc->chip_types[0].regs.ack = GPIO_PORTS_EOI; > gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit; > - gc->chip_types[0].chip.irq_mask = rockchip_irq_gc_mask_set_bit; > - gc->chip_types[0].chip.irq_unmask = > - rockchip_irq_gc_mask_clr_bit; > + gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit; > + gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit; > + gc->chip_types[0].chip.irq_enable = rockchip_irq_enable; > + gc->chip_types[0].chip.irq_disable = rockchip_irq_disable; > gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake; > gc->chip_types[0].chip.irq_suspend = rockchip_irq_suspend; > gc->chip_types[0].chip.irq_resume = rockchip_irq_resume; From mboxrd@z Thu Jan 1 00:00:00 1970 From: heiko@sntech.de (Heiko Stuebner) Date: Wed, 15 Mar 2017 18:46:38 +0100 Subject: [PATCH] pinctrl: rockchip: add irq_enable & irq_disable ops In-Reply-To: <1488434212-6567-1-git-send-email-jeffy.chen@rock-chips.com> References: <1488434212-6567-1-git-send-email-jeffy.chen@rock-chips.com> Message-ID: <2305921.lIYBC71m00@phil> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Am Donnerstag, 2. M?rz 2017, 13:56:52 CET schrieb Jeffy Chen: > Currently we are trying to enable/disable the clk of irq's gpio bank when > unmask/mask irq. But the kernel's "lazy disable approach" will skip masking > irq when the irq chip doesn't support irq_disable ops. > > So we may hit this case: > irq_enable-> enable clk > irq_disable-> noop > irq_enable-> enable clk again > irq_disable-> noop > > Signed-off-by: Jeffy Chen While I'm not that versed in irqchip details, reading through some other irqchips makes this look correct handling, so also makes sense in the context of the pinctrl irqs here Reviewed-by: Heiko Stuebner > --- > > drivers/pinctrl/pinctrl-rockchip.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-rockchip.c > b/drivers/pinctrl/pinctrl-rockchip.c index 7813599..a9a7dfa 100644 > --- a/drivers/pinctrl/pinctrl-rockchip.c > +++ b/drivers/pinctrl/pinctrl-rockchip.c > @@ -2061,7 +2061,7 @@ static void rockchip_irq_resume(struct irq_data *d) > clk_disable(bank->clk); > } > > -static void rockchip_irq_gc_mask_clr_bit(struct irq_data *d) > +static void rockchip_irq_enable(struct irq_data *d) > { > struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); > struct rockchip_pin_bank *bank = gc->private; > @@ -2070,7 +2070,7 @@ static void rockchip_irq_gc_mask_clr_bit(struct > irq_data *d) irq_gc_mask_clr_bit(d); > } > > -static void rockchip_irq_gc_mask_set_bit(struct irq_data *d) > +static void rockchip_irq_disable(struct irq_data *d) > { > struct irq_chip_generic *gc = irq_data_get_irq_chip_data(d); > struct rockchip_pin_bank *bank = gc->private; > @@ -2137,9 +2137,10 @@ static int rockchip_interrupts_register(struct > platform_device *pdev, gc->chip_types[0].regs.mask = GPIO_INTMASK; > gc->chip_types[0].regs.ack = GPIO_PORTS_EOI; > gc->chip_types[0].chip.irq_ack = irq_gc_ack_set_bit; > - gc->chip_types[0].chip.irq_mask = rockchip_irq_gc_mask_set_bit; > - gc->chip_types[0].chip.irq_unmask = > - rockchip_irq_gc_mask_clr_bit; > + gc->chip_types[0].chip.irq_mask = irq_gc_mask_set_bit; > + gc->chip_types[0].chip.irq_unmask = irq_gc_mask_clr_bit; > + gc->chip_types[0].chip.irq_enable = rockchip_irq_enable; > + gc->chip_types[0].chip.irq_disable = rockchip_irq_disable; > gc->chip_types[0].chip.irq_set_wake = irq_gc_set_wake; > gc->chip_types[0].chip.irq_suspend = rockchip_irq_suspend; > gc->chip_types[0].chip.irq_resume = rockchip_irq_resume;