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

[Mark Brown]

On Mon, Jun 15, 2026 at 12:40:39PM +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)
>  {
> -	u16 irq_status = dw_readl(dws, DW_SPI_ISR);
> +	struct spi_controller *ctlr = dev_id;
> +	struct dw_spi *dws = spi_controller_get_devdata(ctlr);
> +	u16 irq_status = dw_readl(dws, DW_SPI_RISR);
> +	u32 rx, tx, imask, mask = 0;

> +	do {

> +		rx = dw_reader(dws);
> +		if (!dws->rx_len) {
> +			mask = DW_SPI_INT_MASK;
> +			spi_finalize_current_transfer(dws->ctlr);

It should actually be safe I think but it's not 100% clear from the code
that we can't take an additional trip through the loop and report the
same transfer finalized twice.

> +		/*
> +		 * Send data out if Tx FIFO Empty IRQ is received. The IRQ will be
> +		 * disabled after the data transmission is finished so not to
> +		 * have the TXE IRQ flood at the final stage of the transfer.
> +		 */
> +		if (irq_status & DW_SPI_INT_TXEI) {
> +			tx = dw_writer(dws);

This is the only place where tx gets initialised...

> +			if (!dws->tx_len)
> +				mask = DW_SPI_INT_TXEI;
> +		}
> +	} while (rx != 0 || tx != 0);

...but we read it on every run through the loop, so if there's no TX
interrupt we'll be buggy.

> +	ret = request_threaded_irq(dws->irq, dw_spi_irq, dw_spi_irq_thread_fn,
> +				   IRQF_SHARED, dev_name(dev), ctlr);
> +	if (ret < 0 && ret != -ENOTCONN) {
> +		dev_err(dev, "can not request IRQ\n");
> +		goto err_free_ctlr;
> +	}
> +

Do we need to adjust when we free the DMA resources as well?

Attachment: signature.asc
Description: PGP signature