| Message ID | 20201107081420.60325-5-damien.lemoal@wdc.com |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V Kendryte K210 support improvments | expand |
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 can avoid a lot > of RX FIFO overflow errors while keeping the device SPI frequency > reasonnably high (for speed). 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: This isn't something that looks like it should be configured via DT as a separate property, this is more of a tuning property as far as I can see - even on the same hardware there might be cases where people prefer to keep using interrupts but for example allow transfers to stall while waiting for the interrupt controller giving lower throughput but more CPU cycles available for other things. Unfortunately we don't have any information about how much interrupt latency we should expect which makes this a bit annoying. I do see that there's already some existing attempt in the DMA code to tune burst sizes to avoid FIFO overflows - it looks like the hardware is doing something unusual and possibly unfortunate here though I've never looked at it so I'm not sure exactly what is going on there but perhaps ther's some tuning that can be done in there? TBH I'm not clear what the failure mode is where we need software to service interrupts promptly in the middle of a DMA transfer in the first place, that seems really strange.
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@wdc.com> > --- > 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 >
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))
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 can avoid a lot of RX FIFO overflow errors while keeping the device SPI frequency reasonnably high (for speed). 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@wdc.com> --- drivers/spi/spi-dw-mmio.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)