Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 27 May 2026 18:22:58 +0000
From:      Mitchell Horne <mhorne@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Cc:        Kyle Crenshaw <B1nc0d3x@gmail.com>
Subject:   git: ccda002ca10f - main - rk_gpio: implement PIC masking methods and mask unhandled IRQs
Message-ID:  <6a173682.3fb6f.4ddf8fab@gitrepo.freebsd.org>

index | next in thread | raw e-mail

The branch main has been updated by mhorne:

URL: https://cgit.FreeBSD.org/src/commit/?id=ccda002ca10f9fb88e65d4bc27f0676e6f97d16d

commit ccda002ca10f9fb88e65d4bc27f0676e6f97d16d
Author:     Kyle Crenshaw <B1nc0d3x@gmail.com>
AuthorDate: 2026-05-14 21:07:42 +0000
Commit:     Mitchell Horne <mhorne@FreeBSD.org>
CommitDate: 2026-05-27 18:21:19 +0000

    rk_gpio: implement PIC masking methods and mask unhandled IRQs
    
    The Rockchip GPIO controller implements PIC operations for the INTRNG
    framework but is missing four masking methods that INTRNG calls during
    the filter/ithread handoff: pic_disable_intr, pic_enable_intr,
    pic_pre_ithread, pic_post_ithread.
    
    Without them, level-sensitive interrupt sources connected to a
    Rockchip GPIO pin re-fire continuously while their ithread runs.  On
    a RockPro64 with a FUSB302B Type-C controller (i2c) attached to
    gpio1 INT_N, the system enters a ~210 kHz interrupt storm the moment
    the fusb302 driver attaches and INT_N goes low.
    
    Two complementary changes:
    
    1. Add the four pic_disable_intr/pic_enable_intr/pic_pre_ithread/
       pic_post_ithread method bodies.  Each toggles the pin's
       RK_GPIO_INTMASK bit so the source is masked during the in-flight
       ithread window and unmasked on return, honouring the generic
       INTRNG mask/unmask sequence.
    
    2. When the GPIO IRQ filter dispatches a pin and finds no consumer
       registered, mask the pin at the controller (INTMASK=1, INTEN=0)
       before continuing.  Level-triggered sources keep asserting until
       acked, so a single stuck pin used to flood the console with
       thousands of "Interrupt pin=N unhandled" lines per second.  The
       mask survives until something re-attaches and re-enables the IRQ
       via the standard pic_enable_intr path.
    
    Affects all level-triggered IRQs on Rockchip GPIO banks; edge-
    triggered IRQs were already self-acking and unaffected.
    
    Signed-off-by:  Kyle Crenshaw <B1nc0d3x@gmail.com>
    Reviewed by:    mhorne
    MFC after:      2 weeks
    Pull Request:   https://github.com/freebsd/freebsd-src/pull/2197
---
 sys/arm64/rockchip/rk_gpio.c | 106 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 100 insertions(+), 6 deletions(-)

diff --git a/sys/arm64/rockchip/rk_gpio.c b/sys/arm64/rockchip/rk_gpio.c
index 8da37d516802..7c2071d2d178 100644
--- a/sys/arm64/rockchip/rk_gpio.c
+++ b/sys/arm64/rockchip/rk_gpio.c
@@ -227,8 +227,22 @@ rk_gpio_intr(void *arg)
 
 		status &= ~(1 << pin);
 		if (intr_isrc_dispatch(RK_GPIO_ISRC(sc, pin), tf)) {
-			device_printf(sc->sc_dev, "Interrupt pin=%d unhandled\n",
-			    pin);
+			/*
+			 * Pin asserted but no consumer is registered for it
+			 * yet (or anymore).  Level-triggered sources keep
+			 * firing on every interrupt cycle, so a single stuck
+			 * pin floods the console with thousands of these
+			 * messages per second.  Mask the pin's IRQ at the
+			 * controller and disable further dispatches; if a
+			 * consumer attaches later it will re-enable through
+			 * pic_enable_intr / rk_gpio_pic_enable_intr.
+			 */
+			RK_GPIO_LOCK(sc);
+			rk_gpio_write_bit(sc, RK_GPIO_INTMASK, pin, 1);
+			rk_gpio_write_bit(sc, RK_GPIO_INTEN, pin, 0);
+			RK_GPIO_UNLOCK(sc);
+			device_printf(sc->sc_dev,
+			    "Interrupt pin=%d unhandled — masked\n", pin);
 			continue;
 		}
 
@@ -818,10 +832,14 @@ rk_pic_setup_intr(device_t dev, struct intr_irqsrc *isrc,
 		return (EINVAL);
 	}
 	rk_gpio_write_bit(sc, RK_GPIO_DEBOUNCE, pin, 1);
-	rk_gpio_write_bit(sc, RK_GPIO_INTMASK, pin, 0);
-	rk_gpio_write_bit(sc, RK_GPIO_INTEN, pin, 1);
 	RK_GPIO_UNLOCK(sc);
 
+	/*
+	 * Leave the interrupt masked + disabled here.  INTRNG will call
+	 * pic_enable_intr() next to make it live.  That keeps the
+	 * masking responsibility cleanly in enable/disable rather than
+	 * split between setup and disable.
+	 */
 	return (0);
 }
 
