From: Marc Zyngier <maz@kernel.org>
To: Prabhakar <prabhakar.csengg@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Rob Herring <robh+dt@kernel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
Geert Uytterhoeven <geert+renesas@glider.be>,
Magnus Damm <magnus.damm@gmail.com>,
Linus Walleij <linus.walleij@linaro.org>,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-renesas-soc@vger.kernel.org, linux-gpio@vger.kernel.org,
Biju Das <biju.das.jz@bp.renesas.com>,
Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: Re: [PATCH v2 3/9] irqchip: irq-renesas-rzg2l: Skip mapping NMI interrupt as part of hierarchy domain
Date: Wed, 21 Dec 2022 10:31:10 +0000 [thread overview]
Message-ID: <86mt7haw0h.wl-maz@kernel.org> (raw)
In-Reply-To: <20221221000242.340202-4-prabhakar.mahadev-lad.rj@bp.renesas.com>
On Wed, 21 Dec 2022 00:02:36 +0000,
Prabhakar <prabhakar.csengg@gmail.com> wrote:
>
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> NMI interrupt is not an external interrupt as compared to the IRQ0-7 and
> TINT0-31, this means we need to install the irq handler for NMI in the
> IRQC driver and not include it as part of IRQ domain.
>
> This patch skips mapping NMI interrupt as part of the IRQ domain
> hierarchy.
Does it mean nobody can connect anything to it? Where is the handler
you're mentioning for this NMI?
>
> Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt Controller driver")
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v1 -> v2
> * New patch
> ---
> drivers/irqchip/irq-renesas-rzg2l.c | 24 +++++++++++++-----------
> 1 file changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/irqchip/irq-renesas-rzg2l.c b/drivers/irqchip/irq-renesas-rzg2l.c
> index 25fd8ee66565..7918fe201218 100644
> --- a/drivers/irqchip/irq-renesas-rzg2l.c
> +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> @@ -23,7 +23,8 @@
> #define IRQC_IRQ_COUNT 8
> #define IRQC_TINT_START (IRQC_IRQ_START + IRQC_IRQ_COUNT)
> #define IRQC_TINT_COUNT 32
> -#define IRQC_NUM_IRQ (IRQC_TINT_START + IRQC_TINT_COUNT)
> + /* IRQ0-7 + TINT0-31 */
> +#define IRQC_NUM_HIERARCHY_IRQ (IRQC_TINT_START + IRQC_TINT_COUNT - 1)
>
> #define ISCR 0x10
> #define IITSR 0x14
> @@ -58,7 +59,8 @@
>
> struct rzg2l_irqc_priv {
> void __iomem *base;
> - struct irq_fwspec fwspec[IRQC_NUM_IRQ];
> + /* IRQ0-7 + TINT0-31 will be part of hierarchy domain */
> + struct irq_fwspec fwspec[IRQC_NUM_HIERARCHY_IRQ];
> raw_spinlock_t lock;
> };
>
> @@ -99,7 +101,7 @@ static void rzg2l_irqc_eoi(struct irq_data *d)
> raw_spin_lock(&priv->lock);
> if (hw_irq >= IRQC_IRQ_START && hw_irq <= IRQC_IRQ_COUNT)
> rzg2l_irq_eoi(d);
> - else if (hw_irq >= IRQC_TINT_START && hw_irq < IRQC_NUM_IRQ)
> + else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_NUM_HIERARCHY_IRQ)
> rzg2l_tint_eoi(d);
> raw_spin_unlock(&priv->lock);
> irq_chip_eoi_parent(d);
> @@ -109,7 +111,7 @@ static void rzg2l_irqc_irq_disable(struct irq_data *d)
> {
> unsigned int hw_irq = irqd_to_hwirq(d);
>
> - if (hw_irq >= IRQC_TINT_START && hw_irq < IRQC_NUM_IRQ) {
> + if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_NUM_HIERARCHY_IRQ) {
> struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> u32 offset = hw_irq - IRQC_TINT_START;
> u32 tssr_offset = TSSR_OFFSET(offset);
> @@ -129,7 +131,7 @@ static void rzg2l_irqc_irq_enable(struct irq_data *d)
> {
> unsigned int hw_irq = irqd_to_hwirq(d);
>
> - if (hw_irq >= IRQC_TINT_START && hw_irq < IRQC_NUM_IRQ) {
> + if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_NUM_HIERARCHY_IRQ) {
> struct rzg2l_irqc_priv *priv = irq_data_to_priv(d);
> unsigned long tint = (uintptr_t)d->chip_data;
> u32 offset = hw_irq - IRQC_TINT_START;
> @@ -228,7 +230,7 @@ static int rzg2l_irqc_set_type(struct irq_data *d, unsigned int type)
>
> 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)
> + else if (hw_irq >= IRQC_TINT_START && hw_irq <= IRQC_NUM_HIERARCHY_IRQ)
How about you define a "tint_hwirq()" helper that checks got the
boundaries? Same thing for the other IRQ type.
> ret = rzg2l_tint_set_edge(d, type);
> if (ret)
> return ret;
> @@ -280,7 +282,7 @@ static int rzg2l_irqc_alloc(struct irq_domain *domain, unsigned int virq,
> return -EINVAL;
> }
>
> - if (hwirq > (IRQC_NUM_IRQ - 1))
> + if (!hwirq || hwirq > IRQC_NUM_HIERARCHY_IRQ)
> return -EINVAL;
>
> ret = irq_domain_set_hwirq_and_chip(domain, virq, hwirq, &irqc_chip,
> @@ -288,7 +290,7 @@ static int rzg2l_irqc_alloc(struct irq_domain *domain, unsigned int virq,
> if (ret)
> return ret;
>
> - return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &priv->fwspec[hwirq]);
> + return irq_domain_alloc_irqs_parent(domain, virq, nr_irqs, &priv->fwspec[hwirq - 1]);
> }
>
> static const struct irq_domain_ops rzg2l_irqc_domain_ops = {
> @@ -304,12 +306,12 @@ static int rzg2l_irqc_parse_interrupts(struct rzg2l_irqc_priv *priv,
> unsigned int i;
> int ret;
>
> - for (i = 0; i < IRQC_NUM_IRQ; i++) {
> + for (i = 1; i <= IRQC_NUM_HIERARCHY_IRQ; i++) {
> ret = of_irq_parse_one(np, i, &map);
> if (ret)
> return ret;
> of_phandle_args_to_fwspec(np, map.args, map.args_count,
> - &priv->fwspec[i]);
> + &priv->fwspec[i - 1]);
Starting the loop at 1 really is non-idiomatic, and I'd rather see
something like this:
for (i = 0; i < IRQC_NUM_HIERARCHY_IRQ; i++) {
ret = of_irq_parse_one(np, i + 1, &map);
if (ret)
return ret;
of_phandle_args_to_fwspec(np, map.args, map.args_count,
&priv->fwspec[i]);
}
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
next prev parent reply other threads:[~2022-12-21 10:32 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-21 0:02 [PATCH v2 0/9] Add IRQC support to RZ/G2UL SoC Prabhakar
2022-12-21 0:02 ` [PATCH v2 1/9] dt-bindings: interrupt-controller: renesas,rzg2l-irqc: Document " Prabhakar
2022-12-21 12:37 ` Geert Uytterhoeven
2022-12-21 21:06 ` Lad, Prabhakar
2022-12-22 8:19 ` Geert Uytterhoeven
2022-12-22 11:53 ` Lad, Prabhakar
2022-12-21 0:02 ` [PATCH v2 2/9] dt-bindings: interrupt-controller: irqc-rzg2l: Drop RZG2L_NMI macro Prabhakar
2022-12-29 8:46 ` Krzysztof Kozlowski
2023-01-03 8:43 ` Geert Uytterhoeven
2023-01-03 10:33 ` Lad, Prabhakar
2022-12-21 0:02 ` [PATCH v2 3/9] irqchip: irq-renesas-rzg2l: Skip mapping NMI interrupt as part of hierarchy domain Prabhakar
2022-12-21 10:31 ` Marc Zyngier [this message]
2022-12-22 11:52 ` Lad, Prabhakar
2022-12-21 0:02 ` [PATCH v2 4/9] irqchip: irq-renesas-rzg2l: Add support for RZ/G2UL SoC Prabhakar
2022-12-21 10:20 ` Marc Zyngier
2022-12-21 12:18 ` Geert Uytterhoeven
2022-12-22 11:49 ` Lad, Prabhakar
2022-12-22 12:51 ` Geert Uytterhoeven
2022-12-21 0:02 ` [PATCH v2 5/9] pinctrl: renesas: rzg2l: Fix configuring the GPIO pins as interrupts Prabhakar
2022-12-21 0:02 ` [PATCH v2 6/9] pinctrl: renesas: rzg2l: Add BUILD_BUG_ON() checks Prabhakar
2022-12-21 0:02 ` [PATCH v2 7/9] arm64: dts: renesas: r9a07g043u: Add IRQC node Prabhakar
2022-12-21 0:02 ` [PATCH v2 8/9] arm64: dts: renesas: r9a07g043[u]: Update pinctrl node to handle GPIO interrupts Prabhakar
2022-12-21 0:02 ` [PATCH v2 9/9] arm64: dts: renesas: rzg2ul-smarc-som: Add PHY interrupt support for ETH{0/1} Prabhakar
2022-12-27 13:02 ` Geert Uytterhoeven
2022-12-28 23:36 ` Lad, Prabhakar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=86mt7haw0h.wl-maz@kernel.org \
--to=maz@kernel.org \
--cc=biju.das.jz@bp.renesas.com \
--cc=devicetree@vger.kernel.org \
--cc=geert+renesas@glider.be \
--cc=krzysztof.kozlowski+dt@linaro.org \
--cc=linus.walleij@linaro.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=magnus.damm@gmail.com \
--cc=prabhakar.csengg@gmail.com \
--cc=prabhakar.mahadev-lad.rj@bp.renesas.com \
--cc=robh+dt@kernel.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).