2014-02-20 10:03:34

by Ivan T. Ivanov

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

From: "Ivan T. Ivanov" <[email protected]>

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 <[email protected]>
---
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


2014-02-22 23:37:03

by Mark Brown

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

On Thu, Feb 20, 2014 at 12:02:08PM +0200, Ivan T. Ivanov wrote:
> From: "Ivan T. Ivanov" <[email protected]>
>
> 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!


Attachments:
(No filename) (386.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2014-02-25 14:16:41

by Atsushi Nemoto

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

On Thu, 20 Feb 2014 12:02:08 +0200, "Ivan T. Ivanov" <[email protected]> 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

2014-02-26 00:32:43

by Mark Brown

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

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.


Attachments:
(No filename) (741.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2014-02-27 13:38:41

by Atsushi Nemoto

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

On Wed, 26 Feb 2014 09:14:07 +0900, Mark Brown <[email protected]> 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

2014-02-28 06:19:34

by Mark Brown

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

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?


Attachments:
(No filename) (394.00 B)
signature.asc (836.00 B)
Digital signature
Download all attachments

2014-02-28 13:55:47

by Atsushi Nemoto

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

On Fri, 28 Feb 2014 15:19:20 +0900, Mark Brown <[email protected]> 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