diff mbox series

spi: meson-spicc: move wait completion in driver to take bursts delay in account

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

Commit Message

Neil Armstrong Oct. 26, 2022, 7:58 a.m. UTC
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,

Comments

Mark Brown Oct. 26, 2022, 12:39 p.m. UTC | #1
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.
Neil Armstrong Oct. 26, 2022, 12:48 p.m. UTC | #2
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
Mark Brown Oct. 26, 2022, 12:51 p.m. UTC | #3
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.
Mark Brown Oct. 26, 2022, 5:54 p.m. UTC | #4
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 mbox series

Patch

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");