[PATCH 00/10] spi: finalize 'delay_usecs' removal/transition

[PATCH 00/10] spi: finalize 'delay_usecs' removal/transition

[Alexandru Ardelean]

A while back I started the introduction of the 'spi_delay' data type:
  https://lore.kernel.org/linux-spi/20190926105147.7839-1-alexandru.ardelean@analog.com/

Users of the 'delay_usecs' were removed from drivers.

Now it's time to remove the 'delay_usecs' from the SPI subsystem and use
only the 'delay' field.

This changeset adapts all SPI drivers to do without 'delay_usecs'.
Additionally, for greybus we need to adapt it to use the 'delay' in
nano-seconds and convert it to micro-seconds.

Alexandru Ardelean (10):
  spi: spi-axi-spi-engine: remove usage of delay_usecs
  spi: bcm63xx-spi: don't check 'delay_usecs' field
  spi: spi-bcm-qspi: replace 'delay_usecs' with 'delay.value' check
  spi: spi-sh: replace 'delay_usecs' with 'delay.value' in pr_debug
  spi: spi-tegra20-flash: don't check 'delay_usecs' field for spi
    transfer
  staging: greybus: spilib: use 'spi_delay_to_ns' for getting xfer delay
  spi: spi-falcon: remove check for 'delay_usecs'
  spi: fsl-espi: remove usage of 'delay_usecs' field
  spi: core: remove 'delay_usecs' field from spi_transfer
  spi: docs: update info about 'delay_usecs'

 Documentation/spi/spi-summary.rst |  7 +++++--
 drivers/spi/spi-axi-spi-engine.c  | 12 ++++--------
 drivers/spi/spi-bcm-qspi.c        |  2 +-
 drivers/spi/spi-bcm63xx.c         |  2 +-
 drivers/spi/spi-falcon.c          |  2 +-
 drivers/spi/spi-fsl-espi.c        | 17 +++++------------
 drivers/spi/spi-sh.c              |  4 ++--
 drivers/spi/spi-tegra20-sflash.c  |  3 +--
 drivers/spi/spi.c                 |  1 -
 drivers/staging/greybus/spilib.c  |  5 ++++-
 include/linux/spi/spi.h           | 12 ------------
 11 files changed, 24 insertions(+), 43 deletions(-)

-- 
2.29.2

[PATCH 01/10] spi: spi-axi-spi-engine: remove usage of delay_usecs

[Alexandru Ardelean]

The 'delay_usecs' field was handled for backwards compatibility in case
there were some users that still configured SPI delay transfers with
this field.

