[PATCH v2 0/2] spi: imx: support word delay in ecspi

[PATCH v2 0/2] spi: imx: support word delay in ecspi

[Jonas Rebmann]

The i.MX SPI controller supports inserting a configurable delay between
subsequent words, which is needed for some slower devices that couldn't
keep up otherwise.

This patch series introduces support for the word delay parameters for
i.MX51 onwards.

The SPI clock (CSRC=0) was chosen as the clock source over the also
available 32.768 KHz Low-Frequency Reference Clock (CSRC=1). The sample
period control bits (SAMPLE_PERIOD) are set to the selected word delay
converted to SPI clock cycles. A deviation from the requested number of
wait cycles and the actual word delay was observed via both software
timings and oscilloscope measurements and accounted for.

The Chip Select Delay Control bits in the Sample Period Control Register
remain zero.

Behaviour on i.MX35 and earlier, where the CSPI interface is used,
remains unchanged.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
Changes in v2:
- Remove accidentally added CCs
- spi-imx.c: Add missing includes, Rb kernel test robot
- Link to v1: https://lore.kernel.org/r/20241107-imx-spi-word-delay-v1-0-2a969214d796@pengutronix.de

---
Jonas Rebmann (2):
      spi: imx: pass struct spi_transfer to prepare_transfer()
      spi: imx: support word delay

 drivers/spi/spi-imx.c | 108 ++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 92 insertions(+), 16 deletions(-)
---
base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc
change-id: 20241009-imx-spi-word-delay-21dc01f098cc

Best regards,
-- 
Jonas Rebmann <jre@pengutronix.de>

[PATCH v2 1/2] spi: imx: pass struct spi_transfer to prepare_transfer()

[Jonas Rebmann]

In an upcoming patch, mx51_ecspi_prepare_transfer() needs access to the
word_delay parameter. To enable controller-specific handling of such
per-transfer parameters, extend the prepare_transfer() function of the
spi_imx_devtype_data interface to take a struct spi_transfer argument,
update all controller-specific implementations accordingly.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 drivers/spi/spi-imx.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 85bd1a82a34eb4bc76a4b4528e087fc2ebfa8b85..65a8303b80b1191cd2c19d61f88836e7fd3c7ae9 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -71,7 +71,8 @@ struct spi_imx_data;
 struct spi_imx_devtype_data {
 	void (*intctrl)(struct spi_imx_data *spi_imx, int enable);
 	int (*prepare_message)(struct spi_imx_data *spi_imx, struct spi_message *msg);
-	int (*prepare_transfer)(struct spi_imx_data *spi_imx, struct spi_device *spi);
+	int (*prepare_transfer)(struct spi_imx_data *spi_imx, struct spi_device *spi,
+				struct spi_transfer *t);
 	void (*trigger)(struct spi_imx_data *spi_imx);
 	int (*rx_available)(struct spi_imx_data *spi_imx);
 	void (*reset)(struct spi_imx_data *spi_imx);
@@ -649,7 +650,7 @@ static void mx51_configure_cpha(struct spi_imx_data *spi_imx,
 }
 
 static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx,
-				       struct spi_device *spi)
+				       struct spi_device *spi, struct spi_transfer *t)
 {
 	u32 ctrl = readl(spi_imx->base + MX51_ECSPI_CTRL);
 	u32 clk;
@@ -774,7 +775,7 @@ static int mx31_prepare_message(struct spi_imx_data *spi_imx,
 }
 
 static int mx31_prepare_transfer(struct spi_imx_data *spi_imx,
-				 struct spi_device *spi)
+				 struct spi_device *spi, struct spi_transfer *t)
 {
 	unsigned int reg = MX31_CSPICTRL_ENABLE | MX31_CSPICTRL_HOST;
 	unsigned int clk;
@@ -878,7 +879,7 @@ static int mx21_prepare_message(struct spi_imx_data *spi_imx,
 }
 
 static int mx21_prepare_transfer(struct spi_imx_data *spi_imx,
-				 struct spi_device *spi)
+				 struct spi_device *spi, struct spi_transfer *t)
 {
 	unsigned int reg = MX21_CSPICTRL_ENABLE | MX21_CSPICTRL_HOST;
 	unsigned int max = is_imx27_cspi(spi_imx) ? 16 : 18;
@@ -953,7 +954,7 @@ static int mx1_prepare_message(struct spi_imx_data *spi_imx,
 }
 
 static int mx1_prepare_transfer(struct spi_imx_data *spi_imx,
-				struct spi_device *spi)
+				struct spi_device *spi, struct spi_transfer *t)
 {
 	unsigned int reg = MX1_CSPICTRL_ENABLE | MX1_CSPICTRL_HOST;
 	unsigned int clk;
@@ -1304,7 +1305,7 @@ static int spi_imx_setupxfer(struct spi_device *spi,
 		spi_imx->target_burst = t->len;
 	}
 
-	spi_imx->devtype_data->prepare_transfer(spi_imx, spi);
+	spi_imx->devtype_data->prepare_transfer(spi_imx, spi, t);
 
 	return 0;
 }

