From: Marc Zyngier <maz@kernel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Anup Patel <apatel@ventanamicro.com>,
Konrad Dybcio <konrad.dybcio@linaro.org>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH] irqchip/gic-v3: handle DOMAIN_BUS_ANY in gic_irq_domain_select
Date: Mon, 19 Feb 2024 15:53:48 +0000 [thread overview]
Message-ID: <868r3g4fhv.wl-maz@kernel.org> (raw)
In-Reply-To: <20240219-gic-fix-child-domain-v1-1-09f8fd2d9a8f@linaro.org>
On Mon, 19 Feb 2024 14:47:37 +0000,
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote:
>
> Before the commit de1ff306dcf4 ("genirq/irqdomain: Remove the param
> count restriction from select()") the irq_find_matching_fwspec() was
> handling the DOMAIN_BUS_ANY on its own. After this commit it is a job of
> the select() callback. However the callback of GICv3 (even though it got
> modified to handle zero param_count) wasn't prepared to return true for
> DOMAIN_BUS_ANY bus_token.
>
> This breaks probing of any of the child IRQ domains, since
> platform_irqchip_probe() uses irq_find_matching_host(par_np,
> DOMAIN_BUS_ANY) to check for the presence of the parent IRQ domain.
>
> Fixes: 151378251004 ("irqchip/gic-v3: Make gic_irq_domain_select() robust for zero parameter count")
> Fixes: de1ff306dcf4 ("genirq/irqdomain: Remove the param count restriction from select()")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/irqchip/irq-gic-v3.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 6fb276504bcc..e9e9643c653f 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -1696,7 +1696,8 @@ static int gic_irq_domain_select(struct irq_domain *d,
>
> /* Handle pure domain searches */
> if (!fwspec->param_count)
> - return d->bus_token == bus_token;
> + return d->bus_token == bus_token ||
> + bus_token == DOMAIN_BUS_ANY;
>
> /* If this is not DT, then we have a single domain */
> if (!is_of_node(fwspec->fwnode))
>
I really dislike the look of this. If that's the case, any irqchip
that has a 'select' method (such as imx-intmux) should be similarly
hacked. And at this point, this should be handled by the core code.
Can you try this instead? I don't have any HW that relies on
behaviour, but I'd expect this to work.
Thanks,
M.
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index aeb41655d6de..3dd1c871e091 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -449,7 +449,7 @@ struct irq_domain *irq_find_matching_fwspec(struct irq_fwspec *fwspec,
*/
mutex_lock(&irq_domain_mutex);
list_for_each_entry(h, &irq_domain_list, link) {
- if (h->ops->select)
+ if (h->ops->select && bus_token != DOMAIN_BUS_ANY)
rc = h->ops->select(h, fwspec, bus_token);
else if (h->ops->match)
rc = h->ops->match(h, to_of_node(fwnode), bus_token);
--
Without deviation from the norm, progress is not possible.
WARNING: multiple messages have this Message-ID (diff)
From: Marc Zyngier <maz@kernel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Anup Patel <apatel@ventanamicro.com>,
Konrad Dybcio <konrad.dybcio@linaro.org>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH] irqchip/gic-v3: handle DOMAIN_BUS_ANY in gic_irq_domain_select
Date: Mon, 19 Feb 2024 15:53:48 +0000 [thread overview]
Message-ID: <868r3g4fhv.wl-maz@kernel.org> (raw)
In-Reply-To: <20240219-gic-fix-child-domain-v1-1-09f8fd2d9a8f@linaro.org>
On Mon, 19 Feb 2024 14:47:37 +0000,
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote:
>
> Before the commit de1ff306dcf4 ("genirq/irqdomain: Remove the param
> count restriction from select()") the irq_find_matching_fwspec() was
> handling the DOMAIN_BUS_ANY on its own. After this commit it is a job of
> the select() callback. However the callback of GICv3 (even though it got
> modified to handle zero param_count) wasn't prepared to return true for
> DOMAIN_BUS_ANY bus_token.
>
> This breaks probing of any of the child IRQ domains, since
> platform_irqchip_probe() uses irq_find_matching_host(par_np,
> DOMAIN_BUS_ANY) to check for the presence of the parent IRQ domain.
>
> Fixes: 151378251004 ("irqchip/gic-v3: Make gic_irq_domain_select() robust for zero parameter count")
> Fixes: de1ff306dcf4 ("genirq/irqdomain: Remove the param count restriction from select()")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> drivers/irqchip/irq-gic-v3.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 6fb276504bcc..e9e9643c653f 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -1696,7 +1696,8 @@ static int gic_irq_domain_select(struct irq_domain *d,
>
> /* Handle pure domain searches */
> if (!fwspec->param_count)
> - return d->bus_token == bus_token;
> + return d->bus_token == bus_token ||
> + bus_token == DOMAIN_BUS_ANY;
>
> /* If this is not DT, then we have a single domain */
> if (!is_of_node(fwspec->fwnode))
>
I really dislike the look of this. If that's the case, any irqchip
that has a 'select' method (such as imx-intmux) should be similarly
hacked. And at this point, this should be handled by the core code.
Can you try this instead? I don't have any HW that relies on
behaviour, but I'd expect this to work.
Thanks,
M.
diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index aeb41655d6de..3dd1c871e091 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -449,7 +449,7 @@ struct irq_domain *irq_find_matching_fwspec(struct irq_fwspec *fwspec,
*/
mutex_lock(&irq_domain_mutex);
list_for_each_entry(h, &irq_domain_list, link) {
- if (h->ops->select)
+ if (h->ops->select && bus_token != DOMAIN_BUS_ANY)
rc = h->ops->select(h, fwspec, bus_token);
else if (h->ops->match)
rc = h->ops->match(h, to_of_node(fwnode), bus_token);
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2024-02-19 15:53 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-19 14:47 [PATCH] irqchip/gic-v3: handle DOMAIN_BUS_ANY in gic_irq_domain_select Dmitry Baryshkov
2024-02-19 14:47 ` Dmitry Baryshkov
2024-02-19 15:53 ` Marc Zyngier [this message]
2024-02-19 15:53 ` Marc Zyngier
2024-02-19 16:21 ` Dmitry Baryshkov
2024-02-19 16:21 ` Dmitry Baryshkov
2024-02-19 16:37 ` Marc Zyngier
2024-02-19 16:37 ` Marc Zyngier
2024-02-19 17:41 ` Dmitry Baryshkov
2024-02-19 17:41 ` Dmitry Baryshkov
2024-02-19 17:51 ` Marc Zyngier
2024-02-19 17:51 ` Marc Zyngier
2024-02-19 19:28 ` Dmitry Baryshkov
2024-02-19 19:28 ` Dmitry Baryshkov
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=868r3g4fhv.wl-maz@kernel.org \
--to=maz@kernel.org \
--cc=apatel@ventanamicro.com \
--cc=dmitry.baryshkov@linaro.org \
--cc=konrad.dybcio@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.