| Message ID | 20221026-spicc-burst-delay-v1-0-1be5ffb7051a@linaro.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 04694e50020b62b10bd0d46ff9e9708a6e1c7eb3 |
| Headers | show |
| Series | spi: meson-spicc: move wait completion in driver to take bursts delay in account | expand |
On Wed, Oct 26, 2022 at 09:58:28AM +0200, Neil Armstrong wrote: > - spi_finalize_current_transfer(spicc->master); > + complete(&spicc->done); No, you need to call spi_finalize_current_transfer() - you need to block inside the transfer function if you want to open code this stuff.
Hi, On 26/10/2022 14:39, Mark Brown wrote: > On Wed, Oct 26, 2022 at 09:58:28AM +0200, Neil Armstrong wrote: > >> - spi_finalize_current_transfer(spicc->master); >> + complete(&spicc->done); > > No, you need to call spi_finalize_current_transfer() - you need to block > inside the transfer function if you want to open code this stuff. It's the case, I added a wait_for_completion_timeout() + return 0 in meson_spicc_transfer_one. Neil
On Wed, Oct 26, 2022 at 02:48:07PM +0200, Neil Armstrong wrote:
> It's the case, I added a wait_for_completion_timeout() + return 0 in meson_spicc_transfer_one.
Ah, so you did sorry - it was a bit masked in the diff.
On Wed, 26 Oct 2022 09:58:28 +0200, Neil Armstrong wrote: > Some delay occurs between each bursts, thus the default delay is wrong > and a timeout will occur with big enough transfers. > > The solution is to handle the timeout management in the driver and > add some delay for each bursts in the timeout calculation. > > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/1] spi: meson-spicc: move wait completion in driver to take bursts delay in account commit: 04694e50020b62b10bd0d46ff9e9708a6e1c7eb3 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
diff --git a/drivers/spi/spi-meson-spicc.c b/drivers/spi/spi-meson-spicc.c index bad201510a99..52bffab18329 100644 --- a/drivers/spi/spi-meson-spicc.c +++ b/drivers/spi/spi-meson-spicc.c @@ -160,6 +160,7 @@ struct meson_spicc_device { struct clk *clk; struct spi_message *message; struct spi_transfer *xfer; + struct completion done; const struct meson_spicc_data *data; u8 *tx_buf; u8 *rx_buf; @@ -282,7 +283,7 @@ static irqreturn_t meson_spicc_irq(int irq, void *data) /* Disable all IRQs */ writel(0, spicc->base + SPICC_INTREG); - spi_finalize_current_transfer(spicc->master); + complete(&spicc->done); return IRQ_HANDLED; } @@ -386,6 +387,7 @@ static int meson_spicc_transfer_one(struct spi_master *master, struct spi_transfer *xfer) { struct meson_spicc_device *spicc = spi_master_get_devdata(master); + unsigned long timeout; /* Store current transfer */ spicc->xfer = xfer; @@ -410,13 +412,29 @@ static int meson_spicc_transfer_one(struct spi_master *master, /* Setup burst */ meson_spicc_setup_burst(spicc); + /* Setup wait for completion */ + reinit_completion(&spicc->done); + + /* For each byte we wait for 8 cycles of the SPI clock */ + timeout = 8LL * MSEC_PER_SEC * xfer->len; + do_div(timeout, xfer->speed_hz); + + /* Add 10us delay between each fifo bursts */ + timeout += ((xfer->len >> 4) * 10) / MSEC_PER_SEC; + + /* Increase it twice and add 200 ms tolerance */ + timeout += timeout + 200; + /* Start burst */ writel_bits_relaxed(SPICC_XCH, SPICC_XCH, spicc->base + SPICC_CONREG); /* Enable interrupts */ writel_relaxed(SPICC_TC_EN, spicc->base + SPICC_INTREG); - return 1; + if (!wait_for_completion_timeout(&spicc->done, msecs_to_jiffies(timeout))) + return -ETIMEDOUT; + + return 0; } static int meson_spicc_prepare_message(struct spi_master *master, @@ -743,6 +761,8 @@ static int meson_spicc_probe(struct platform_device *pdev) spicc->pdev = pdev; platform_set_drvdata(pdev, spicc); + init_completion(&spicc->done); + spicc->base = devm_platform_ioremap_resource(pdev, 0); if (IS_ERR(spicc->base)) { dev_err(&pdev->dev, "io resource mapping failed\n");
Some delay occurs between each bursts, thus the default delay is wrong and a timeout will occur with big enough transfers. The solution is to handle the timeout management in the driver and add some delay for each bursts in the timeout calculation. Reported-by: Da Xue <da@libre.computer> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- To: Mark Brown <broonie@kernel.org> To: Kevin Hilman <khilman@baylibre.com> To: Jerome Brunet <jbrunet@baylibre.com> To: Martin Blumenstingl <martin.blumenstingl@googlemail.com> Cc: linux-spi@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-amlogic@lists.infradead.org Cc: linux-kernel@vger.kernel.org --- drivers/spi/spi-meson-spicc.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) --- base-commit: 247f34f7b80357943234f93f247a1ae6b6c3a740 change-id: 20221026-spicc-burst-delay-ea0526602760 Best regards,