-- 
2.39.5

Re: [PATCH v2 1/2] spi: imx: pass struct spi_transfer to prepare_transfer()

[Frank Li]

On Wed, Nov 13, 2024 at 01:18:31PM +0100, Jonas Rebmann wrote:
> In an upcoming patch, mx51_ecspi_prepare_transfer() needs access to the
> word_delay parameter. To enable controller-specific handling of such
> per-transfer parameters, extend the prepare_transfer() function of the
> spi_imx_devtype_data interface to take a struct spi_transfer argument,
> update all controller-specific implementations accordingly.
>
> Signed-off-by: Jonas Rebmann <jre@pengutronix.dei

Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/spi/spi-imx.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index 85bd1a82a34eb4bc76a4b4528e087fc2ebfa8b85..65a8303b80b1191cd2c19d61f88836e7fd3c7ae9 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -71,7 +71,8 @@ struct spi_imx_data;
>  struct spi_imx_devtype_data {
>  	void (*intctrl)(struct spi_imx_data *spi_imx, int enable);
>  	int (*prepare_message)(struct spi_imx_data *spi_imx, struct spi_message *msg);
> -	int (*prepare_transfer)(struct spi_imx_data *spi_imx, struct spi_device *spi);
> +	int (*prepare_transfer)(struct spi_imx_data *spi_imx, struct spi_device *spi,
> +				struct spi_transfer *t);
>  	void (*trigger)(struct spi_imx_data *spi_imx);
>  	int (*rx_available)(struct spi_imx_data *spi_imx);
>  	void (*reset)(struct spi_imx_data *spi_imx);
> @@ -649,7 +650,7 @@ static void mx51_configure_cpha(struct spi_imx_data *spi_imx,
>  }
>
>  static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx,
> -				       struct spi_device *spi)
> +				       struct spi_device *spi, struct spi_transfer *t)
>  {
>  	u32 ctrl = readl(spi_imx->base + MX51_ECSPI_CTRL);
>  	u32 clk;
> @@ -774,7 +775,7 @@ static int mx31_prepare_message(struct spi_imx_data *spi_imx,
>  }
>
>  static int mx31_prepare_transfer(struct spi_imx_data *spi_imx,
> -				 struct spi_device *spi)
> +				 struct spi_device *spi, struct spi_transfer *t)
>  {
>  	unsigned int reg = MX31_CSPICTRL_ENABLE | MX31_CSPICTRL_HOST;
>  	unsigned int clk;
> @@ -878,7 +879,7 @@ static int mx21_prepare_message(struct spi_imx_data *spi_imx,
>  }
>
>  static int mx21_prepare_transfer(struct spi_imx_data *spi_imx,
> -				 struct spi_device *spi)
> +				 struct spi_device *spi, struct spi_transfer *t)
>  {
>  	unsigned int reg = MX21_CSPICTRL_ENABLE | MX21_CSPICTRL_HOST;
>  	unsigned int max = is_imx27_cspi(spi_imx) ? 16 : 18;
> @@ -953,7 +954,7 @@ static int mx1_prepare_message(struct spi_imx_data *spi_imx,
>  }
>
>  static int mx1_prepare_transfer(struct spi_imx_data *spi_imx,
> -				struct spi_device *spi)
> +				struct spi_device *spi, struct spi_transfer *t)
>  {
>  	unsigned int reg = MX1_CSPICTRL_ENABLE | MX1_CSPICTRL_HOST;
>  	unsigned int clk;
> @@ -1304,7 +1305,7 @@ static int spi_imx_setupxfer(struct spi_device *spi,
>  		spi_imx->target_burst = t->len;
>  	}
>
> -	spi_imx->devtype_data->prepare_transfer(spi_imx, spi);
> +	spi_imx->devtype_data->prepare_transfer(spi_imx, spi, t);
>
>  	return 0;
>  }
>
> --
> 2.39.5
>

