Re: [PATCH 04/32] spi: dw: Introduce polling device tree property

[Serge Semin <fancer.lancer@xxxxxxxxx>]

On Sat, Nov 07, 2020 at 05:13:52PM +0900, Damien Le Moal wrote:
> With boards that have slow interrupts context switch, and a fast device
> connected to a spi master, e.g. an SD card through mmc-spi,

> using
> dw_spi_poll_transfer() intead of the regular interrupt based
> dw_spi_transfer_handler() function is more efficient and

I can believe in that. But the next part seems questionable:

> can avoid a lot
> of RX FIFO overflow errors while keeping the device SPI frequency
> reasonnably high (for speed).

No matter whether it's an IRQ-based or poll-based transfer, as long as a
client SPI-device is connected with a GPIO-based chip-select (or the
DW APB SSI-controller feature of the automatic chip-select toggling is
fixed), the Rx FIFO should never overrun. It's ensured by the transfer
algorithm design by calculating the rxtx_gap in the dw_writer()
method. If the error still happens then there must be some bug in
the code.

It's also strange to hear that the polling-based transfer helps
to avoid the Rx FIFO overflow errors, while the IRQ-based transfer
causes them. Both of those methods are based on the same dw_writer()
and dw_reader() methods. So basically they both should either work
well or cause the errors at same time.

So to speak could you more through debug your case?

On the other hand the errors (but not the Rx FIFO overflow) might be
caused by the DW APB SSI nasty feature of the native chip-select
automatic assertion/de-assertion. So if your MMC-spi port is handled
by the native DW APB SSI CS lane, then it won't work well for sure.
No matter whether you use the poll- or IRQ-based transfers.

-Sergey


> Introduce the "polling" device tree
> property to allow requesting polled processing of transfer depending on
> the connected device while keeping the spi master interrupts property
> unschanged. E.g. device trees such as:
> 
> Generic soc.dtsi dts:
> 
> spi0: spi@53000000 {
> 	#address-cells = <1>;
> 	#size-cells = <0>;
> 	compatible = "snps,dw-apb-ssi";
> 	reg = <0x53000000 0x100>;
> 	interrupts = <2>;
> 	...
> }
> 
> Board specific dts:
> 
> ...
> &spi0 {
> 	polling;
> 	status = "okay";
> 
> 	slot@0 {
> 		compatible = "mmc-spi-slot";
> 		reg = <0>;
> 		voltage-ranges = <3300 3300>;
> 		spi-max-frequency = <4000000>;
> 	};
> }
> 
> will result in using polled transfers for the SD card while other boards
> using spi0 for different peripherals can use interrupt based transfers
> without needing to change the generic base soc dts.
> 
> Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxx>
> ---
>  drivers/spi/spi-dw-mmio.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/spi-dw-mmio.c b/drivers/spi/spi-dw-mmio.c
> index d0cc5bf4fa4e..3f1bc384cb45 100644
> --- a/drivers/spi/spi-dw-mmio.c
> +++ b/drivers/spi/spi-dw-mmio.c
> @@ -20,6 +20,7 @@
>  #include <linux/property.h>
>  #include <linux/regmap.h>
>  #include <linux/reset.h>
> +#include <linux/interrupt.h>
>  
>  #include "spi-dw.h"
>  
> @@ -246,9 +247,13 @@ static int dw_spi_mmio_probe(struct platform_device *pdev)
>  
>  	dws->paddr = mem->start;
>  
> -	dws->irq = platform_get_irq(pdev, 0);
> -	if (dws->irq < 0)
> -		return dws->irq; /* -ENXIO */
> +	if (device_property_read_bool(&pdev->dev, "polling")) {
> +		dws->irq = IRQ_NOTCONNECTED;
> +	} else {
> +		dws->irq = platform_get_irq(pdev, 0);
> +		if (dws->irq < 0)
> +			return dws->irq; /* -ENXIO */
> +	}
>  
>  	dwsmmio->clk = devm_clk_get(&pdev->dev, NULL);
>  	if (IS_ERR(dwsmmio->clk))
> -- 
> 2.28.0
>