Re: [PATCH v1] irqchip: loongson-liointc: add hierarchy irq support

From: Yinbo Zhu
Date: Thu Dec 08 2022 - 21:19:10 EST



在 2022/12/7 19:07, Marc Zyngier 写道:
On Wed, 07 Dec 2022 10:50:56 +0000,
Yinbo Zhu <zhuyinbo@xxxxxxxxxxx> wrote:

在 2022/12/7 16:08, Marc Zyngier 写道:
On Wed, 07 Dec 2022 01:45:55 +0000,
Yinbo Zhu <zhuyinbo@xxxxxxxxxxx> wrote:
When the irq of hierarchical interrupt chip was routed to liointc
that asked liointc driver to support hierarchy irq and this patch
was to add such support.

In addition, this patch only consider dts, and acpi hierarchy irq
support will be added later as required.

Signed-off-by: Yinbo Zhu <zhuyinbo@xxxxxxxxxxx>
---
drivers/irqchip/irq-loongson-liointc.c | 31 ++++++++++++++++++++++++++
1 file changed, 31 insertions(+)

diff --git a/drivers/irqchip/irq-loongson-liointc.c b/drivers/irqchip/irq-loongson-liointc.c
index 0da8716f8f24..58e43a2cd02e 100644
--- a/drivers/irqchip/irq-loongson-liointc.c
+++ b/drivers/irqchip/irq-loongson-liointc.c
@@ -177,6 +177,32 @@ static const struct irq_domain_ops acpi_irq_gc_ops = {
.xlate = liointc_domain_xlate,
};
+#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
+static int liointc_domain_alloc(struct irq_domain *domain, unsigned int virq,
+ unsigned int nr_irqs, void *arg)
+{
+ int i, ret;
+ irq_hw_number_t hwirq;
+ unsigned int type = IRQ_TYPE_NONE;
+ struct irq_fwspec *fwspec = arg;
+
+ ret = irq_domain_translate_twocell(domain, fwspec, &hwirq, &type);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < nr_irqs; i++)
+ irq_map_generic_chip(domain, virq + i, hwirq + i);
+
+ return 0;
+}
+
+static const struct irq_domain_ops of_irq_gc_ops = {
+ .translate = irq_domain_translate_twocell,
+ .alloc = liointc_domain_alloc,
+ .free = irq_domain_free_irqs_top,
+};
+#endif
+
static int liointc_init(phys_addr_t addr, unsigned long size, int revision,
struct fwnode_handle *domain_handle, struct device_node *node)
{
@@ -218,8 +244,13 @@ static int liointc_init(phys_addr_t addr, unsigned long size, int revision,
domain = irq_domain_create_linear(domain_handle, LIOINTC_CHIP_IRQ,
&acpi_irq_gc_ops, priv);
else
+#ifdef CONFIG_IRQ_DOMAIN_HIERARCHY
+ domain = irq_domain_create_linear(domain_handle, LIOINTC_CHIP_IRQ,
+ &of_irq_gc_ops, priv);
+#else
domain = irq_domain_create_linear(domain_handle, LIOINTC_CHIP_IRQ,
&irq_generic_chip_ops, priv);
+#endif
Two things:

- Why do we need three calls to create the same domains depending on
what firmware is used and kernel configuration?
yes, It depend on firmeware and kernel configuration.
Read again:

why do we need 3 different calls to irq_domain_create_linear when you
can *indirect* them with a pointer to the correct structure?
It was not considered comprehensively before, one call is enough.

- who is going to decide whether to select the
CONFIG_IRQ_DOMAIN_HIERARCHY option?
The latest gpio driver will select  CONFIG_IRQ_DOMAIN_HIERARCHY
Then why do we need two different behaviours? The same kernel should
run everywhere.

in fact, A behaviours can handle it, and I will add proper change

in v2.


I'd really like to see a statement from the Loongson folks about what
this whole DT stuff is all about. AFAICT, the core ACPICA stuff isn't
even fully merged (i.e. we still rely on arch-specific hacks).
The support of dts is mainly for Loongson embedded chips, such as
LoongArch Loongson-2 series SoC.  and it use dts to descripte device
and don't support acpi.
That doesn't answer my question. Please have a *consistent* approach
to your interrupt handling, and work with your ACPI colleagues.

I have a talk with ACPI colleagues that the dts and acpi can keep consistent

approach and I will add it in v2.


M.