public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] spi: meson-spicc: move wait completion in driver to take bursts delay in account
@ 2022-10-26  7:58 Neil Armstrong
  2022-10-26 12:39 ` Mark Brown
  2022-10-26 17:54 ` Mark Brown
  0 siblings, 2 replies; 5+ messages in thread
From: Neil Armstrong @ 2022-10-26  7:58 UTC (permalink / raw)
  To: Jerome Brunet, Martin Blumenstingl, Mark Brown, Kevin Hilman
  Cc: linux-arm-kernel, Da Xue, linux-kernel, linux-amlogic,
	Neil Armstrong, linux-spi

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(-)

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

---
base-commit: 247f34f7b80357943234f93f247a1ae6b6c3a740
change-id: 20221026-spicc-burst-delay-ea0526602760

Best regards,
-- 
Neil Armstrong <neil.armstrong@linaro.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] spi: meson-spicc: move wait completion in driver to take bursts delay in account
  2022-10-26  7:58 [PATCH] spi: meson-spicc: move wait completion in driver to take bursts delay in account Neil Armstrong
@ 2022-10-26 12:39 ` Mark Brown
  2022-10-26 12:48   ` Neil Armstrong
  2022-10-26 17:54 ` Mark Brown
  1 sibling, 1 reply; 5+ messages in thread
From: Mark Brown @ 2022-10-26 12:39 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Jerome Brunet, Martin Blumenstingl, Kevin Hilman,
	linux-arm-kernel, Da Xue, linux-kernel, linux-amlogic, linux-spi


[-- Attachment #1.1: Type: text/plain, Size: 285 bytes --]

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.

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] spi: meson-spicc: move wait completion in driver to take bursts delay in account
  2022-10-26 12:39 ` Mark Brown
@ 2022-10-26 12:48   ` Neil Armstrong
  2022-10-26 12:51     ` Mark Brown
  0 siblings, 1 reply; 5+ messages in thread
From: Neil Armstrong @ 2022-10-26 12:48 UTC (permalink / raw)
  To: Mark Brown
  Cc: Jerome Brunet, Martin Blumenstingl, Kevin Hilman,
	linux-arm-kernel, Da Xue, linux-kernel, linux-amlogic, linux-spi

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] spi: meson-spicc: move wait completion in driver to take bursts delay in account
  2022-10-26 12:48   ` Neil Armstrong
@ 2022-10-26 12:51     ` Mark Brown
  0 siblings, 0 replies; 5+ messages in thread
From: Mark Brown @ 2022-10-26 12:51 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: Jerome Brunet, Martin Blumenstingl, Kevin Hilman,
	linux-arm-kernel, Da Xue, linux-kernel, linux-amlogic, linux-spi


[-- Attachment #1.1: Type: text/plain, Size: 219 bytes --]

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.

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] spi: meson-spicc: move wait completion in driver to take bursts delay in account
  2022-10-26  7:58 [PATCH] spi: meson-spicc: move wait completion in driver to take bursts delay in account Neil Armstrong
  2022-10-26 12:39 ` Mark Brown
@ 2022-10-26 17:54 ` Mark Brown
  1 sibling, 0 replies; 5+ messages in thread
From: Mark Brown @ 2022-10-26 17:54 UTC (permalink / raw)
  To: Neil Armstrong, Jerome Brunet, Kevin Hilman, Martin Blumenstingl
  Cc: linux-kernel, linux-amlogic, Da Xue, linux-spi, linux-arm-kernel

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-10-26 17:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-26  7:58 [PATCH] spi: meson-spicc: move wait completion in driver to take bursts delay in account Neil Armstrong
2022-10-26 12:39 ` Mark Brown
2022-10-26 12:48   ` Neil Armstrong
2022-10-26 12:51     ` Mark Brown
2022-10-26 17:54 ` Mark Brown

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox