From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 39934CE7A88 for ; Sun, 24 Sep 2023 09:27:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229570AbjIXJ1b (ORCPT ); Sun, 24 Sep 2023 05:27:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:32982 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229437AbjIXJ12 (ORCPT ); Sun, 24 Sep 2023 05:27:28 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CE442107; Sun, 24 Sep 2023 02:27:21 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 561A3C433C8; Sun, 24 Sep 2023 09:27:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1695547641; bh=OtKqRE8d8XyKijo02N0v5qsYYrxnvoYjWA5m+RhuMbA=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=Fw28rO4+0BcWKTqRtJ11UQd3yu8tYz0YafTOJiUhCVPahmTdtfqBG0kv5yk7K2uLP vhfOiVZjwUntx0nkT00PKJMca5e69fEqwi4Nf0Fck4gKhkgm8zNQIqtaHipH+KWZN/ CHOImXvIcyMVwFFDQ/f5o5bi0TpEh50nMSX1NGsdkhFtvRiipqldcl1immYEOXLEyu TMnNfHUTZ34JCc8mOpI9Q8huT/4lnzJ3n+kYO6vFjzUGVaiT+3ADARUBPmh9o1KbNh 0ckacHPExsD2VBXYMijC3w325KbqHLD8kAmSG7UdKyFP5u4b3a1x1dqqqx7esezVmM bgZVFmUgpBKLQ== Received: from [85.255.234.76] (helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1qkLOY-00FfQs-Sb; Sun, 24 Sep 2023 10:27:19 +0100 Date: Sun, 24 Sep 2023 10:27:13 +0100 Message-ID: <87wmwf53bi.wl-maz@kernel.org> From: Marc Zyngier To: Biju Das Cc: Thomas Gleixner , Lad Prabhakar , Claudiu Beznea , Geert Uytterhoeven , Biju Das , linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org Subject: Re: [PATCH 2/3] irqchip: renesas-rzg2l: Mask interrupts for changing interrupt settings In-Reply-To: <20230918122411.237635-3-biju.das.jz@bp.renesas.com> References: <20230918122411.237635-1-biju.das.jz@bp.renesas.com> <20230918122411.237635-3-biju.das.jz@bp.renesas.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 85.255.234.76 X-SA-Exim-Rcpt-To: biju.das.jz@bp.renesas.com, tglx@linutronix.de, prabhakar.mahadev-lad.rj@bp.renesas.com, claudiu.beznea.uj@bp.renesas.com, geert+renesas@glider.be, biju.das.au@gmail.com, linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 18 Sep 2023 13:24:10 +0100, Biju Das 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 > Tested-by: Claudiu Beznea > --- > 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. -- Without deviation from the norm, progress is not possible.