[PATCH v3] spi: core: Validate length of the transfers in message

[PATCH v3] spi: core: Validate length of the transfers in message

[Ivan T. Ivanov]

From: "Ivan T. Ivanov" <iivanov@mm-sol.com>

SPI transfer length should be multiple of SPI word size,
where SPI word size should be power-of-two multiple

Signed-off-by: Ivan T. Ivanov <iivanov@mm-sol.com>
---
 drivers/spi/spi.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index d0b28bb..45b1610 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1617,6 +1617,7 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message)
 {
 	struct spi_master *master = spi->master;
 	struct spi_transfer *xfer;
+	int w_size, n_words;
 
 	if (list_empty(&message->transfers))
 		return -EINVAL;
@@ -1668,6 +1669,22 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message)
 				return -EINVAL;
 		}
 
+		/*
+		 * SPI transfer length should be multiple of SPI word size
+		 * where SPI word size should be power-of-two multiple
+		 */
+		if (xfer->bits_per_word <= 8)
+			w_size = 1;
+		else if (xfer->bits_per_word <= 16)
+			w_size = 2;
+		else
+			w_size = 4;
+
+		n_words = xfer->len / w_size;
+		/* No partial transfers accepted */
+		if (!n_words || xfer->len % w_size)
+			return -EINVAL;
+
 		if (xfer->speed_hz && master->min_speed_hz &&
 		    xfer->speed_hz < master->min_speed_hz)
 			return -EINVAL;
-- 
1.7.9.5

[PATCH v3] spi: core: Validate length of the transfers in message

[Ivan T. Ivanov]

From: "Ivan T. Ivanov" <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>

SPI transfer length should be multiple of SPI word size,
where SPI word size should be power-of-two multiple

Signed-off-by: Ivan T. Ivanov <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
---
 drivers/spi/spi.c |   17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index d0b28bb..45b1610 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1617,6 +1617,7 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message)
 {
 	struct spi_master *master = spi->master;
 	struct spi_transfer *xfer;
+	int w_size, n_words;
 
 	if (list_empty(&message->transfers))
 		return -EINVAL;
@@ -1668,6 +1669,22 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message)
 				return -EINVAL;
 		}
 
+		/*
+		 * SPI transfer length should be multiple of SPI word size
+		 * where SPI word size should be power-of-two multiple
+		 */
+		if (xfer->bits_per_word <= 8)
+			w_size = 1;
+		else if (xfer->bits_per_word <= 16)
+			w_size = 2;
+		else
+			w_size = 4;
+
+		n_words = xfer->len / w_size;
+		/* No partial transfers accepted */
+		if (!n_words || xfer->len % w_size)
+			return -EINVAL;
+
 		if (xfer->speed_hz && master->min_speed_hz &&
 		    xfer->speed_hz < master->min_speed_hz)
 			return -EINVAL;
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] spi: core: Validate length of the transfers in message

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 395 bytes --]

On Thu, Feb 20, 2014 at 12:02:08PM +0200, Ivan T. Ivanov wrote:
> From: "Ivan T. Ivanov" <iivanov@mm-sol.com>
> 
> SPI transfer length should be multiple of SPI word size,
> where SPI word size should be power-of-two multiple

OK, I checked all the existing users and everything using unusual bits
per word settings seems to be doing the right thing here so I've applied
this - thanks!

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3] spi: core: Validate length of the transfers in message

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 424 bytes --]

On Thu, Feb 20, 2014 at 12:02:08PM +0200, Ivan T. Ivanov wrote:
> From: "Ivan T. Ivanov" <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org>
> 
> SPI transfer length should be multiple of SPI word size,
> where SPI word size should be power-of-two multiple

OK, I checked all the existing users and everything using unusual bits
per word settings seems to be doing the right thing here so I've applied
this - thanks!

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3] spi: core: Validate length of the transfers in message

[Atsushi Nemoto]

On Thu, 20 Feb 2014 12:02:08 +0200, "Ivan T. Ivanov" <iivanov@mm-sol.com> wrote:
> SPI transfer length should be multiple of SPI word size,
> where SPI word size should be power-of-two multiple
...
> +		n_words = xfer->len / w_size;
> +		/* No partial transfers accepted */
> +		if (!n_words || xfer->len % w_size)
> +			return -EINVAL;

Is xfer->len == 0 invalid?
Long time ago I have fixed atmel spi driver to support zero length
transfer (commit 06719814 atmel_spi: support zero length transfer).