[PATCH v2 2/2] spi: imx: support word delay

[Jonas Rebmann]

Implement support for the word delay feature of i.MX51 (and onwards) via
the ECSPI interface.

Convert the requested delay to SPI cycles and account for an extra
inter-word delay inserted by the controller in addition to the requested
number of cycles, which was observed when testing this patch.

Disable dynamic burst when word delay is set. As the configurable delay
period in the controller is inserted after bursts, the burst length must
equal the word length.

Account for word delay in the transfer time estimation for
polling_limit_us.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 drivers/spi/spi-imx.c | 95 +++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 85 insertions(+), 10 deletions(-)

diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
index 65a8303b80b1191cd2c19d61f88836e7fd3c7ae9..29b83659b8036d1cffe076012ad5cb229509abd8 100644
--- a/drivers/spi/spi-imx.c
+++ b/drivers/spi/spi-imx.c
@@ -3,6 +3,7 @@
 // Copyright (C) 2008 Juergen Beisert
 
 #include <linux/bits.h>
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/delay.h>
@@ -13,7 +14,10 @@
 #include <linux/io.h>
 #include <linux/irq.h>
 #include <linux/kernel.h>
+#include <linux/math.h>
+#include <linux/math64.h>
 #include <linux/module.h>
+#include <linux/overflow.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
@@ -302,6 +306,18 @@ static bool spi_imx_can_dma(struct spi_controller *controller, struct spi_device
 #define MX51_ECSPI_STAT		0x18
 #define MX51_ECSPI_STAT_RR		(1 <<  3)
 
+#define MX51_ECSPI_PERIOD	0x1c
+#define MX51_ECSPI_PERIOD_MASK		0x7fff
+/*
+ * As measured on the i.MX6, the SPI host controller inserts a 4 SPI-Clock
+ * (SCLK) delay after each burst if the PERIOD reg is 0x0. This value will be
+ * called MX51_ECSPI_PERIOD_MIN_DELAY_SCK.
+ *
+ * If the PERIOD register is != 0, the controller inserts a delay of
+ * MX51_ECSPI_PERIOD_MIN_DELAY_SCK + register value + 1 SCLK after each burst.
+ */
+#define MX51_ECSPI_PERIOD_MIN_DELAY_SCK 4
+
 #define MX51_ECSPI_TESTREG	0x20
 #define MX51_ECSPI_TESTREG_LBC	BIT(31)
 
@@ -653,6 +669,7 @@ static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx,
 				       struct spi_device *spi, struct spi_transfer *t)
 {
 	u32 ctrl = readl(spi_imx->base + MX51_ECSPI_CTRL);
+	u64 word_delay_sck;
 	u32 clk;
 
 	/* Clear BL field and set the right value */
@@ -684,6 +701,49 @@ static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx,
 
 	writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL);
 
