| Message ID | 1392302806-17267-1-git-send-email-iivanov@mm-sol.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Mark Brown |
| Headers | show |
On Thu, Feb 13, 2014 at 16:46 +0200, Ivan T. Ivanov wrote: > > From: "Ivan T. Ivanov" <iivanov@mm-sol.com> > > SPI transfer lenght should be a power-of-two multiple > of eight bits. Are you suggesting that an SPI transfer cannot consist of e.g. three bytes? This would be surprising, and certainly would be rather wrong an assumption. Am I missing/misunderstanding something? virtually yours Gerhard Sittig
On Thu, Feb 13, 2014 at 10:27:57PM +0100, Gerhard Sittig wrote: > On Thu, Feb 13, 2014 at 16:46 +0200, Ivan T. Ivanov wrote: > > SPI transfer lenght should be a power-of-two multiple > > of eight bits. > Are you suggesting that an SPI transfer cannot consist of e.g. > three bytes? This would be surprising, and certainly would be > rather wrong an assumption. Am I missing/misunderstanding > something? The check is supposed to be verifying that the transfer is a whole number of the current SPI word size as I understand it (not actually looked at the change yet but that's where it came from, factored out of a driver).
On Fri, 2014-02-14 at 00:26 +0000, Mark Brown wrote: > On Thu, Feb 13, 2014 at 10:27:57PM +0100, Gerhard Sittig wrote: > > On Thu, Feb 13, 2014 at 16:46 +0200, Ivan T. Ivanov wrote: > > > > SPI transfer lenght should be a power-of-two multiple > > > of eight bits. > > > Are you suggesting that an SPI transfer cannot consist of e.g. > > three bytes? This would be surprising, and certainly would be > > rather wrong an assumption. Am I missing/misunderstanding > > something? > > The check is supposed to be verifying that the transfer is a whole > number of the current SPI word size as I understand it (not actually > looked at the change yet but that's where it came from, factored out of > a driver). Yes. Comment is not correct. Is this better? "SPI transfer length should be multiple of SPI word size, where SPI word size should be power-of-two multiple" Regards, Ivan -- To unsubscribe from this list: send the line "unsubscribe linux-spi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 14, 2014 at 10:16:05AM +0200, Ivan T. Ivanov wrote: > "SPI transfer length should be multiple of SPI word size, where SPI > word size should be power-of-two multiple" Yes, that's clearer though you could in theory have a three byte word (I'm not sure that anyone would actually do that but it's possible). I do have a comment on the code as well.
On Thu, Feb 13, 2014 at 04:46:46PM +0200, Ivan T. Ivanov wrote: > + /* No partial transfers accepted */ > + if (!n_words || xfer->len & (w_size - 1)) > + return -EINVAL; Please write this using % rather than the & - it's a lot clearer what it's checking for, I had to think what the above meant. Hopefully the compiler will make it equally fast either way.
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 23756b0..474c0b0 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1619,6 +1619,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; @@ -1670,6 +1671,22 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message) return -EINVAL; } + /* + * Word size of the SPI transfer should be a power-of-two + * multiple of eight bits, + */ + 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 - 1)) + return -EINVAL; + if (xfer->speed_hz && master->min_speed_hz && xfer->speed_hz < master->min_speed_hz) return -EINVAL;