Re: [PATCH] spi: atmel: poll short PIO transfers

["Jens Emil Schulz Østergaard" <jensemil.schulzostergaard@microchip.com>]

On Thu Nov 12, 2026 at 9:12 PM CET, Mark Brown wrote:
>> Every PIO transfer enables the controller interrupt and sleeps in
>> wait_for_completion_timeout() until the interrupt handler has drained the
>> receive data. For a transfer of a few bytes, the interrupt, the
>> completion and the two context switches take several times longer than
>> the transfer on the wire. Devices that do their register IO over SPI,
>> such as SPI attached Ethernet switches, issue one message of a few short
>> transfers per register access, so this overhead dominates their register
>> access time.
>
>> Busy-wait on the status register instead of enabling the interrupt when
>> the transfer is estimated to take less than polling_limit_us, 30 us by
>> default, as spi-imx does. The estimate uses the effective clock rate and
>
> How big is the FIFO?
>

It depends on the SoC. The SPI in lan966x, lan969x and sama7 has a 32
data FIFO, sam9x60 and sam9x7 have 16, and sama5d2 has 16 or 32 depending
on the instance. The older parts (sam9x5, sama5d3, sama5d4 and earlier)
have none and take one interrupt per word.

>> the word delay. Setting the module parameter to 0 disables polling.
>
> If this is dubious enough to require the module parameter we shouldn't
> be doing this.  Most likely it's perfectly sensible and we should just
> do it as standard, other drivers don't make this optional.  If we do
> have a parameter it needs to be documented.
>

I will drop the parameter in the next version and poll below a fixed 30
us.

>> @@ -725,6 +736,9 @@ static void atmel_spi_next_xfer_fifo(struct spi_controller *host,
>>  
>>  	dev_vdbg(host->dev.parent, "atmel_spi_next_xfer_fifo\n");
>>  
>> +	if (as->poll)
>> +		spi_writel(as, IDR, SPI_BIT(RXFTHF) | SPI_BIT(OVRES));
>
> We should ensure there's no interrupt handler running or that could run
> in future, especially with PREEMPT_RT which puts everything into a
> thread.  If we timed out a transfer then an interrupt could be
> delivered while we're polling and try to also process things.  A lock
> while updating the flag and checking the flag before doing anything in
> the interrupt would probably do the trick.

Done as you suggest in v2. The poll flag is set and the interrupts are
masked in the same locked section before the transfer starts. The PIO
interrupt handler takes the lock before it reads the status register and
returns IRQ_NONE without touching the controller while the flag is set.

Thanks,
Emil