They should all be removed by now.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/spi/spi-axi-spi-engine.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
index af86e6d6e16b..80c3e38f5c1b 100644
--- a/drivers/spi/spi-axi-spi-engine.c
+++ b/drivers/spi/spi-axi-spi-engine.c
@@ -170,14 +170,10 @@ static void spi_engine_gen_sleep(struct spi_engine_program *p, bool dry,
 	unsigned int t;
 	int delay;
 
-	if (xfer->delay_usecs) {
-		delay = xfer->delay_usecs;
-	} else {
-		delay = spi_delay_to_ns(&xfer->delay, xfer);
-		if (delay < 0)
-			return;
-		delay /= 1000;
-	}
+	delay = spi_delay_to_ns(&xfer->delay, xfer);
+	if (delay < 0)
+		return;
+	delay /= 1000;
 
 	if (delay == 0)
 		return;
-- 
2.29.2

[PATCH 02/10] spi: bcm63xx-spi: don't check 'delay_usecs' field

[Alexandru Ardelean]

The 'delay_usecs' field was handled for backwards compatibility in case
there were some users that still configured SPI delay transfers with
this field.

They should all be removed by now.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/spi/spi-bcm63xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-bcm63xx.c b/drivers/spi/spi-bcm63xx.c
index d08bb7600150..80fa0ef8909c 100644
--- a/drivers/spi/spi-bcm63xx.c
+++ b/drivers/spi/spi-bcm63xx.c
@@ -369,7 +369,7 @@ static int bcm63xx_spi_transfer_one(struct spi_master *master,
 		}
 
 		/* CS will be deasserted directly after transfer */
-		if (t->delay_usecs || t->delay.value) {
+		if (t->delay.value) {
 			dev_err(&spi->dev, "unable to keep CS asserted after transfer\n");
 			status = -EINVAL;
 			goto exit;
-- 
2.29.2

[PATCH 03/10] spi: spi-bcm-qspi: replace 'delay_usecs' with 'delay.value' check

[Alexandru Ardelean]

The 'delay_usecs' field is going away. The replacement for it is the
'delay' field. So, we should check for 'delay.value' being non-zero.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/spi/spi-bcm-qspi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-bcm-qspi.c b/drivers/spi/spi-bcm-qspi.c
index 707fe3a5d8ef..a78e56f566dd 100644
--- a/drivers/spi/spi-bcm-qspi.c
+++ b/drivers/spi/spi-bcm-qspi.c
@@ -671,7 +671,7 @@ static int update_qspi_trans_byte_count(struct bcm_qspi *qspi,
 	if (qt->byte >= qt->trans->len) {
 		/* we're at the end of the spi_transfer */
 		/* in TX mode, need to pause for a delay or CS change */
-		if (qt->trans->delay_usecs &&
+		if (qt->trans->delay.value &&
 		    (flags & TRANS_STATUS_BREAK_DELAY))
 			ret |= TRANS_STATUS_BREAK_DELAY;
 		if (qt->trans->cs_change &&
-- 
2.29.2

[PATCH 04/10] spi: spi-sh: replace 'delay_usecs' with 'delay.value' in pr_debug

[Alexandru Ardelean]

The 'delay_usecs' field is going away. The replacement for it is the
'delay' field. So, we should print the 'delay.value' value instead.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/spi/spi-sh.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-sh.c b/drivers/spi/spi-sh.c
index 15123a8f41e1..45f304935332 100644
--- a/drivers/spi/spi-sh.c
+++ b/drivers/spi/spi-sh.c
@@ -290,8 +290,8 @@ static void spi_sh_work(struct work_struct *work)
 		list_for_each_entry(t, &mesg->transfers, transfer_list) {
 			pr_debug("tx_buf = %p, rx_buf = %p\n",
 					t->tx_buf, t->rx_buf);
-			pr_debug("len = %d, delay_usecs = %d\n",
-					t->len, t->delay_usecs);
+			pr_debug("len = %d, delay.value = %d\n",
+					t->len, t->delay.value);
 
 			if (t->tx_buf) {
 				ret = spi_sh_send(ss, mesg, t);
-- 
2.29.2

[PATCH 05/10] spi: spi-tegra20-flash: don't check 'delay_usecs' field for spi transfer

[Alexandru Ardelean]

The 'delay_usecs' field was handled for backwards compatibility in case
there were some users that still configured SPI delay transfers with
this field.

They should all be removed by now. So we can remove the 'delay_usecs'
handling in this driver.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/spi/spi-tegra20-sflash.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/spi/spi-tegra20-sflash.c b/drivers/spi/spi-tegra20-sflash.c
index cfb7de737937..2888d8a8dc6d 100644
--- a/drivers/spi/spi-tegra20-sflash.c
+++ b/drivers/spi/spi-tegra20-sflash.c
@@ -341,8 +341,7 @@ static int tegra_sflash_transfer_one_message(struct spi_master *master,
 			goto exit;
 		}
 		msg->actual_length += xfer->len;
-		if (xfer->cs_change &&
-		    (xfer->delay_usecs || xfer->delay.value)) {
+		if (xfer->cs_change && xfer->delay.value) {
 			tegra_sflash_writel(tsd, tsd->def_command_reg,
 					SPI_COMMAND);
 			spi_transfer_delay_exec(xfer);
-- 
2.29.2

[PATCH 06/10] staging: greybus: spilib: use 'spi_delay_to_ns' for getting xfer delay

[Alexandru Ardelean]

The intent is the removal of the 'delay_usecs' field from the
spi_transfer struct, as there is a 'delay' field that does the same
thing.

The spi_delay_to_ns() can be used to get the transfer delay. It works by
using the 'delay_usecs' field first (if it is non-zero), and finally
uses the 'delay' field.

Since the 'delay_usecs' field is going away, this change makes use of the
spi_delay_to_ns() function. This also means dividing the return value of
the function by 1000, to convert it to microseconds.
To prevent any potential faults when converting to microseconds and since
the result of spi_delay_to_ns() is int, the delay is being computed in 32
bits and then clamped between 0 & U16_MAX.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/staging/greybus/spilib.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/greybus/spilib.c b/drivers/staging/greybus/spilib.c
index 672d540d3365..30655153df6a 100644
--- a/drivers/staging/greybus/spilib.c
+++ b/drivers/staging/greybus/spilib.c
@@ -245,6 +245,7 @@ static struct gb_operation *gb_spi_operation_create(struct gb_spilib *spi,
 	/* Fill in the transfers array */
 	xfer = spi->first_xfer;
 	while (msg->state != GB_SPI_STATE_OP_DONE) {
+		int xfer_delay;
 		if (xfer == spi->last_xfer)
 			xfer_len = spi->last_xfer_size;
 		else
@@ -259,7 +260,9 @@ static struct gb_operation *gb_spi_operation_create(struct gb_spilib *spi,
 
 		gb_xfer->speed_hz = cpu_to_le32(xfer->speed_hz);
 		gb_xfer->len = cpu_to_le32(xfer_len);
-		gb_xfer->delay_usecs = cpu_to_le16(xfer->delay_usecs);
+		xfer_delay = spi_delay_to_ns(&xfer->delay, xfer) / 1000;
+		xfer_delay = clamp_t(u16, xfer_delay, 0, U16_MAX);
+		gb_xfer->delay_usecs = cpu_to_le16(xfer_delay);
 		gb_xfer->cs_change = xfer->cs_change;
 		gb_xfer->bits_per_word = xfer->bits_per_word;
 
-- 
2.29.2

[PATCH 07/10] spi: spi-falcon: remove check for 'delay_usecs'

[Alexandru Ardelean]

The 'delay_usecs' field is being removed from the spi_transfer struct.
This change removes it from the SPI Falcon driver.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/spi/spi-falcon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi-falcon.c b/drivers/spi/spi-falcon.c
index d3336a63f462..a7d4dffac66b 100644
--- a/drivers/spi/spi-falcon.c
+++ b/drivers/spi/spi-falcon.c
@@ -377,7 +377,7 @@ static int falcon_sflash_xfer_one(struct spi_master *master,
 
 		m->actual_length += t->len;
 
-		WARN_ON(t->delay_usecs || t->delay.value || t->cs_change);
+		WARN_ON(t->delay.value || t->cs_change);
 		spi_flags = 0;
 	}
 
-- 
2.29.2

[PATCH 08/10] spi: fsl-espi: remove usage of 'delay_usecs' field

[Alexandru Ardelean]

The 'delay_usecs' field is being removed from the spi_transfer struct.
This change removes it from the SPI FSL ESPI driver.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/spi/spi-fsl-espi.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index cf2b947c600e..f7066bef7b06 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -435,8 +435,7 @@ static int fsl_espi_trans(struct spi_message *m, struct spi_transfer *trans)
 static int fsl_espi_do_one_msg(struct spi_master *master,
 			       struct spi_message *m)
 {
-	unsigned int delay_usecs = 0, rx_nbits = 0;
-	unsigned int delay_nsecs = 0, delay_nsecs1 = 0;
+	unsigned int rx_nbits = 0, delay_nsecs = 0;
 	struct spi_transfer *t, trans = {};
 	int ret;
 
@@ -445,16 +444,10 @@ static int fsl_espi_do_one_msg(struct spi_master *master,
 		goto out;
 
 	list_for_each_entry(t, &m->transfers, transfer_list) {
-		if (t->delay_usecs) {
-			if (t->delay_usecs > delay_usecs) {
-				delay_usecs = t->delay_usecs;
-				delay_nsecs = delay_usecs * 1000;
-			}
-		} else {
-			delay_nsecs1 = spi_delay_to_ns(&t->delay, t);
-			if (delay_nsecs1 > delay_nsecs)
-				delay_nsecs = delay_nsecs1;
-		}
+		unsigned int delay = spi_delay_to_ns(&t->delay, t);
+
+		if (delay > delay_nsecs)
+			delay_nsecs = delay;
 		if (t->rx_nbits > rx_nbits)
 			rx_nbits = t->rx_nbits;
 	}
-- 
2.29.2

[PATCH 09/10] spi: core: remove 'delay_usecs' field from spi_transfer

[Alexandru Ardelean]

The 'delay' field in the spi_transfer struct is meant to replace the
'delay_usecs' field. However some cleanup was required to remove the
uses of 'delay_usecs'. Now that it's been cleaned up, we can remove it
from the kernel tree.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 drivers/spi/spi.c       |  1 -
 include/linux/spi/spi.h | 12 ------------
 2 files changed, 13 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index b08efe88ccd6..481427780162 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -3178,7 +3178,6 @@ struct spi_replaced_transfers *spi_replace_transfers(
 		/* clear cs_change and delay for all but the last */
 		if (i) {
 			xfer->cs_change = false;
-			xfer->delay_usecs = 0;
 			xfer->delay.value = 0;
 		}
 	}
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index 592897fa4f03..ea1784a43267 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -832,9 +832,6 @@ extern void spi_res_release(struct spi_controller *ctlr,
  * @delay: delay to be introduced after this transfer before
  *	(optionally) changing the chipselect status, then starting
  *	the next transfer or completing this @spi_message.
- * @delay_usecs: microseconds to delay after this transfer before
- *	(optionally) changing the chipselect status, then starting
- *	the next transfer or completing this @spi_message.
  * @word_delay: inter word delay to be introduced after each word size
  *	(set by bits_per_word) transmission.
  * @effective_speed_hz: the effective SCK-speed that was used to
@@ -946,7 +943,6 @@ struct spi_transfer {
 #define	SPI_NBITS_DUAL		0x02 /* 2bits transfer */
 #define	SPI_NBITS_QUAD		0x04 /* 4bits transfer */
 	u8		bits_per_word;
-	u16		delay_usecs;
 	struct spi_delay	delay;
 	struct spi_delay	cs_change_delay;
 	struct spi_delay	word_delay;
@@ -1060,14 +1056,6 @@ spi_transfer_del(struct spi_transfer *t)
 static inline int
 spi_transfer_delay_exec(struct spi_transfer *t)
 {
-	struct spi_delay d;
-
-	if (t->delay_usecs) {
-		d.value = t->delay_usecs;
-		d.unit = SPI_DELAY_UNIT_USECS;
-		return spi_delay_exec(&d, NULL);
-	}
-
 	return spi_delay_exec(&t->delay, t);
 }
 
-- 
2.29.2

[PATCH 10/10] spi: docs: update info about 'delay_usecs'

[Alexandru Ardelean]

The 'delay_usecs' field is no longer present on the spi_transfer struct.
This change updates the doc to mention the usage of the (relatively) new
'delay' field.

Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
---
 Documentation/spi/spi-summary.rst | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/spi/spi-summary.rst b/Documentation/spi/spi-summary.rst
index f1daffe10d78..d4239025461d 100644
--- a/Documentation/spi/spi-summary.rst
+++ b/Documentation/spi/spi-summary.rst
@@ -411,8 +411,11 @@ any more such messages.
         duplex (one pointer is NULL) transfers;
 
       + optionally defining short delays after transfers ... using
-        the spi_transfer.delay_usecs setting (this delay can be the
-        only protocol effect, if the buffer length is zero);
+        the spi_transfer.delay.value setting (this delay can be the
+        only protocol effect, if the buffer length is zero) ...
+        when specifying this delay the default spi_transfer.delay.unit
+        is microseconds, however this can be adjusted to clock cycles
+        or nanoseconds if needed;
 
       + whether the chipselect becomes inactive after a transfer and
         any delay ... by using the spi_transfer.cs_change flag;
-- 
2.29.2

Re: [PATCH 01/10] spi: spi-axi-spi-engine: remove usage of delay_usecs

[Lars-Peter Clausen]

On 3/8/21 3:54 PM, Alexandru Ardelean wrote:
> The 'delay_usecs' field was handled for backwards compatibility in case
> there were some users that still configured SPI delay transfers with
> this field.
>
> They should all be removed by now.
>
> Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
> ---
>   drivers/spi/spi-axi-spi-engine.c | 12 ++++--------
>   1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
> index af86e6d6e16b..80c3e38f5c1b 100644
> --- a/drivers/spi/spi-axi-spi-engine.c
> +++ b/drivers/spi/spi-axi-spi-engine.c
> @@ -170,14 +170,10 @@ static void spi_engine_gen_sleep(struct spi_engine_program *p, bool dry,
>   	unsigned int t;
>   	int delay;
>   
> -	if (xfer->delay_usecs) {
> -		delay = xfer->delay_usecs;
> -	} else {
> -		delay = spi_delay_to_ns(&xfer->delay, xfer);
> -		if (delay < 0)
> -			return;
> -		delay /= 1000;
> -	}
> +	delay = spi_delay_to_ns(&xfer->delay, xfer);
> +	if (delay < 0)
> +		return;

Bit of a nit, but this could be `delay <= 0` and then drop the check for 
`delay == 0` below.

> +	delay /= 1000;
>   
>   	if (delay == 0)
>   		return;

Re: [PATCH 06/10] staging: greybus: spilib: use 'spi_delay_to_ns' for getting xfer delay

[Viresh Kumar]

On 08-03-21, 16:54, Alexandru Ardelean wrote:
> The intent is the removal of the 'delay_usecs' field from the
> spi_transfer struct, as there is a 'delay' field that does the same
> thing.
> 
> The spi_delay_to_ns() can be used to get the transfer delay. It works by
> using the 'delay_usecs' field first (if it is non-zero), and finally
> uses the 'delay' field.
> 
> Since the 'delay_usecs' field is going away, this change makes use of the
> spi_delay_to_ns() function. This also means dividing the return value of
> the function by 1000, to convert it to microseconds.
> To prevent any potential faults when converting to microseconds and since
> the result of spi_delay_to_ns() is int, the delay is being computed in 32
> bits and then clamped between 0 & U16_MAX.
> 
> Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
> ---
>  drivers/staging/greybus/spilib.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/greybus/spilib.c b/drivers/staging/greybus/spilib.c
> index 672d540d3365..30655153df6a 100644
> --- a/drivers/staging/greybus/spilib.c
> +++ b/drivers/staging/greybus/spilib.c
> @@ -245,6 +245,7 @@ static struct gb_operation *gb_spi_operation_create(struct gb_spilib *spi,
>  	/* Fill in the transfers array */
>  	xfer = spi->first_xfer;
>  	while (msg->state != GB_SPI_STATE_OP_DONE) {
> +		int xfer_delay;
>  		if (xfer == spi->last_xfer)
>  			xfer_len = spi->last_xfer_size;
>  		else
> @@ -259,7 +260,9 @@ static struct gb_operation *gb_spi_operation_create(struct gb_spilib *spi,
>  
>  		gb_xfer->speed_hz = cpu_to_le32(xfer->speed_hz);
>  		gb_xfer->len = cpu_to_le32(xfer_len);
> -		gb_xfer->delay_usecs = cpu_to_le16(xfer->delay_usecs);
> +		xfer_delay = spi_delay_to_ns(&xfer->delay, xfer) / 1000;
> +		xfer_delay = clamp_t(u16, xfer_delay, 0, U16_MAX);
> +		gb_xfer->delay_usecs = cpu_to_le16(xfer_delay);
>  		gb_xfer->cs_change = xfer->cs_change;
>  		gb_xfer->bits_per_word = xfer->bits_per_word;

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH 06/10] staging: greybus: spilib: use 'spi_delay_to_ns' for getting xfer delay

[Rui Miguel Silva]

Hi,
On Tue, Mar 09, 2021 at 09:58:09AM +0530, Viresh Kumar wrote:
> On 08-03-21, 16:54, Alexandru Ardelean wrote:
> > The intent is the removal of the 'delay_usecs' field from the
> > spi_transfer struct, as there is a 'delay' field that does the same
> > thing.
> > 
> > The spi_delay_to_ns() can be used to get the transfer delay. It works by
> > using the 'delay_usecs' field first (if it is non-zero), and finally
> > uses the 'delay' field.
> > 
> > Since the 'delay_usecs' field is going away, this change makes use of the
> > spi_delay_to_ns() function. This also means dividing the return value of
> > the function by 1000, to convert it to microseconds.
> > To prevent any potential faults when converting to microseconds and since
> > the result of spi_delay_to_ns() is int, the delay is being computed in 32
> > bits and then clamped between 0 & U16_MAX.
> > 
> > Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
> > ---
> >  drivers/staging/greybus/spilib.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/greybus/spilib.c b/drivers/staging/greybus/spilib.c
> > index 672d540d3365..30655153df6a 100644
> > --- a/drivers/staging/greybus/spilib.c
> > +++ b/drivers/staging/greybus/spilib.c
> > @@ -245,6 +245,7 @@ static struct gb_operation *gb_spi_operation_create(struct gb_spilib *spi,
> >  	/* Fill in the transfers array */
> >  	xfer = spi->first_xfer;
> >  	while (msg->state != GB_SPI_STATE_OP_DONE) {
> > +		int xfer_delay;
> >  		if (xfer == spi->last_xfer)
> >  			xfer_len = spi->last_xfer_size;
> >  		else
> > @@ -259,7 +260,9 @@ static struct gb_operation *gb_spi_operation_create(struct gb_spilib *spi,
> >  
> >  		gb_xfer->speed_hz = cpu_to_le32(xfer->speed_hz);
> >  		gb_xfer->len = cpu_to_le32(xfer_len);
> > -		gb_xfer->delay_usecs = cpu_to_le16(xfer->delay_usecs);
> > +		xfer_delay = spi_delay_to_ns(&xfer->delay, xfer) / 1000;
> > +		xfer_delay = clamp_t(u16, xfer_delay, 0, U16_MAX);
> > +		gb_xfer->delay_usecs = cpu_to_le16(xfer_delay);
> >  		gb_xfer->cs_change = xfer->cs_change;
> >  		gb_xfer->bits_per_word = xfer->bits_per_word;
> 
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Acked-by: Rui Miguel Silva <rmfrfs@gmail.com>

------
Cheers,
     Rui

Re: [PATCH 06/10] staging: greybus: spilib: use 'spi_delay_to_ns' for getting xfer delay

[Greg KH]

On Mon, Mar 08, 2021 at 04:54:58PM +0200, Alexandru Ardelean wrote:
> The intent is the removal of the 'delay_usecs' field from the
> spi_transfer struct, as there is a 'delay' field that does the same
> thing.
> 
> The spi_delay_to_ns() can be used to get the transfer delay. It works by
> using the 'delay_usecs' field first (if it is non-zero), and finally
> uses the 'delay' field.
> 
> Since the 'delay_usecs' field is going away, this change makes use of the
> spi_delay_to_ns() function. This also means dividing the return value of
> the function by 1000, to convert it to microseconds.
> To prevent any potential faults when converting to microseconds and since
> the result of spi_delay_to_ns() is int, the delay is being computed in 32
> bits and then clamped between 0 & U16_MAX.
> 
> Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
> ---
>  drivers/staging/greybus/spilib.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/greybus/spilib.c b/drivers/staging/greybus/spilib.c
> index 672d540d3365..30655153df6a 100644
> --- a/drivers/staging/greybus/spilib.c
> +++ b/drivers/staging/greybus/spilib.c
> @@ -245,6 +245,7 @@ static struct gb_operation *gb_spi_operation_create(struct gb_spilib *spi,
>  	/* Fill in the transfers array */
>  	xfer = spi->first_xfer;
>  	while (msg->state != GB_SPI_STATE_OP_DONE) {
> +		int xfer_delay;
>  		if (xfer == spi->last_xfer)
>  			xfer_len = spi->last_xfer_size;
>  		else
> @@ -259,7 +260,9 @@ static struct gb_operation *gb_spi_operation_create(struct gb_spilib *spi,
>  
>  		gb_xfer->speed_hz = cpu_to_le32(xfer->speed_hz);
>  		gb_xfer->len = cpu_to_le32(xfer_len);
> -		gb_xfer->delay_usecs = cpu_to_le16(xfer->delay_usecs);
> +		xfer_delay = spi_delay_to_ns(&xfer->delay, xfer) / 1000;
> +		xfer_delay = clamp_t(u16, xfer_delay, 0, U16_MAX);
> +		gb_xfer->delay_usecs = cpu_to_le16(xfer_delay);
>  		gb_xfer->cs_change = xfer->cs_change;
>  		gb_xfer->bits_per_word = xfer->bits_per_word;
>  

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH 01/10] spi: spi-axi-spi-engine: remove usage of delay_usecs

[Alexandru Ardelean]

On Mon, 8 Mar 2021 at 18:42, Lars-Peter Clausen <lars@metafoo.de> wrote:
>
> On 3/8/21 3:54 PM, Alexandru Ardelean wrote:
> > The 'delay_usecs' field was handled for backwards compatibility in case
> > there were some users that still configured SPI delay transfers with
> > this field.
> >
> > They should all be removed by now.
> >
> > Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
> > ---
> >   drivers/spi/spi-axi-spi-engine.c | 12 ++++--------
> >   1 file changed, 4 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
> > index af86e6d6e16b..80c3e38f5c1b 100644
> > --- a/drivers/spi/spi-axi-spi-engine.c
> > +++ b/drivers/spi/spi-axi-spi-engine.c
> > @@ -170,14 +170,10 @@ static void spi_engine_gen_sleep(struct spi_engine_program *p, bool dry,
> >       unsigned int t;
> >       int delay;
> >
> > -     if (xfer->delay_usecs) {
> > -             delay = xfer->delay_usecs;
> > -     } else {
> > -             delay = spi_delay_to_ns(&xfer->delay, xfer);
> > -             if (delay < 0)
> > -                     return;
> > -             delay /= 1000;
> > -     }
> > +     delay = spi_delay_to_ns(&xfer->delay, xfer);
> > +     if (delay < 0)
> > +             return;
>
> Bit of a nit, but this could be `delay <= 0` and then drop the check for
> `delay == 0` below.

hmm, that's a bit debatable, because the `delay == 0` check comes
after `delay /= 1000` ;
to do what you're suggesting, it would probably need a
DIV_ROUND_UP(delay, 1000) to make sure that even sub-microsecond
delays don't become zero;

if you're acking this suggestion i'll implement it;
i'll wait a few more days to see if there are any other acks or
complaints on the set before sending a V2;

btw: this new spi_delay struct supports delays in microseconds,
nanoseconds and clock cycles;
at some point it may be interesting to use a
`spi_delay_to_clk_cycles()` for this driver and other similar;

>
> > +     delay /= 1000;
> >
> >       if (delay == 0)
> >               return;
>
>

Re: [PATCH 01/10] spi: spi-axi-spi-engine: remove usage of delay_usecs

[Lars-Peter Clausen]

On 3/10/21 8:16 AM, Alexandru Ardelean wrote:
> On Mon, 8 Mar 2021 at 18:42, Lars-Peter Clausen <lars@metafoo.de> wrote:
>> On 3/8/21 3:54 PM, Alexandru Ardelean wrote:
>>> The 'delay_usecs' field was handled for backwards compatibility in case
>>> there were some users that still configured SPI delay transfers with
>>> this field.
>>>
>>> They should all be removed by now.
>>>
>>> Signed-off-by: Alexandru Ardelean <aardelean@deviqon.com>
>>> ---
>>>    drivers/spi/spi-axi-spi-engine.c | 12 ++++--------
>>>    1 file changed, 4 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/spi/spi-axi-spi-engine.c b/drivers/spi/spi-axi-spi-engine.c
>>> index af86e6d6e16b..80c3e38f5c1b 100644
>>> --- a/drivers/spi/spi-axi-spi-engine.c
>>> +++ b/drivers/spi/spi-axi-spi-engine.c
>>> @@ -170,14 +170,10 @@ static void spi_engine_gen_sleep(struct spi_engine_program *p, bool dry,
>>>        unsigned int t;
>>>        int delay;
>>>
>>> -     if (xfer->delay_usecs) {
>>> -             delay = xfer->delay_usecs;
>>> -     } else {
>>> -             delay = spi_delay_to_ns(&xfer->delay, xfer);
>>> -             if (delay < 0)
>>> -                     return;
>>> -             delay /= 1000;
>>> -     }
>>> +     delay = spi_delay_to_ns(&xfer->delay, xfer);
>>> +     if (delay < 0)
>>> +             return;
>> Bit of a nit, but this could be `delay <= 0` and then drop the check for
>> `delay == 0` below.
> hmm, that's a bit debatable, because the `delay == 0` check comes
> after `delay /= 1000` ;
> to do what you're suggesting, it would probably need a
> DIV_ROUND_UP(delay, 1000) to make sure that even sub-microsecond
> delays don't become zero;

Ah, true. Lets keep the code as it is.

On the other hand you could argue that we should round up to ensure the 
delay is at least as long as requested. But that is something that 
should be changed independently from this series.

Re: [PATCH 00/10] spi: finalize 'delay_usecs' removal/transition

[Mark Brown]

On Mon, 8 Mar 2021 16:54:52 +0200, Alexandru Ardelean wrote:
> A while back I started the introduction of the 'spi_delay' data type:
>   https://lore.kernel.org/linux-spi/20190926105147.7839-1-alexandru.ardelean@analog.com/
> 
> Users of the 'delay_usecs' were removed from drivers.
> 
> Now it's time to remove the 'delay_usecs' from the SPI subsystem and use
> only the 'delay' field.
> 
> [...]

Applied to

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

Thanks!

[01/10] spi: spi-axi-spi-engine: remove usage of delay_usecs
        commit: 93c941448994a728e691f7dce9ea6475e352b09c
[02/10] spi: bcm63xx-spi: don't check 'delay_usecs' field
        commit: e7f2d4c6aacd0a2cded363bb14ef9b6e752798fd
[03/10] spi: spi-bcm-qspi: replace 'delay_usecs' with 'delay.value' check
        commit: 66a3aadec42aa001c62ae9a637398d853880a02b
[04/10] spi: spi-sh: replace 'delay_usecs' with 'delay.value' in pr_debug
        commit: 506d1a1b441e058e318d8d81141295ff76927367
[05/10] spi: spi-tegra20-flash: don't check 'delay_usecs' field for spi transfer
        commit: 7ca660f8212b2fbeb0f3133c3a6fa8805777a877
[06/10] staging: greybus: spilib: use 'spi_delay_to_ns' for getting xfer delay
        commit: 33a23423ca0a08b488791fc9d4ca53f4bea4e45b
[07/10] spi: spi-falcon: remove check for 'delay_usecs'
        commit: a886010c69718988756fd7873522caa0f26af398
[08/10] spi: fsl-espi: remove usage of 'delay_usecs' field
        commit: 55a47532fa4c5dc3291d796dd21cc80034b5d067
[09/10] spi: core: remove 'delay_usecs' field from spi_transfer
        commit: 3ab1cce553378fc0df1b1d26d7e23d03bd4dd3b6
[10/10] spi: docs: update info about 'delay_usecs'
        commit: 05d8a019eb057d14cdf9483318a7ee8b35a69cda

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