From 6df534cc7136fc9e023cbd4e0011a04e3659e74d Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Fri, 5 Jan 2024 11:32:50 +0100 Subject: spi: make spi_bus_type const Now that the driver core can properly handle constant struct bus_type, move the spi_bus_type variable to be a constant structure as well, placing it into read-only memory which can not be modified at runtime. Cc: Mark Brown Cc: Signed-off-by: Greg Kroah-Hartman Link: https://msgid.link/r/2024010549-erasure-swoop-1cc6@gregkh Signed-off-by: Mark Brown --- drivers/spi/spi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 7477a11e12be..bc3d7c0ce6b4 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -459,7 +459,7 @@ static void spi_shutdown(struct device *dev) } } -struct bus_type spi_bus_type = { +const struct bus_type spi_bus_type = { .name = "spi", .dev_groups = spi_dev_groups, .match = spi_match_device, -- cgit v1.2.3 From b204aa0f99cfe3c9d796ecfc0bc6f3f89585789e Mon Sep 17 00:00:00 2001 From: David Lechner Date: Tue, 23 Jan 2024 15:49:46 -0600 Subject: spi: consolidate setting message->spi Previously, __spi_sync() and __spi_async() set message->spi to the spi device independently after calling __spi_validate(). __spi_validate() also would conditionally set this if it needed to split the message since it wasn't set yet. Since both __spi_sync() and __spi_async() call __spi_validate(), we can consolidate this into only setting message->spi once (unconditionally) in __spi_validate(). This will also save any future callers of __spi_validate() from also needing to set message->spi. Signed-off-by: David Lechner Link: https://msgid.link/r/20240123214946.2616786-1-dlechner@baylibre.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index bc3d7c0ce6b4..7a70ef47cdf6 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -4059,6 +4059,8 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message) if (list_empty(&message->transfers)) return -EINVAL; + message->spi = spi; + /* * If an SPI controller does not support toggling the CS line on each * transfer (indicated by the SPI_CS_WORD flag) or we are using a GPIO @@ -4071,9 +4073,6 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message) size_t maxsize = BITS_TO_BYTES(spi->bits_per_word); int ret; - /* spi_split_transfers_maxsize() requires message->spi */ - message->spi = spi; - ret = spi_split_transfers_maxsize(ctlr, message, maxsize, GFP_KERNEL); if (ret) @@ -4210,8 +4209,6 @@ static int __spi_async(struct spi_device *spi, struct spi_message *message) if (!ctlr->transfer) return -ENOTSUPP; - message->spi = spi; - SPI_STATISTICS_INCREMENT_FIELD(ctlr->pcpu_statistics, spi_async); SPI_STATISTICS_INCREMENT_FIELD(spi->pcpu_statistics, spi_async); @@ -4391,8 +4388,6 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message) if (status != 0) return status; - message->spi = spi; - SPI_STATISTICS_INCREMENT_FIELD(ctlr->pcpu_statistics, spi_sync); SPI_STATISTICS_INCREMENT_FIELD(spi->pcpu_statistics, spi_sync); -- cgit v1.2.3 From 0da9a5794cfda615668eaefde811e8ef378134fe Mon Sep 17 00:00:00 2001 From: David Lechner Date: Thu, 25 Jan 2024 17:47:31 -0600 Subject: spi: avoid double validation in __spi_sync() The __spi_sync() function calls __spi_validate() early in the function. Later, it can call spi_async_locked() which calls __spi_validate() again. __spi_validate() is an expensive function, so we can improve performance measurably by avoiding calling it twice. Instead of calling spi_async_locked(), we can call __spi_async() with the spin lock held. spi_async_locked() is removed since there are no more callers. Signed-off-by: David Lechner Link: https://msgid.link/r/20240125234732.3530278-2-dlechner@baylibre.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 58 ++++++------------------------------------------------- 1 file changed, 6 insertions(+), 52 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 7a70ef47cdf6..6610aeced765 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -4278,57 +4278,6 @@ int spi_async(struct spi_device *spi, struct spi_message *message) } EXPORT_SYMBOL_GPL(spi_async); -/** - * spi_async_locked - version of spi_async with exclusive bus usage - * @spi: device with which data will be exchanged - * @message: describes the data transfers, including completion callback - * Context: any (IRQs may be blocked, etc) - * - * This call may be used in_irq and other contexts which can't sleep, - * as well as from task contexts which can sleep. - * - * The completion callback is invoked in a context which can't sleep. - * Before that invocation, the value of message->status is undefined. - * When the callback is issued, message->status holds either zero (to - * indicate complete success) or a negative error code. After that - * callback returns, the driver which issued the transfer request may - * deallocate the associated memory; it's no longer in use by any SPI - * core or controller driver code. - * - * Note that although all messages to a spi_device are handled in - * FIFO order, messages may go to different devices in other orders. - * Some device might be higher priority, or have various "hard" access - * time requirements, for example. - * - * On detection of any fault during the transfer, processing of - * the entire message is aborted, and the device is deselected. - * Until returning from the associated message completion callback, - * no other spi_message queued to that device will be processed. - * (This rule applies equally to all the synchronous transfer calls, - * which are wrappers around this core asynchronous primitive.) - * - * Return: zero on success, else a negative error code. - */ -static int spi_async_locked(struct spi_device *spi, struct spi_message *message) -{ - struct spi_controller *ctlr = spi->controller; - int ret; - unsigned long flags; - - ret = __spi_validate(spi, message); - if (ret != 0) - return ret; - - spin_lock_irqsave(&ctlr->bus_lock_spinlock, flags); - - ret = __spi_async(spi, message); - - spin_unlock_irqrestore(&ctlr->bus_lock_spinlock, flags); - - return ret; - -} - static void __spi_transfer_message_noqueue(struct spi_controller *ctlr, struct spi_message *msg) { bool was_busy; @@ -4376,6 +4325,7 @@ static void spi_complete(void *arg) static int __spi_sync(struct spi_device *spi, struct spi_message *message) { DECLARE_COMPLETION_ONSTACK(done); + unsigned long flags; int status; struct spi_controller *ctlr = spi->controller; @@ -4419,7 +4369,11 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message) */ message->complete = spi_complete; message->context = &done; - status = spi_async_locked(spi, message); + + spin_lock_irqsave(&ctlr->bus_lock_spinlock, flags); + status = __spi_async(spi, message); + spin_unlock_irqrestore(&ctlr->bus_lock_spinlock, flags); + if (status == 0) { wait_for_completion(&done); status = message->status; -- cgit v1.2.3 From c8bec3355f08ddb887d5c13b7095dfa79e6db108 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Fri, 26 Jan 2024 15:23:57 -0600 Subject: spi: move split xfers for CS_WORD emulation This moves splitting transfers for CS_WORD software emulation to the same place where we split transfers for controller-specific reasons. This fixes a few subtle bugs. The calculation for maxsize was wrong for bit sizes between 17 and 24. This is fixed by making use of spi_split_transfers_maxwords() which already has the correct calculation. Also, since this indirectly calls spi_res_alloc(), to avoid leaking resources, spi_finalize_current_message() would need to be called on all error paths in __spi_validate() and callers of __spi_validate() would need to do the same. This is fixed by moving the call to __spi_pump_transfer_message() where it is already splitting transfers for other reasons and correctly releases resources in the subsequent error paths. Fixes: cbaa62e0094a ("spi: add software implementation for SPI_CS_WORD") Signed-off-by: David Lechner Link: https://lore.kernel.org/r/20240126212358.3916280-2-dlechner@baylibre.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 63 +++++++++++++++++++++++++++---------------------------- 1 file changed, 31 insertions(+), 32 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 6610aeced765..517558e01559 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1743,13 +1743,37 @@ static int __spi_pump_transfer_message(struct spi_controller *ctlr, trace_spi_message_start(msg); - ret = spi_split_transfers_maxsize(ctlr, msg, - spi_max_transfer_size(msg->spi), - GFP_KERNEL | GFP_DMA); - if (ret) { - msg->status = ret; - spi_finalize_current_message(ctlr); - return ret; + /* + * If an SPI controller does not support toggling the CS line on each + * transfer (indicated by the SPI_CS_WORD flag) or we are using a GPIO + * for the CS line, we can emulate the CS-per-word hardware function by + * splitting transfers into one-word transfers and ensuring that + * cs_change is set for each transfer. + */ + if ((msg->spi->mode & SPI_CS_WORD) && (!(ctlr->mode_bits & SPI_CS_WORD) || + spi_is_csgpiod(msg->spi))) { + ret = spi_split_transfers_maxwords(ctlr, msg, 1, GFP_KERNEL); + if (ret) { + msg->status = ret; + spi_finalize_current_message(ctlr); + return ret; + } + + list_for_each_entry(xfer, &msg->transfers, transfer_list) { + /* Don't change cs_change on the last entry in the list */ + if (list_is_last(&xfer->transfer_list, &msg->transfers)) + break; + xfer->cs_change = 1; + } + } else { + ret = spi_split_transfers_maxsize(ctlr, msg, + spi_max_transfer_size(msg->spi), + GFP_KERNEL | GFP_DMA); + if (ret) { + msg->status = ret; + spi_finalize_current_message(ctlr); + return ret; + } } if (ctlr->prepare_message) { @@ -4061,31 +4085,6 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message) message->spi = spi; - /* - * If an SPI controller does not support toggling the CS line on each - * transfer (indicated by the SPI_CS_WORD flag) or we are using a GPIO - * for the CS line, we can emulate the CS-per-word hardware function by - * splitting transfers into one-word transfers and ensuring that - * cs_change is set for each transfer. - */ - if ((spi->mode & SPI_CS_WORD) && (!(ctlr->mode_bits & SPI_CS_WORD) || - spi_is_csgpiod(spi))) { - size_t maxsize = BITS_TO_BYTES(spi->bits_per_word); - int ret; - - ret = spi_split_transfers_maxsize(ctlr, message, maxsize, - GFP_KERNEL); - if (ret) - return ret; - - list_for_each_entry(xfer, &message->transfers, transfer_list) { - /* Don't change cs_change on the last entry in the list */ - if (list_is_last(&xfer->transfer_list, &message->transfers)) - break; - xfer->cs_change = 1; - } - } - /* * Half-duplex links include original MicroWire, and ones with * only one data pin like SPI_3WIRE (switches direction) or where -- cgit v1.2.3 From c0c0293cf7a0f21ef461956d44e4add718574f3f Mon Sep 17 00:00:00 2001 From: David Lechner Date: Tue, 6 Feb 2024 14:06:46 -0600 Subject: spi: drop gpf arg from __spi_split_transfer_maxsize() The __spi_split_transfer_maxsize() function has a gpf argument to allow callers to specify the type of memory allocation that needs to be used. However, this function only allocates struct spi_transfer and is not intended to be used from atomic contexts so this type should always be GFP_KERNEL, so we can just drop the argument. Some callers of these functions also passed GFP_DMA, but since only struct spi_transfer is allocated and not any tx/rx buffers, this is not actually necessary and is removed in this commit. Signed-off-by: David Lechner Link: https://lore.kernel.org/r/20240206200648.1782234-1-dlechner@baylibre.com Signed-off-by: Mark Brown --- drivers/spi/spi-stm32.c | 4 +--- drivers/spi/spi.c | 22 ++++++++-------------- include/linux/spi/spi.h | 6 ++---- 3 files changed, 11 insertions(+), 21 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c index e61302ef3c21..c32e57bb38bd 100644 --- a/drivers/spi/spi-stm32.c +++ b/drivers/spi/spi-stm32.c @@ -1170,9 +1170,7 @@ static int stm32_spi_prepare_msg(struct spi_controller *ctrl, if (spi->cfg->set_number_of_data) { int ret; - ret = spi_split_transfers_maxwords(ctrl, msg, - spi->t_size_max, - GFP_KERNEL | GFP_DMA); + ret = spi_split_transfers_maxwords(ctrl, msg, spi->t_size_max); if (ret) return ret; } diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 517558e01559..3270a1337a5b 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1752,7 +1752,7 @@ static int __spi_pump_transfer_message(struct spi_controller *ctlr, */ if ((msg->spi->mode & SPI_CS_WORD) && (!(ctlr->mode_bits & SPI_CS_WORD) || spi_is_csgpiod(msg->spi))) { - ret = spi_split_transfers_maxwords(ctlr, msg, 1, GFP_KERNEL); + ret = spi_split_transfers_maxwords(ctlr, msg, 1); if (ret) { msg->status = ret; spi_finalize_current_message(ctlr); @@ -1767,8 +1767,7 @@ static int __spi_pump_transfer_message(struct spi_controller *ctlr, } } else { ret = spi_split_transfers_maxsize(ctlr, msg, - spi_max_transfer_size(msg->spi), - GFP_KERNEL | GFP_DMA); + spi_max_transfer_size(msg->spi)); if (ret) { msg->status = ret; spi_finalize_current_message(ctlr); @@ -3707,8 +3706,7 @@ static struct spi_replaced_transfers *spi_replace_transfers( static int __spi_split_transfer_maxsize(struct spi_controller *ctlr, struct spi_message *msg, struct spi_transfer **xferp, - size_t maxsize, - gfp_t gfp) + size_t maxsize) { struct spi_transfer *xfer = *xferp, *xfers; struct spi_replaced_transfers *srt; @@ -3719,7 +3717,7 @@ static int __spi_split_transfer_maxsize(struct spi_controller *ctlr, count = DIV_ROUND_UP(xfer->len, maxsize); /* Create replacement */ - srt = spi_replace_transfers(msg, xfer, 1, count, NULL, 0, gfp); + srt = spi_replace_transfers(msg, xfer, 1, count, NULL, 0, GFP_KERNEL); if (IS_ERR(srt)) return PTR_ERR(srt); xfers = srt->inserted_transfers; @@ -3779,14 +3777,12 @@ static int __spi_split_transfer_maxsize(struct spi_controller *ctlr, * @ctlr: the @spi_controller for this transfer * @msg: the @spi_message to transform * @maxsize: the maximum when to apply this - * @gfp: GFP allocation flags * * Return: status of transformation */ int spi_split_transfers_maxsize(struct spi_controller *ctlr, struct spi_message *msg, - size_t maxsize, - gfp_t gfp) + size_t maxsize) { struct spi_transfer *xfer; int ret; @@ -3801,7 +3797,7 @@ int spi_split_transfers_maxsize(struct spi_controller *ctlr, list_for_each_entry(xfer, &msg->transfers, transfer_list) { if (xfer->len > maxsize) { ret = __spi_split_transfer_maxsize(ctlr, msg, &xfer, - maxsize, gfp); + maxsize); if (ret) return ret; } @@ -3819,14 +3815,12 @@ EXPORT_SYMBOL_GPL(spi_split_transfers_maxsize); * @ctlr: the @spi_controller for this transfer * @msg: the @spi_message to transform * @maxwords: the number of words to limit each transfer to - * @gfp: GFP allocation flags * * Return: status of transformation */ int spi_split_transfers_maxwords(struct spi_controller *ctlr, struct spi_message *msg, - size_t maxwords, - gfp_t gfp) + size_t maxwords) { struct spi_transfer *xfer; @@ -3844,7 +3838,7 @@ int spi_split_transfers_maxwords(struct spi_controller *ctlr, maxsize = maxwords * roundup_pow_of_two(BITS_TO_BYTES(xfer->bits_per_word)); if (xfer->len > maxsize) { ret = __spi_split_transfer_maxsize(ctlr, msg, &xfer, - maxsize, gfp); + maxsize); if (ret) return ret; } diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 29c3e4dd5d93..9339c8ed1f8f 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -1365,12 +1365,10 @@ struct spi_replaced_transfers { extern int spi_split_transfers_maxsize(struct spi_controller *ctlr, struct spi_message *msg, - size_t maxsize, - gfp_t gfp); + size_t maxsize); extern int spi_split_transfers_maxwords(struct spi_controller *ctlr, struct spi_message *msg, - size_t maxwords, - gfp_t gfp); + size_t maxwords); /*---------------------------------------------------------------------------*/ -- cgit v1.2.3 From 620d269f29a569ba37419cc03cf1da2d55f6252a Mon Sep 17 00:00:00 2001 From: Uwe Kleine-König Date: Wed, 7 Feb 2024 19:40:45 +0100 Subject: spi: Drop compat layer from renaming "master" to "controller" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that all in-tree users followed the rename, the compat stuff can go away. This completes the renaming started with commit 8caab75fd2c2 ("spi: Generalize SPI "master" to "controller"") Acked-by: Jonathan Cameron Signed-off-by: Uwe Kleine-König Link: https://lore.kernel.org/r/ad1d949325b61a4682e8d6ecf9d05da751e6a99f.1707324794.git.u.kleine-koenig@pengutronix.de Signed-off-by: Mark Brown --- Documentation/driver-api/driver-model/devres.rst | 2 +- drivers/spi/spi.c | 2 +- include/linux/spi/spi.h | 20 +------------------- 3 files changed, 3 insertions(+), 21 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst index c5f99d834ec5..49c6fd82cd5d 100644 --- a/Documentation/driver-api/driver-model/devres.rst +++ b/Documentation/driver-api/driver-model/devres.rst @@ -462,7 +462,7 @@ SLAVE DMA ENGINE SPI devm_spi_alloc_master() devm_spi_alloc_slave() - devm_spi_register_master() + devm_spi_register_controller() WATCHDOG devm_watchdog_register_device() diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index f2170f4b5077..4b3e7a624de2 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -584,7 +584,7 @@ struct spi_device *spi_alloc_device(struct spi_controller *ctlr) return NULL; } - spi->master = spi->controller = ctlr; + spi->controller = ctlr; spi->dev.parent = &ctlr->dev; spi->dev.bus = &spi_bus_type; spi->dev.release = spidev_release; diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 600fbd5daf68..30ada46b51cd 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -131,7 +131,6 @@ extern void spi_transfer_cs_change_delay_exec(struct spi_message *msg, * struct spi_device - Controller side proxy for an SPI slave device * @dev: Driver model representation of the device. * @controller: SPI controller used with the device. - * @master: Copy of controller, for backwards compatibility. * @max_speed_hz: Maximum clock rate to be used with this chip * (on this board); may be changed by the device's driver. * The spi_transfer.speed_hz can override this for each transfer. @@ -185,7 +184,6 @@ extern void spi_transfer_cs_change_delay_exec(struct spi_message *msg, struct spi_device { struct device dev; struct spi_controller *controller; - struct spi_controller *master; /* Compatibility layer */ u32 max_speed_hz; u8 chip_select[SPI_CS_CNT_MAX]; u8 bits_per_word; @@ -1298,7 +1296,7 @@ spi_max_transfer_size(struct spi_device *spi) */ static inline bool spi_is_bpw_supported(struct spi_device *spi, u32 bpw) { - u32 bpw_mask = spi->master->bits_per_word_mask; + u32 bpw_mask = spi->controller->bits_per_word_mask; if (bpw == 8 || (bpw <= 32 && bpw_mask & SPI_BPW_MASK(bpw))) return true; @@ -1670,20 +1668,4 @@ spi_transfer_is_last(struct spi_controller *ctlr, struct spi_transfer *xfer) return list_is_last(&xfer->transfer_list, &ctlr->cur_msg->transfers); } -/* Compatibility layer */ -#define spi_master spi_controller - -#define spi_master_get_devdata(_ctlr) spi_controller_get_devdata(_ctlr) -#define spi_master_set_devdata(_ctlr, _data) \ - spi_controller_set_devdata(_ctlr, _data) -#define spi_master_get(_ctlr) spi_controller_get(_ctlr) -#define spi_master_put(_ctlr) spi_controller_put(_ctlr) -#define spi_master_suspend(_ctlr) spi_controller_suspend(_ctlr) -#define spi_master_resume(_ctlr) spi_controller_resume(_ctlr) - -#define spi_register_master(_ctlr) spi_register_controller(_ctlr) -#define devm_spi_register_master(_dev, _ctlr) \ - devm_spi_register_controller(_dev, _ctlr) -#define spi_unregister_master(_ctlr) spi_unregister_controller(_ctlr) - #endif /* __LINUX_SPI_H */ -- cgit v1.2.3 From 7b1d87af14d9ae902ed0c5dc5fabf4eea5abdf02 Mon Sep 17 00:00:00 2001 From: David Lechner Date: Mon, 19 Feb 2024 16:33:18 -0600 Subject: spi: add spi_optimize_message() APIs This adds a new spi_optimize_message() function that can be used to optimize SPI messages that are used more than once. Peripheral drivers that use the same message multiple times can use this API to perform SPI message validation and controller-specific optimizations once and then reuse the message while avoiding the overhead of revalidating the message on each spi_(a)sync() call. Internally, the SPI core will also call this function for each message if the peripheral driver did not explicitly call it. This is done to so that controller drivers don't have to have multiple code paths for optimized and non-optimized messages. A hook is provided for controller drivers to perform controller-specific optimizations. Suggested-by: Martin Sperl Link: https://lore.kernel.org/linux-spi/39DEC004-10A1-47EF-9D77-276188D2580C@martin.sperl.org/ Signed-off-by: David Lechner Link: https://msgid.link/r/20240219-mainline-spi-precook-message-v2-1-4a762c6701b9@baylibre.com Reviewed-by: Jonathan Cameron Signed-off-by: Mark Brown --- drivers/spi/spi.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++-- include/linux/spi/spi.h | 20 +++++++ 2 files changed, 167 insertions(+), 4 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index c2b10e2c75f0..f68d92b57543 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -2106,6 +2106,41 @@ struct spi_message *spi_get_next_queued_message(struct spi_controller *ctlr) } EXPORT_SYMBOL_GPL(spi_get_next_queued_message); +/* + * __spi_unoptimize_message - shared implementation of spi_unoptimize_message() + * and spi_maybe_unoptimize_message() + * @msg: the message to unoptimize + * + * Peripheral drivers should use spi_unoptimize_message() and callers inside + * core should use spi_maybe_unoptimize_message() rather than calling this + * function directly. + * + * It is not valid to call this on a message that is not currently optimized. + */ +static void __spi_unoptimize_message(struct spi_message *msg) +{ + struct spi_controller *ctlr = msg->spi->controller; + + if (ctlr->unoptimize_message) + ctlr->unoptimize_message(msg); + + msg->optimized = false; + msg->opt_state = NULL; +} + +/* + * spi_maybe_unoptimize_message - unoptimize msg not managed by a peripheral + * @msg: the message to unoptimize + * + * This function is used to unoptimize a message if and only if it was + * optimized by the core (via spi_maybe_optimize_message()). + */ +static void spi_maybe_unoptimize_message(struct spi_message *msg) +{ + if (!msg->pre_optimized && msg->optimized) + __spi_unoptimize_message(msg); +} + /** * spi_finalize_current_message() - the current message is complete * @ctlr: the controller to return the message to @@ -2153,6 +2188,8 @@ void spi_finalize_current_message(struct spi_controller *ctlr) mesg->prepared = false; + spi_maybe_unoptimize_message(mesg); + WRITE_ONCE(ctlr->cur_msg_incomplete, false); smp_mb(); /* See __spi_pump_transfer_message()... */ if (READ_ONCE(ctlr->cur_msg_need_completion)) @@ -4194,6 +4231,110 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message) return 0; } +/* + * __spi_optimize_message - shared implementation for spi_optimize_message() + * and spi_maybe_optimize_message() + * @spi: the device that will be used for the message + * @msg: the message to optimize + * + * Peripheral drivers will call spi_optimize_message() and the spi core will + * call spi_maybe_optimize_message() instead of calling this directly. + * + * It is not valid to call this on a message that has already been optimized. + * + * Return: zero on success, else a negative error code + */ +static int __spi_optimize_message(struct spi_device *spi, + struct spi_message *msg) +{ + struct spi_controller *ctlr = spi->controller; + int ret; + + ret = __spi_validate(spi, msg); + if (ret) + return ret; + + if (ctlr->optimize_message) { + ret = ctlr->optimize_message(msg); + if (ret) + return ret; + } + + msg->optimized = true; + + return 0; +} + +/* + * spi_maybe_optimize_message - optimize message if it isn't already pre-optimized + * @spi: the device that will be used for the message + * @msg: the message to optimize + * Return: zero on success, else a negative error code + */ +static int spi_maybe_optimize_message(struct spi_device *spi, + struct spi_message *msg) +{ + if (msg->pre_optimized) + return 0; + + return __spi_optimize_message(spi, msg); +} + +/** + * spi_optimize_message - do any one-time validation and setup for a SPI message + * @spi: the device that will be used for the message + * @msg: the message to optimize + * + * Peripheral drivers that reuse the same message repeatedly may call this to + * perform as much message prep as possible once, rather than repeating it each + * time a message transfer is performed to improve throughput and reduce CPU + * usage. + * + * Once a message has been optimized, it cannot be modified with the exception + * of updating the contents of any xfer->tx_buf (the pointer can't be changed, + * only the data in the memory it points to). + * + * Calls to this function must be balanced with calls to spi_unoptimize_message() + * to avoid leaking resources. + * + * Context: can sleep + * Return: zero on success, else a negative error code + */ +int spi_optimize_message(struct spi_device *spi, struct spi_message *msg) +{ + int ret; + + ret = __spi_optimize_message(spi, msg); + if (ret) + return ret; + + /* + * This flag indicates that the peripheral driver called spi_optimize_message() + * and therefore we shouldn't unoptimize message automatically when finalizing + * the message but rather wait until spi_unoptimize_message() is called + * by the peripheral driver. + */ + msg->pre_optimized = true; + + return 0; +} +EXPORT_SYMBOL_GPL(spi_optimize_message); + +/** + * spi_unoptimize_message - releases any resources allocated by spi_optimize_message() + * @msg: the message to unoptimize + * + * Calls to this function must be balanced with calls to spi_optimize_message(). + * + * Context: can sleep + */ +void spi_unoptimize_message(struct spi_message *msg) +{ + __spi_unoptimize_message(msg); + msg->pre_optimized = false; +} +EXPORT_SYMBOL_GPL(spi_unoptimize_message); + static int __spi_async(struct spi_device *spi, struct spi_message *message) { struct spi_controller *ctlr = spi->controller; @@ -4258,8 +4399,8 @@ int spi_async(struct spi_device *spi, struct spi_message *message) int ret; unsigned long flags; - ret = __spi_validate(spi, message); - if (ret != 0) + ret = spi_maybe_optimize_message(spi, message); + if (ret) return ret; spin_lock_irqsave(&ctlr->bus_lock_spinlock, flags); @@ -4271,6 +4412,8 @@ int spi_async(struct spi_device *spi, struct spi_message *message) spin_unlock_irqrestore(&ctlr->bus_lock_spinlock, flags); + spi_maybe_unoptimize_message(message); + return ret; } EXPORT_SYMBOL_GPL(spi_async); @@ -4331,8 +4474,8 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message) return -ESHUTDOWN; } - status = __spi_validate(spi, message); - if (status != 0) + status = spi_maybe_optimize_message(spi, message); + if (status) return status; SPI_STATISTICS_INCREMENT_FIELD(ctlr->pcpu_statistics, spi_sync); diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h index 2b8e2746769a..ddfb66dd4caf 100644 --- a/include/linux/spi/spi.h +++ b/include/linux/spi/spi.h @@ -475,6 +475,8 @@ extern struct spi_device *spi_new_ancillary_device(struct spi_device *spi, u8 ch * * @set_cs: set the logic level of the chip select line. May be called * from interrupt context. + * @optimize_message: optimize the message for reuse + * @unoptimize_message: release resources allocated by optimize_message * @prepare_message: set up the controller to transfer a single message, * for example doing DMA mapping. Called from threaded * context. @@ -715,6 +717,8 @@ struct spi_controller { struct completion xfer_completion; size_t max_dma_len; + int (*optimize_message)(struct spi_message *msg); + int (*unoptimize_message)(struct spi_message *msg); int (*prepare_transfer_hardware)(struct spi_controller *ctlr); int (*transfer_one_message)(struct spi_controller *ctlr, struct spi_message *mesg); @@ -1111,6 +1115,8 @@ struct spi_transfer { * @spi: SPI device to which the transaction is queued * @is_dma_mapped: if true, the caller provided both DMA and CPU virtual * addresses for each transfer buffer + * @pre_optimized: peripheral driver pre-optimized the message + * @optimized: the message is in the optimized state * @prepared: spi_prepare_message was called for the this message * @status: zero for success, else negative errno * @complete: called to report transaction completions @@ -1120,6 +1126,7 @@ struct spi_transfer { * successful segments * @queue: for use by whichever driver currently owns the message * @state: for use by whichever driver currently owns the message + * @opt_state: for use by whichever driver currently owns the message * @resources: for resource management when the SPI message is processed * * A @spi_message is used to execute an atomic sequence of data transfers, @@ -1143,6 +1150,11 @@ struct spi_message { unsigned is_dma_mapped:1; + /* spi_optimize_message() was called for this message */ + bool pre_optimized; + /* __spi_optimize_message() was called for this message */ + bool optimized; + /* spi_prepare_message() was called for this message */ bool prepared; @@ -1172,6 +1184,11 @@ struct spi_message { */ struct list_head queue; void *state; + /* + * Optional state for use by controller driver between calls to + * __spi_optimize_message() and __spi_unoptimize_message(). + */ + void *opt_state; /* List of spi_res resources when the SPI message is processed */ struct list_head resources; @@ -1255,6 +1272,9 @@ static inline void spi_message_free(struct spi_message *m) kfree(m); } +extern int spi_optimize_message(struct spi_device *spi, struct spi_message *msg); +extern void spi_unoptimize_message(struct spi_message *msg); + extern int spi_setup(struct spi_device *spi); extern int spi_async(struct spi_device *spi, struct spi_message *message); extern int spi_slave_abort(struct spi_device *spi); -- cgit v1.2.3 From fab53fea21a909e4e0656764a8ee7c356fe89d6f Mon Sep 17 00:00:00 2001 From: David Lechner Date: Mon, 19 Feb 2024 16:33:19 -0600 Subject: spi: move splitting transfers to spi_optimize_message() Splitting transfers is an expensive operation so we can potentially optimize it by doing it only once per optimization of the message instead of repeating each time the message is transferred. The transfer splitting functions are currently the only user of spi_res_alloc() so spi_res_release() can be safely moved at this time from spi_finalize_current_message() to spi_unoptimize_message(). The doc comments of the public functions for splitting transfers are also updated so that callers will know when it is safe to call them to ensure proper resource management. Reviewed-by: Jonathan Cameron Signed-off-by: David Lechner Link: https://msgid.link/r/20240219-mainline-spi-precook-message-v2-2-4a762c6701b9@baylibre.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 110 +++++++++++++++++++++++++++++++++--------------------- 1 file changed, 68 insertions(+), 42 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index f68d92b57543..ba4d3fde2054 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1747,38 +1747,6 @@ static int __spi_pump_transfer_message(struct spi_controller *ctlr, trace_spi_message_start(msg); - /* - * If an SPI controller does not support toggling the CS line on each - * transfer (indicated by the SPI_CS_WORD flag) or we are using a GPIO - * for the CS line, we can emulate the CS-per-word hardware function by - * splitting transfers into one-word transfers and ensuring that - * cs_change is set for each transfer. - */ - if ((msg->spi->mode & SPI_CS_WORD) && (!(ctlr->mode_bits & SPI_CS_WORD) || - spi_is_csgpiod(msg->spi))) { - ret = spi_split_transfers_maxwords(ctlr, msg, 1); - if (ret) { - msg->status = ret; - spi_finalize_current_message(ctlr); - return ret; - } - - list_for_each_entry(xfer, &msg->transfers, transfer_list) { - /* Don't change cs_change on the last entry in the list */ - if (list_is_last(&xfer->transfer_list, &msg->transfers)) - break; - xfer->cs_change = 1; - } - } else { - ret = spi_split_transfers_maxsize(ctlr, msg, - spi_max_transfer_size(msg->spi)); - if (ret) { - msg->status = ret; - spi_finalize_current_message(ctlr); - return ret; - } - } - if (ctlr->prepare_message) { ret = ctlr->prepare_message(ctlr, msg); if (ret) { @@ -2124,6 +2092,8 @@ static void __spi_unoptimize_message(struct spi_message *msg) if (ctlr->unoptimize_message) ctlr->unoptimize_message(msg); + spi_res_release(ctlr, msg); + msg->optimized = false; msg->opt_state = NULL; } @@ -2169,15 +2139,6 @@ void spi_finalize_current_message(struct spi_controller *ctlr) spi_unmap_msg(ctlr, mesg); - /* - * In the prepare_messages callback the SPI bus has the opportunity - * to split a transfer to smaller chunks. - * - * Release the split transfers here since spi_map_msg() is done on - * the split transfers. - */ - spi_res_release(ctlr, mesg); - if (mesg->prepared && ctlr->unprepare_message) { ret = ctlr->unprepare_message(ctlr, mesg); if (ret) { @@ -3819,6 +3780,10 @@ static int __spi_split_transfer_maxsize(struct spi_controller *ctlr, * @msg: the @spi_message to transform * @maxsize: the maximum when to apply this * + * This function allocates resources that are automatically freed during the + * spi message unoptimize phase so this function should only be called from + * optimize_message callbacks. + * * Return: status of transformation */ int spi_split_transfers_maxsize(struct spi_controller *ctlr, @@ -3857,6 +3822,10 @@ EXPORT_SYMBOL_GPL(spi_split_transfers_maxsize); * @msg: the @spi_message to transform * @maxwords: the number of words to limit each transfer to * + * This function allocates resources that are automatically freed during the + * spi message unoptimize phase so this function should only be called from + * optimize_message callbacks. + * * Return: status of transformation */ int spi_split_transfers_maxwords(struct spi_controller *ctlr, @@ -4231,6 +4200,57 @@ static int __spi_validate(struct spi_device *spi, struct spi_message *message) return 0; } +/* + * spi_split_transfers - generic handling of transfer splitting + * @msg: the message to split + * + * Under certain conditions, a SPI controller may not support arbitrary + * transfer sizes or other features required by a peripheral. This function + * will split the transfers in the message into smaller transfers that are + * supported by the controller. + * + * Controllers with special requirements not covered here can also split + * transfers in the optimize_message() callback. + * + * Context: can sleep + * Return: zero on success, else a negative error code + */ +static int spi_split_transfers(struct spi_message *msg) +{ + struct spi_controller *ctlr = msg->spi->controller; + struct spi_transfer *xfer; + int ret; + + /* + * If an SPI controller does not support toggling the CS line on each + * transfer (indicated by the SPI_CS_WORD flag) or we are using a GPIO + * for the CS line, we can emulate the CS-per-word hardware function by + * splitting transfers into one-word transfers and ensuring that + * cs_change is set for each transfer. + */ + if ((msg->spi->mode & SPI_CS_WORD) && + (!(ctlr->mode_bits & SPI_CS_WORD) || spi_is_csgpiod(msg->spi))) { + ret = spi_split_transfers_maxwords(ctlr, msg, 1); + if (ret) + return ret; + + list_for_each_entry(xfer, &msg->transfers, transfer_list) { + /* Don't change cs_change on the last entry in the list */ + if (list_is_last(&xfer->transfer_list, &msg->transfers)) + break; + + xfer->cs_change = 1; + } + } else { + ret = spi_split_transfers_maxsize(ctlr, msg, + spi_max_transfer_size(msg->spi)); + if (ret) + return ret; + } + + return 0; +} + /* * __spi_optimize_message - shared implementation for spi_optimize_message() * and spi_maybe_optimize_message() @@ -4254,10 +4274,16 @@ static int __spi_optimize_message(struct spi_device *spi, if (ret) return ret; + ret = spi_split_transfers(msg); + if (ret) + return ret; + if (ctlr->optimize_message) { ret = ctlr->optimize_message(msg); - if (ret) + if (ret) { + spi_res_release(ctlr, msg); return ret; + } } msg->optimized = true; -- cgit v1.2.3 From 5ee91605ad9ad363766a7ed13dc7d47f5102982a Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 6 Mar 2024 17:59:40 +0200 Subject: spi: Exctract spi_set_all_cs_unused() helper It seems a few functions implement the similar for-loop to mark all chip select pins unused. Let's deduplicate that code in order to have a single place of that for better maintenance. Signed-off-by: Andy Shevchenko Link: https://msgid.link/r/20240306160114.3471398-2-andriy.shevchenko@linux.intel.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 74 ++++++++++++++++++------------------------------------- 1 file changed, 24 insertions(+), 50 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index ba4d3fde2054..5e530fa790b0 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -745,6 +745,23 @@ int spi_add_device(struct spi_device *spi) } EXPORT_SYMBOL_GPL(spi_add_device); +static void spi_set_all_cs_unused(struct spi_device *spi) +{ + u8 idx; + + /* + * Zero(0) is a valid physical CS value and can be located at any + * logical CS in the spi->chip_select[]. If all the physical CS + * are initialized to 0 then It would be difficult to differentiate + * between a valid physical CS 0 & an unused logical CS whose physical + * CS can be 0. As a solution to this issue initialize all the CS to 0xFF. + * Now all the unused logical CS will have 0xFF physical CS value & can be + * ignore while performing physical CS validity checks. + */ + for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) + spi_set_chipselect(spi, idx, 0xFF); +} + /** * spi_new_device - instantiate one new SPI device * @ctlr: Controller to which device is connected @@ -764,7 +781,6 @@ struct spi_device *spi_new_device(struct spi_controller *ctlr, { struct spi_device *proxy; int status; - u8 idx; /* * NOTE: caller did any chip->bus_num checks necessary. @@ -780,19 +796,10 @@ struct spi_device *spi_new_device(struct spi_controller *ctlr, WARN_ON(strlen(chip->modalias) >= sizeof(proxy->modalias)); - /* - * Zero(0) is a valid physical CS value and can be located at any - * logical CS in the spi->chip_select[]. If all the physical CS - * are initialized to 0 then It would be difficult to differentiate - * between a valid physical CS 0 & an unused logical CS whose physical - * CS can be 0. As a solution to this issue initialize all the CS to 0xFF. - * Now all the unused logical CS will have 0xFF physical CS value & can be - * ignore while performing physical CS validity checks. - */ - for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) - spi_set_chipselect(proxy, idx, 0xFF); - + /* Use provided chip-select for proxy device */ + spi_set_all_cs_unused(proxy); spi_set_chipselect(proxy, 0, chip->chip_select); + proxy->max_speed_hz = chip->max_speed_hz; proxy->mode = chip->mode; proxy->irq = chip->irq; @@ -2418,17 +2425,7 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi, return -EINVAL; } - /* - * Zero(0) is a valid physical CS value and can be located at any - * logical CS in the spi->chip_select[]. If all the physical CS - * are initialized to 0 then It would be difficult to differentiate - * between a valid physical CS 0 & an unused logical CS whose physical - * CS can be 0. As a solution to this issue initialize all the CS to 0xFF. - * Now all the unused logical CS will have 0xFF physical CS value & can be - * ignore while performing physical CS validity checks. - */ - for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) - spi_set_chipselect(spi, idx, 0xFF); + spi_set_all_cs_unused(spi); /* Device address */ rc = of_property_read_variable_u32_array(nc, "reg", &cs[0], 1, @@ -2565,7 +2562,6 @@ struct spi_device *spi_new_ancillary_device(struct spi_device *spi, struct spi_controller *ctlr = spi->controller; struct spi_device *ancillary; int rc = 0; - u8 idx; /* Alloc an spi_device */ ancillary = spi_alloc_device(ctlr); @@ -2576,19 +2572,8 @@ struct spi_device *spi_new_ancillary_device(struct spi_device *spi, strscpy(ancillary->modalias, "dummy", sizeof(ancillary->modalias)); - /* - * Zero(0) is a valid physical CS value and can be located at any - * logical CS in the spi->chip_select[]. If all the physical CS - * are initialized to 0 then It would be difficult to differentiate - * between a valid physical CS 0 & an unused logical CS whose physical - * CS can be 0. As a solution to this issue initialize all the CS to 0xFF. - * Now all the unused logical CS will have 0xFF physical CS value & can be - * ignore while performing physical CS validity checks. - */ - for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) - spi_set_chipselect(ancillary, idx, 0xFF); - /* Use provided chip-select for ancillary device */ + spi_set_all_cs_unused(ancillary); spi_set_chipselect(ancillary, 0, chip_select); /* Take over SPI mode/speed from SPI main device */ @@ -2805,7 +2790,6 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr, struct acpi_spi_lookup lookup = {}; struct spi_device *spi; int ret; - u8 idx; if (!ctlr && index == -1) return ERR_PTR(-EINVAL); @@ -2841,24 +2825,14 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr, return ERR_PTR(-ENOMEM); } - /* - * Zero(0) is a valid physical CS value and can be located at any - * logical CS in the spi->chip_select[]. If all the physical CS - * are initialized to 0 then It would be difficult to differentiate - * between a valid physical CS 0 & an unused logical CS whose physical - * CS can be 0. As a solution to this issue initialize all the CS to 0xFF. - * Now all the unused logical CS will have 0xFF physical CS value & can be - * ignore while performing physical CS validity checks. - */ - for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) - spi_set_chipselect(spi, idx, 0xFF); + spi_set_all_cs_unused(spi); + spi_set_chipselect(spi, 0, lookup.chip_select); ACPI_COMPANION_SET(&spi->dev, adev); spi->max_speed_hz = lookup.max_speed_hz; spi->mode |= lookup.mode; spi->irq = lookup.irq; spi->bits_per_word = lookup.bits_per_word; - spi_set_chipselect(spi, 0, lookup.chip_select); /* * spi->chip_select[i] gives the corresponding physical CS for logical CS i * logical CS number is represented by setting the ith bit in spi->cs_index_mask -- cgit v1.2.3 From 9086d0f23b7c292f162a828967975e29e97c0680 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 6 Mar 2024 17:59:41 +0200 Subject: spi: Exctract spi_dev_check_cs() helper It seems a few functions implement the similar for-loop to validate chip select pins for uniqueness. Let's deduplicate that code in order to have a single place of that for better maintenance. Signed-off-by: Andy Shevchenko Link: https://msgid.link/r/20240306160114.3471398-3-andriy.shevchenko@linux.intel.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 47 +++++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 20 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 5e530fa790b0..eb7e3ddf909b 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -608,23 +608,35 @@ static void spi_dev_set_name(struct spi_device *spi) spi_get_chipselect(spi, 0)); } +static inline int spi_dev_check_cs(struct device *dev, + struct spi_device *spi, u8 idx, + struct spi_device *new_spi, u8 new_idx) +{ + u8 cs, cs_new; + u8 idx_new; + + cs = spi_get_chipselect(spi, idx); + for (idx_new = new_idx; idx_new < SPI_CS_CNT_MAX; idx_new++) { + cs_new = spi_get_chipselect(new_spi, idx_new); + if (cs != 0xFF && cs_new != 0xFF && cs == cs_new) { + dev_err(dev, "chipselect %u already in use\n", cs_new); + return -EBUSY; + } + } + return 0; +} + static int spi_dev_check(struct device *dev, void *data) { struct spi_device *spi = to_spi_device(dev); struct spi_device *new_spi = data; - int idx, nw_idx; - u8 cs, cs_nw; + int status, idx; if (spi->controller == new_spi->controller) { for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) { - cs = spi_get_chipselect(spi, idx); - for (nw_idx = 0; nw_idx < SPI_CS_CNT_MAX; nw_idx++) { - cs_nw = spi_get_chipselect(new_spi, nw_idx); - if (cs != 0xFF && cs_nw != 0xFF && cs == cs_nw) { - dev_err(dev, "chipselect %d already in use\n", cs_nw); - return -EBUSY; - } - } + status = spi_dev_check_cs(dev, spi, idx, new_spi, 0); + if (status) + return status; } } return 0; @@ -640,8 +652,8 @@ static int __spi_add_device(struct spi_device *spi) { struct spi_controller *ctlr = spi->controller; struct device *dev = ctlr->dev.parent; - int status, idx, nw_idx; - u8 cs, nw_cs; + int status, idx; + u8 cs; for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) { /* Chipselects are numbered 0..max; validate. */ @@ -658,14 +670,9 @@ static int __spi_add_device(struct spi_device *spi) * For example, spi->chip_select[0] != spi->chip_select[1] and so on. */ for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) { - cs = spi_get_chipselect(spi, idx); - for (nw_idx = idx + 1; nw_idx < SPI_CS_CNT_MAX; nw_idx++) { - nw_cs = spi_get_chipselect(spi, nw_idx); - if (cs != 0xFF && nw_cs != 0xFF && cs == nw_cs) { - dev_err(dev, "chipselect %d already in use\n", nw_cs); - return -EBUSY; - } - } + status = spi_dev_check_cs(dev, spi, idx, spi, idx + 1); + if (status) + return status; } /* Set the bus ID string */ -- cgit v1.2.3 From 1209c5566f9b244bd047478b7fc90318c9a310f0 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 7 Mar 2024 17:00:59 +0200 Subject: spi: Consistently use BIT for cs_index_mask Some of the parts related to the chip select are using BIT() macro the rest are using plain numbers. Unify all of them to use BIT(). While at it, make the (repetitive) comment clearer when assigning cs_index_mask during SPI target device enumeration. Signed-off-by: Andy Shevchenko Link: https://msgid.link/r/20240307150256.3789138-2-andriy.shevchenko@linux.intel.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 35 +++++++++++------------------------ 1 file changed, 11 insertions(+), 24 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index eb7e3ddf909b..4ab155f698c8 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1021,7 +1021,7 @@ static inline bool spi_is_last_cs(struct spi_device *spi) bool last = false; for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) { - if ((spi->cs_index_mask >> idx) & 0x01) { + if (spi->cs_index_mask & BIT(idx)) { if (spi->controller->last_cs[idx] == spi_get_chipselect(spi, idx)) last = true; } @@ -1072,8 +1072,7 @@ static void spi_set_cs(struct spi_device *spi, bool enable, bool force) * into account. */ for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) { - if (((spi->cs_index_mask >> idx) & 0x01) && - spi_get_csgpiod(spi, idx)) { + if ((spi->cs_index_mask & BIT(idx)) && spi_get_csgpiod(spi, idx)) { if (has_acpi_companion(&spi->dev)) gpiod_set_value_cansleep(spi_get_csgpiod(spi, idx), !enable); @@ -2456,14 +2455,10 @@ static int of_spi_parse_dt(struct spi_controller *ctlr, struct spi_device *spi, spi_set_chipselect(spi, idx, cs[idx]); /* - * spi->chip_select[i] gives the corresponding physical CS for logical CS i - * logical CS number is represented by setting the ith bit in spi->cs_index_mask - * So, for example, if spi->cs_index_mask = 0x01 then logical CS number is 0 and - * spi->chip_select[0] will give the physical CS. - * By default spi->chip_select[0] will hold the physical CS number so, set - * spi->cs_index_mask as 0x01. + * By default spi->chip_select[0] will hold the physical CS number, + * so set bit 0 in spi->cs_index_mask. */ - spi->cs_index_mask = 0x01; + spi->cs_index_mask = BIT(0); /* Device speed */ if (!of_property_read_u32(nc, "spi-max-frequency", &value)) @@ -2587,14 +2582,10 @@ struct spi_device *spi_new_ancillary_device(struct spi_device *spi, ancillary->max_speed_hz = spi->max_speed_hz; ancillary->mode = spi->mode; /* - * spi->chip_select[i] gives the corresponding physical CS for logical CS i - * logical CS number is represented by setting the ith bit in spi->cs_index_mask - * So, for example, if spi->cs_index_mask = 0x01 then logical CS number is 0 and - * spi->chip_select[0] will give the physical CS. - * By default spi->chip_select[0] will hold the physical CS number so, set - * spi->cs_index_mask as 0x01. + * By default spi->chip_select[0] will hold the physical CS number, + * so set bit 0 in spi->cs_index_mask. */ - ancillary->cs_index_mask = 0x01; + ancillary->cs_index_mask = BIT(0); WARN_ON(!mutex_is_locked(&ctlr->add_lock)); @@ -2841,14 +2832,10 @@ struct spi_device *acpi_spi_device_alloc(struct spi_controller *ctlr, spi->irq = lookup.irq; spi->bits_per_word = lookup.bits_per_word; /* - * spi->chip_select[i] gives the corresponding physical CS for logical CS i - * logical CS number is represented by setting the ith bit in spi->cs_index_mask - * So, for example, if spi->cs_index_mask = 0x01 then logical CS number is 0 and - * spi->chip_select[0] will give the physical CS. - * By default spi->chip_select[0] will hold the physical CS number so, set - * spi->cs_index_mask as 0x01. + * By default spi->chip_select[0] will hold the physical CS number, + * so set bit 0 in spi->cs_index_mask. */ - spi->cs_index_mask = 0x01; + spi->cs_index_mask = BIT(0); return spi; } -- cgit v1.2.3 From be84be4a35fa99cca7e81e6dd21516a324cca413 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 7 Mar 2024 17:01:01 +0200 Subject: spi: Introduce SPI_INVALID_CS and is_valid_cs() The SPI core inconsistently uses the marker value for unused chip select pin. Define a constant (with appropriate type) and introduce is_valid_cs() helper function to avoid spreading this inconsistency in the future. Signed-off-by: Andy Shevchenko Link: https://msgid.link/r/20240307150256.3789138-4-andriy.shevchenko@linux.intel.com Signed-off-by: Mark Brown --- drivers/spi/spi.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 4ab155f698c8..f18738ae95f8 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -608,6 +608,22 @@ static void spi_dev_set_name(struct spi_device *spi) spi_get_chipselect(spi, 0)); } +/* + * Zero(0) is a valid physical CS value and can be located at any + * logical CS in the spi->chip_select[]. If all the physical CS + * are initialized to 0 then It would be difficult to differentiate + * between a valid physical CS 0 & an unused logical CS whose physical + * CS can be 0. As a solution to this issue initialize all the CS to -1. + * Now all the unused logical CS will have -1 physical CS value & can be + * ignored while performing physical CS validity checks. + */ +#define SPI_INVALID_CS ((s8)-1) + +static inline bool is_valid_cs(s8 chip_select) +{ + return chip_select != SPI_INVALID_CS; +} + static inline int spi_dev_check_cs(struct device *dev, struct spi_device *spi, u8 idx, struct spi_device *new_spi, u8 new_idx) @@ -618,7 +634,7 @@ static inline int spi_dev_check_cs(struct device *dev, cs = spi_get_chipselect(spi, idx); for (idx_new = new_idx; idx_new < SPI_CS_CNT_MAX; idx_new++) { cs_new = spi_get_chipselect(new_spi, idx_new); - if (cs != 0xFF && cs_new != 0xFF && cs == cs_new) { + if (is_valid_cs(cs) && is_valid_cs(cs_new) && cs == cs_new) { dev_err(dev, "chipselect %u already in use\n", cs_new); return -EBUSY; } @@ -658,7 +674,7 @@ static int __spi_add_device(struct spi_device *spi) for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) { /* Chipselects are numbered 0..max; validate. */ cs = spi_get_chipselect(spi, idx); - if (cs != 0xFF && cs >= ctlr->num_chipselect) { + if (is_valid_cs(cs) && cs >= ctlr->num_chipselect) { dev_err(dev, "cs%d >= max %d\n", spi_get_chipselect(spi, idx), ctlr->num_chipselect); return -EINVAL; @@ -698,7 +714,7 @@ static int __spi_add_device(struct spi_device *spi) for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) { cs = spi_get_chipselect(spi, idx); - if (cs != 0xFF) + if (is_valid_cs(cs)) spi_set_csgpiod(spi, idx, ctlr->cs_gpiods[cs]); } } @@ -756,17 +772,8 @@ static void spi_set_all_cs_unused(struct spi_device *spi) { u8 idx; - /* - * Zero(0) is a valid physical CS value and can be located at any - * logical CS in the spi->chip_select[]. If all the physical CS - * are initialized to 0 then It would be difficult to differentiate - * between a valid physical CS 0 & an unused logical CS whose physical - * CS can be 0. As a solution to this issue initialize all the CS to 0xFF. - * Now all the unused logical CS will have 0xFF physical CS value & can be - * ignore while performing physical CS validity checks. - */ for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) - spi_set_chipselect(spi, idx, 0xFF); + spi_set_chipselect(spi, idx, SPI_INVALID_CS); } /** @@ -1050,7 +1057,7 @@ static void spi_set_cs(struct spi_device *spi, bool enable, bool force) spi->controller->last_cs_index_mask = spi->cs_index_mask; for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) - spi->controller->last_cs[idx] = enable ? spi_get_chipselect(spi, 0) : -1; + spi->controller->last_cs[idx] = enable ? spi_get_chipselect(spi, 0) : SPI_INVALID_CS; spi->controller->last_cs_mode_high = spi->mode & SPI_CS_HIGH; if (spi->mode & SPI_CS_HIGH) @@ -3333,9 +3340,9 @@ int spi_register_controller(struct spi_controller *ctlr) goto free_bus_id; } - /* Setting last_cs to -1 means no chip selected */ + /* Setting last_cs to SPI_INVALID_CS means no chip selected */ for (idx = 0; idx < SPI_CS_CNT_MAX; idx++) - ctlr->last_cs[idx] = -1; + ctlr->last_cs[idx] = SPI_INVALID_CS; status = device_add(&ctlr->dev); if (status < 0) -- cgit v1.2.3 From aa0162dc0dd95c3bf248e3c78068760094e8f64b Mon Sep 17 00:00:00 2001 From: Janne Grunau Date: Mon, 11 Mar 2024 23:53:17 +0100 Subject: spi: Restore delays for non-GPIO chip select SPI controller with integrated chip select handling still need to adhere to SPI device's CS setup, hold and inactive delays. For controller without set_cs_timing spi core shall handle the delays to avoid duplicated delay handling in each controller driver. Fixes a regression for the out of tree SPI controller and SPI HID transport on Apple M1/M1 Pro/Max notebooks. Fixes: 4d8ff6b0991d ("spi: Add multi-cs memories support in SPI core") Signed-off-by: Janne Grunau Link: https://msgid.link/r/20240311-spi-cs-delays-regression-v1-1-0075020a90b2@jannau.net Signed-off-by: Mark Brown --- drivers/spi/spi.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) (limited to 'drivers/spi/spi.c') diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index f18738ae95f8..ff75838c1b5d 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -1063,10 +1063,14 @@ static void spi_set_cs(struct spi_device *spi, bool enable, bool force) if (spi->mode & SPI_CS_HIGH) enable = !enable; - if (spi_is_csgpiod(spi)) { - if (!spi->controller->set_cs_timing && !activate) - spi_delay_exec(&spi->cs_hold, NULL); + /* + * Handle chip select delays for GPIO based CS or controllers without + * programmable chip select timing. + */ + if ((spi_is_csgpiod(spi) || !spi->controller->set_cs_timing) && !activate) + spi_delay_exec(&spi->cs_hold, NULL); + if (spi_is_csgpiod(spi)) { if (!(spi->mode & SPI_NO_CS)) { /* * Historically ACPI has no means of the GPIO polarity and @@ -1099,16 +1103,16 @@ static void spi_set_cs(struct spi_device *spi, bool enable, bool force) if ((spi->controller->flags & SPI_CONTROLLER_GPIO_SS) && spi->controller->set_cs) spi->controller->set_cs(spi, !enable); - - if (!spi->controller->set_cs_timing) { - if (activate) - spi_delay_exec(&spi->cs_setup, NULL); - else - spi_delay_exec(&spi->cs_inactive, NULL); - } } else if (spi->controller->set_cs) { spi->controller->set_cs(spi, !enable); } + + if (spi_is_csgpiod(spi) || !spi->controller->set_cs_timing) { + if (activate) + spi_delay_exec(&spi->cs_setup, NULL); + else + spi_delay_exec(&spi->cs_inactive, NULL); + } } #ifdef CONFIG_HAS_DMA -- cgit v1.2.3