Thread (29 messages) 29 messages, 6 authors, 2021-12-06

Re: [PATCH] of/irq: Add a quirk for controllers with their own definition of interrupt-map

From: Rob Herring <robh@kernel.org>
Date: 2021-11-29 19:17:46
Also in: lkml

On Mon, Nov 22, 2021 at 4:30 AM Marc Zyngier [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Since 041284181226 ("of/irq: Allow matching of an interrupt-map local
to an interrupt controller"), a handful of interrupt controllers have
stopped working correctly. This is due to the DT exposing a non-sensical
interrupt-map property, and their drivers relying on the kernel ignoring
this property.

Since we cannot realistically fix this terrible behaviour, add a quirk
for the limited set of devices that have implemented this monster,
and document that this is a pretty bad practice.

Cc: Rob Herring <robh@kernel.org>
Cc: John Crispin <john@phrozen.org>
Cc: Biwen Li <redacted>
Cc: Chris Brandt <chris.brandt@renesas.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/of/irq.c | 37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index b10f015b2e37..27a5173c813c 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -76,6 +76,36 @@ struct device_node *of_irq_find_parent(struct device_node *child)
 }
 EXPORT_SYMBOL_GPL(of_irq_find_parent);

+/*
+ * These interrupt controllers abuse interrupt-map for unspeakable
+ * reasons and rely on the core code to *ignore* it (the drivers do
+ * their own parsing of the property).
+ *
+ * If you think of adding to the list for something *new*, think
+ * again. There is a high chance that you will be sent back to the
+ * drawing board.
+ */
+static const char * const of_irq_imap_abusers[] = {
+       "CBEA,platform-spider-pic",
+       "sti,platform-spider-pic",
+       "realtek,rtl-intc",
+       "fsl,ls1021a-extirq",
+       "fsl,ls1043a-extirq",
+       "fsl,ls1088a-extirq",
+       "renesas,rza1-irqc",
+};
I guess this list was obtained by with a: git grep '"interrupt-map"'

I suppose that should be sufficient to find all the cases. I'd like to
be able to identify this case just from a DT file, but it's not really
clear

Perhaps a simpler solution to all this is only handle interrupt-map
with interrupt-controller if it points to its own node. That works for
Apple and I don't see a need beyond that case.

+static bool of_irq_abuses_interrupt_map(struct device_node *np)
+{
+       int i;
+
+       for (i = 0; i < ARRAY_SIZE(of_irq_imap_abusers); i++)
+               if (of_device_is_compatible(np, of_irq_imap_abusers[i]))
+                       return true;
+
+       return false;
With a NULL terminated list, you can use of_device_compatible_match() instead .

Rob
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help