Re: [PATCH v3 5/5] spi: dw: use threaded interrupt and optimize the threaded ISR

[Mark Brown]

On Wed, Sep 09, 2026 at 10:36:52PM +0800, Jisheng Zhang wrote:
> To avoid blocking for an excessive amount of time, eventually impacting
> on system responsiveness, hard interrupt handlers should finish
> executing in as little time as possible.

> +static irqreturn_t dw_spi_irq_thread_fn(int irq, void *dev_id)
> +{
> +	struct spi_controller *ctlr = dev_id;
> +	struct dw_spi *dws = spi_controller_get_devdata(ctlr);
> +	u32 rx, tx, imask, mask = 0;
> +	bool finalize = false;
> +
> +	do {

...

> +	} while (rx != 0 || tx != 0);

This will sit and drive the FIFOs for as long as we manage to push data
through them.  Usually we should have DMA so any long transfers wouldn't
be here but if there's systems with this controller wired up without DMA
we could potentially end up doing very large transfers, that can trigger
the hung task detector.  The simplistic solution is to use cond_resched()
to give other things a chance to run.

The hardirq handler would just do one round of FIFO operations and
return, allowing other things to happen until the device interrupts
again.

> +static irqreturn_t dw_spi_host_handler(struct dw_spi *dws)
> +{
> +	if (dw_spi_check_status(dws, false)) {
> +		spi_finalize_current_transfer(dws->ctlr);
> +		return IRQ_HANDLED;
> +	}
> +
> +	dw_spi_mask_intr(dws, 0xff);
> +
> +	return IRQ_WAKE_THREAD;
> +}

It feels like it might be good to do one round of FIFO in the hardirq
handler before deferring for a longer transfer, that would avoid needing
to schedule for very short transfers.  Essentially a copybreak like we'd
use for choosing between PIO and DMA.  This is entirely based on vibes
rather than having benchmarked anything, but all your numbers seem to be
for larger transfers rather than a continual stream of short transfers.
The people doing stuff like running CAN buses often run into trying to
saturate the bus which means they are very concerned with the latency
from transaction initiation to completion, and from completion to
initiating the next transfer.

TBH along those lines with really short transfers (eg, read/write 16 bit
registers) we might not want to bother with the interrupt at all and
poll but that's a separate thing.

Attachment: signature.asc
Description: PGP signature