[PATCH 0/3] irqchip/econet-en751221: Deliver odd interrupts without VEIC

[PATCH 0/3] irqchip/econet-en751221: Deliver odd interrupts without VEIC

[Akshat Singh via B4 Relay]

the en751221 intc puts the number of the pending irq plus one, in binary,
on IP2-IP7. with VEIC the cpu takes that as the vector, but without it the
driver only chains IP2, so odd irqs (qdma 21, xhci 17, first pcie port 23)
only get handled when an even one runs the handler, usually the next timer
tick.

on a nokia g-140w-c the 34kc doesn't report VEIC (no veic in
/proc/cpuinfo), and ping to the box kept cycling through 1-11ms. chaining
the handler to IP2-IP6 brings it under 1ms. VEIC mode is untouched.

tested on the g-140w-c with openwrt's 6.18 kernel, where the non-VEIC path
is the same as in mainline. on 7.4-rc2 it's build tested (W=1, no new
warnings), and dt_binding_check, CHECK_DTBS and checkpatch --strict are
clean. Caleb offered to test it on his boards:
https://github.com/openwrt/openwrt/pull/26744

Caleb: arch/mips/econet/init.c says the en751221 is a 34kc with VEIC, but
this one doesn't report it. is that something the bootloader or a soc
register turns on? if VEIC can be enabled here that's probably the better
fix for this board, but the non-VEIC path should still deliver everything.

the binding patch has to go in first, the dts change fails CHECK_DTBS
without it.

Signed-off-by: Akshat Singh <automobilehacks@gmail.com>
---
Akshat Singh (3):
      dt-bindings: interrupt-controller: econet,en751221-intc: Allow up to 6 parent interrupts
      irqchip/econet-en751221: Chain the handler to all parent interrupts
      mips: dts: Connect the EcoNet EN751221 intc to IP2-IP6

 .../interrupt-controller/econet,en751221-intc.yaml |  9 ++++--
 arch/mips/boot/dts/econet/en751221.dtsi            |  7 ++++-
 drivers/irqchip/irq-econet-en751221.c              | 33 ++++++++++++++++++++--
 3 files changed, 43 insertions(+), 6 deletions(-)
---
base-commit: fa2933aac170dd19c6a7f0d7b81a8f537e40915c
change-id: 20261115-en751221-intc-all-lines-2a35252b0b9c

Best regards,
--  
Akshat Singh <automobilehacks@gmail.com>

[PATCH 1/3] dt-bindings: interrupt-controller: econet,en751221-intc: Allow up to 6 parent interrupts

[Akshat Singh via B4 Relay]

From: Akshat Singh <automobilehacks@gmail.com>

The controller is not connected to its parent through a single
interrupt line: it presents the number of the pending interrupt plus
one, in binary, on the CPU interrupt lines IP2-IP7. A CPU in VEIC mode
takes that number as the vector, but without VEIC each line is a
separate interrupt, and the controller has to be chained to all the
lines that can carry it.

Allow listing up to six parent interrupts, one per line.

Signed-off-by: Akshat Singh <automobilehacks@gmail.com>
---
 .../bindings/interrupt-controller/econet,en751221-intc.yaml      | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/econet,en751221-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/econet,en751221-intc.yaml
index 44c09785e..098fdeaa3 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/econet,en751221-intc.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/econet,en751221-intc.yaml
@@ -34,8 +34,13 @@ properties:
   interrupt-controller: true
 
   interrupts:
-    maxItems: 1
-    description: Interrupt line connecting this controller to its parent.
+    minItems: 1
+    maxItems: 6
+    description:
+      CPU interrupt lines connecting this controller to its parent. The
+      controller presents the number of the pending interrupt plus one, in
+      binary, on the CPU interrupt lines IP2-IP7, so it can be connected to
+      up to six of them.
 
   econet,shadow-interrupts:
     $ref: /schemas/types.yaml#/definitions/uint32-matrix

-- 
2.43.0

[PATCH 2/3] irqchip/econet-en751221: Chain the handler to all parent interrupts

[Akshat Singh via B4 Relay]

From: Akshat Singh <automobilehacks@gmail.com>

The EN751221 interrupt controller does not drive one CPU interrupt line
per source. It presents the number of the pending interrupt plus one, in
binary, on the CPU interrupt lines IP2-IP7. In VEIC mode the CPU takes
that number as the vector, but without VEIC the driver chains its handler
to the first parent interrupt only, IP2 in the existing device trees. An
odd-numbered interrupt then never interrupts the CPU by itself: it is
handled only when an even-numbered one happens to run the handler, which
on an idle system is the next timer interrupt. That covers the frame
engine's QDMA (21), xHCI (17) and the first PCIe port (23).

