From 90808738fd242ef2533e86f2f481bebe8a7aa11b Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Wed, 13 Nov 2013 23:44:15 +0000 Subject: spi: Factor validation and initialisation of messages outside lock Currently we do a bunch of per-message validation and initialisation in __spi_async() which is called with the bus lock held. Since none of this validation depends on the current bus status there's no need to hold the lock to do it so split it out into a separate __spi_validate() function which is called prior to taking the bus lock. This could be slightly neater but keep things simple for now to show the code motion clearly. Based on observations from Martin Sperl. Signed-off-by: Mark Brown --- drivers/spi/spi.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 18cc625d887..857ee8c407b 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1596,15 +1596,11 @@ int spi_setup(struct spi_device *spi) } EXPORT_SYMBOL_GPL(spi_setup); -static int __spi_async(struct spi_device *spi, struct spi_message *message) +static int __spi_validate(struct spi_device *spi, struct spi_message *message) { struct spi_master *master = spi->master; struct spi_transfer *xfer; - message->spi = spi; - - trace_spi_message_submit(message); - if (list_empty(&message->transfers)) return -EINVAL; if (!message->complete) @@ -1705,6 +1701,18 @@ static int __spi_async(struct spi_device *spi, struct spi_message *message) } message->status = -EINPROGRESS; + + return 0; +} + +static int __spi_async(struct spi_device *spi, struct spi_message *message) +{ + struct spi_master *master = spi->master; + + message->spi = spi; + + trace_spi_message_submit(message); + return master->transfer(spi, message); } @@ -1743,6 +1751,10 @@ int spi_async(struct spi_device *spi, struct spi_message *message) int ret; unsigned long flags; + ret = __spi_validate(spi, message); + if (ret != 0) + return ret; + spin_lock_irqsave(&master->bus_lock_spinlock, flags); if (master->bus_lock_flag) @@ -1791,6 +1803,10 @@ int spi_async_locked(struct spi_device *spi, struct spi_message *message) int ret; unsigned long flags; + ret = __spi_validate(spi, message); + if (ret != 0) + return ret; + spin_lock_irqsave(&master->bus_lock_spinlock, flags); ret = __spi_async(spi, message); -- cgit v1.2.3-70-g09d2 From 0c64bc1b5eace5f48dec7305b83ee4d613d58c94 Mon Sep 17 00:00:00 2001 From: John Whitmore Date: Fri, 6 Dec 2013 13:33:50 +0000 Subject: spi: Correction to typos in Documentation/spi/spi-summary Just a few simple typo corrections. Signed-off-by: John Whitmore Signed-off-by: Mark Brown --- Documentation/spi/spi-summary | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/spi/spi-summary b/Documentation/spi/spi-summary index f21edb98341..f72e0d1e0da 100644 --- a/Documentation/spi/spi-summary +++ b/Documentation/spi/spi-summary @@ -34,7 +34,7 @@ SPI slave functions are usually not interoperable between vendors - It may also be used to stream data in either direction (half duplex), or both of them at the same time (full duplex). - - Some devices may use eight bit words. Others may different word + - Some devices may use eight bit words. Others may use different word lengths, such as streams of 12-bit or 20-bit digital samples. - Words are usually sent with their most significant bit (MSB) first, @@ -121,7 +121,7 @@ active. So the master must set the clock to inactive before selecting a slave, and the slave can tell the chosen polarity by sampling the clock level when its select line goes active. That's why many devices support for example both modes 0 and 3: they don't care about polarity, -and alway clock data in/out on rising clock edges. +and always clock data in/out on rising clock edges. How do these driver programming interfaces work? @@ -139,7 +139,7 @@ a command and then reading its response. There are two types of SPI driver, here called: - Controller drivers ... controllers may be built in to System-On-Chip + Controller drivers ... controllers may be built into System-On-Chip processors, and often support both Master and Slave roles. These drivers touch hardware registers and may use DMA. Or they can be PIO bitbangers, needing just GPIO pins. @@ -548,7 +548,7 @@ SPI MASTER METHODS DEPRECATED METHODS master->transfer(struct spi_device *spi, struct spi_message *message) - This must not sleep. Its responsibility is arrange that the + This must not sleep. Its responsibility is to arrange that the transfer happens and its complete() callback is issued. The two will normally happen later, after other transfers complete, and if the controller is idle it will need to be kickstarted. This -- cgit v1.2.3-70-g09d2 From 89c1f60746106755e29dcc3db9f22e37000891ef Mon Sep 17 00:00:00 2001 From: Trent Piepho Date: Fri, 13 Dec 2013 18:27:44 -0800 Subject: spi: Order fields in spi_device for better packing Now that spi_device->mode is a u16, the chip_select, bits_per_mode, and mode fields pack poorly, taking 8 bytes: four data and four padding. By moving (u8)bits_per_word up one position, to after (u8)chip_select, they pack better and only use 4 bytes. Signed-off-by: Trent Piepho Reviewed-by: Sourav Poddar Tested-by: Sourav Poddar g Signed-off-by: Mark Brown --- include/linux/spi/spi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 8c62ba74dd9..27a882978c1 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -75,6 +75,7 @@ struct spi_device { struct spi_master *master; u32 max_speed_hz; u8 chip_select; + u8 bits_per_word; u16 mode; #define SPI_CPHA 0x01 /* clock phase */ #define SPI_CPOL 0x02 /* clock polarity */ @@ -92,7 +93,6 @@ struct spi_device { #define SPI_TX_QUAD 0x200 /* transmit with 4 wires */ #define SPI_RX_DUAL 0x400 /* receive with 2 wires */ #define SPI_RX_QUAD 0x800 /* receive with 4 wires */ - u8 bits_per_word; int irq; void *controller_state; void *controller_data; -- cgit v1.2.3-70-g09d2 From 368ca4e0c75612c0a4d6bbcef7efb944604340c2 Mon Sep 17 00:00:00 2001 From: Trent Piepho Date: Thu, 26 Dec 2013 21:51:06 -0800 Subject: spi: Eliminate 3WIRE spi_transfer check Checking for SPI_3WIRE isn't needed. spi_setup() already prevents 3WIRE mode from being combined with DUAL or QUAD mode support. So there is no need to differentiate between a single bit device with SPI_3WIRE set and one with without. It doesn't change the allowed bit widths. Signed-off-by: Trent Piepho Signed-off-by: Mark Brown --- drivers/spi/spi.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 857ee8c407b..2cd9fdc9c7f 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1678,9 +1678,6 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message) if ((xfer->tx_nbits == SPI_NBITS_QUAD) && !(spi->mode & SPI_TX_QUAD)) return -EINVAL; - if ((spi->mode & SPI_3WIRE) && - (xfer->tx_nbits != SPI_NBITS_SINGLE)) - return -EINVAL; } /* check transfer rx_nbits */ if (xfer->rx_buf) { @@ -1694,9 +1691,6 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message) if ((xfer->rx_nbits == SPI_NBITS_QUAD) && !(spi->mode & SPI_RX_QUAD)) return -EINVAL; - if ((spi->mode & SPI_3WIRE) && - (xfer->rx_nbits != SPI_NBITS_SINGLE)) - return -EINVAL; } } -- cgit v1.2.3-70-g09d2 From 1cfd97f93e36b3f8b5d2a26147aaccae0c847a9c Mon Sep 17 00:00:00 2001 From: Axel Lin Date: Thu, 2 Jan 2014 15:16:40 +0800 Subject: spi: core: Use list_first_entry_or_null() instead of open-coded Use list_first_entry_or_null() to save a few lines. Signed-off-by: Axel Lin Signed-off-by: Mark Brown --- drivers/spi/spi.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 2cd9fdc9c7f..401cd66770f 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -791,11 +791,8 @@ struct spi_message *spi_get_next_queued_message(struct spi_master *master) /* get a pointer to the next message, if any */ spin_lock_irqsave(&master->queue_lock, flags); - if (list_empty(&master->queue)) - next = NULL; - else - next = list_entry(master->queue.next, - struct spi_message, queue); + next = list_first_entry_or_null(&master->queue, struct spi_message, + queue); spin_unlock_irqrestore(&master->queue_lock, flags); return next; -- cgit v1.2.3-70-g09d2 From a89e2d2754246645959f1a2f91e37e9b367bfd36 Mon Sep 17 00:00:00 2001 From: Axel Lin Date: Thu, 9 Jan 2014 16:03:58 +0800 Subject: spi: core: Use list_first_entry to extract head of queue For slightly better readability. Signed-off-by: Axel Lin Signed-off-by: Mark Brown --- drivers/spi/spi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 401cd66770f..36bfa7f820a 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -685,7 +685,7 @@ static void spi_pump_messages(struct kthread_work *work) } /* Extract head of queue */ master->cur_msg = - list_entry(master->queue.next, struct spi_message, queue); + list_first_entry(&master->queue, struct spi_message, queue); list_del_init(&master->cur_msg->queue); if (master->busy) -- cgit v1.2.3-70-g09d2 From d3fbd457013c0f6d96de461ce0a9cb1f8b73cd0b Mon Sep 17 00:00:00 2001 From: Mark Brown Date: Fri, 10 Jan 2014 17:09:53 +0000 Subject: spi: Use bitfields for multiple data lines Trent Piepho observed that since the current realistic maximum number of data lines is four we can pack the spi_transfer struct more efficiently if we use a bitfield for the number of bits, allowing the fields to fit in a single byte along with cs_change. If space becomes an issue further optimiation is possible by only using the constants and packing the values chosen for them. Reported-by: Trent Piepho Signed-off-by: Mark Brown --- include/linux/spi/spi.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 27a882978c1..77b529e418d 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -576,8 +576,8 @@ struct spi_transfer { dma_addr_t rx_dma; unsigned cs_change:1; - u8 tx_nbits; - u8 rx_nbits; + unsigned tx_nbits:3; + unsigned rx_nbits:3; #define SPI_NBITS_SINGLE 0x01 /* 1bit transfer */ #define SPI_NBITS_DUAL 0x02 /* 2bits transfer */ #define SPI_NBITS_QUAD 0x04 /* 4bits transfer */ -- cgit v1.2.3-70-g09d2 From 269ccca8ec1a3991e07eaf5f90326134bd17781a Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Sun, 12 Jan 2014 13:59:06 +0100 Subject: spi: Kill superfluous cast in spi_w8r16() spi_write_then_read() takes a "void *" for rxbuf, so there's no need to cast the buffer pointer to "u8 *". Signed-off-by: Geert Uytterhoeven Signed-off-by: Mark Brown --- include/linux/spi/spi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 77b529e418d..21a7251d85e 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -847,7 +847,7 @@ static inline ssize_t spi_w8r16(struct spi_device *spi, u8 cmd) ssize_t status; u16 result; - status = spi_write_then_read(spi, &cmd, 1, (u8 *) &result, 2); + status = spi_write_then_read(spi, &cmd, 1, &result, 2); /* return negative errno or unsigned value */ return (status < 0) ? status : result; -- cgit v1.2.3-70-g09d2 From 1afd9989a6a2561183be82e420d4d2f3889b7ee7 Mon Sep 17 00:00:00 2001 From: Geert Uytterhoeven Date: Sun, 12 Jan 2014 14:00:29 +0100 Subject: spi: core: Improve tx/rx_nbits check comments - Rephrase the comments about tx/rx_nbits validity checks, - Remove the stale comment about SPI_3WIRE (the code it refers to was removed in commit 368ca4e0c75612c0a4d6bbcef7efb944604340c2 ("spi: Eliminate 3WIRE spi_transfer check")). Signed-off-by: Geert Uytterhoeven Signed-off-by: Mark Brown --- drivers/spi/spi.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 36bfa7f820a..9f26797e431 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1660,9 +1660,8 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message) if (xfer->rx_buf && !xfer->rx_nbits) xfer->rx_nbits = SPI_NBITS_SINGLE; /* check transfer tx/rx_nbits: - * 1. keep the value is not out of single, dual and quad - * 2. keep tx/rx_nbits is contained by mode in spi_device - * 3. if SPI_3WIRE, tx/rx_nbits should be in single + * 1. check the value matches one of single, dual and quad + * 2. check tx/rx_nbits match the mode in spi_device */ if (xfer->tx_buf) { if (xfer->tx_nbits != SPI_NBITS_SINGLE && -- cgit v1.2.3-70-g09d2