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

[Mark Brown]

> 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?

> 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.

> @@ -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.

-- 
Mark Brown <broonie@xxxxxxxxxx>