| Message ID | 20201107081420.60325-6-damien.lemoal@wdc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [01/32] of: Fix property supplier parsing | expand |
On Sat, Nov 07, 2020 at 05:13:53PM +0900, Damien Le Moal wrote: > On slow systems, i.e. systems with a slow CPU resulting in slow context > switches, calling spi_delay_exec() when executing polled transfers > using dw_spi_poll_transfer() can lead to RX FIFO overflows. Allow > platforms to opt out of delayed polling by introducing the > DW_SPI_CAP_POLL_NODELAY DW SPI capability flag to disable > the execution of spi_delay_exec() in dw_spi_poll_transfer(). This feels like it should be done based on something like a cutoff for sufficiently small delays rather than with a capability - it's tuning, not a property of the system.
On Sat, Nov 07, 2020 at 05:13:53PM +0900, Damien Le Moal wrote: > On slow systems, i.e. systems with a slow CPU resulting in slow context > switches, calling spi_delay_exec() when executing polled transfers > using dw_spi_poll_transfer() can lead to RX FIFO overflows. Allow > platforms to opt out of delayed polling by introducing the > DW_SPI_CAP_POLL_NODELAY DW SPI capability flag to disable > the execution of spi_delay_exec() in dw_spi_poll_transfer(). Please, have a more thorough problem review. Rx FIFO shouldn't overflow even for the CPUs with slow context switch. If it does, then most likely there is a bug in the code. So it's not a good idea to work it around by introducing a dts-property in any case. Just to note in case of our hardware no matter what CPU frequency I specified (it can be varied from 200MHz to 1500MHz), there have never been seen the Rx FIFO overflow error even with heavy background tasks executed. I am really puzzled why some context switch causes the error in your case... As I said in another thread, some logical MMC-SPI drivers errors may happen if the native CS is utilized... On the other hand, if you have a DMA-engine utilized together with your controller and the Tx DMA-channel is tuned to work faster than the Rx DMA-channel, then the Rx FIFO overflow will eventually happen. So replacing the DMA-based transfers with the poll- or IRQ-based ones shall indeed solve the problem. But the better solution would be to balance the DMA-channels priority if your DMA-controller is capable of doing that. -Sergey > > Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> > --- > drivers/spi/spi-dw-core.c | 12 ++++++++---- > drivers/spi/spi-dw.h | 1 + > 2 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c > index c2ef1d8d46d5..16a6fd569145 100644 > --- a/drivers/spi/spi-dw-core.c > +++ b/drivers/spi/spi-dw-core.c > @@ -385,14 +385,18 @@ static int dw_spi_poll_transfer(struct dw_spi *dws, > u16 nbits; > int ret; > > - delay.unit = SPI_DELAY_UNIT_SCK; > - nbits = dws->n_bytes * BITS_PER_BYTE; > + if (!(dws->caps & DW_SPI_CAP_POLL_NODELAY)) { > + delay.unit = SPI_DELAY_UNIT_SCK; > + nbits = dws->n_bytes * BITS_PER_BYTE; > + } > > do { > dw_writer(dws); > > - delay.value = nbits * (dws->rx_len - dws->tx_len); > - spi_delay_exec(&delay, transfer); > + if (!(dws->caps & DW_SPI_CAP_POLL_NODELAY)) { > + delay.value = nbits * (dws->rx_len - dws->tx_len); > + spi_delay_exec(&delay, transfer); > + } > > dw_reader(dws); > > diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h > index 48a11a51a407..25f6372b993a 100644 > --- a/drivers/spi/spi-dw.h > +++ b/drivers/spi/spi-dw.h > @@ -130,6 +130,7 @@ enum dw_ssi_type { > #define DW_SPI_CAP_KEEMBAY_MST BIT(1) > #define DW_SPI_CAP_DWC_SSI BIT(2) > #define DW_SPI_CAP_DFS_32 BIT(3) > +#define DW_SPI_CAP_POLL_NODELAY BIT(4) > > /* Slave spi_transfer/spi_mem_op related */ > struct dw_spi_cfg { > -- > 2.28.0 >
diff --git a/drivers/spi/spi-dw-core.c b/drivers/spi/spi-dw-core.c index c2ef1d8d46d5..16a6fd569145 100644 --- a/drivers/spi/spi-dw-core.c +++ b/drivers/spi/spi-dw-core.c @@ -385,14 +385,18 @@ static int dw_spi_poll_transfer(struct dw_spi *dws, u16 nbits; int ret; - delay.unit = SPI_DELAY_UNIT_SCK; - nbits = dws->n_bytes * BITS_PER_BYTE; + if (!(dws->caps & DW_SPI_CAP_POLL_NODELAY)) { + delay.unit = SPI_DELAY_UNIT_SCK; + nbits = dws->n_bytes * BITS_PER_BYTE; + } do { dw_writer(dws); - delay.value = nbits * (dws->rx_len - dws->tx_len); - spi_delay_exec(&delay, transfer); + if (!(dws->caps & DW_SPI_CAP_POLL_NODELAY)) { + delay.value = nbits * (dws->rx_len - dws->tx_len); + spi_delay_exec(&delay, transfer); + } dw_reader(dws); diff --git a/drivers/spi/spi-dw.h b/drivers/spi/spi-dw.h index 48a11a51a407..25f6372b993a 100644 --- a/drivers/spi/spi-dw.h +++ b/drivers/spi/spi-dw.h @@ -130,6 +130,7 @@ enum dw_ssi_type { #define DW_SPI_CAP_KEEMBAY_MST BIT(1) #define DW_SPI_CAP_DWC_SSI BIT(2) #define DW_SPI_CAP_DFS_32 BIT(3) +#define DW_SPI_CAP_POLL_NODELAY BIT(4) /* Slave spi_transfer/spi_mem_op related */ struct dw_spi_cfg {
On slow systems, i.e. systems with a slow CPU resulting in slow context switches, calling spi_delay_exec() when executing polled transfers using dw_spi_poll_transfer() can lead to RX FIFO overflows. Allow platforms to opt out of delayed polling by introducing the DW_SPI_CAP_POLL_NODELAY DW SPI capability flag to disable the execution of spi_delay_exec() in dw_spi_poll_transfer(). Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> --- drivers/spi/spi-dw-core.c | 12 ++++++++---- drivers/spi/spi-dw.h | 1 + 2 files changed, 9 insertions(+), 4 deletions(-)