[v7,04/16] mtd: rawnand: sunxi: drain interrupts before reusing the completion
Commit Message
The controller uses one completion for successive interrupt-driven
waits. Some transfers wait for both command completion and DMA
completion. The handler acknowledges each event and masks it, leaving
any outstanding event enabled; it calls complete() when all requested
events have arrived.
sunxi_nfc_interrupt() currently calls complete() before updating those
registers. A successful waiter can wake on another CPU and start the
next operation before the old handler acknowledges status or rewrites
the mask. Move complete() after those updates so waking the waiter
cannot precede the handler's last controller access.
A timeout also needs to exclude an unfinished handler. With PREEMPT_RT,
the flags-zero IRQ request is forced threaded. Near the timeout
boundary, that thread can be preempted after reading status and enable
bits while the waiter times out, clears the device mask and rearms the
next wait. The saved snapshot can then restore an obsolete
remaining-event mask or, if it satisfies the old wait, complete the new
wait. Masking the device does not invalidate the running handler's local
values. Repeating init_completion() can also reset its waitqueue lock
during complete(). This lifetime problem was identified by code
inspection during review.
Initialize the completion once in probe and use reinit_completion() for
each wait. On timeout, call disable_irq() before clearing controller
state so existing handling finishes and no new handler can enter during
cleanup. Then mask and acknowledge the events, read back the mask to
flush those writes, and re-enable the IRQ line. Keep partial-event
handling in the IRQ handler and leave polling and controller/DMA abort
handling unchanged.
This keeps additional synchronization on the timeout path, without a
driver lock in every interrupt and wait. The caller can sleep, this IRQ
is not registered as shared, and the handler does not acquire the
controller mutex held by the caller. The tradeoff is unbounded cleanup
latency and no priority inheritance: a starved IRQ thread can prevent
timeout cleanup from returning and block other operations on the
controller. This is a handler-lifetime boundary, not an RT
recovery-latency guarantee.
Fixes: 1fef62c1423b ("mtd: nand: add sunxi NAND flash controller support")
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
drivers/mtd/nand/raw/sunxi_nand.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
Comments
On 2026-10-14 19:33:51 [-0600], James Hilliard wrote:
> --- a/drivers/mtd/nand/raw/sunxi_nand.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> @@ -454,16 +457,19 @@ static int sunxi_nfc_wait_events(struct sunxi_nfc *nfc, u32 events,
> timeout_ms = NFC_DEFAULT_TIMEOUT_MS;
>
> if (!use_polling) {
> - init_completion(&nfc->complete);
> + reinit_completion(&nfc->complete);
>
> writel(events, nfc->regs + NFC_REG_INT);
>
> ret = wait_for_completion_timeout(&nfc->complete,
> msecs_to_jiffies(timeout_ms));
> - if (!ret)
> + if (!ret) {
> + /* Drain the handler before it can restore an old IRQ mask. */
> + disable_irq(nfc->irq);
> ret = -ETIMEDOUT;
> - else
> + } else {
> ret = 0;
> + }
>
> writel(0, nfc->regs + NFC_REG_INT);
> } else {
> @@ -476,6 +482,12 @@ static int sunxi_nfc_wait_events(struct sunxi_nfc *nfc, u32 events,
>
> writel(events & NFC_INT_MASK, nfc->regs + NFC_REG_ST);
>
> + if (!use_polling && ret) {
> + /* Flush the mask and acknowledgment before re-enabling the IRQ. */
> + readl(nfc->regs + NFC_REG_INT);
> + enable_irq(nfc->irq);
Shouldn't you re-read NFC_REG_ST, checking if the event became true even
in the case of a timeout?
Say the interrupt wake the thread but the thread never had a chance
to run. The timer (on another CPU) had a chance to run and did wake this
thread. Checking the event in the timeout case would help to prevent
that.
> + }
> +
> if (ret)
> dev_err(nfc->dev, "wait interrupt timedout\n");
>
Sebastian
On Thu, Oct 15, 2026 at 1:19 AM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> On 2026-10-14 19:33:51 [-0600], James Hilliard wrote:
> > --- a/drivers/mtd/nand/raw/sunxi_nand.c
> > +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> > @@ -454,16 +457,19 @@ static int sunxi_nfc_wait_events(struct sunxi_nfc *nfc, u32 events,
> > timeout_ms = NFC_DEFAULT_TIMEOUT_MS;
> >
> > if (!use_polling) {
> > - init_completion(&nfc->complete);
> > + reinit_completion(&nfc->complete);
> >
> > writel(events, nfc->regs + NFC_REG_INT);
> >
> > ret = wait_for_completion_timeout(&nfc->complete,
> > msecs_to_jiffies(timeout_ms));
> > - if (!ret)
> > + if (!ret) {
> > + /* Drain the handler before it can restore an old IRQ mask. */
> > + disable_irq(nfc->irq);
> > ret = -ETIMEDOUT;
> > - else
> > + } else {
> > ret = 0;
> > + }
> >
> > writel(0, nfc->regs + NFC_REG_INT);
> > } else {
> > @@ -476,6 +482,12 @@ static int sunxi_nfc_wait_events(struct sunxi_nfc *nfc, u32 events,
> >
> > writel(events & NFC_INT_MASK, nfc->regs + NFC_REG_ST);
> >
> > + if (!use_polling && ret) {
> > + /* Flush the mask and acknowledgment before re-enabling the IRQ. */
> > + readl(nfc->regs + NFC_REG_INT);
> > + enable_irq(nfc->irq);
>
> Shouldn't you re-read NFC_REG_ST, checking if the event became true even
> in the case of a timeout?
> Say the interrupt wake the thread but the thread never had a chance
> to run. The timer (on another CPU) had a chance to run and did wake this
> thread. Checking the event in the timeout case would help to prevent
> that.
Yes. I've folded a recheck into the IRQ fix for v8, after draining the
handler but before masking and acknowledging the controller events.
One detail is that the handler acknowledges each event and removes it
from NFC_REG_INT. For transfers waiting for both command and DMA
completion, one event may therefore have been consumed already.
Checking status against the original event mask would miss that case.
The timeout path now checks only the remaining enabled events:
if (!ret) {
u32 pending, status;
disable_irq(nfc->irq);
irq_disabled = true;
pending = readl(nfc->regs + NFC_REG_INT) & events;
status = readl(nfc->regs + NFC_REG_ST);
ret = (status & pending) == pending ? 0 : -ETIMEDOUT;
} else {
ret = 0;
}
An empty remaining mask also counts as success: it means the handler
processed all requested events while the timeout path was draining it.
irq_disabled is initialized to false at function entry. After masking
and acknowledging the events, cleanup uses that flag rather than ret:
if (irq_disabled) {
readl(nfc->regs + NFC_REG_INT);
enable_irq(nfc->irq);
}
That preserves the write flush and ensures the IRQ is re-enabled even
when the recheck changes the timeout result to success. Successful
waits and polling are otherwise unchanged.
>
> > + }
> > +
> > if (ret)
> > dev_err(nfc->dev, "wait interrupt timedout\n");
> >
>
> Sebastian
@@ -393,6 +393,7 @@ struct sunxi_nfc_caps {
* @chips: a list containing all the NAND chips attached to this NAND
* controller
* @complete: a completion object used to wait for NAND controller events
+ * @irq: NAND controller interrupt
* @dmac: the DMA channel attached to the NAND controller
* @use_mdma: use an internal MBUS DMA backend
* @mdma_desc: H6-style MBUS DMA descriptor
@@ -412,6 +413,7 @@ struct sunxi_nfc {
unsigned long clk_rate;
struct list_head chips;
struct completion complete;
+ int irq;
struct dma_chan *dmac;
bool use_mdma;
struct sunxi_nfc_mdma_desc *mdma_desc;
@@ -433,12 +435,13 @@ static irqreturn_t sunxi_nfc_interrupt(int irq, void *dev_id)
if (!(ien & st))
return IRQ_NONE;
- if ((ien & st) == ien)
- complete(&nfc->complete);
-
+ /* Finish updating the interrupt state before waking the next operation. */
writel(st & NFC_INT_MASK, nfc->regs + NFC_REG_ST);
writel(~st & ien & NFC_INT_MASK, nfc->regs + NFC_REG_INT);
+ if ((ien & st) == ien)
+ complete(&nfc->complete);
+
return IRQ_HANDLED;
}
@@ -454,16 +457,19 @@ static int sunxi_nfc_wait_events(struct sunxi_nfc *nfc, u32 events,
timeout_ms = NFC_DEFAULT_TIMEOUT_MS;
if (!use_polling) {
- init_completion(&nfc->complete);
+ reinit_completion(&nfc->complete);
writel(events, nfc->regs + NFC_REG_INT);
ret = wait_for_completion_timeout(&nfc->complete,
msecs_to_jiffies(timeout_ms));
- if (!ret)
+ if (!ret) {
+ /* Drain the handler before it can restore an old IRQ mask. */
+ disable_irq(nfc->irq);
ret = -ETIMEDOUT;
- else
+ } else {
ret = 0;
+ }
writel(0, nfc->regs + NFC_REG_INT);
} else {
@@ -476,6 +482,12 @@ static int sunxi_nfc_wait_events(struct sunxi_nfc *nfc, u32 events,
writel(events & NFC_INT_MASK, nfc->regs + NFC_REG_ST);
+ if (!use_polling && ret) {
+ /* Flush the mask and acknowledgment before re-enabling the IRQ. */
+ readl(nfc->regs + NFC_REG_INT);
+ enable_irq(nfc->irq);
+ }
+
if (ret)
dev_err(nfc->dev, "wait interrupt timedout\n");
@@ -2650,6 +2662,7 @@ static int sunxi_nfc_probe(struct platform_device *pdev)
nfc->dev = dev;
nand_controller_init(&nfc->controller);
INIT_LIST_HEAD(&nfc->chips);
+ init_completion(&nfc->complete);
nfc->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &r);
if (IS_ERR(nfc->regs))
@@ -2658,6 +2671,7 @@ static int sunxi_nfc_probe(struct platform_device *pdev)
irq = platform_get_irq(pdev, 0);
if (irq < 0)
return irq;
+ nfc->irq = irq;
nfc->caps = of_device_get_match_data(dev);
if (!nfc->caps)