On a Nokia G-140W-C, whose 34Kc does not report VEIC, this delayed
received packets by up to 10 ms, and ping round-trip times to the device
cycled through 1-11 ms.

Without VEIC, chain the handler to every parent interrupt listed in the
device tree. One interrupt usually raises several lines, so the later
runs of the handler find nothing pending; don't count those as spurious.

VEIC mode is unchanged.

Signed-off-by: Akshat Singh <automobilehacks@gmail.com>
---
 drivers/irqchip/irq-econet-en751221.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-econet-en751221.c b/drivers/irqchip/irq-econet-en751221.c
index 2ca5d9018..5930c67c3 100644
--- a/drivers/irqchip/irq-econet-en751221.c
+++ b/drivers/irqchip/irq-econet-en751221.c
@@ -18,6 +18,15 @@
  * When VPE#1 requests IRQ 30, the driver manipulates the mask bit for IRQ 29,
  * telling the hardware to mask VPE#1's view of IRQ 30.
  *
+ * The controller does not give each source its own CPU interrupt line. It
+ * presents the number of the pending interrupt plus one, in binary, on the
+ * CPU interrupt lines IP2-IP7, which a CPU in VEIC mode takes as the vector.
+ * Without VEIC, IRQ 21, for example, shows up as 22 = 0b010110 on IP3, IP4
+ * and IP6, so the controller is chained to every CPU line listed in its
+ * "interrupts" property. Chained to IP2 alone, it would see the odd-numbered
+ * IRQs only when an even-numbered one, such as the timer, happened to run
+ * the handler.
+ *
  * Copyright (C) 2025 Caleb James DeLisle <cjd@cjdns.fr>
  */
 
@@ -145,9 +154,12 @@ static void econet_intc_from_parent(struct irq_desc *desc)
 	pending0 = ioread32(econet_intc.membase + REG_PENDING0);
 	pending1 = ioread32(econet_intc.membase + REG_PENDING1);
 