+	/* calculate word delay in SPI Clock (SCLK) cycles */
+	if (t->word_delay.value == 0) {
+		word_delay_sck = 0;
+	} else if (t->word_delay.unit == SPI_DELAY_UNIT_SCK) {
+		word_delay_sck = t->word_delay.value;
+
+		if (word_delay_sck <= MX51_ECSPI_PERIOD_MIN_DELAY_SCK)
+			word_delay_sck = 0;
+		else if (word_delay_sck <= MX51_ECSPI_PERIOD_MIN_DELAY_SCK + 1)
+			word_delay_sck = 1;
+		else
+			word_delay_sck -= MX51_ECSPI_PERIOD_MIN_DELAY_SCK + 1;
+	} else {
+		int word_delay_ns;
+
+		word_delay_ns = spi_delay_to_ns(&t->word_delay, t);
+		if (word_delay_ns < 0)
+			return word_delay_ns;
+
+		if (word_delay_ns <= mul_u64_u32_div(NSEC_PER_SEC,
+						     MX51_ECSPI_PERIOD_MIN_DELAY_SCK,
+						     spi_imx->spi_bus_clk)) {
+			word_delay_sck = 0;
+		} else if (word_delay_ns <= mul_u64_u32_div(NSEC_PER_SEC,
+							    MX51_ECSPI_PERIOD_MIN_DELAY_SCK + 1,
+							    spi_imx->spi_bus_clk)) {
+			word_delay_sck = 1;
+		} else {
+			word_delay_ns -= mul_u64_u32_div(NSEC_PER_SEC,
+							 MX51_ECSPI_PERIOD_MIN_DELAY_SCK + 1,
+							 spi_imx->spi_bus_clk);
+
+			word_delay_sck = DIV_U64_ROUND_UP((u64)word_delay_ns * spi_imx->spi_bus_clk,
+							  NSEC_PER_SEC);
+		}
+	}
+
+	if (!FIELD_FIT(MX51_ECSPI_PERIOD_MASK, word_delay_sck))
+		return -EINVAL;
+
+	writel(FIELD_PREP(MX51_ECSPI_PERIOD_MASK, word_delay_sck),
+	       spi_imx->base + MX51_ECSPI_PERIOD);
+
 	return 0;
 }
 
