Re: [PATCH] spi: spi-imx: Revert "spi: spi-imx: add PIO polling support"

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 3473 bytes --]

On 15.11.2022 11:51:53, Frieder Schrempf wrote:
> On 14.11.22 12:29, Mark Brown wrote:
> > On Mon, Nov 14, 2022 at 09:30:26AM +0100, Frieder Schrempf wrote:
> > 
> >> As far as I know Fabio also discovered that disabling SDMA also fixes
> >> the problem.
> > 
> >> I guess I will try to repeat some tests on latest master and see if
> >> there is anything that makes things work again without reducing the
> >> clock. If anyone has some more ideas of how to fix this properly, please
> >> let me know. If nothing else helps we could also reduce the SPI clock.
> > 
> > It sounds like the commit can stay and that everyone is happy
> > that the issue is that the the commit made things run faster and
> > exposed some other misconfiguration for these systems?
> 
> Honestly I'm not really sure how to proceed.
> 
> My first impression was to keep the PIO polling support with its
> benefits if there's just this single issue with the SPI NOR on our board
> and assuming that the performance improvements uncovered a bug somewhere
> else. But at the moment I'm not quite sure this is really the case.
> 
> I did another test on v6.1-rc5 and disabling either PIO polling
> (spi-imx.polling_limit_us=0) or DMA (spi-imx.use_dma=0), or both of them
> makes reading the SPI NOR work again.

That was a good hint, I think I've found something.

Can you check if this fixes your problem? Just a quick hack to, a proper
solution needs some more love.

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 30d82cc7300b..76021b9bb445 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -1270,9 +1270,22 @@ static int spi_imx_setupxfer(struct spi_device *spi,
                spi_imx->dynamic_burst = 0;
        }
 
-       if (spi_imx_can_dma(spi_imx->controller, spi, t))
-               spi_imx->usedma = true;
-       else
+       if (spi_imx_can_dma(spi_imx->controller, spi, t)) {
+               unsigned long hz_per_byte, byte_limit;
+
+               /*
+                * Calculate the estimated time in us the transfer runs. Find
+                * the number of Hz per byte per polling limit.
+                */
+               hz_per_byte = polling_limit_us ? ((8 + 4) * USEC_PER_SEC) / polling_limit_us : 0;
+               byte_limit = hz_per_byte ? t->effective_speed_hz / hz_per_byte : 1;
+
+               /* run in polling mode for short transfers */
+               if (t->len < byte_limit)
+                       spi_imx->usedma = false;
+               else
+                       spi_imx->usedma = true;
+       } else
                spi_imx->usedma = false;
 
        spi_imx->rx_only = ((t->tx_buf == NULL)
@@ -1597,8 +1610,8 @@ static int spi_imx_transfer_one(struct spi_controller *controller,
        struct spi_imx_data *spi_imx = spi_controller_get_devdata(spi->controller);
        unsigned long hz_per_byte, byte_limit;
 
-       spi_imx_setupxfer(spi, transfer);
        transfer->effective_speed_hz = spi_imx->spi_bus_clk;
+       spi_imx_setupxfer(spi, transfer);
 
        /* flush rxfifo before transfer */
        while (spi_imx->devtype_data->rx_available(spi_imx))

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde           |
Embedded Linux                   | https://www.pengutronix.de  |
Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]