@@ -837,14 +855,86 @@ rk_pic_teardown_intr(device_t dev, struct intr_irqsrc *isrc,
 	if (isrc->isrc_handlers == 0) {
 		irqsrc->mode = GPIO_INTR_CONFORM;
 		RK_GPIO_LOCK(sc);
-		rk_gpio_write_bit(sc, RK_GPIO_INTEN, irqsrc->irq, 0);
-		rk_gpio_write_bit(sc, RK_GPIO_INTMASK, irqsrc->irq, 0);
+		/*
+		 * INTEN/INTMASK are already cleared by pic_disable_intr,
+		 * which INTRNG calls before teardown of the last handler.
+		 * We only need to undo what setup_intr configured -- here,
+		 * the debounce filter.
+		 */
 		rk_gpio_write_bit(sc, RK_GPIO_DEBOUNCE, irqsrc->irq, 0);
 		RK_GPIO_UNLOCK(sc);
 	}
 	return (0);
 }
 
+/*
+ * INTRNG calls pic_disable_intr() during teardown of the final handler
+ * for a source, OR when a consumer explicitly wants the source off.
+ * Clear INTEN so the controller will not raise this pin at all.
+ *
+ * The in-flight masking between FILTER_SCHEDULE_THREAD and ithread
+ * completion is handled by pic_pre_ithread() / pic_post_ithread()
+ * below, NOT by this method.
+ */
+static void
+rk_pic_disable_intr(device_t dev, struct intr_irqsrc *isrc)
+{
+	struct rk_gpio_softc *sc = device_get_softc(dev);
+	struct rk_pin_irqsrc *rkisrc = (struct rk_pin_irqsrc *)isrc;
+
+	RK_GPIO_LOCK(sc);
+	rk_gpio_write_bit(sc, RK_GPIO_INTMASK, rkisrc->irq, 1);
+	rk_gpio_write_bit(sc, RK_GPIO_INTEN, rkisrc->irq, 0);
+	RK_GPIO_UNLOCK(sc);
+}
+
+/*
+ * INTRNG calls pic_enable_intr() to make a source live for the first
+ * time (after setup_intr), or to re-enable after a prior
+ * pic_disable_intr().  Set INTEN and unmask so the controller starts
+ * delivering this pin.
+ */
+static void
+rk_pic_enable_intr(device_t dev, struct intr_irqsrc *isrc)
+{
+	struct rk_gpio_softc *sc = device_get_softc(dev);
+	struct rk_pin_irqsrc *rkisrc = (struct rk_pin_irqsrc *)isrc;
+
+	RK_GPIO_LOCK(sc);
+	rk_gpio_write_bit(sc, RK_GPIO_INTEN, rkisrc->irq, 1);
+	rk_gpio_write_bit(sc, RK_GPIO_INTMASK, rkisrc->irq, 0);
+	RK_GPIO_UNLOCK(sc);
+}
+
+/*
+ * Called by INTRNG before delivering to the ithread.  Mask the source
+ * so it cannot re-fire during the ithread window -- without this,
+ * level-low IRQs (e.g. FUSB302 INT_N) re-trigger continuously and
+ * starve the ithread (~210 kHz storm observed via dtrace).
+ * Re-unmasked in pic_post_ithread() once the ithread acks the source.
+ */
+static void
+rk_pic_pre_ithread(device_t dev, struct intr_irqsrc *isrc)
+{
+	struct rk_gpio_softc *sc = device_get_softc(dev);
+	struct rk_pin_irqsrc *rkisrc = (struct rk_pin_irqsrc *)isrc;
+
+	RK_GPIO_LOCK(sc);
+	rk_gpio_write_bit(sc, RK_GPIO_INTMASK, rkisrc->irq, 1);
+	RK_GPIO_UNLOCK(sc);
+}
+
+static void
+rk_pic_post_ithread(device_t dev, struct intr_irqsrc *isrc)
+{
+	struct rk_gpio_softc *sc = device_get_softc(dev);
+	struct rk_pin_irqsrc *rkisrc = (struct rk_pin_irqsrc *)isrc;
+
+	RK_GPIO_LOCK(sc);
+	rk_gpio_write_bit(sc, RK_GPIO_INTMASK, rkisrc->irq, 0);
+	RK_GPIO_UNLOCK(sc);
+}
+
 static device_method_t rk_gpio_methods[] = {
 	/* Device interface */
 	DEVMETHOD(device_probe,		rk_gpio_probe),
@@ -873,6 +963,10 @@ static device_method_t rk_gpio_methods[] = {
 	DEVMETHOD(pic_map_intr,		rk_pic_map_intr),
 	DEVMETHOD(pic_setup_intr,	rk_pic_setup_intr),
 	DEVMETHOD(pic_teardown_intr,	rk_pic_teardown_intr),
+	DEVMETHOD(pic_disable_intr,	rk_pic_disable_intr),
+	DEVMETHOD(pic_enable_intr,	rk_pic_enable_intr),
+	DEVMETHOD(pic_pre_ithread,	rk_pic_pre_ithread),
+	DEVMETHOD(pic_post_ithread,	rk_pic_post_ithread),
 
 	/* ofw_bus interface */
 	DEVMETHOD(ofw_bus_get_node,	rk_gpio_get_node),


home | help

Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a173682.3fb6f.4ddf8fab>