@@ -1264,11 +1324,13 @@ static int spi_imx_setupxfer(struct spi_device *spi,
 
 	/*
 	 * Initialize the functions for transfer. To transfer non byte-aligned
-	 * words, we have to use multiple word-size bursts, we can't use
-	 * dynamic_burst in that case.
+	 * words, we have to use multiple word-size bursts. To insert word
+	 * delay, the burst size has to equal the word size. We can't use
+	 * dynamic_burst in these cases.
 	 */
 	if (spi_imx->devtype_data->dynamic_burst && !spi_imx->target_mode &&
 	    !(spi->mode & SPI_CS_WORD) &&
+	    !(t->word_delay.value) &&
 	    (spi_imx->bits_per_word == 8 ||
 	    spi_imx->bits_per_word == 16 ||
 	    spi_imx->bits_per_word == 32)) {
@@ -1611,12 +1673,30 @@ static int spi_imx_pio_transfer_target(struct spi_device *spi,
 	return ret;
 }
 
+static unsigned int spi_imx_transfer_estimate_time_us(struct spi_transfer *transfer)
+{
+	u64 result;
+
+	result = DIV_U64_ROUND_CLOSEST((u64)USEC_PER_SEC * transfer->len * BITS_PER_BYTE,
+				       transfer->effective_speed_hz);
+	if (transfer->word_delay.value) {
+		unsigned int word_delay_us;
+		unsigned int words;
+
+		words = DIV_ROUND_UP(transfer->len * BITS_PER_BYTE, transfer->bits_per_word);
+		word_delay_us = DIV_ROUND_CLOSEST(spi_delay_to_ns(&transfer->word_delay, transfer),
+						  NSEC_PER_USEC);
+		result += words * word_delay_us;
+	}
+
+	return min(result, U32_MAX);
+}
+
 static int spi_imx_transfer_one(struct spi_controller *controller,
 				struct spi_device *spi,
 				struct spi_transfer *transfer)
 {
 	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;
@@ -1635,15 +1715,10 @@ static int spi_imx_transfer_one(struct spi_controller *controller,
 	 */
 	if (spi_imx->usedma)
 		return spi_imx_dma_transfer(spi_imx, transfer);
-	/*
-	 * 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 ? transfer->effective_speed_hz / hz_per_byte : 1;
 
 	/* run in polling mode for short transfers */
-	if (transfer->len < byte_limit)
+	if (transfer->len == 1 || (polling_limit_us &&
+				   spi_imx_transfer_estimate_time_us(transfer) < polling_limit_us))
 		return spi_imx_poll_transfer(spi, transfer);
 
 	return spi_imx_pio_transfer(spi, transfer);

-- 
2.39.5

Re: [PATCH v2 2/2] spi: imx: support word delay

[Frank Li]

On Wed, Nov 13, 2024 at 01:18:32PM +0100, Jonas Rebmann wrote:
> Implement support for the word delay feature of i.MX51 (and onwards) via
> the ECSPI interface.
>
> Convert the requested delay to SPI cycles and account for an extra
> inter-word delay inserted by the controller in addition to the requested
> number of cycles, which was observed when testing this patch.
>
> Disable dynamic burst when word delay is set. As the configurable delay
> period in the controller is inserted after bursts, the burst length must
> equal the word length.
>
> Account for word delay in the transfer time estimation for
> polling_limit_us.
>
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
>  drivers/spi/spi-imx.c | 95 +++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 85 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c
> index 65a8303b80b1191cd2c19d61f88836e7fd3c7ae9..29b83659b8036d1cffe076012ad5cb229509abd8 100644
> --- a/drivers/spi/spi-imx.c
> +++ b/drivers/spi/spi-imx.c
> @@ -3,6 +3,7 @@
>  // Copyright (C) 2008 Juergen Beisert
>
>  #include <linux/bits.h>
> +#include <linux/bitfield.h>
>  #include <linux/clk.h>
>  #include <linux/completion.h>
>  #include <linux/delay.h>
> @@ -13,7 +14,10 @@
>  #include <linux/io.h>
>  #include <linux/irq.h>
>  #include <linux/kernel.h>
> +#include <linux/math.h>
> +#include <linux/math64.h>
>  #include <linux/module.h>
> +#include <linux/overflow.h>
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> @@ -302,6 +306,18 @@ static bool spi_imx_can_dma(struct spi_controller *controller, struct spi_device
>  #define MX51_ECSPI_STAT		0x18
>  #define MX51_ECSPI_STAT_RR		(1 <<  3)
>
> +#define MX51_ECSPI_PERIOD	0x1c
> +#define MX51_ECSPI_PERIOD_MASK		0x7fff
> +/*
> + * As measured on the i.MX6, the SPI host controller inserts a 4 SPI-Clock
> + * (SCLK) delay after each burst if the PERIOD reg is 0x0. This value will be
> + * called MX51_ECSPI_PERIOD_MIN_DELAY_SCK.
> + *
> + * If the PERIOD register is != 0, the controller inserts a delay of
> + * MX51_ECSPI_PERIOD_MIN_DELAY_SCK + register value + 1 SCLK after each burst.
> + */
> +#define MX51_ECSPI_PERIOD_MIN_DELAY_SCK 4
> +
>  #define MX51_ECSPI_TESTREG	0x20
>  #define MX51_ECSPI_TESTREG_LBC	BIT(31)
>
> @@ -653,6 +669,7 @@ static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx,
>  				       struct spi_device *spi, struct spi_transfer *t)
>  {
>  	u32 ctrl = readl(spi_imx->base + MX51_ECSPI_CTRL);
> +	u64 word_delay_sck;
>  	u32 clk;
>
>  	/* Clear BL field and set the right value */
> @@ -684,6 +701,49 @@ static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx,
>
>  	writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL);
>
> +	/* calculate word delay in SPI Clock (SCLK) cycles */
> +	if (t->word_delay.value == 0) {
> +		word_delay_sck = 0;
> +	} else if (t->word_delay.unit == SPI_DELAY_UNIT_SCK) {
> +		word_delay_sck = t->word_delay.value;
> +
> +		if (word_delay_sck <= MX51_ECSPI_PERIOD_MIN_DELAY_SCK)
> +			word_delay_sck = 0;
> +		else if (word_delay_sck <= MX51_ECSPI_PERIOD_MIN_DELAY_SCK + 1)
> +			word_delay_sck = 1;
> +		else
> +			word_delay_sck -= MX51_ECSPI_PERIOD_MIN_DELAY_SCK + 1;
> +	} else {
> +		int word_delay_ns;
> +
> +		word_delay_ns = spi_delay_to_ns(&t->word_delay, t);
> +		if (word_delay_ns < 0)
> +			return word_delay_ns;
> +
> +		if (word_delay_ns <= mul_u64_u32_div(NSEC_PER_SEC,
> +						     MX51_ECSPI_PERIOD_MIN_DELAY_SCK,
> +						     spi_imx->spi_bus_clk)) {
> +			word_delay_sck = 0;
> +		} else if (word_delay_ns <= mul_u64_u32_div(NSEC_PER_SEC,
> +							    MX51_ECSPI_PERIOD_MIN_DELAY_SCK + 1,
> +							    spi_imx->spi_bus_clk)) {
> +			word_delay_sck = 1;
> +		} else {
> +			word_delay_ns -= mul_u64_u32_div(NSEC_PER_SEC,
> +							 MX51_ECSPI_PERIOD_MIN_DELAY_SCK + 1,
> +							 spi_imx->spi_bus_clk);
> +
> +			word_delay_sck = DIV_U64_ROUND_UP((u64)word_delay_ns * spi_imx->spi_bus_clk,
> +							  NSEC_PER_SEC);
> +		}
> +	}
> +
> +	if (!FIELD_FIT(MX51_ECSPI_PERIOD_MASK, word_delay_sck))
> +		return -EINVAL;
> +
> +	writel(FIELD_PREP(MX51_ECSPI_PERIOD_MASK, word_delay_sck),
> +	       spi_imx->base + MX51_ECSPI_PERIOD);
> +
>  	return 0;
>  }
>
> @@ -1264,11 +1324,13 @@ static int spi_imx_setupxfer(struct spi_device *spi,
>
>  	/*
>  	 * Initialize the functions for transfer. To transfer non byte-aligned
> -	 * words, we have to use multiple word-size bursts, we can't use
> -	 * dynamic_burst in that case.
> +	 * words, we have to use multiple word-size bursts. To insert word
> +	 * delay, the burst size has to equal the word size. We can't use
> +	 * dynamic_burst in these cases.
>  	 */
>  	if (spi_imx->devtype_data->dynamic_burst && !spi_imx->target_mode &&
>  	    !(spi->mode & SPI_CS_WORD) &&
> +	    !(t->word_delay.value) &&
>  	    (spi_imx->bits_per_word == 8 ||
>  	    spi_imx->bits_per_word == 16 ||
>  	    spi_imx->bits_per_word == 32)) {
> @@ -1611,12 +1673,30 @@ static int spi_imx_pio_transfer_target(struct spi_device *spi,
>  	return ret;
>  }
>
> +static unsigned int spi_imx_transfer_estimate_time_us(struct spi_transfer *transfer)
> +{
> +	u64 result;
> +
> +	result = DIV_U64_ROUND_CLOSEST((u64)USEC_PER_SEC * transfer->len * BITS_PER_BYTE,
> +				       transfer->effective_speed_hz);
> +	if (transfer->word_delay.value) {
> +		unsigned int word_delay_us;
> +		unsigned int words;
> +
> +		words = DIV_ROUND_UP(transfer->len * BITS_PER_BYTE, transfer->bits_per_word);
> +		word_delay_us = DIV_ROUND_CLOSEST(spi_delay_to_ns(&transfer->word_delay, transfer),
> +						  NSEC_PER_USEC);
> +		result += words * word_delay_us;
> +	}
> +
> +	return min(result, U32_MAX);
> +}
> +
>  static int spi_imx_transfer_one(struct spi_controller *controller,
>  				struct spi_device *spi,
>  				struct spi_transfer *transfer)
>  {
>  	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;
> @@ -1635,15 +1715,10 @@ static int spi_imx_transfer_one(struct spi_controller *controller,
>  	 */
>  	if (spi_imx->usedma)
>  		return spi_imx_dma_transfer(spi_imx, transfer);
> -	/*
> -	 * 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 ? transfer->effective_speed_hz / hz_per_byte : 1;
>
>  	/* run in polling mode for short transfers */
> -	if (transfer->len < byte_limit)
> +	if (transfer->len == 1 || (polling_limit_us &&
> +				   spi_imx_transfer_estimate_time_us(transfer) < polling_limit_us))
>  		return spi_imx_poll_transfer(spi, transfer);
>
>  	return spi_imx_pio_transfer(spi, transfer);
>
> --
> 2.39.5
>

Re: [PATCH v2 2/2] spi: imx: support word delay

[Kees Bakker]

Op 13-11-2024 om 13:18 schreef Jonas Rebmann:
> Implement support for the word delay feature of i.MX51 (and onwards) via
> the ECSPI interface.
>
> Convert the requested delay to SPI cycles and account for an extra
> inter-word delay inserted by the controller in addition to the requested
> number of cycles, which was observed when testing this patch.
>
> Disable dynamic burst when word delay is set. As the configurable delay
> period in the controller is inserted after bursts, the burst length must
> equal the word length.
>
> Account for word delay in the transfer time estimation for
> polling_limit_us.
>
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
> ---
>   drivers/spi/spi-imx.c | 95 +++++++++++++++++++++++++++++++++++++++++++++------
>   1 file changed, 85 insertions(+), 10 deletions(-)
>
> [...]
> +static unsigned int spi_imx_transfer_estimate_time_us(struct spi_transfer *transfer)
> +{
> +	u64 result;
> +
> +	result = DIV_U64_ROUND_CLOSEST((u64)USEC_PER_SEC * transfer->len * BITS_PER_BYTE,
> +				       transfer->effective_speed_hz);
> +	if (transfer->word_delay.value) {
> +		unsigned int word_delay_us;
> +		unsigned int words;
> +
> +		words = DIV_ROUND_UP(transfer->len * BITS_PER_BYTE, transfer->bits_per_word);
> +		word_delay_us = DIV_ROUND_CLOSEST(spi_delay_to_ns(&transfer->word_delay, transfer),
> +						  NSEC_PER_USEC);
> +		result += words * word_delay_us;
If the multiplication can overflow 32 bits to need to force a 64 bits 
multiply.
     result += (u64)words * word_delay_us;

But I'm wondering if `result` needs to be u64.
> +	}
> +
> +	return min(result, U32_MAX);
Do you really expect this much? You're clipping to U32_MAX.
U32_MAX microsecs is already more than an hour.
> +}
> [...]

Re: [PATCH v2 0/2] spi: imx: support word delay in ecspi

[Mark Brown]

On Wed, 13 Nov 2024 13:18:30 +0100, Jonas Rebmann wrote:
> The i.MX SPI controller supports inserting a configurable delay between
> subsequent words, which is needed for some slower devices that couldn't
> keep up otherwise.
> 
> This patch series introduces support for the word delay parameters for
> i.MX51 onwards.
> 
> [...]

Applied to

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

Thanks!

[1/2] spi: imx: pass struct spi_transfer to prepare_transfer()
      commit: 7b94af24a7a4d12a76183f1b2f0d363d2c9ced43
[2/2] spi: imx: support word delay
      commit: a3bb4e663df318b232746478e7b191bcf6e3af40

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