-	if (unlikely(!(pending0 | pending1))) {
-		spurious_interrupt();
-	} else {
+	/*
+	 * An interrupt usually raises several CPU lines. It is handled when
+	 * the first of them runs this function, and the others find nothing
+	 * pending.
+	 */
+	if (pending0 | pending1) {
 		domain = irq_desc_get_handler_data(desc);
 		econet_intc_handle_pending(domain, pending0, 0);
 		econet_intc_handle_pending(domain, pending1, 32);
@@ -461,7 +473,22 @@ static int __init econet_intc_of_init(struct device_node *node, struct device_no
 		if (ret)
 			return ret;
 	} else {
+		int nr_parents = of_irq_count(node);
+
 		irq_set_chained_handler_and_data(irq, econet_intc_from_parent, domain);
+
+		for (int i = 1; i < nr_parents; i++) {
+			int parent = irq_of_parse_and_map(node, i);
+
+			if (!parent) {
+				pr_warn("%pOF: DT: Failed to get IRQ %d from 'interrupts'\n",
+					node, i);
+				continue;
+			}
+
+			irq_set_chained_handler_and_data(parent, econet_intc_from_parent,
+							 domain);
+		}
 	}
 
 	econet_intc.domain = domain;

-- 
2.43.0

[PATCH 3/3] mips: dts: Connect the EcoNet EN751221 intc to IP2-IP6

[Akshat Singh via B4 Relay]

From: Akshat Singh <automobilehacks@gmail.com>

The interrupt controller presents the number of the pending interrupt
plus one, in binary, on the CPU interrupt lines IP2-IP7. Without VEIC,
chaining it to IP2 alone leaves the odd-numbered interrupts, such as the
QDMA, xHCI and the first PCIe port, waiting for an even-numbered one to
run the handler.

List IP2-IP6. IP7 is left out: the CPU can also signal its own timer on
it, and only hwirq 31, which is unused, needs it.

Signed-off-by: Akshat Singh <automobilehacks@gmail.com>
---
 arch/mips/boot/dts/econet/en751221.dtsi | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/mips/boot/dts/econet/en751221.dtsi b/arch/mips/boot/dts/econet/en751221.dtsi
index 57ae84a8a..5e3f626b5 100644
--- a/arch/mips/boot/dts/econet/en751221.dtsi
+++ b/arch/mips/boot/dts/econet/en751221.dtsi
@@ -66,7 +66,12 @@ intc: interrupt-controller@1fb40000 {
 		compatible = "econet,en751221-intc";
 		reg = <0x1fb40000 0x100>;
 		interrupt-parent = <&cpuintc>;
-		interrupts = <2>;
+		/*
+		 * The number of the pending interrupt plus one is presented in
+		 * binary on IP2-IP7. IP7 is left out: the CPU can also signal its
+		 * own timer on it, and only hwirq 31, which is unused, needs it.
+		 */
+		interrupts = <2>, <3>, <4>, <5>, <6>;
 
 		interrupt-controller;
 		#interrupt-cells = <1>;

-- 
2.43.0

Re: [PATCH 3/3] mips: dts: Connect the EcoNet EN751221 intc to IP2-IP6

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] mips: dts: econet: missing Fixes trailer for EN751221 intc
  interrupt lines fix
--

commit ef45b82519d1615a5175bd08b37ea85f1395050e
Author: Akshat Singh <automobilehacks@gmail.com>

> mips: dts: Connect the EcoNet EN751221 intc to IP2-IP6
>
> The interrupt controller presents the number of the pending interrupt
> plus one, in binary, on the CPU interrupt lines IP2-IP7. Without VEIC,
> chaining it to IP2 alone leaves the odd-numbered interrupts, such as the
> QDMA, xHCI and the first PCIe port, waiting for an even-numbered one to
> run the handler.
>
> List IP2-IP6. IP7 is left out: the CPU can also signal its own timer on
> it, and only hwirq 31, which is unused, needs it.
>
> Signed-off-by: Akshat Singh <automobilehacks@gmail.com>

[Severity: Low]
This fixes interrupt delivery for odd-numbered interrupts on the EN751221
in non-VEIC mode.  Should this include:

  Fixes: 0ec488700972 ("mips: dts: Add EcoNet DTS with EN751221 and SmartFiber XP8421-B board")

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115-en751221-intc-all-lines-v1-0-dc03a40f47f0@gmail.com?part=3

Re: [PATCH 2/3] irqchip/econet-en751221: Chain the handler to all parent interrupts

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Low] irqchip/econet-en751221: local parent variable shadows parameter
  in econet_intc_of_init()
- [Low] irqchip/econet-en751221: missing Fixes trailer on non-VEIC IRQ
  chaining fix
--

commit 341fd2b16f35f804366b103223f85a72065ed255
Author: Akshat Singh <automobilehacks@gmail.com>

> irqchip/econet-en751221: Chain the handler to all parent interrupts
>
> The EN751221 interrupt controller does not drive one CPU interrupt line
> per source. It presents the number of the pending interrupt plus one, in
> binary, on the CPU interrupt lines IP2-IP7. In VEIC mode the CPU takes
> that number as the vector, but without VEIC the driver chains its handler
> to the first parent interrupt only, IP2 in the existing device trees. An
> odd-numbered interrupt then never interrupts the CPU by itself: it is
> handled only when an even-numbered one happens to run the handler, which
> on an idle system is the next timer interrupt. That covers the frame
> engine's QDMA (21), xHCI (17) and the first PCIe port (23).
>
> On a Nokia G-140W-C, whose 34Kc does not report VEIC, this delayed
> received packets by up to 10 ms, and ping round-trip times to the device
> cycled through 1-11 ms.
>
> Without VEIC, chain the handler to every parent interrupt listed in the
> device tree. One interrupt usually raises several lines, so the later
> runs of the handler find nothing pending; don't count those as spurious.
>
> VEIC mode is unchanged.
>
> Signed-off-by: Akshat Singh <automobilehacks@gmail.com>

[Severity: Low]
This fixes broken delivery of odd-numbered interrupts on non-VEIC systems.
Should this include:

  Fixes: 1902a59cf5f9 ("irqchip: Add EcoNet EN751221 INTC")

> diff --git a/drivers/irqchip/irq-econet-en751221.c b/drivers/irqchip/irq-econet-en751221.c
> index 2ca5d901866fe..5930c67c37261 100644
> --- a/drivers/irqchip/irq-econet-en751221.c
> +++ b/drivers/irqchip/irq-econet-en751221.c

[ ... ]

> @@ -461,7 +473,22 @@ static int __init econet_intc_of_init(struct device_node *node, struct device_no
>  		if (ret)
>  			return ret;
>  	} else {
> +		int nr_parents = of_irq_count(node);
> +
>  		irq_set_chained_handler_and_data(irq, econet_intc_from_parent, domain);
> +
> +		for (int i = 1; i < nr_parents; i++) {
> +			int parent = irq_of_parse_and_map(node, i);

[Severity: Low]
Does declaring the local variable parent here trigger a -Wshadow warning
under W=2 builds because econet_intc_of_init() already takes a
struct device_node *parent parameter?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115-en751221-intc-all-lines-v1-0-dc03a40f47f0@gmail.com?part=2