Message ID | 20230918122411.237635-3-biju.das.jz@bp.renesas.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:172:b0:3f2:4152:657d with SMTP id h50csp2625311vqi; Mon, 18 Sep 2023 05:40:31 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGVSA4bgXYTY2br5JSzUIemk5Kzwm6TFAgui6vte2zb7TKFjNMdawg9HbBsTSmNTunGC350 X-Received: by 2002:a17:902:e744:b0:1bb:30c5:835a with SMTP id p4-20020a170902e74400b001bb30c5835amr9041520plf.7.1695040830911; Mon, 18 Sep 2023 05:40:30 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1695040830; cv=none; d=google.com; s=arc-20160816; b=mZOUG2ruRnCtjUlbPhDBJ6FcdxZWwkOLE71GxqjEz3YIng9lQ50nj0tZEZPS8WFX9Q 3cP7E7lJdICFhT9AjeOEEnrjB+ZzM1pcSZwU55UxguX7mla5jwiunKn2rbbqgSh6V1cG gXzzmaK7VBgCMEcfoVetFz6GZDXQTxF9CIZFP/paz8TLnCUlGjkvnoA7nmQWCbqN1CJ5 6opdARoh+L9uiK+L//EF/rQBeBQIFQ9II6SZnwVEgohjbQkt35svOVoI1Dmhm9SmFTuc DnFtCZl1FG0yMjoXJIwkrDGD6PjxC4pGihOqcJDwUD0kBhyKd4wEhvxEBBtFdfPOnYbK 79hg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=kWAYsRcLVJHP56QGfOuoyxt3WMyTP8X5tyuHao+rugo=; fh=sWH11FnHMfCkHSU9kXlYjtdyZVAjuuR7mMUGHavtm3Q=; b=0c30csJMOk1j6ltzr0hlOypM1OkKjTfZDV+QLPrIXMAqAA/7TJDNyQorNRnI0LEQDz xUAPUVYUQymgPWYRp62eeWYSnL+1hQVAoWcJ5Cqne1MacNWjObRvgEQrmoT0wUa9Hgva 2idcPrMsFRI8bEBLLdWbwbnlNznfuMHcmbOo/+cZndxnsOfmajfFbX/wdLQSvMXNhPqY c0NRdi3Yr8rQAOTxpjAUmzlFEVM/dgBTC41OUN2OUWH8JlS/GxFjQjRd5p477E/ES3ip kXOze4zvDhL25W8Ea25vae6qMrggATyDZ8Lhx43IzINYvJGt1LZAB2mQ8TbM7yT6SN7i TszA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=renesas.com Received: from snail.vger.email (snail.vger.email. [23.128.96.37]) by mx.google.com with ESMTPS id l15-20020a170903244f00b001b178baf356si8359997pls.95.2023.09.18.05.40.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 18 Sep 2023 05:40:30 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=renesas.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id 0DCEB81A7ADC; Mon, 18 Sep 2023 05:25:16 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241956AbjIRMYu (ORCPT <rfc822;kernel.ruili@gmail.com> + 27 others); Mon, 18 Sep 2023 08:24:50 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59348 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242000AbjIRMYf (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 18 Sep 2023 08:24:35 -0400 Received: from relmlie5.idc.renesas.com (relmlor1.renesas.com [210.160.252.171]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 9B88B106; Mon, 18 Sep 2023 05:24:25 -0700 (PDT) X-IronPort-AV: E=Sophos;i="6.02,156,1688396400"; d="scan'208";a="176397781" Received: from unknown (HELO relmlir6.idc.renesas.com) ([10.200.68.152]) by relmlie5.idc.renesas.com with ESMTP; 18 Sep 2023 21:24:25 +0900 Received: from localhost.localdomain (unknown [10.226.92.107]) by relmlir6.idc.renesas.com (Postfix) with ESMTP id 3569641F6AEF; Mon, 18 Sep 2023 21:24:21 +0900 (JST) From: Biju Das <biju.das.jz@bp.renesas.com> To: Thomas Gleixner <tglx@linutronix.de>, Marc Zyngier <maz@kernel.org> Cc: Biju Das <biju.das.jz@bp.renesas.com>, Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>, Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>, Geert Uytterhoeven <geert+renesas@glider.be>, Biju Das <biju.das.au@gmail.com>, linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org Subject: [PATCH 2/3] irqchip: renesas-rzg2l: Mask interrupts for changing interrupt settings Date: Mon, 18 Sep 2023 13:24:10 +0100 Message-Id: <20230918122411.237635-3-biju.das.jz@bp.renesas.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20230918122411.237635-1-biju.das.jz@bp.renesas.com> References: <20230918122411.237635-1-biju.das.jz@bp.renesas.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Mon, 18 Sep 2023 05:25:16 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1777379134183572826 X-GMAIL-MSGID: 1777379134183572826 |
Series |
Fix IRQ storm with GPIO interrupts
|
|
Commit Message
Biju Das
Sept. 18, 2023, 12:24 p.m. UTC
As per RZ/G2L hardware manual Rev.1.30 section 8.8.3 Precaution when changing interrupt settings, we need to mask the interrupts for any changes in below settings: * When changing the noise filter settings. * When switching the GPIO pins to IRQ or GPIOINT. * When changing the source of TINT. * When changing the interrupt detection method. This patch masks the interrupts when there is a change in the interrupt detection method and changing the source of TINT. Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver") Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> --- drivers/irqchip/irq-renesas-rzg2l.c | 6 ++++++ 1 file changed, 6 insertions(+)
Comments
On Mon, 18 Sep 2023 13:24:10 +0100, Biju Das <biju.das.jz@bp.renesas.com> wrote: > > As per RZ/G2L hardware manual Rev.1.30 section 8.8.3 Precaution when > changing interrupt settings, we need to mask the interrupts for > any changes in below settings: > > * When changing the noise filter settings. > * When switching the GPIO pins to IRQ or GPIOINT. > * When changing the source of TINT. > * When changing the interrupt detection method. > > This patch masks the interrupts when there is a change in the interrupt > detection method and changing the source of TINT. > > Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver") > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- > drivers/irqchip/irq-renesas-rzg2l.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c > index 2cee5477be6b..33a22bafedcd 100644 > --- a/drivers/irqchip/irq-renesas-rzg2l.c > +++ b/drivers/irqchip/irq-renesas-rzg2l.c > @@ -116,11 +116,13 @@ static void rzg2l_irqc_irq_disable(struct irq_data *d) > u8 tssr_index = TSSR_INDEX(offset); > u32 reg; > > + irq_chip_mask_parent(d); > raw_spin_lock(&priv->lock); > reg = readl_relaxed(priv->base + TSSR(tssr_index)); > reg &= ~(TSSEL_MASK << TSSEL_SHIFT(tssr_offset)); > writel_relaxed(reg, priv->base + TSSR(tssr_index)); > raw_spin_unlock(&priv->lock); > + irq_chip_unmask_parent(d); What guarantees that the parent irqchip state has been correctly restored? Nothing refcounts the nesting of mask/unmask. > } > irq_chip_disable_parent(d); I'd rather you start by *disabling* the parent, and then none of that matters at all. > } > @@ -137,11 +139,13 @@ static void rzg2l_irqc_irq_enable(struct irq_data *d) > u8 tssr_index = TSSR_INDEX(offset); > u32 reg; > > + irq_chip_mask_parent(d); > raw_spin_lock(&priv->lock); > reg = readl_relaxed(priv->base + TSSR(tssr_index)); > reg |= (TIEN | tint) << TSSEL_SHIFT(tssr_offset); > writel_relaxed(reg, priv->base + TSSR(tssr_index)); > raw_spin_unlock(&priv->lock); > + irq_chip_unmask_parent(d); > } > irq_chip_enable_parent(d); Same thing: if the parent was disabled, why do we need to do anything? > } > @@ -226,10 +230,12 @@ static int rzg2l_irqc_set_type(struct irq_data *d, unsigned int type) > unsigned int hw_irq = irqd_to_hwirq(d); > int ret = -EINVAL; > > + irq_chip_mask_parent(d); > if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT) > ret = rzg2l_irq_set_type(d, type); > else if (hw_irq >= IRQC_TINT_START && hw_irq < IRQC_NUM_IRQ) > ret = rzg2l_tint_set_edge(d, type); > + irq_chip_unmask_parent(d); This one is the only interesting one: why don't you mask the interrupt at the local level rather than on the parent? And this should be conditioned on the interrupt state itself (enabled or disabled), not done unconditionally. Thanks, M.
Hi Marc Zyngier, Thanks for the feedback. > Subject: Re: [PATCH 2/3] irqchip: renesas-rzg2l: Mask interrupts for > changing interrupt settings > > On Mon, 18 Sep 2023 13:24:10 +0100, > Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > > As per RZ/G2L hardware manual Rev.1.30 section 8.8.3 Precaution when > > changing interrupt settings, we need to mask the interrupts for any > > changes in below settings: > > > > * When changing the noise filter settings. > > * When switching the GPIO pins to IRQ or GPIOINT. > > * When changing the source of TINT. > > * When changing the interrupt detection method. > > > > This patch masks the interrupts when there is a change in the > > interrupt detection method and changing the source of TINT. > > > > Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller > > driver") > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com> > > Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > --- > > drivers/irqchip/irq-renesas-rzg2l.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/drivers/irqchip/irq-renesas-rzg2l.c > > b/drivers/irqchip/irq-renesas-rzg2l.c > > index 2cee5477be6b..33a22bafedcd 100644 > > --- a/drivers/irqchip/irq-renesas-rzg2l.c > > +++ b/drivers/irqchip/irq-renesas-rzg2l.c > > @@ -116,11 +116,13 @@ static void rzg2l_irqc_irq_disable(struct irq_data > *d) > > u8 tssr_index = TSSR_INDEX(offset); > > u32 reg; > > > > + irq_chip_mask_parent(d); > > raw_spin_lock(&priv->lock); > > reg = readl_relaxed(priv->base + TSSR(tssr_index)); > > reg &= ~(TSSEL_MASK << TSSEL_SHIFT(tssr_offset)); > > writel_relaxed(reg, priv->base + TSSR(tssr_index)); > > raw_spin_unlock(&priv->lock); > > + irq_chip_unmask_parent(d); > > What guarantees that the parent irqchip state has been correctly restored? > Nothing refcounts the nesting of mask/unmask. > > > } > > irq_chip_disable_parent(d); > > I'd rather you start by *disabling* the parent, and then none of that > matters at all. Agreed. Will do this in next version. > > > } > > @@ -137,11 +139,13 @@ static void rzg2l_irqc_irq_enable(struct irq_data > *d) > > u8 tssr_index = TSSR_INDEX(offset); > > u32 reg; > > > > + irq_chip_mask_parent(d); > > raw_spin_lock(&priv->lock); > > reg = readl_relaxed(priv->base + TSSR(tssr_index)); > > reg |= (TIEN | tint) << TSSEL_SHIFT(tssr_offset); > > writel_relaxed(reg, priv->base + TSSR(tssr_index)); > > raw_spin_unlock(&priv->lock); > > + irq_chip_unmask_parent(d); > > } > > irq_chip_enable_parent(d); > > Same thing: if the parent was disabled, why do we need to do anything? OK. It is not required. > > > > } > > @@ -226,10 +230,12 @@ static int rzg2l_irqc_set_type(struct irq_data *d, > unsigned int type) > > unsigned int hw_irq = irqd_to_hwirq(d); > > int ret = -EINVAL; > > > > + irq_chip_mask_parent(d); > > if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT) > > ret = rzg2l_irq_set_type(d, type); > > else if (hw_irq >= IRQC_TINT_START && hw_irq < IRQC_NUM_IRQ) > > ret = rzg2l_tint_set_edge(d, type); > > + irq_chip_unmask_parent(d); > > This one is the only interesting one: why don't you mask the interrupt at > the local level rather than on the parent? And this should be conditioned > on the interrupt state itself (enabled or disabled), not done > unconditionally. OK. Will do this locally by conditioned on the interrupt state. Cheers, Biju
diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c index 2cee5477be6b..33a22bafedcd 100644 --- a/drivers/irqchip/irq-renesas-rzg2l.c +++ b/drivers/irqchip/irq-renesas-rzg2l.c @@ -116,11 +116,13 @@ static void rzg2l_irqc_irq_disable(struct irq_data *d) u8 tssr_index = TSSR_INDEX(offset); u32 reg; + irq_chip_mask_parent(d); raw_spin_lock(&priv->lock); reg = readl_relaxed(priv->base + TSSR(tssr_index)); reg &= ~(TSSEL_MASK << TSSEL_SHIFT(tssr_offset)); writel_relaxed(reg, priv->base + TSSR(tssr_index)); raw_spin_unlock(&priv->lock); + irq_chip_unmask_parent(d); } irq_chip_disable_parent(d); } @@ -137,11 +139,13 @@ static void rzg2l_irqc_irq_enable(struct irq_data *d) u8 tssr_index = TSSR_INDEX(offset); u32 reg; + irq_chip_mask_parent(d); raw_spin_lock(&priv->lock); reg = readl_relaxed(priv->base + TSSR(tssr_index)); reg |= (TIEN | tint) << TSSEL_SHIFT(tssr_offset); writel_relaxed(reg, priv->base + TSSR(tssr_index)); raw_spin_unlock(&priv->lock); + irq_chip_unmask_parent(d); } irq_chip_enable_parent(d); } @@ -226,10 +230,12 @@ static int rzg2l_irqc_set_type(struct irq_data *d, unsigned int type) unsigned int hw_irq = irqd_to_hwirq(d); int ret = -EINVAL; + irq_chip_mask_parent(d); if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT) ret = rzg2l_irq_set_type(d, type); else if (hw_irq >= IRQC_TINT_START && hw_irq < IRQC_NUM_IRQ) ret = rzg2l_tint_set_edge(d, type); + irq_chip_unmask_parent(d); if (ret) return ret;