According to Documentation/spi/spi-summary, zeto length transfer seems
valid.

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

---
Atsushi Nemoto

Re: [PATCH v3] spi: core: Validate length of the transfers in message

[Atsushi Nemoto]

On Thu, 20 Feb 2014 12:02:08 +0200, "Ivan T. Ivanov" <iivanov-NEYub+7Iv8PQT0dZR+AlfA@public.gmane.org> wrote:
> SPI transfer length should be multiple of SPI word size,
> where SPI word size should be power-of-two multiple
...
> +		n_words = xfer->len / w_size;
> +		/* No partial transfers accepted */
> +		if (!n_words || xfer->len % w_size)
> +			return -EINVAL;

Is xfer->len == 0 invalid?
Long time ago I have fixed atmel spi driver to support zero length
transfer (commit 06719814 atmel_spi: support zero length transfer).

According to Documentation/spi/spi-summary, zeto length transfer seems
valid.

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

---
Atsushi Nemoto
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] spi: core: Validate length of the transfers in message

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]

On Tue, Feb 25, 2014 at 10:55:28PM +0900, Atsushi Nemoto wrote:

> Is xfer->len == 0 invalid?
> Long time ago I have fixed atmel spi driver to support zero length
> transfer (commit 06719814 atmel_spi: support zero length transfer).

> According to Documentation/spi/spi-summary, zeto length transfer seems
> valid.

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

*sigh*  I guess it is valid, though frankly I'm concerned that this
isn't a good idea - it's certainly not going to work reliably given the
need for every driver to open code this, most of them get the delay
stuff wrong.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3] spi: core: Validate length of the transfers in message

[Atsushi Nemoto]

On Wed, 26 Feb 2014 09:14:07 +0900, Mark Brown <broonie@kernel.org> wrote:
>>       + 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);
> 
> *sigh*  I guess it is valid, though frankly I'm concerned that this
> isn't a good idea - it's certainly not going to work reliably given the
> need for every driver to open code this, most of them get the delay
> stuff wrong.

I don't object to the whole patch.  Validating in spi core is good of
course, and "xfer->len % w_size" part looks no problem.

I just want to keep ways to handle an odd device, for example, which
requires long delay between chipselect and the first transfer, etc.

---
Atsushi Nemoto

Re: [PATCH v3] spi: core: Validate length of the transfers in message

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 394 bytes --]

On Thu, Feb 27, 2014 at 10:38:26PM +0900, Atsushi Nemoto wrote:

> I don't object to the whole patch.  Validating in spi core is good of
> course, and "xfer->len % w_size" part looks no problem.

> I just want to keep ways to handle an odd device, for example, which
> requires long delay between chipselect and the first transfer, etc.

Can you submit a patch adding that support back please?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3] spi: core: Validate length of the transfers in message

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 394 bytes --]

On Thu, Feb 27, 2014 at 10:38:26PM +0900, Atsushi Nemoto wrote:

> I don't object to the whole patch.  Validating in spi core is good of
> course, and "xfer->len % w_size" part looks no problem.

> I just want to keep ways to handle an odd device, for example, which
> requires long delay between chipselect and the first transfer, etc.

Can you submit a patch adding that support back please?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v3] spi: core: Validate length of the transfers in message

[Atsushi Nemoto]

On Fri, 28 Feb 2014 15:19:20 +0900, Mark Brown <broonie@kernel.org> wrote:
>> I don't object to the whole patch.  Validating in spi core is good of
>> course, and "xfer->len % w_size" part looks no problem.
> 
>> I just want to keep ways to handle an odd device, for example, which
>> requires long delay between chipselect and the first transfer, etc.
> 
> Can you submit a patch adding that support back please?

OK, I will send a patch against spi.git/for-next branch.

---
Atsushi Nemoto

Re: [PATCH v3] spi: core: Validate length of the transfers in message

[Atsushi Nemoto]

On Fri, 28 Feb 2014 15:19:20 +0900, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> I don't object to the whole patch.  Validating in spi core is good of
>> course, and "xfer->len % w_size" part looks no problem.
> 
>> I just want to keep ways to handle an odd device, for example, which
>> requires long delay between chipselect and the first transfer, etc.
> 
> Can you submit a patch adding that support back please?

OK, I will send a patch against spi.git/for-next branch.

---
Atsushi Nemoto
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html