spi: axi-spi-engine: fix sleep calculation

Commit Message

David Lechner June 20, 2024, 4:43 p.m. UTC

The sleep calculation was not taking into account increased delay when
the SPI device is not running at the maximum SCLK frequency.

Rounding down when one SCLK tick was the same as the instruction
execution time was fine, but it rounds down too much when SCLK is
slower. This changes the rounding to round up instead while still
taking into account the instruction execution time so that small
delays remain accurate.

Fixes: be9070bcf670 ("spi: axi-spi-engine: fix sleep ticks calculation")
Signed-off-by: David Lechner <[e-mail]>
---
 drivers/spi/spi-axi-spi-engine.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)


---
base-commit: 3f685a501f2ee00111ce930e9a210b9eab5e759d
change-id: 20240620-spi-axi-spi-engine-fix-sleep-time-6f0b7c147a0f

Comments

Mark Brown June 25, 2024, 11:34 a.m. UTC | #1

On Thu, 20 Jun 2024 11:43:58 -0500, David Lechner wrote:
> The sleep calculation was not taking into account increased delay when
> the SPI device is not running at the maximum SCLK frequency.
> 
> Rounding down when one SCLK tick was the same as the instruction
> execution time was fine, but it rounds down too much when SCLK is
> slower. This changes the rounding to round up instead while still
> taking into account the instruction execution time so that small
> delays remain accurate.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: axi-spi-engine: fix sleep calculation
      commit: 40b3d0838a1ff242e61f341e49226074bbdd319f

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

Patch

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index 3231f67ae265..103a68bd4f19 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -169,16 +169,20 @@  static void spi_engine_gen_xfer(struct spi_engine_program *p, bool dry,
 }
 
 static void spi_engine_gen_sleep(struct spi_engine_program *p, bool dry,
-				 int delay_ns, u32 sclk_hz)
+				 int delay_ns, int inst_ns, u32 sclk_hz)
 {
 	unsigned int t;
 
-	/* negative delay indicates error, e.g. from spi_delay_to_ns() */
-	if (delay_ns <= 0)
+	/*
+	 * Negative delay indicates error, e.g. from spi_delay_to_ns(). And if
+	 * delay is less that the instruction execution time, there is no need
+	 * for an extra sleep instruction since the instruction execution time
+	 * will already cover the required delay.
+	 */
+	if (delay_ns < 0 || delay_ns <= inst_ns)
 		return;
 
-	/* rounding down since executing the instruction adds a couple of ticks delay */
-	t = DIV_ROUND_DOWN_ULL((u64)delay_ns * sclk_hz, NSEC_PER_SEC);
+	t = DIV_ROUND_UP_ULL((u64)(delay_ns - inst_ns) * sclk_hz, NSEC_PER_SEC);
 	while (t) {
 		unsigned int n = min(t, 256U);
 
@@ -225,10 +229,16 @@  static void spi_engine_compile_message(struct spi_message *msg, bool dry,
 	struct spi_device *spi = msg->spi;
 	struct spi_controller *host = spi->controller;
 	struct spi_transfer *xfer;
-	int clk_div, new_clk_div;
+	int clk_div, new_clk_div, inst_ns;
 	bool keep_cs = false;
 	u8 bits_per_word = 0;
 
+	/*
+	 * Take into account instruction execution time for more accurate sleep
+	 * times, especially when the delay is small.
+	 */
+	inst_ns = DIV_ROUND_UP(NSEC_PER_SEC, host->max_speed_hz);
+
 	clk_div = 1;
 
 	spi_engine_program_add_cmd(p, dry,
@@ -257,7 +267,7 @@  static void spi_engine_compile_message(struct spi_message *msg, bool dry,
 
 		spi_engine_gen_xfer(p, dry, xfer);
 		spi_engine_gen_sleep(p, dry, spi_delay_to_ns(&xfer->delay, xfer),
-				     xfer->effective_speed_hz);
+				     inst_ns, xfer->effective_speed_hz);
 
 		if (xfer->cs_change) {
 			if (list_is_last(&xfer->transfer_list, &msg->transfers)) {
@@ -267,7 +277,7 @@  static void spi_engine_compile_message(struct spi_message *msg, bool dry,
 					spi_engine_gen_cs(p, dry, spi, false);
 
 				spi_engine_gen_sleep(p, dry, spi_delay_to_ns(
-					&xfer->cs_change_delay, xfer),
+					&xfer->cs_change_delay, xfer), inst_ns,
 					xfer->effective_speed_hz);
 
 				if (!list_next_entry(xfer, transfer_list)->cs_off)