* [PATCH 01/14] spi: unoptimize message when queuing it fails
2026-11-02 16:30 [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations Joseph CT Chan
@ 2026-11-02 16:30 ` Joseph CT Chan
2026-11-02 17:00 ` David Lechner
2026-11-02 16:30 ` [PATCH 02/14] spi: spi-mux: release mux when parent setup fails Joseph CT Chan
` (13 subsequent siblings)
14 siblings, 1 reply; 20+ messages in thread
From: Joseph CT Chan @ 2026-11-02 16:30 UTC (permalink / raw)
To: Mark Brown
Cc: Bart Van Assche, David Lechner, Conor Dooley, linux-spi,
Joseph CT Chan, stable
From: Joseph CT Chan <josephctchan@google.com>
spi_async() and spi_sync() optimize a message before they queue it.
When queuing then fails, the message is never finalized, but finalizing
is the only place where the core undoes its own optimization
(spi_finalize_current_message() calls spi_maybe_unoptimize_message()).
So the error paths leak the split transfers and the controller's
optimize state, and leave msg->optimized set. A caller that resends the
message then optimizes an already optimized message.
This happens when spi_async() returns -EBUSY because another device
holds spi_bus_lock() on the controller (for example mmc_spi), when the
controller's message queue is stopped (-ESHUTDOWN), when the controller
has no ->transfer() callback (-ENOTSUPP), and when __spi_sync() gets an
error from a controller's legacy ->transfer() callback.
Call spi_maybe_unoptimize_message() on these two error paths. It only
acts on messages the core optimized itself, so messages optimized by
the peripheral driver and controllers that defer optimization (such as
spi-mux) are not affected. A queued message is never unoptimized here.
Tested with KUnit (spi_core_optimize: spi_core_queue_error_test,
added later in this series) on ARCH=um; builds with W=1 on x86_64.
Not tested on hardware.
Assisted-by: Claude, Gemini
Fixes: 7b1d87af14d9 ("spi: add spi_optimize_message() APIs")
Fixes: c86a918b1bdb ("spi: don't unoptimize message in spi_async()")
Cc: stable@vger.kernel.org
Signed-off-by: Joseph CT Chan <josephctchan@google.com>
---
drivers/spi/spi.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 40cda0037ea5..0d36460e2c87 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -4906,6 +4906,10 @@ int spi_async(struct spi_device *spi, struct spi_message *message)
spin_unlock_irqrestore(&ctlr->bus_lock_spinlock, flags);
+ /* The message was not queued, so it will not be finalized. */
+ if (ret)
+ spi_maybe_unoptimize_message(message);
+
return ret;
}
EXPORT_SYMBOL_GPL(spi_async);
@@ -5009,6 +5013,9 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message)
if (status == 0) {
wait_for_completion(&done);
status = message->status;
+ } else {
+ /* The message was not queued, so it will not be finalized. */
+ spi_maybe_unoptimize_message(message);
}
message->complete = NULL;
message->context = NULL;
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 01/14] spi: unoptimize message when queuing it fails
2026-11-02 16:30 ` [PATCH 01/14] spi: unoptimize message when queuing it fails Joseph CT Chan
@ 2026-11-02 17:00 ` David Lechner
0 siblings, 0 replies; 20+ messages in thread
From: David Lechner @ 2026-11-02 17:00 UTC (permalink / raw)
To: Joseph CT Chan, Mark Brown
Cc: Bart Van Assche, Conor Dooley, linux-spi, Joseph CT Chan, stable
On 11/2/26 10:30 AM, Joseph CT Chan wrote:
> From: Joseph CT Chan <josephctchan@google.com>
>
> spi_async() and spi_sync() optimize a message before they queue it.
> When queuing then fails, the message is never finalized, but finalizing
> is the only place where the core undoes its own optimization
> (spi_finalize_current_message() calls spi_maybe_unoptimize_message()).
> So the error paths leak the split transfers and the controller's
> optimize state, and leave msg->optimized set. A caller that resends the
> message then optimizes an already optimized message.
>
> This happens when spi_async() returns -EBUSY because another device
> holds spi_bus_lock() on the controller (for example mmc_spi), when the
> controller's message queue is stopped (-ESHUTDOWN), when the controller
> has no ->transfer() callback (-ENOTSUPP), and when __spi_sync() gets an
> error from a controller's legacy ->transfer() callback.
>
> Call spi_maybe_unoptimize_message() on these two error paths. It only
> acts on messages the core optimized itself, so messages optimized by
> the peripheral driver and controllers that defer optimization (such as
> spi-mux) are not affected. A queued message is never unoptimized here.
>
> Tested with KUnit (spi_core_optimize: spi_core_queue_error_test,
> added later in this series) on ARCH=um; builds with W=1 on x86_64.
> Not tested on hardware.
>
> Assisted-by: Claude, Gemini
> Fixes: 7b1d87af14d9 ("spi: add spi_optimize_message() APIs")
> Fixes: c86a918b1bdb ("spi: don't unoptimize message in spi_async()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Joseph CT Chan <josephctchan@google.com>
> ---
> drivers/spi/spi.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 40cda0037ea5..0d36460e2c87 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -4906,6 +4906,10 @@ int spi_async(struct spi_device *spi, struct spi_message *message)
>
> spin_unlock_irqrestore(&ctlr->bus_lock_spinlock, flags);
>
> + /* The message was not queued, so it will not be finalized. */
> + if (ret)
> + spi_maybe_unoptimize_message(message);
> +
> return ret;
> }
> EXPORT_SYMBOL_GPL(spi_async);
> @@ -5009,6 +5013,9 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message)
> if (status == 0) {
> wait_for_completion(&done);
> status = message->status;
> + } else {
> + /* The message was not queued, so it will not be finalized. */
> + spi_maybe_unoptimize_message(message);
> }
> message->complete = NULL;
> message->context = NULL;
Reviewed-by: David Lechner <dlechner@baylibre.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 02/14] spi: spi-mux: release mux when parent setup fails
2026-11-02 16:30 [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations Joseph CT Chan
2026-11-02 16:30 ` [PATCH 01/14] spi: unoptimize message when queuing it fails Joseph CT Chan
@ 2026-11-02 16:30 ` Joseph CT Chan
2026-11-02 16:30 ` [PATCH 03/14] spi: spi-mux: re-apply settings after child calls spi_setup() Joseph CT Chan
` (12 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Joseph CT Chan @ 2026-11-02 16:30 UTC (permalink / raw)
To: Mark Brown
Cc: Bart Van Assche, David Lechner, Conor Dooley, linux-spi,
Joseph CT Chan, stable
From: Joseph CT Chan <josephctchan@google.com>
When spi_mux_select() switches to a different child, it locks the mux
with mux_control_select(), copies the child's settings into the spi-mux
device and calls spi_setup() on the parent controller. If spi_setup()
fails, spi_mux_select() returns the error without calling
mux_control_deselect(), so the mux stays locked. The next message on
any child of this mux then blocks forever in mux_control_select(), and
so does every other user of the same mux control.
spi_mux_select() also records the new chip select in current_cs before
spi_setup() runs. After a failure, the next message for the same child
skips the copy and spi_setup(), and is sent with a parent that was
never set up for it.
On spi_setup() failure, set current_cs to SPI_MUX_NO_CS, so the next
message sets up the parent again, and release the mux. Record the new
chip select only after spi_setup() succeeds.
Tested with KUnit (spi_mux: spi_mux_error_test, added later in this
series) on ARCH=um; that case also needs "spi: spi-mux: finalize
message when forwarding fails" to see the error status. Builds with
W=1 on x86_64. Not tested on hardware.
Assisted-by: Claude, Gemini
Fixes: 5a5fc308418a ("spi: spi-mux: Fix coverity issue, unchecked return value")
Cc: stable@vger.kernel.org
Signed-off-by: Joseph CT Chan <josephctchan@google.com>
---
drivers/spi/spi-mux.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi-mux.c b/drivers/spi/spi-mux.c
index 08fe1fa32dea..28d7718138a5 100644
--- a/drivers/spi/spi-mux.c
+++ b/drivers/spi/spi-mux.c
@@ -66,9 +66,16 @@ static int spi_mux_select(struct spi_device *spi)
priv->spi->mode = spi->mode;
priv->spi->bits_per_word = spi->bits_per_word;
+ ret = spi_setup(priv->spi);
+ if (ret) {
+ priv->current_cs = SPI_MUX_NO_CS;
+ mux_control_deselect(priv->mux);
+ return ret;
+ }
+
priv->current_cs = spi_get_chipselect(spi, 0);
- return spi_setup(priv->spi);
+ return 0;
}
static int spi_mux_setup(struct spi_device *spi)
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 03/14] spi: spi-mux: re-apply settings after child calls spi_setup()
2026-11-02 16:30 [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations Joseph CT Chan
2026-11-02 16:30 ` [PATCH 01/14] spi: unoptimize message when queuing it fails Joseph CT Chan
2026-11-02 16:30 ` [PATCH 02/14] spi: spi-mux: release mux when parent setup fails Joseph CT Chan
@ 2026-11-02 16:30 ` Joseph CT Chan
2026-11-02 16:30 ` [PATCH 04/14] spi: spi-mux: finalize message when forwarding fails Joseph CT Chan
` (11 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Joseph CT Chan @ 2026-11-02 16:30 UTC (permalink / raw)
To: Mark Brown
Cc: Bart Van Assche, David Lechner, Conor Dooley, linux-spi,
Joseph CT Chan, stable
From: Joseph CT Chan <josephctchan@google.com>
spi_mux_select() copies a child's max_speed_hz, mode and bits_per_word
into the spi-mux device and sets up the parent controller only when
the mux switches to a different child (current_cs differs). If the
child that is already selected changes its settings and calls
spi_setup(), spi_mux_setup() only calls spi_setup() on the spi-mux
device with its old settings, and current_cs still matches. The next
transfers of that child then run with the old speed and mode.
Set current_cs to SPI_MUX_NO_CS in spi_mux_setup(), so the next
transfer copies the child's settings and sets up the parent again.
spi_mux_setup() runs under the spi-mux controller's io_mutex, which the
message pump holds while a message is forwarded, so this does not race
with spi_mux_select(). The cost is one extra parent spi_setup() on the
next transfer.
Tested with KUnit (spi_mux: spi_mux_settings_lifecycle_test, added
later in this series) on ARCH=um; builds with W=1 on x86_64. Not tested
on hardware.
Assisted-by: Claude, Gemini
Fixes: e9e40543ad5b ("spi: Add generic SPI multiplexer")
Cc: stable@vger.kernel.org
Signed-off-by: Joseph CT Chan <josephctchan@google.com>
---
drivers/spi/spi-mux.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/spi/spi-mux.c b/drivers/spi/spi-mux.c
index 28d7718138a5..a7ea6585b374 100644
--- a/drivers/spi/spi-mux.c
+++ b/drivers/spi/spi-mux.c
@@ -86,7 +86,13 @@ static int spi_mux_setup(struct spi_device *spi)
* can be called multiple times, won't do a valid setup now but we will
* change the settings when we do a transfer (necessary because we
* can't predict from which device it will be anyway)
+ *
+ * The child may have changed its settings, so make the next transfer
+ * copy them again even if this child is the current one. This runs
+ * under our io_mutex, so no message is being forwarded right now.
*/
+ priv->current_cs = SPI_MUX_NO_CS;
+
return spi_setup(priv->spi);
}
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 04/14] spi: spi-mux: finalize message when forwarding fails
2026-11-02 16:30 [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations Joseph CT Chan
` (2 preceding siblings ...)
2026-11-02 16:30 ` [PATCH 03/14] spi: spi-mux: re-apply settings after child calls spi_setup() Joseph CT Chan
@ 2026-11-02 16:30 ` Joseph CT Chan
2026-11-02 16:30 ` [PATCH 05/14] spi: spi-ti-qspi: finalize message on transfer error Joseph CT Chan
` (10 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Joseph CT Chan @ 2026-11-02 16:30 UTC (permalink / raw)
To: Mark Brown
Cc: Bart Van Assche, David Lechner, Conor Dooley, linux-spi,
Joseph CT Chan, stable
From: Joseph CT Chan <josephctchan@google.com>
spi_mux_transfer_one_message() forwards a child's message to the parent
controller. When spi_mux_select() or spi_async() on the parent fails,
it returns the error without finalizing the message. The SPI core then
logs "failed to transfer one message from queue" and does not finalize
it either, so the child's completion callback never runs and spi_sync()
on the child waits forever. After a spi_async() failure the mux also
stays locked, so the next message on any child of the mux blocks the
spi-mux message pump in mux_control_select(), and the message keeps the
spi-mux device and callback in place of the child's.
This is easy to hit: spi_async() on the parent returns -EBUSY whenever
another device on the parent bus holds spi_bus_lock(), which mmc_spi
does for every request. It also returns an error when the parent
rejects the message, for example a transfer with 16-bit words and an
odd length, which spi-mux does not validate because it defers message
optimization to the parent.
On select failure, set the message status and finalize it; there is
nothing to restore yet and, with patch 2 of this series ("spi: spi-mux:
release mux when parent setup fails"), spi_mux_select() has already
released the mux. On spi_async() failure, set the status and call
spi_mux_complete_cb(), which restores the child's pointers, finalizes
the message and releases the mux, in the same order as a successful
transfer. In both cases return 0: the message is already finalized
with m->status set, so the error reaches the caller through the
message, and the core does not wait for it. Returning the error would
only make the core log "failed to transfer one message from queue"
once per -EBUSY while another device holds the bus lock.
Behavior change: a device behind spi-mux now gets -EBUSY while another
device holds spi_bus_lock() on the parent, instead of hanging. A device
wired directly to the parent would instead wait in spi_sync(), because
spi_sync() takes the bus_lock_mutex that spi_bus_lock() holds
(drivers/spi/spi.c spi_sync() and spi_bus_lock()); spi-mux forwards
with spi_async(), which returns -EBUSY while the bus is locked. Making
spi-mux wait for the bus lock needs a core helper and is left for a
follow-up.
This depends on patch 1 of this series ("spi: unoptimize message when
queuing it fails"): spi_async() on the parent optimizes the message
for the parent, and without patch 1 a failed spi_async() leaves it in
that state when it is handed back to the child.
Tested with KUnit (spi_mux: spi_mux_error_test, added later in this
series) on ARCH=um; before this patch they time out with the message
never finalized. Builds with W=1 on x86_64. Not tested on hardware.
Assisted-by: Claude, Gemini
Fixes: e9e40543ad5b ("spi: Add generic SPI multiplexer")
Cc: stable@vger.kernel.org
Signed-off-by: Joseph CT Chan <josephctchan@google.com>
---
drivers/spi/spi-mux.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/drivers/spi/spi-mux.c b/drivers/spi/spi-mux.c
index a7ea6585b374..0e770137b5bb 100644
--- a/drivers/spi/spi-mux.c
+++ b/drivers/spi/spi-mux.c
@@ -117,8 +117,12 @@ static int spi_mux_transfer_one_message(struct spi_controller *ctlr,
int ret;
ret = spi_mux_select(spi);
- if (ret)
- return ret;
+ if (ret) {
+ /* spi_mux_select() released the mux on failure */
+ m->status = ret;
+ spi_finalize_current_message(ctlr);
+ return 0;
+ }
/*
* Replace the complete callback, context and spi_device with our own
@@ -133,7 +137,23 @@ static int spi_mux_transfer_one_message(struct spi_controller *ctlr,
m->spi = priv->spi;
/* do the transfer */
- return spi_async(priv->spi, m);
+ ret = spi_async(priv->spi, m);
+ if (ret) {
+ /*
+ * The parent did not take the message, so it will not call
+ * spi_mux_complete_cb(). Call it here to restore the child's
+ * pointers, finalize the message and release the mux.
+ */
+ m->status = ret;
+ spi_mux_complete_cb(priv);
+ }
+
+ /*
+ * On error the message is already finalized with m->status set, so
+ * the error reaches the caller; returning it would only make the core
+ * log it once more.
+ */
+ return 0;
}
static int spi_mux_probe(struct spi_device *spi)
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 05/14] spi: spi-ti-qspi: finalize message on transfer error
2026-11-02 16:30 [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations Joseph CT Chan
` (3 preceding siblings ...)
2026-11-02 16:30 ` [PATCH 04/14] spi: spi-mux: finalize message when forwarding fails Joseph CT Chan
@ 2026-11-02 16:30 ` Joseph CT Chan
2026-11-02 16:30 ` [PATCH 06/14] spi: spi-mux: pass child CS and word delays to the parent Joseph CT Chan
` (9 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Joseph CT Chan @ 2026-11-02 16:30 UTC (permalink / raw)
To: Mark Brown
Cc: Bart Van Assche, David Lechner, Conor Dooley, linux-spi,
Joseph CT Chan, stable
From: Joseph CT Chan <josephctchan@google.com>
When ti_qspi_start_transfer_one() hits an error in qspi_transfer_msg(),
such as a transfer timeout (-ETIMEDOUT), it unlocks qspi->list_lock and
returns ret directly without writing QSPI_INVAL to QSPI_SPI_CMD_REG,
setting m->status, or calling spi_finalize_current_message(host).
Because the message is never finalized, spi_sync() or the spi_async()
callback waits forever on queued transfers. If spi_sync() took the
no-queue path, it returns -EINPROGRESS instead of the error, and the
message's optimize state is never released. The frame is also left
open, because the QSPI_INVAL write that ends it is skipped.
Set status to the transfer error and leave the transfer loop, so the
error goes through the existing tail: unlock list_lock, write
QSPI_INVAL, set m->status, and finalize the message. This fixes both
paths, and the caller now gets the error from qspi_transfer_msg()
instead of -EINVAL or -EINPROGRESS.
Tested at runtime with KUnit (spi_ti_qspi, ti_qspi_test, added later in
this series) by injecting a QSPI_SPI_STATUS_REG WC poll timeout
(-ETIMEDOUT) via logic_iomem emulated MMIO on ARCH=um.
Assisted-by: Claude, Gemini
Fixes: 505a14954e2d ("spi/qspi: Add qspi flash controller")
Cc: stable@vger.kernel.org
Signed-off-by: Joseph CT Chan <josephctchan@google.com>
---
drivers/spi/spi-ti-qspi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-ti-qspi.c b/drivers/spi/spi-ti-qspi.c
index 091a8b8842f5..7360ee6bd122 100644
--- a/drivers/spi/spi-ti-qspi.c
+++ b/drivers/spi/spi-ti-qspi.c
@@ -708,8 +708,8 @@ static int ti_qspi_start_transfer_one(struct spi_controller *host,
ret = qspi_transfer_msg(qspi, t, transfer_len_words * wlen);
if (ret) {
dev_dbg(qspi->dev, "transfer message failed\n");
- mutex_unlock(&qspi->list_lock);
- return -EINVAL;
+ status = ret;
+ break;
}
m->actual_length += transfer_len_words * wlen;
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 06/14] spi: spi-mux: pass child CS and word delays to the parent
2026-11-02 16:30 [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations Joseph CT Chan
` (4 preceding siblings ...)
2026-11-02 16:30 ` [PATCH 05/14] spi: spi-ti-qspi: finalize message on transfer error Joseph CT Chan
@ 2026-11-02 16:30 ` Joseph CT Chan
2026-11-02 16:30 ` [PATCH 07/14] spi: add KUnit tests for message optimize state on queue errors Joseph CT Chan
` (8 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Joseph CT Chan @ 2026-11-02 16:30 UTC (permalink / raw)
To: Mark Brown
Cc: Bart Van Assche, David Lechner, Conor Dooley, linux-spi,
Joseph CT Chan
From: Joseph CT Chan <josephctchan@google.com>
spi_mux_select() copies only max_speed_hz, mode and bits_per_word from
the child to the spi-mux device on the parent controller. The child's
cs_setup, cs_hold, cs_inactive and word_delay are never passed on, so
the parent controller uses the spi-mux device's own delays (usually
zero) for every child. A child that needs a CS setup time or a delay
between words, for example from spi-cs-setup-delay-ns in its device
tree node, does not get it when it sits behind a mux.
Save the spi-mux device's own delays at probe time, and when switching
to a child, merge them with the child's in a new helper,
spi_mux_merge_cs_delay(): use the longer CS delay of the two, so that
the needs of the mux and of the child are both met; on a tie the
spi-mux delay is kept. A delay given in SCK cycles cannot be converted
to time without a transfer, so it cannot be compared: a child delay in
SCK cycles is used when the spi-mux delay is zero, and otherwise the
spi-mux delay is kept and a warning is printed once. If only the
spi-mux delay is in SCK cycles, it is kept without a warning; device
tree only gives delays in ns or us, so this case does not come from
DT. The child's word_delay is used when it is set, else the spi-mux
device's own. The delays are written before spi_setup() on the parent,
so they reach both set_cs_timing() and the software CS delays in
spi_set_cs().
spi_mux_select() overwrites the delays of the spi-mux device, so put
its own delays back when the driver unbinds. Otherwise a later probe
would save the last child's merged delays as the spi-mux device's own.
Unlike a device directly on a controller, whose CS delays are read on
every message, the delays are now copied when the mux switches to the
child or after the child calls spi_setup(). A child that changes its
delays must call spi_setup() for the change to take effect.
This depends on patch 3 of this series ("spi: spi-mux: re-apply
settings after child calls spi_setup()"), which makes the copy happen
again after the child calls spi_setup().
Tested with KUnit (spi_mux: spi_mux_cs_delay_test and
spi_mux_settings_lifecycle_test, added later in this series) on ARCH=um;
they fail without this patch. Builds with W=1 on x86_64. Not tested on
hardware.
Assisted-by: Claude, Gemini
Fixes: e9e40543ad5b ("spi: Add generic SPI multiplexer")
Fixes: 8c33ebfeeb59 ("spi: move cs spi_delay to spi_device")
Signed-off-by: Joseph CT Chan <josephctchan@google.com>
---
drivers/spi/spi-mux.c | 83 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 83 insertions(+)
diff --git a/drivers/spi/spi-mux.c b/drivers/spi/spi-mux.c
index 0e770137b5bb..45d8920727e0 100644
--- a/drivers/spi/spi-mux.c
+++ b/drivers/spi/spi-mux.c
@@ -34,6 +34,10 @@
* @child_msg_dev: Used to store the spi_device pointer to the child
* @mux: mux_control structure used to provide chip selects for
* downstream spi devices
+ * @mux_cs_setup: CS setup delay of the spi-mux device itself
+ * @mux_cs_hold: CS hold delay of the spi-mux device itself
+ * @mux_cs_inactive: CS inactive delay of the spi-mux device itself
+ * @mux_word_delay: word delay of the spi-mux device itself
*/
struct spi_mux_priv {
struct spi_device *spi;
@@ -43,8 +47,55 @@ struct spi_mux_priv {
void *child_msg_context;
struct spi_device *child_msg_dev;
struct mux_control *mux;
+
+ struct spi_delay mux_cs_setup;
+ struct spi_delay mux_cs_hold;
+ struct spi_delay mux_cs_inactive;
+ struct spi_delay mux_word_delay;
};
+/*
+ * Pick the CS delay for the parent from the spi-mux device's own delay and
+ * the child's: the longer of the two, so that both are honored; on a tie the
+ * spi-mux delay is kept. A delay in SCK cycles cannot be converted to time
+ * without a transfer, so it cannot be compared:
+ * - a child delay in SCK cycles is used when the spi-mux delay is zero;
+ * otherwise the spi-mux delay is kept and a warning is printed once;
+ * - if only the spi-mux delay is in SCK cycles, it is kept. Device tree
+ * only gives delays in ns or us, so this case does not come from DT.
+ */
+static void spi_mux_merge_cs_delay(struct spi_mux_priv *priv,
+ struct spi_delay *dst,
+ struct spi_delay *mux,
+ struct spi_delay *child)
+{
+ int mux_ns, child_ns;
+
+ if (!child->value) {
+ *dst = *mux;
+ return;
+ }
+
+ mux_ns = spi_delay_to_ns(mux, NULL);
+ child_ns = spi_delay_to_ns(child, NULL);
+
+ if (mux_ns >= 0 && child_ns >= 0) {
+ *dst = child_ns > mux_ns ? *child : *mux;
+ return;
+ }
+
+ if (child_ns < 0) {
+ if (!mux->value) {
+ *dst = *child;
+ return;
+ }
+ dev_warn_once(&priv->spi->dev,
+ "child CS delay in SCK cycles ignored, using the mux delay\n");
+ }
+
+ *dst = *mux;
+}
+
/* should not get called when the parent controller is doing a transfer */
static int spi_mux_select(struct spi_device *spi)
{
@@ -65,6 +116,14 @@ static int spi_mux_select(struct spi_device *spi)
priv->spi->max_speed_hz = spi->max_speed_hz;
priv->spi->mode = spi->mode;
priv->spi->bits_per_word = spi->bits_per_word;
+ spi_mux_merge_cs_delay(priv, &priv->spi->cs_setup, &priv->mux_cs_setup,
+ &spi->cs_setup);
+ spi_mux_merge_cs_delay(priv, &priv->spi->cs_hold, &priv->mux_cs_hold,
+ &spi->cs_hold);
+ spi_mux_merge_cs_delay(priv, &priv->spi->cs_inactive,
+ &priv->mux_cs_inactive, &spi->cs_inactive);
+ priv->spi->word_delay = spi->word_delay.value ? spi->word_delay :
+ priv->mux_word_delay;
ret = spi_setup(priv->spi);
if (ret) {
@@ -156,10 +215,26 @@ static int spi_mux_transfer_one_message(struct spi_controller *ctlr,
return 0;
}
+/*
+ * spi_mux_select() overwrites the delays of the spi-mux device with the
+ * merged ones. Put its own delays back when the driver unbinds, so that a
+ * later probe does not take the last child's delays as its own.
+ */
+static void spi_mux_restore_delays(void *data)
+{
+ struct spi_mux_priv *priv = data;
+
+ priv->spi->cs_setup = priv->mux_cs_setup;
+ priv->spi->cs_hold = priv->mux_cs_hold;
+ priv->spi->cs_inactive = priv->mux_cs_inactive;
+ priv->spi->word_delay = priv->mux_word_delay;
+}
+
static int spi_mux_probe(struct spi_device *spi)
{
struct spi_controller *ctlr;
struct spi_mux_priv *priv;
+ int ret;
ctlr = devm_spi_alloc_host(&spi->dev, sizeof(*priv));
if (!ctlr)
@@ -168,6 +243,14 @@ static int spi_mux_probe(struct spi_device *spi)
spi_set_drvdata(spi, ctlr);
priv = spi_controller_get_devdata(ctlr);
priv->spi = spi;
+ priv->mux_cs_setup = spi->cs_setup;
+ priv->mux_cs_hold = spi->cs_hold;
+ priv->mux_cs_inactive = spi->cs_inactive;
+ priv->mux_word_delay = spi->word_delay;
+
+ ret = devm_add_action_or_reset(&spi->dev, spi_mux_restore_delays, priv);
+ if (ret)
+ return ret;
/*
* Increase lockdep class as these lock are taken while the parent bus
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 07/14] spi: add KUnit tests for message optimize state on queue errors
2026-11-02 16:30 [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations Joseph CT Chan
` (5 preceding siblings ...)
2026-11-02 16:30 ` [PATCH 06/14] spi: spi-mux: pass child CS and word delays to the parent Joseph CT Chan
@ 2026-11-02 16:30 ` Joseph CT Chan
2026-11-02 17:29 ` David Lechner
2026-11-02 16:30 ` [PATCH 08/14] spi: spi-ti-qspi: add KUnit tests for transfer error finalization Joseph CT Chan
` (7 subsequent siblings)
14 siblings, 1 reply; 20+ messages in thread
From: Joseph CT Chan @ 2026-11-02 16:30 UTC (permalink / raw)
To: Mark Brown
Cc: Bart Van Assche, David Lechner, Conor Dooley, linux-spi,
Joseph CT Chan
From: Joseph CT Chan <josephctchan@google.com>
spi_async() and spi_sync() optimize a message before they queue it. If
queuing fails, for example because another device holds spi_bus_lock()
on the controller, because mem_ops rejects the message, or because the
controller is suspended, the message is never finalized, and finalizing
is where the core normally unoptimizes it.
Add KUnit tests with a fake controller that counts its optimize_message()
and unoptimize_message() calls, splits transfers across a 2-byte boundary
to exercise msg.resources, and tracks msg.opt_state. The tests cover both
async and sync queue errors, verify split transfers are cleaned up and
opt_state cleared, check that a message optimized by the peripheral
driver is left alone on error, and check that a successful spi_sync()
optimizes and unoptimizes the message exactly once on both queued and
unqueued paths.
Without "spi: unoptimize message when queuing it fails" the queue error
cases fail with unoptimize_count == 0, msg.resources leaked, and
msg.optimized still true; the other cases pass.
Tested with KUnit on ARCH=um: all cases pass with the fixes in this series
applied. Builds with W=1 on x86_64.
Assisted-by: Claude, Gemini
Signed-off-by: Joseph CT Chan <josephctchan@google.com>
---
drivers/spi/Kconfig | 13 ++
drivers/spi/Makefile | 1 +
drivers/spi/tests/.kunitconfig | 4 +
drivers/spi/tests/Makefile | 1 +
drivers/spi/tests/spi-core-kunit.c | 308 +++++++++++++++++++++++++++++
5 files changed, 327 insertions(+)
create mode 100644 drivers/spi/tests/.kunitconfig
create mode 100644 drivers/spi/tests/spi-core-kunit.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 49088e5886d3..a7b343d124a9 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -1398,6 +1398,19 @@ config SPI_DMA_KUNIT_TEST
If unsure say N.
+config SPI_CORE_KUNIT_TEST
+ tristate "KUnit tests for SPI core message optimize state" if !KUNIT_ALL_TESTS
+ depends on KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ Build KUnit tests for the SPI core message optimize state. The tests
+ register a fake controller and check that spi_async() and spi_sync()
+ undo the optimization of a message they fail to queue, that a
+ message optimized by the peripheral driver is left alone, and that a
+ successful transfer optimizes and unoptimizes exactly once.
+
+ If unsure say N.
+
config SPI_LOOPBACK_TEST
tristate "spi loopback test framework support"
depends on m
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index b493f50b93e3..d4a806b46b2c 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_SPI_OFFLOAD) += spi-offload.o
obj-$(CONFIG_SPI_SPIDEV) += spidev.o
obj-$(CONFIG_SPI_LOOPBACK_TEST) += spi-loopback-test.o
obj-$(CONFIG_SPI_DMA_KUNIT_TEST) += tests/
+obj-$(CONFIG_SPI_CORE_KUNIT_TEST) += tests/
# SPI master controller drivers (bus)
obj-$(CONFIG_SPI_AIROHA_SNFI) += spi-airoha-snfi.o
diff --git a/drivers/spi/tests/.kunitconfig b/drivers/spi/tests/.kunitconfig
new file mode 100644
index 000000000000..f781425d258e
--- /dev/null
+++ b/drivers/spi/tests/.kunitconfig
@@ -0,0 +1,4 @@
+CONFIG_KUNIT=y
+CONFIG_SPI=y
+CONFIG_SPI_MASTER=y
+CONFIG_SPI_CORE_KUNIT_TEST=y
diff --git a/drivers/spi/tests/Makefile b/drivers/spi/tests/Makefile
index 26689e0cb28a..6dbf53643619 100644
--- a/drivers/spi/tests/Makefile
+++ b/drivers/spi/tests/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_SPI_DMA_KUNIT_TEST) += spi-dma-kunit.o
+obj-$(CONFIG_SPI_CORE_KUNIT_TEST) += spi-core-kunit.o
diff --git a/drivers/spi/tests/spi-core-kunit.c b/drivers/spi/tests/spi-core-kunit.c
new file mode 100644
index 000000000000..e1578f2f8eb3
--- /dev/null
+++ b/drivers/spi/tests/spi-core-kunit.c
@@ -0,0 +1,308 @@
+// SPDX-License-Identifier: GPL-2.0
+// KUnit tests for the SPI core message optimize state.
+#include <kunit/device.h>
+#include <kunit/test.h>
+#include <linux/completion.h>
+#include <linux/errno.h>
+#include <linux/jiffies.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
+
+struct spi_core_test_ctx {
+ struct spi_controller *ctlr;
+ struct spi_device *spi;
+ int optimize_count, unoptimize_count;
+ bool bad_opt_state;
+ struct spi_transfer xfer;
+ struct spi_message msg;
+ struct completion done;
+ u8 tx[4];
+};
+
+static int spi_core_test_optimize(struct spi_message *msg)
+{
+ struct spi_core_test_ctx *ctx = spi_controller_get_devdata(msg->spi->controller);
+
+ ctx->optimize_count++;
+ msg->opt_state = ctx;
+ return 0;
+}
+
+static int spi_core_test_unoptimize(struct spi_message *msg)
+{
+ struct spi_core_test_ctx *ctx = spi_controller_get_devdata(msg->spi->controller);
+
+ ctx->unoptimize_count++;
+ if (msg->opt_state != ctx)
+ ctx->bad_opt_state = true;
+ return 0;
+}
+
+static int spi_core_test_xfer_one(struct spi_controller *ctlr, struct spi_message *msg)
+{
+ struct spi_transfer *xfer;
+
+ list_for_each_entry(xfer, &msg->transfers, transfer_list)
+ msg->actual_length += xfer->len;
+ msg->status = 0;
+ spi_finalize_current_message(ctlr);
+ return 0;
+}
+
+static int spi_core_test_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
+{
+ return -EOPNOTSUPP;
+}
+
+static const struct spi_controller_mem_ops spi_core_test_mem_ops = {
+ .exec_op = spi_core_test_exec_op,
+};
+
+static size_t spi_core_test_max_transfer_size(struct spi_device *spi)
+{
+ return 2;
+}
+
+static void kunit_done(void *p)
+{
+ complete(p);
+}
+
+static struct spi_core_test_ctx *spi_core_test_ctx_new(struct kunit *test, bool mem_only)
+{
+ struct spi_board_info info = { .modalias = "spi-core-kunit", .max_speed_hz = 1000000 };
+ struct device *dev = kunit_device_register(test, "spi-core-kunit");
+ struct spi_controller *ctlr;
+ struct spi_core_test_ctx *ctx;
+
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(test, dev);
+ ctlr = devm_spi_alloc_host(dev, sizeof(struct spi_core_test_ctx));
+ KUNIT_ASSERT_NOT_NULL(test, ctlr);
+ ctx = spi_controller_get_devdata(ctlr);
+
+ ctx->ctlr = ctlr;
+ ctlr->bus_num = -1;
+ ctlr->num_chipselect = 1;
+ ctlr->max_transfer_size = spi_core_test_max_transfer_size;
+ ctlr->optimize_message = spi_core_test_optimize;
+ ctlr->unoptimize_message = spi_core_test_unoptimize;
+ ctlr->mem_ops = mem_only ? &spi_core_test_mem_ops : NULL;
+ ctlr->transfer_one_message = mem_only ? NULL : spi_core_test_xfer_one;
+ KUNIT_ASSERT_EQ(test, 0, devm_spi_register_controller(dev, ctlr));
+ ctx->spi = spi_new_device(ctlr, &info);
+ KUNIT_ASSERT_NOT_NULL(test, ctx->spi);
+ init_completion(&ctx->done);
+ /*
+ * Pair a 4-byte dummy TX buffer with max_transfer_size = 2 so
+ * __spi_optimize_message() splits the 1 transfer into two 2-byte
+ * transfers tracked in msg.resources, allowing check_state() to
+ * verify that spi_res_release() undoes transfer splitting.
+ */
+ ctx->xfer.tx_buf = ctx->tx;
+ ctx->xfer.len = sizeof(ctx->tx);
+ spi_message_init_with_transfers(&ctx->msg, &ctx->xfer, 1);
+ ctx->msg.complete = kunit_done;
+ ctx->msg.context = &ctx->done;
+ return ctx;
+}
+
+enum spi_core_err_mode {
+ ERR_NONE,
+ ERR_BUS_LOCKED,
+ ERR_MEM_OPS,
+ ERR_SUSPEND,
+};
+
+struct spi_core_err_param {
+ const char *name;
+ enum spi_core_err_mode mode;
+ int exp;
+ bool use_sync;
+ bool pre_opt;
+ bool must_async;
+};
+
+static const struct spi_core_err_param spi_core_err_cases[] = {
+ { "bus_locked_async", ERR_BUS_LOCKED, -EBUSY },
+ { "suspend_async", ERR_SUSPEND, -ESHUTDOWN },
+ { "mem_ops_sync", ERR_MEM_OPS, -ENOTSUPP, true },
+ { "pre_opt_locked", ERR_BUS_LOCKED, -EBUSY, false, true },
+ { "pre_opt_mem_ops_sync", ERR_MEM_OPS, -ENOTSUPP, true, true },
+ { "sync_success", ERR_NONE, 0, true },
+ { "sync_success_queued", ERR_NONE, 0, true, false, true },
+};
+
+static void spi_core_err_desc(const struct spi_core_err_param *t, char *desc)
+{
+ strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(spi_core_err, spi_core_err_cases, spi_core_err_desc);
+
+/*
+ * Verify cumulative controller optimize/unoptimize callback counts and the
+ * current optimization state of ctx->msg:
+ * @opt: expected cumulative ctlr->optimize_message() count
+ * (ctx->optimize_count).
+ * @unopt: expected cumulative ctlr->unoptimize_message() count
+ * (ctx->unoptimize_count).
+ * @state: expected current optimization state of ctx->msg:
+ * - true: currently optimized (msg.optimized == true,
+ * msg.opt_state == ctx, split-transfer resource present in
+ * msg.resources, and 2 split transfers in msg.transfers).
+ * - false: unoptimized (msg.optimized == false, msg.opt_state == NULL,
+ * msg.resources is empty, and original 1 transfer restored).
+ */
+static void check_state(struct kunit *test, struct spi_core_test_ctx *ctx,
+ int opt, int unopt, bool state)
+{
+ KUNIT_EXPECT_EQ(test, ctx->optimize_count, opt);
+ KUNIT_EXPECT_EQ(test, ctx->unoptimize_count, unopt);
+ KUNIT_EXPECT_FALSE(test, ctx->bad_opt_state);
+ KUNIT_EXPECT_EQ(test, ctx->msg.optimized, state);
+ KUNIT_EXPECT_EQ(test, !ctx->msg.opt_state, !state);
+ KUNIT_EXPECT_EQ(test, list_empty(&ctx->msg.resources), !state);
+ KUNIT_EXPECT_EQ(test, list_is_singular(&ctx->msg.transfers), !state);
+}
+
+static int spi_core_test_send_locked(struct spi_device *spi, struct spi_message *msg)
+{
+ int ret;
+
+ spi_bus_lock(spi->controller);
+ ret = spi_async(spi, msg);
+ spi_bus_unlock(spi->controller);
+
+ return ret;
+}
+
+static void spi_core_queue_error_test(struct kunit *test)
+{
+ const struct spi_core_err_param *p = test->param_value;
+ struct spi_core_test_ctx *ctx = spi_core_test_ctx_new(test, p->mode == ERR_MEM_OPS);
+ int exp = p->exp, ret;
+
+ ctx->ctlr->must_async = p->must_async;
+ if (p->pre_opt)
+ KUNIT_ASSERT_EQ(test, spi_optimize_message(ctx->spi, &ctx->msg), 0);
+ if (p->mode == ERR_SUSPEND)
+ KUNIT_ASSERT_EQ(test, 0, spi_controller_suspend(ctx->ctlr));
+
+ if (p->mode == ERR_BUS_LOCKED)
+ ret = spi_core_test_send_locked(ctx->spi, &ctx->msg);
+ else if (p->use_sync)
+ ret = spi_sync(ctx->spi, &ctx->msg);
+ else
+ ret = spi_async(ctx->spi, &ctx->msg);
+
+ if (p->mode == ERR_SUSPEND)
+ KUNIT_ASSERT_EQ(test, 0, spi_controller_resume(ctx->ctlr));
+
+ KUNIT_EXPECT_EQ(test, ret, exp);
+ if (!exp) {
+ KUNIT_EXPECT_EQ(test, ctx->msg.actual_length, sizeof(ctx->tx));
+ /*
+ * Normal error-free situation for the ERR_NONE control cases
+ * (sync_success on the noqueue fast-path and
+ * sync_success_queued on the kthread worker path):
+ * - Expected: (opt = 1, unopt = 1, state = false).
+ * Meaning: spi_sync() optimized ctx->msg once before the
+ * transfer (opt = 1) and unoptimized it once on completion
+ * (unopt = 1), leaving ctx->msg unoptimized (state = false:
+ * msg.resources is empty and the single 4-byte transfer is
+ * restored).
+ */
+ check_state(test, ctx, 1, 1, false);
+ return;
+ }
+
+ /*
+ * Check message optimization state immediately after queuing fails
+ * across the 5 error test cases:
+ *
+ * Group A: Core-optimized queue failures (p->pre_opt == false)
+ * 1. bus_locked_async: spi_async() fails with -EBUSY while the bus
+ * lock is held by another caller.
+ * 2. suspend_async: spi_async() fails with -ESHUTDOWN while the
+ * controller queue is stopped by spi_controller_suspend().
+ * 3. mem_ops_sync: spi_sync() fails with -ENOTSUPP on a controller
+ * that only implements mem_ops (ctlr->transfer == NULL).
+ * - Expected (patched): (opt = 1, unopt = 1, state = false).
+ * Meaning: spi_async() / __spi_sync() optimized ctx->msg (opt = 1)
+ * before queuing failed, and spi_maybe_unoptimize_message() on the
+ * error path immediately unoptimized it (unopt = 1), restoring
+ * ctx->msg to the clean unoptimized state (state = false:
+ * msg.optimized == false, msg.opt_state == NULL, split transfers
+ * in msg.resources freed, and original 1 transfer restored).
+ * - Broken behavior watched for (unpatched): (opt = 1, unopt = 0,
+ * state = true).
+ * Meaning: spi_async() / __spi_sync() returned the queue error
+ * without calling spi_maybe_unoptimize_message(), leaving
+ * unopt = 0 and ctx->msg stuck in the optimized state
+ * (state = true: msg.optimized == true, msg.opt_state == ctx, and
+ * the 2 split transfers leaked on msg.resources).
+ *
+ * Group B: Caller pre-optimized queue failures (p->pre_opt == true)
+ * 4. pre_opt_locked: caller pre-optimized ctx->msg via
+ * spi_optimize_message(), then spi_async() fails with -EBUSY.
+ * 5. pre_opt_mem_ops_sync: caller pre-optimized ctx->msg via
+ * spi_optimize_message(), then spi_sync() fails with -ENOTSUPP.
+ * - Expected (patched): (opt = 1, unopt = 0, state = true).
+ * Meaning: the caller explicitly pre-optimized ctx->msg (opt = 1,
+ * msg.pre_optimized == true) before submission, so
+ * spi_maybe_unoptimize_message() on the queue error path must
+ * preserve the caller's pre-optimization (unopt = 0, state = true)
+ * until the caller explicitly calls spi_unoptimize_message() below.
+ * - Broken behavior watched for (if error cleanup unconditionally
+ * called __spi_unoptimize_message() instead of
+ * spi_maybe_unoptimize_message()): (opt = 1, unopt = 1,
+ * state = false).
+ * Meaning: the error path would prematurely destroy the caller's
+ * pre-optimized state, causing a double-unoptimize when the caller
+ * later calls spi_unoptimize_message().
+ */
+ check_state(test, ctx, 1, p->pre_opt ? 0 : 1, p->pre_opt);
+ if (p->pre_opt)
+ spi_unoptimize_message(&ctx->msg);
+ if (p->mode == ERR_MEM_OPS)
+ return;
+
+ reinit_completion(&ctx->done);
+ KUNIT_ASSERT_EQ(test, spi_async(ctx->spi, &ctx->msg), 0);
+ KUNIT_ASSERT_NE(test, wait_for_completion_timeout(&ctx->done, msecs_to_jiffies(1000)), 0);
+ KUNIT_EXPECT_EQ(test, ctx->msg.status, 0);
+ /*
+ * Normal error-free situation after retry for the retryable error
+ * cases (bus_locked_async after spi_bus_unlock(), suspend_async after
+ * spi_controller_resume(), and pre_opt_locked after
+ * spi_unoptimize_message() + spi_bus_unlock()):
+ * - Expected (patched): (opt = 2, unopt = 2, state = false).
+ * Meaning: resubmitting the same ctx->msg after the transient error
+ * clears optimizes it a second time (opt = 2) and unoptimizes it on
+ * completion (unopt = 2), leaving ctx->msg unoptimized
+ * (state = false).
+ * - Broken behavior watched for (unpatched): (opt = 2, unopt = 1,
+ * state = false).
+ * Meaning: in bus_locked_async and suspend_async, the first failed
+ * attempt leaked ctx->msg in the optimized state (opt = 1,
+ * unopt = 0, state = true), so retrying the leaked message
+ * re-optimizes an already-optimized message (opt = 2) and only
+ * unoptimizes once when the retry completes (unopt = 1).
+ */
+ check_state(test, ctx, 2, 2, false);
+}
+
+static struct kunit_case spi_core_optimize_cases[] = {
+ KUNIT_CASE_PARAM(spi_core_queue_error_test, spi_core_err_gen_params),
+ {}
+};
+
+static struct kunit_suite spi_core_optimize_suite = {
+ .name = "spi_core_optimize",
+ .test_cases = spi_core_optimize_cases,
+};
+
+kunit_test_suite(spi_core_optimize_suite);
+
+MODULE_DESCRIPTION("KUnit tests for SPI core message optimize state");
+MODULE_LICENSE("GPL");
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 07/14] spi: add KUnit tests for message optimize state on queue errors
2026-11-02 16:30 ` [PATCH 07/14] spi: add KUnit tests for message optimize state on queue errors Joseph CT Chan
@ 2026-11-02 17:29 ` David Lechner
2026-11-03 22:41 ` Joseph CT Chan
0 siblings, 1 reply; 20+ messages in thread
From: David Lechner @ 2026-11-02 17:29 UTC (permalink / raw)
To: Joseph CT Chan, Mark Brown
Cc: Bart Van Assche, Conor Dooley, linux-spi, Joseph CT Chan
On 11/2/26 10:30 AM, Joseph CT Chan wrote:
> From: Joseph CT Chan <josephctchan@google.com>
>
> spi_async() and spi_sync() optimize a message before they queue it. If
> queuing fails, for example because another device holds spi_bus_lock()
> on the controller, because mem_ops rejects the message, or because the
I was expecting a fake spi controller implementation rather than depending
on an quirk of mem_ops to trigger the errors.
> controller is suspended, the message is never finalized, and finalizing
> is where the core normally unoptimizes it.
>
...
> +static const struct spi_core_err_param spi_core_err_cases[] = {
> + { "bus_locked_async", ERR_BUS_LOCKED, -EBUSY },
> + { "suspend_async", ERR_SUSPEND, -ESHUTDOWN },
> + { "mem_ops_sync", ERR_MEM_OPS, -ENOTSUPP, true },
> + { "pre_opt_locked", ERR_BUS_LOCKED, -EBUSY, false, true },
> + { "pre_opt_mem_ops_sync", ERR_MEM_OPS, -ENOTSUPP, true, true },
> + { "sync_success", ERR_NONE, 0, true },
> + { "sync_success_queued", ERR_NONE, 0, true, false, true },
Can we get the field names here so we can see what true/false are actually
doing?
> + KUNIT_EXPECT_EQ(test, ret, exp);
> + if (!exp) {
> + KUNIT_EXPECT_EQ(test, ctx->msg.actual_length, sizeof(ctx->tx));
> + /*
> + * Normal error-free situation for the ERR_NONE control cases
> + * (sync_success on the noqueue fast-path and
> + * sync_success_queued on the kthread worker path):
> + * - Expected: (opt = 1, unopt = 1, state = false).
> + * Meaning: spi_sync() optimized ctx->msg once before the
> + * transfer (opt = 1) and unoptimized it once on completion
> + * (unopt = 1), leaving ctx->msg unoptimized (state = false:
> + * msg.resources is empty and the single 4-byte transfer is
> + * restored).
> + */
> + check_state(test, ctx, 1, 1, false);
> + return;
> + }
> +
> + /*
> + * Check message optimization state immediately after queuing fails
> + * across the 5 error test cases:
> + *
> + * Group A: Core-optimized queue failures (p->pre_opt == false)
> + * 1. bus_locked_async: spi_async() fails with -EBUSY while the bus
> + * lock is held by another caller.
> + * 2. suspend_async: spi_async() fails with -ESHUTDOWN while the
> + * controller queue is stopped by spi_controller_suspend().
> + * 3. mem_ops_sync: spi_sync() fails with -ENOTSUPP on a controller
> + * that only implements mem_ops (ctlr->transfer == NULL).
> + * - Expected (patched): (opt = 1, unopt = 1, state = false).
> + * Meaning: spi_async() / __spi_sync() optimized ctx->msg (opt = 1)
> + * before queuing failed, and spi_maybe_unoptimize_message() on the
> + * error path immediately unoptimized it (unopt = 1), restoring
> + * ctx->msg to the clean unoptimized state (state = false:
> + * msg.optimized == false, msg.opt_state == NULL, split transfers
> + * in msg.resources freed, and original 1 transfer restored).
> + * - Broken behavior watched for (unpatched): (opt = 1, unopt = 0,
> + * state = true).
> + * Meaning: spi_async() / __spi_sync() returned the queue error
> + * without calling spi_maybe_unoptimize_message(), leaving
> + * unopt = 0 and ctx->msg stuck in the optimized state
> + * (state = true: msg.optimized == true, msg.opt_state == ctx, and
> + * the 2 split transfers leaked on msg.resources).
> + *
> + * Group B: Caller pre-optimized queue failures (p->pre_opt == true)
> + * 4. pre_opt_locked: caller pre-optimized ctx->msg via
> + * spi_optimize_message(), then spi_async() fails with -EBUSY.
> + * 5. pre_opt_mem_ops_sync: caller pre-optimized ctx->msg via
> + * spi_optimize_message(), then spi_sync() fails with -ENOTSUPP.
> + * - Expected (patched): (opt = 1, unopt = 0, state = true).
> + * Meaning: the caller explicitly pre-optimized ctx->msg (opt = 1,
> + * msg.pre_optimized == true) before submission, so
> + * spi_maybe_unoptimize_message() on the queue error path must
> + * preserve the caller's pre-optimization (unopt = 0, state = true)
> + * until the caller explicitly calls spi_unoptimize_message() below.
> + * - Broken behavior watched for (if error cleanup unconditionally
> + * called __spi_unoptimize_message() instead of
> + * spi_maybe_unoptimize_message()): (opt = 1, unopt = 1,
> + * state = false).
> + * Meaning: the error path would prematurely destroy the caller's
> + * pre-optimized state, causing a double-unoptimize when the caller
> + * later calls spi_unoptimize_message().
> + */
Do we really need the huge AI-generated comments?
> + check_state(test, ctx, 1, p->pre_opt ? 0 : 1, p->pre_opt);
> + if (p->pre_opt)
> + spi_unoptimize_message(&ctx->msg);
> + if (p->mode == ERR_MEM_OPS)
> + return;
> +
> + reinit_completion(&ctx->done);
> + KUNIT_ASSERT_EQ(test, spi_async(ctx->spi, &ctx->msg), 0);
> + KUNIT_ASSERT_NE(test, wait_for_completion_timeout(&ctx->done, msecs_to_jiffies(1000)), 0);
> + KUNIT_EXPECT_EQ(test, ctx->msg.status, 0);
> + /*
> + * Normal error-free situation after retry for the retryable error
> + * cases (bus_locked_async after spi_bus_unlock(), suspend_async after
> + * spi_controller_resume(), and pre_opt_locked after
> + * spi_unoptimize_message() + spi_bus_unlock()):
> + * - Expected (patched): (opt = 2, unopt = 2, state = false).
> + * Meaning: resubmitting the same ctx->msg after the transient error
> + * clears optimizes it a second time (opt = 2) and unoptimizes it on
> + * completion (unopt = 2), leaving ctx->msg unoptimized
> + * (state = false).
> + * - Broken behavior watched for (unpatched): (opt = 2, unopt = 1,
> + * state = false).
> + * Meaning: in bus_locked_async and suspend_async, the first failed
> + * attempt leaked ctx->msg in the optimized state (opt = 1,
> + * unopt = 0, state = true), so retrying the leaked message
> + * re-optimizes an already-optimized message (opt = 2) and only
> + * unoptimizes once when the retry completes (unopt = 1).
> + */
> + check_state(test, ctx, 2, 2, false);
> +}
> +
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 07/14] spi: add KUnit tests for message optimize state on queue errors
2026-11-02 17:29 ` David Lechner
@ 2026-11-03 22:41 ` Joseph CT Chan
0 siblings, 0 replies; 20+ messages in thread
From: Joseph CT Chan @ 2026-11-03 22:41 UTC (permalink / raw)
To: linux-spi
On 11/2/26 11:29 AM, David Lechner wrote:
> I was expecting a fake spi controller implementation rather than depending
> on an quirk of mem_ops to trigger the errors.
Thanks for the review. For v2 I am trying to address this with a small
change: the mem_ops controller is gone, and the test controller gets its
own transfer() that returns -EIO when a case asks for an error and
otherwise hands the message to the normal queue. That keeps one fake
controller for both the error and the retry. If you would rather see a
separate controller with its own legacy transfer() set up before
registration, let me know and I will do that instead.
> Can we get the field names here so we can see what true/false are actually
> doing?
Will do in v2, here and in the spi-mux test tables.
> Do we really need the huge AI-generated comments?
No. v2 cuts them down to one-line comments in all three test files.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 08/14] spi: spi-ti-qspi: add KUnit tests for transfer error finalization
2026-11-02 16:30 [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations Joseph CT Chan
` (6 preceding siblings ...)
2026-11-02 16:30 ` [PATCH 07/14] spi: add KUnit tests for message optimize state on queue errors Joseph CT Chan
@ 2026-11-02 16:30 ` Joseph CT Chan
2026-11-02 16:30 ` [PATCH 09/14] spi: spi-mux: add KUnit tests for forwarding errors and settings Joseph CT Chan
` (6 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Joseph CT Chan @ 2026-11-02 16:30 UTC (permalink / raw)
To: Mark Brown
Cc: Bart Van Assche, David Lechner, Conor Dooley, linux-spi,
Joseph CT Chan
From: Joseph CT Chan <josephctchan@google.com>
Add KUnit tests for ti_qspi_start_transfer_one() covering the transfer
error handling path.
On ARCH=um, emulate the TI QSPI controller MMIO via logic_iomem:
- ti_qspi_test: verify that queued and synchronous transfers hitting a
ti_qspi_poll_wc() write completion timeout complete with -ETIMEDOUT,
that QSPI_INVAL is written to end the frame, and that subsequent
transfers succeed cleanly once QSPI_SPI_STATUS_REG reports QSPI_WC.
Assisted-by: Claude, Gemini
Signed-off-by: Joseph CT Chan <josephctchan@google.com>
---
drivers/spi/Kconfig | 12 ++
drivers/spi/Makefile | 1 +
drivers/spi/tests/.kunitconfig | 4 +
drivers/spi/tests/Makefile | 1 +
drivers/spi/tests/spi-ti-qspi-kunit.c | 225 ++++++++++++++++++++++++++
5 files changed, 243 insertions(+)
create mode 100644 drivers/spi/tests/spi-ti-qspi-kunit.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index a7b343d124a9..feb9265b7d0f 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -1258,6 +1258,18 @@ config SPI_TI_QSPI
This device supports single, dual and quad read support, while
it only supports single write mode.
+config SPI_TI_QSPI_KUNIT_TEST
+ bool "KUnit test for TI QSPI controller driver" if !KUNIT_ALL_TESTS
+ depends on KUNIT=y && SPI_TI_QSPI=y && INDIRECT_IOMEM && !HAVE_CLK
+ default KUNIT_ALL_TESTS
+ help
+ This option builds the KUnit test suite for the TI QSPI controller
+ driver. It verifies that transfer timeout errors are properly
+ finalized with -ETIMEDOUT and that QSPI_INVAL is written to the
+ command register.
+
+ If unsure, say N.
+
config SPI_TLE62X0
tristate "Infineon TLE62X0 (for power switching)"
depends on SYSFS
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index d4a806b46b2c..24497d3936f6 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_SPI_SPIDEV) += spidev.o
obj-$(CONFIG_SPI_LOOPBACK_TEST) += spi-loopback-test.o
obj-$(CONFIG_SPI_DMA_KUNIT_TEST) += tests/
obj-$(CONFIG_SPI_CORE_KUNIT_TEST) += tests/
+obj-$(CONFIG_SPI_TI_QSPI_KUNIT_TEST) += tests/
# SPI master controller drivers (bus)
obj-$(CONFIG_SPI_AIROHA_SNFI) += spi-airoha-snfi.o
diff --git a/drivers/spi/tests/.kunitconfig b/drivers/spi/tests/.kunitconfig
index f781425d258e..354bafc42c2b 100644
--- a/drivers/spi/tests/.kunitconfig
+++ b/drivers/spi/tests/.kunitconfig
@@ -1,4 +1,8 @@
CONFIG_KUNIT=y
+CONFIG_KUNIT_UML_PCI=y
CONFIG_SPI=y
CONFIG_SPI_MASTER=y
CONFIG_SPI_CORE_KUNIT_TEST=y
+CONFIG_COMPILE_TEST=y
+CONFIG_SPI_TI_QSPI=y
+CONFIG_SPI_TI_QSPI_KUNIT_TEST=y
diff --git a/drivers/spi/tests/Makefile b/drivers/spi/tests/Makefile
index 6dbf53643619..a5a8fc9435d4 100644
--- a/drivers/spi/tests/Makefile
+++ b/drivers/spi/tests/Makefile
@@ -2,3 +2,4 @@
obj-$(CONFIG_SPI_DMA_KUNIT_TEST) += spi-dma-kunit.o
obj-$(CONFIG_SPI_CORE_KUNIT_TEST) += spi-core-kunit.o
+obj-$(CONFIG_SPI_TI_QSPI_KUNIT_TEST) += spi-ti-qspi-kunit.o
diff --git a/drivers/spi/tests/spi-ti-qspi-kunit.c b/drivers/spi/tests/spi-ti-qspi-kunit.c
new file mode 100644
index 000000000000..71ec2464a34a
--- /dev/null
+++ b/drivers/spi/tests/spi-ti-qspi-kunit.c
@@ -0,0 +1,225 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <kunit/test.h>
+#include <linux/completion.h>
+#include <linux/jiffies.h>
+#include <linux/kthread.h>
+#include <linux/logic_iomem.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/spi/spi.h>
+
+#define QSPI_SPI_CMD_REG 0x48
+#define QSPI_SPI_STATUS_REG 0x4c
+#define QSPI_WC BIT(1)
+#define QSPI_INVAL (4 << 16)
+
+struct ti_qspi_test_ctx {
+ struct platform_device *pdev;
+ struct spi_device *spi;
+ struct spi_message msg;
+ struct spi_transfer xfer;
+ struct completion done;
+ u8 tx[4];
+};
+
+static bool g_err, g_inval, g_region_registered;
+static struct resource g_iomem_res = DEFINE_RES_MEM(0x30000000, 0x2000);
+
+static unsigned long ti_qspi_io_r(void *p, unsigned int off, int sz)
+{
+ return (off == QSPI_SPI_STATUS_REG && !g_err) ? QSPI_WC : 0;
+}
+
+static void ti_qspi_io_w(void *p, unsigned int off, int sz, unsigned long val)
+{
+ if (off == QSPI_SPI_CMD_REG && (val & QSPI_INVAL))
+ g_inval = true;
+}
+
+static const struct logic_iomem_ops g_iops = {
+ .read = ti_qspi_io_r,
+ .write = ti_qspi_io_w,
+};
+
+static long ti_qspi_map(unsigned long o, size_t s, const struct logic_iomem_ops **ops, void **p)
+{
+ *ops = &g_iops;
+ return 0;
+}
+
+static const struct logic_iomem_region_ops g_rops = {
+ .map = ti_qspi_map,
+};
+
+static int ti_qspi_find_ctlr(struct device *d, void *data)
+{
+ *(struct spi_controller **)data = container_of(d, struct spi_controller, dev);
+ return 1;
+}
+
+static void kunit_done(void *p)
+{
+ complete(p);
+}
+
+static int ti_qspi_init(struct kunit *t)
+{
+ const struct resource res[] = {
+ DEFINE_RES_MEM(0x30000000, 0x1000),
+ DEFINE_RES_MEM(0x30001000, 0x1000),
+ DEFINE_RES_IRQ(1),
+ };
+ struct ti_qspi_test_ctx *ctx;
+ struct spi_board_info info = { .modalias = "ti-qspi-test" };
+ struct spi_controller *ctlr = NULL;
+ struct platform_device *pdev;
+
+ g_err = false;
+ g_inval = false;
+ ctx = kunit_kzalloc(t, sizeof(*ctx), GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(t, ctx);
+ t->priv = ctx;
+
+ pdev = platform_device_register_simple("ti-qspi", -1, res, ARRAY_SIZE(res));
+ KUNIT_ASSERT_NOT_ERR_OR_NULL(t, pdev);
+ ctx->pdev = pdev;
+
+ device_for_each_child(&pdev->dev, &ctlr, ti_qspi_find_ctlr);
+ if (!ctlr) {
+ platform_device_unregister(pdev);
+ ctx->pdev = NULL;
+ KUNIT_FAIL_AND_ABORT(t, "TI QSPI controller device not found");
+ }
+
+ ctlr->max_speed_hz = 50000000;
+ ctx->spi = spi_new_device(ctlr, &info);
+ if (!ctx->spi) {
+ pm_runtime_get_noresume(&pdev->dev);
+ platform_device_unregister(pdev);
+ ctx->pdev = NULL;
+ KUNIT_FAIL_AND_ABORT(t, "Failed to create spi child device");
+ }
+
+ init_completion(&ctx->done);
+ memcpy(ctx->tx, "ABCD", sizeof(ctx->tx));
+ ctx->xfer.tx_buf = ctx->tx;
+ ctx->xfer.len = sizeof(ctx->tx);
+ spi_message_init_with_transfers(&ctx->msg, &ctx->xfer, 1);
+ return 0;
+}
+
+static void ti_qspi_exit(struct kunit *t)
+{
+ struct ti_qspi_test_ctx *ctx = t->priv;
+
+ if (!ctx)
+ return;
+ if (ctx->spi)
+ spi_unregister_device(ctx->spi);
+ if (ctx->pdev) {
+ pm_runtime_get_noresume(&ctx->pdev->dev);
+ platform_device_unregister(ctx->pdev);
+ }
+}
+
+static void check_sync(struct kunit *t, struct spi_device *s, struct spi_message *m, int exp)
+{
+ g_inval = false;
+ /*
+ * Good situation (patched): ti_qspi_start_transfer_one() sets
+ * m->status = status (exp, either -ETIMEDOUT on error or 0 on
+ * recovery) before calling spi_finalize_current_message(host), and
+ * spi_sync() returns m->status.
+ * Bad situation (unpatched): on transfer error (exp == -ETIMEDOUT),
+ * ti_qspi_start_transfer_one() returns early without updating
+ * m->status or finalizing the message, leaving m->status stuck at
+ * -EINPROGRESS (-115), which __spi_sync() returns on the noqueue path.
+ */
+ KUNIT_EXPECT_EQ(t, spi_sync(s, m), exp);
+ KUNIT_EXPECT_EQ(t, m->status, exp);
+ /*
+ * Good situation (patched): spi_sync() calls
+ * spi_maybe_optimize_message() at entry (setting m->optimized = true
+ * while the transfer is in flight), and ti_qspi_start_transfer_one()
+ * calls spi_finalize_current_message(host) on both success and error
+ * paths, which runs spi_maybe_unoptimize_message() and resets
+ * m->optimized = false before spi_sync() returns.
+ * Bad situation (unpatched): when qspi_transfer_msg() fails with
+ * -ETIMEDOUT, ti_qspi_start_transfer_one() returns early without
+ * calling spi_finalize_current_message(host), so
+ * spi_maybe_unoptimize_message() never runs and m->optimized remains
+ * stuck at true after spi_sync() returns.
+ */
+ KUNIT_EXPECT_FALSE(t, m->optimized);
+ /*
+ * Good situation (patched): ti_qspi_start_transfer_one() writes
+ * QSPI_INVAL to QSPI_SPI_CMD_REG (g_inval = true) in its epilogue to
+ * terminate the hardware frame even when a transfer fails.
+ * Bad situation (unpatched): the early return on error skips writing
+ * QSPI_INVAL, leaving g_inval == false.
+ */
+ KUNIT_EXPECT_TRUE(t, g_inval);
+}
+
+static void ti_qspi_test(struct kunit *t)
+{
+ struct ti_qspi_test_ctx *ctx = t->priv;
+ unsigned long timeout;
+
+ g_err = true;
+ check_sync(t, ctx->spi, &ctx->msg, -ETIMEDOUT);
+
+ /*
+ * Exercise the queued kworker path via spi_async() with g_err = true:
+ * - Good situation (patched): ti_qspi_start_transfer_one() finalizes
+ * the failed message, completing ctx->done, setting
+ * ctx->msg.status = -ETIMEDOUT, resetting ctx->msg.optimized = false,
+ * and writing QSPI_INVAL (g_inval = true).
+ * - Bad situation (unpatched): skipping spi_finalize_current_message()
+ * never signals ctx->done (causing wait_for_completion_timeout() to
+ * time out) and leaves ctx->msg.optimized == true.
+ */
+ ctx->msg.complete = kunit_done;
+ ctx->msg.context = &ctx->done;
+ g_inval = false;
+ reinit_completion(&ctx->done);
+ KUNIT_ASSERT_EQ(t, spi_async(ctx->spi, &ctx->msg), 0);
+
+ timeout = wait_for_completion_timeout(&ctx->done, msecs_to_jiffies(4000));
+ kthread_flush_worker(ctx->spi->controller->kworker);
+ if (!timeout)
+ KUNIT_FAIL_AND_ABORT(t, "ti-qspi async transfer timed out");
+
+ KUNIT_EXPECT_EQ(t, ctx->msg.status, -ETIMEDOUT);
+ KUNIT_EXPECT_FALSE(t, ctx->msg.optimized);
+ KUNIT_EXPECT_TRUE(t, g_inval);
+ /* Clear the injected fault and verify a subsequent transfer succeeds. */
+ g_err = false;
+ check_sync(t, ctx->spi, &ctx->msg, 0);
+}
+
+static int ti_qspi_suite_init(struct kunit_suite *s)
+{
+ int ret = g_region_registered ? 0 : logic_iomem_add_region(&g_iomem_res, &g_rops);
+
+ g_region_registered = !ret;
+ return ret;
+}
+
+static struct kunit_case ti_qspi_test_cases[] = {
+ KUNIT_CASE_SLOW(ti_qspi_test),
+ {}
+};
+
+static struct kunit_suite ti_qspi_test_suite = {
+ .name = "spi_ti_qspi",
+ .suite_init = ti_qspi_suite_init,
+ .init = ti_qspi_init,
+ .exit = ti_qspi_exit,
+ .test_cases = ti_qspi_test_cases,
+};
+
+kunit_test_suite(ti_qspi_test_suite);
+
+MODULE_DESCRIPTION("KUnit tests for TI QSPI controller driver");
+MODULE_LICENSE("GPL");
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 09/14] spi: spi-mux: add KUnit tests for forwarding errors and settings
2026-11-02 16:30 [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations Joseph CT Chan
` (7 preceding siblings ...)
2026-11-02 16:30 ` [PATCH 08/14] spi: spi-ti-qspi: add KUnit tests for transfer error finalization Joseph CT Chan
@ 2026-11-02 16:30 ` Joseph CT Chan
2026-11-02 16:30 ` [PATCH 10/14] spi: Split spi_write_then_read() Joseph CT Chan
` (5 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Joseph CT Chan @ 2026-11-02 16:30 UTC (permalink / raw)
To: Mark Brown
Cc: Bart Van Assche, David Lechner, Conor Dooley, linux-spi,
Joseph CT Chan
From: Joseph CT Chan <josephctchan@google.com>
Add a KUnit test suite (spi_mux) with a Device Tree overlay that
instantiates a fake mux controller, a fake parent SPI controller, the
real spi-mux driver, and two child SPI devices.
The suite covers three areas:
- spi_mux_error_test: forwarding errors (locked parent bus, invalid
transfer length, mux select failure, and parent spi_setup() failure),
verifying that the message is finalized, the mux is released, the
caller's message fields and optimize state are restored, and
subsequent transfers succeed.
- spi_mux_cs_delay_test: CS setup, hold, and inactive delay merging
across ns, us, and SCK units, including zero-value SCK delays.
- spi_mux_settings_lifecycle_test: re-applying child mode, speed,
bits_per_word, and delay changes after spi_setup(), switching
between children with different delays, and restoring the spi-mux
device's own delays on driver unbind.
Assisted-by: Claude, Gemini
Signed-off-by: Joseph CT Chan <josephctchan@google.com>
---
drivers/spi/Kconfig | 14 +
drivers/spi/Makefile | 1 +
drivers/spi/tests/.kunitconfig | 4 +
drivers/spi/tests/Makefile | 2 +
drivers/spi/tests/kunit_spi_mux.dtso | 42 ++
drivers/spi/tests/spi-mux-kunit.c | 581 +++++++++++++++++++++++++++
6 files changed, 644 insertions(+)
create mode 100644 drivers/spi/tests/kunit_spi_mux.dtso
create mode 100644 drivers/spi/tests/spi-mux-kunit.c
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index feb9265b7d0f..9a36bf9cf94f 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -1386,6 +1386,20 @@ config SPI_MUX
to be chip selects on this controller. It is still necessary to
select one or more specific mux-controller drivers.
+config SPI_MUX_KUNIT_TEST
+ tristate "KUnit tests for spi-mux" if !KUNIT_ALL_TESTS
+ depends on KUNIT && SPI_MUX && OF_OVERLAY
+ select DTC
+ default KUNIT_ALL_TESTS
+ help
+ Build KUnit tests for the SPI multiplexer driver. A device tree
+ overlay adds a fake parent controller, a test mux chip and an
+ spi-mux device with two children. The tests check that a message
+ which cannot be forwarded is completed with an error and releases
+ the mux, and that child settings and delays reach the parent.
+
+ If unsure say N.
+
#
# There are lots of SPI device types, with sensors and memory
# being probably the most widely used ones.
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 24497d3936f6..9826626995a9 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_SPI_LOOPBACK_TEST) += spi-loopback-test.o
obj-$(CONFIG_SPI_DMA_KUNIT_TEST) += tests/
obj-$(CONFIG_SPI_CORE_KUNIT_TEST) += tests/
obj-$(CONFIG_SPI_TI_QSPI_KUNIT_TEST) += tests/
+obj-$(CONFIG_SPI_MUX_KUNIT_TEST) += tests/
# SPI master controller drivers (bus)
obj-$(CONFIG_SPI_AIROHA_SNFI) += spi-airoha-snfi.o
diff --git a/drivers/spi/tests/.kunitconfig b/drivers/spi/tests/.kunitconfig
index 354bafc42c2b..1631a16ce6ec 100644
--- a/drivers/spi/tests/.kunitconfig
+++ b/drivers/spi/tests/.kunitconfig
@@ -6,3 +6,7 @@ CONFIG_SPI_CORE_KUNIT_TEST=y
CONFIG_COMPILE_TEST=y
CONFIG_SPI_TI_QSPI=y
CONFIG_SPI_TI_QSPI_KUNIT_TEST=y
+CONFIG_OF=y
+CONFIG_OF_OVERLAY=y
+CONFIG_SPI_MUX=y
+CONFIG_SPI_MUX_KUNIT_TEST=y
diff --git a/drivers/spi/tests/Makefile b/drivers/spi/tests/Makefile
index a5a8fc9435d4..0ec04ecd4d55 100644
--- a/drivers/spi/tests/Makefile
+++ b/drivers/spi/tests/Makefile
@@ -3,3 +3,5 @@
obj-$(CONFIG_SPI_DMA_KUNIT_TEST) += spi-dma-kunit.o
obj-$(CONFIG_SPI_CORE_KUNIT_TEST) += spi-core-kunit.o
obj-$(CONFIG_SPI_TI_QSPI_KUNIT_TEST) += spi-ti-qspi-kunit.o
+obj-$(CONFIG_SPI_MUX_KUNIT_TEST) += spi-mux-kunit-test.o
+spi-mux-kunit-test-y := spi-mux-kunit.o kunit_spi_mux.dtbo.o
diff --git a/drivers/spi/tests/kunit_spi_mux.dtso b/drivers/spi/tests/kunit_spi_mux.dtso
new file mode 100644
index 000000000000..48e72ba8e217
--- /dev/null
+++ b/drivers/spi/tests/kunit_spi_mux.dtso
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0
+/dts-v1/;
+/plugin/;
+
+&{/} {
+ tmux: kunit-spi-mux-chip {
+ compatible = "test,spi-mux-kunit-chip";
+ #mux-control-cells = <0>;
+ };
+
+ kunit-spi-mux-parent {
+ compatible = "test,spi-mux-kunit-parent";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ spi@0 {
+ compatible = "spi-mux";
+ reg = <0>;
+ spi-max-frequency = <1000000>;
+ spi-cs-setup-delay-ns = <1000>;
+ spi-cs-hold-delay-ns = <300>;
+ mux-controls = <&tmux>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ device@0 {
+ compatible = "test,spi-mux-kunit-child";
+ reg = <0>;
+ spi-max-frequency = <1000000>;
+ };
+
+ device@1 {
+ compatible = "test,spi-mux-kunit-child";
+ reg = <1>;
+ spi-max-frequency = <2000000>;
+ spi-cs-setup-delay-ns = <1500>;
+ spi-cs-hold-delay-ns = <100>;
+ spi-cs-inactive-delay-ns = <700>;
+ };
+ };
+ };
+};
diff --git a/drivers/spi/tests/spi-mux-kunit.c b/drivers/spi/tests/spi-mux-kunit.c
new file mode 100644
index 000000000000..53a4e490ddb2
--- /dev/null
+++ b/drivers/spi/tests/spi-mux-kunit.c
@@ -0,0 +1,581 @@
+// SPDX-License-Identifier: GPL-2.0
+// KUnit tests for spi-mux error handling and settings forwarding.
+#include <kunit/of.h>
+#include <kunit/platform_device.h>
+#include <kunit/test.h>
+#include <linux/completion.h>
+#include <linux/jiffies.h>
+#include <linux/kthread.h>
+#include <linux/mux/consumer.h>
+#include <linux/mux/driver.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/spi/spi.h>
+
+#define PFX "/kunit-spi-mux-parent/spi@0"
+
+struct spi_mux_test_ctx {
+ struct mux_control *mux;
+ struct spi_controller *parent;
+ struct spi_device *child[2], *mux_spi;
+ int opt_cnt, unopt_cnt, mux_err, setup_err;
+ u32 last_speed_hz, last_mode;
+ u8 last_bpw;
+ struct spi_delay last_w, last_s, last_h, last_in;
+ struct spi_transfer xfer;
+ struct spi_message msg;
+ struct completion done;
+ u8 tx[4];
+};
+
+static struct spi_mux_test_ctx *g_mux_ctx;
+
+static void kunit_done(void *p)
+{
+ complete(p);
+}
+
+/*
+ * Verify the CS and word delay values captured on the parent controller during
+ * the most recent transfer:
+ * @s: expected cs_setup.value (ctx->last_s.value).
+ * @h: expected cs_hold.value (ctx->last_h.value).
+ * @in: expected cs_inactive.value (ctx->last_in.value).
+ * @w: expected word_delay.value (ctx->last_w.value).
+ *
+ * Baseline DT defaults from kunit_spi_mux.dtso:
+ * - ctx->mux_spi (spi@0): (s = 1000 ns, h = 300 ns, in = 0, w = 0)
+ * - child[0] (device@0, 1 MHz): (s = 0, h = 0, in = 0, w = 0)
+ * - child[1] (device@1, 2 MHz): (s = 1500 ns, h = 100 ns, in = 700 ns, w = 0)
+ */
+static void check_delays(struct kunit *test, struct spi_mux_test_ctx *ctx,
+ u16 s, u16 h, u16 in, u16 w)
+{
+ KUNIT_EXPECT_EQ(test, ctx->last_s.value, s);
+ KUNIT_EXPECT_EQ(test, ctx->last_h.value, h);
+ KUNIT_EXPECT_EQ(test, ctx->last_in.value, in);
+ KUNIT_EXPECT_EQ(test, ctx->last_w.value, w);
+}
+
+static void check_d(struct kunit *test, struct spi_delay d, struct spi_delay exp)
+{
+ KUNIT_EXPECT_EQ(test, d.value, exp.value);
+ KUNIT_EXPECT_EQ(test, d.unit, exp.unit);
+}
+
+static void check_msg_restored(struct kunit *test, struct spi_mux_test_ctx *ctx,
+ struct spi_device *spi)
+{
+ kthread_flush_worker(spi->controller->kworker);
+ kthread_flush_worker(ctx->parent->kworker);
+ /*
+ * Verify the mux semaphore was released after the failed transfer:
+ * - Good situation (patched): spi-mux released the mux semaphore on
+ * error (spi_mux_select() failure or spi_async() failure), so
+ * mux_control_try_select() succeeds with 0.
+ * - Bad situation (unpatched): ERR_LOCKED, ERR_ODD_LEN, and ERR_SETUP
+ * return early with the mux semaphore still held, so
+ * mux_control_try_select() fails with -EBUSY.
+ */
+ KUNIT_ASSERT_EQ(test, 0, mux_control_try_select(ctx->mux, 0));
+ mux_control_deselect(ctx->mux);
+ /*
+ * Verify the child caller's spi_message fields were restored:
+ * - Good situation (patched): spi_mux_complete_cb() restored the child
+ * caller's spi, complete, and context pointers on spi_async()
+ * failure.
+ * - Bad situation (unpatched): ERR_LOCKED and ERR_ODD_LEN return early
+ * after overwriting msg.spi = priv->spi,
+ * msg.complete = spi_mux_complete_cb, and msg.context = priv.
+ */
+ KUNIT_EXPECT_PTR_EQ(test, ctx->msg.spi, spi);
+ KUNIT_EXPECT_PTR_EQ(test, ctx->msg.complete, &kunit_done);
+ KUNIT_EXPECT_PTR_EQ(test, ctx->msg.context, &ctx->done);
+ /*
+ * Verify parent message optimization state was cleaned up:
+ * - Good situation (patched): ctx->parent has max_transfer_size = 2,
+ * so when ERR_LOCKED fails inside spi_async(priv->spi, m) after
+ * __spi_optimize_message() splits the 4-byte transfer, spi_async()
+ * unoptimizes m (optimized = false, resources empty,
+ * opt_cnt == unopt_cnt).
+ * - Bad situation (unpatched): spi_async() leaks the optimized state
+ * on -EBUSY, leaving msg.optimized == true, split transfers in
+ * msg.resources, and opt_cnt (1) != unopt_cnt (0).
+ */
+ KUNIT_EXPECT_FALSE(test, ctx->msg.optimized);
+ KUNIT_EXPECT_TRUE(test, list_empty(&ctx->msg.resources));
+ KUNIT_EXPECT_EQ(test, ctx->opt_cnt, ctx->unopt_cnt);
+}
+
+static int spi_mux_test_chip_set(struct mux_control *mux, int state)
+{
+ return g_mux_ctx ? READ_ONCE(g_mux_ctx->mux_err) : 0;
+}
+
+static const struct mux_control_ops spi_mux_test_chip_ops = {
+ .set = spi_mux_test_chip_set,
+};
+
+static int spi_mux_test_parent_opt(struct spi_message *msg)
+{
+ if (!g_mux_ctx)
+ return -ENODEV;
+ g_mux_ctx->opt_cnt++;
+ return 0;
+}
+
+static int spi_mux_test_parent_unopt(struct spi_message *msg)
+{
+ if (!g_mux_ctx)
+ return -ENODEV;
+ g_mux_ctx->unopt_cnt++;
+ return 0;
+}
+
+static int spi_mux_test_parent_setup(struct spi_device *spi)
+{
+ if (!g_mux_ctx)
+ return -ENODEV;
+ return xchg(&g_mux_ctx->setup_err, 0);
+}
+
+static void spi_mux_test_parent_set_cs(struct spi_device *spi, bool enable)
+{
+ if (!g_mux_ctx)
+ return;
+ g_mux_ctx->last_mode = spi->mode;
+ g_mux_ctx->last_s = spi->cs_setup;
+ g_mux_ctx->last_h = spi->cs_hold;
+ g_mux_ctx->last_in = spi->cs_inactive;
+}
+
+static int spi_mux_parent_xfer(struct spi_controller *c, struct spi_device *s,
+ struct spi_transfer *x)
+{
+ if (!g_mux_ctx)
+ return -ENODEV;
+ g_mux_ctx->last_speed_hz = x->speed_hz;
+ g_mux_ctx->last_bpw = x->bits_per_word;
+ g_mux_ctx->last_w = x->word_delay;
+ return 0;
+}
+
+static size_t spi_mux_test_parent_max_transfer_size(struct spi_device *spi)
+{
+ return 2;
+}
+
+static int spi_mux_test_probe(struct platform_device *pdev)
+{
+ struct spi_controller *ctlr;
+ struct mux_chip *chip;
+
+ if (!g_mux_ctx)
+ return -ENODEV;
+ if (of_device_is_compatible(pdev->dev.of_node, "test,spi-mux-kunit-chip")) {
+ chip = devm_mux_chip_alloc(&pdev->dev, 1, 0);
+ if (IS_ERR(chip))
+ return PTR_ERR(chip);
+ chip->ops = &spi_mux_test_chip_ops;
+ chip->mux[0].states = 2;
+ g_mux_ctx->mux = &chip->mux[0];
+ return devm_mux_chip_register(&pdev->dev, chip);
+ }
+ ctlr = devm_spi_alloc_host(&pdev->dev, 0);
+ if (!ctlr)
+ return -ENOMEM;
+ ctlr->bus_num = -1;
+ ctlr->mode_bits = SPI_CPOL | SPI_CPHA | SPI_CS_HIGH | SPI_CS_WORD;
+ ctlr->bits_per_word_mask = SPI_BPW_MASK(8) | SPI_BPW_MASK(16);
+ ctlr->max_transfer_size = spi_mux_test_parent_max_transfer_size;
+ ctlr->setup = spi_mux_test_parent_setup;
+ ctlr->set_cs = spi_mux_test_parent_set_cs;
+ ctlr->transfer_one = spi_mux_parent_xfer;
+ ctlr->optimize_message = spi_mux_test_parent_opt;
+ ctlr->unoptimize_message = spi_mux_test_parent_unopt;
+ g_mux_ctx->parent = ctlr;
+ return devm_spi_register_controller(&pdev->dev, ctlr);
+}
+
+static const struct of_device_id spi_mux_test_match[] = {
+ { .compatible = "test,spi-mux-kunit-chip" },
+ { .compatible = "test,spi-mux-kunit-parent" },
+ {}
+};
+
+static struct platform_driver spi_mux_test_driver = {
+ .probe = spi_mux_test_probe,
+ .driver = {
+ .name = "spi-mux-kunit-driver",
+ .of_match_table = spi_mux_test_match,
+ .probe_type = PROBE_FORCE_SYNCHRONOUS,
+ },
+};
+
+KUNIT_DEFINE_ACTION_WRAPPER(spi_mux_test_put_dev, put_device, struct device *);
+
+static struct spi_device *spi_mux_test_find_dev(struct kunit *test, const char *path)
+{
+ struct device_node *np = of_find_node_by_path(path);
+ struct device *dev = bus_find_device_by_of_node(&spi_bus_type, np);
+
+ of_node_put(np);
+ KUNIT_ASSERT_NOT_NULL(test, dev);
+ KUNIT_ASSERT_EQ(test, 0, kunit_add_action_or_reset(test, spi_mux_test_put_dev, dev));
+ return to_spi_device(dev);
+}
+
+static int spi_mux_test_init(struct kunit *test)
+{
+ struct spi_mux_test_ctx *ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
+
+ KUNIT_ASSERT_NOT_NULL(test, ctx);
+ test->priv = ctx;
+ g_mux_ctx = ctx;
+ init_completion(&ctx->done);
+ ctx->xfer.tx_buf = ctx->tx;
+ ctx->xfer.len = sizeof(ctx->tx);
+ spi_message_init_with_transfers(&ctx->msg, &ctx->xfer, 1);
+ ctx->msg.complete = kunit_done;
+ ctx->msg.context = &ctx->done;
+ KUNIT_ASSERT_EQ(test, 0, kunit_platform_driver_register(test, &spi_mux_test_driver));
+ KUNIT_ASSERT_EQ(test, 0, of_overlay_apply_kunit(test, kunit_spi_mux));
+ ctx->child[0] = spi_mux_test_find_dev(test, PFX "/device@0");
+ ctx->child[1] = spi_mux_test_find_dev(test, PFX "/device@1");
+ ctx->mux_spi = to_spi_device(ctx->child[0]->controller->dev.parent);
+ return 0;
+}
+
+static void spi_mux_test_exit(struct kunit *test)
+{
+ struct spi_mux_test_ctx *ctx = test->priv;
+
+ if (ctx) {
+ if (ctx->child[0])
+ kthread_flush_worker(ctx->child[0]->controller->kworker);
+ if (ctx->parent)
+ kthread_flush_worker(ctx->parent->kworker);
+ if (ctx->mux) {
+ int ret;
+
+ ctx->mux_err = 0;
+ ret = mux_control_try_select(ctx->mux, 0);
+ if (!ret || ret == -EBUSY)
+ mux_control_deselect(ctx->mux);
+ }
+ }
+ g_mux_ctx = NULL;
+}
+
+static int spi_mux_test_send(struct kunit *test, struct spi_device *spi,
+ unsigned int len, u8 bpw)
+{
+ struct spi_mux_test_ctx *ctx = test->priv;
+ int ret;
+
+ reinit_completion(&ctx->done);
+ ctx->xfer.len = len;
+ ctx->xfer.bits_per_word = bpw;
+ ctx->xfer.speed_hz = 0;
+ ctx->xfer.word_delay = (struct spi_delay){ 0 };
+ ctx->msg.status = -EINPROGRESS;
+ ctx->msg.actual_length = 0;
+ ctx->msg.spi = spi;
+ ret = spi_async(spi, &ctx->msg);
+ if (ret)
+ return ret;
+ return wait_for_completion_timeout(&ctx->done, msecs_to_jiffies(5000)) ?
+ ctx->msg.status : -ETIMEDOUT;
+}
+
+static int spi_mux_test_send_locked(struct kunit *test, struct spi_device *spi,
+ struct spi_controller *parent,
+ unsigned int len, u8 bpw)
+{
+ int ret;
+
+ spi_bus_lock(parent);
+ ret = spi_mux_test_send(test, spi, len, bpw);
+ spi_bus_unlock(parent);
+
+ return ret;
+}
+
+static void send_4b(struct kunit *test, struct spi_device *spi)
+{
+ struct spi_mux_test_ctx *ctx = test->priv;
+
+ KUNIT_ASSERT_EQ(test, 0, spi_mux_test_send(test, spi, 4, 0));
+ kthread_flush_worker(spi->controller->kworker);
+ kthread_flush_worker(ctx->parent->kworker);
+}
+
+static void check_sync_resend(struct kunit *test, struct spi_device *spi, u32 exp_hz)
+{
+ struct spi_mux_test_ctx *ctx = test->priv;
+
+ ctx->xfer.len = 4;
+ ctx->xfer.bits_per_word = 0;
+ ctx->xfer.speed_hz = 0;
+ ctx->msg.spi = spi;
+ KUNIT_EXPECT_EQ(test, 0, spi_sync(spi, &ctx->msg));
+ KUNIT_EXPECT_EQ(test, ctx->msg.actual_length, 4);
+ KUNIT_EXPECT_EQ(test, ctx->last_speed_hz, exp_hz);
+}
+
+enum spi_mux_err_type {
+ ERR_LOCKED,
+ ERR_ODD_LEN,
+ ERR_SELECT,
+ ERR_SETUP,
+};
+
+struct spi_mux_err_case {
+ const char *name;
+ enum spi_mux_err_type type;
+ int exp;
+};
+
+static const struct spi_mux_err_case spi_mux_err_cases[] = {
+ { "bus_locked", ERR_LOCKED, -EBUSY },
+ { "rejects_odd_len", ERR_ODD_LEN, -EINVAL },
+ { "select_fails", ERR_SELECT, -EIO },
+ { "parent_setup_fails", ERR_SETUP, -EIO },
+};
+
+struct spi_mux_delay_case {
+ const char *name;
+ u8 child;
+ struct spi_delay set_s, set_h, set_in, exp_s, exp_h, exp_in;
+};
+
+#define NS(v) { (v), SPI_DELAY_UNIT_NSECS }
+#define US(v) { (v), SPI_DELAY_UNIT_USECS }
+#define SCK(v) { (v), SPI_DELAY_UNIT_SCK }
+
+/*
+ * Each case exercises a distinct branch of spi_mux_merge_cs_delay() against
+ * mux_spi's DT baseline delays (s = 1000 ns, h = 300 ns, in = 0):
+ * 1. longer (child[1]): child[1] DT s = 1500 ns (> mux 1000 ns), runtime
+ * h = 500 ns (> mux 300 ns), DT in = 700 ns (> mux 0 ns) -> selects child
+ * delays (NS(1500), NS(500), NS(700)).
+ * 2. tie (child[0]): runtime s = US(1) (1000 ns, tied with mux NS(1000)),
+ * h = 0, in = NS(200) -> on tie (child_ns > mux_ns is false), keeps mux's
+ * NS(1000) unit/value, resets h to mux NS(300), and selects child
+ * in = NS(200).
+ * 3. sck_zero (child[0]): runtime in = SCK(4) while mux in is 0 (!mux->value)
+ * -> spi_delay_to_ns() returns -EINVAL (child_ns < 0), but because mux
+ * delay is 0, passes through SCK(4).
+ * 4. sck_set (child[0]): runtime s = SCK(4) while mux s is non-zero
+ * (NS(1000)), and h = NS(500) -> logs warning and falls back to mux
+ * NS(1000) for s, while selecting NS(500) for h.
+ *
+ * Bad situation (unpatched): spi_mux_select() never copies CS delays from the
+ * child, leaving ctx->mux_spi at (NS(1000), NS(300), 0) for all 4 cases.
+ */
+static const struct spi_mux_delay_case spi_mux_delay_cases[] = {
+ { "longer", 1, {}, NS(500), {}, NS(1500), NS(500), NS(700) },
+ { "tie", 0, US(1), {}, NS(200), NS(1000), NS(300), NS(200) },
+ { "sck_zero", 0, {}, {}, SCK(4), NS(1000), NS(300), SCK(4) },
+ { "sck_set", 0, SCK(4), NS(500), {}, NS(1000), NS(500), {} },
+};
+
+static void spi_mux_err_desc(const struct spi_mux_err_case *p, char *desc)
+{
+ strscpy(desc, p->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(spi_mux_err, spi_mux_err_cases, spi_mux_err_desc);
+
+static void spi_mux_delay_desc(const struct spi_mux_delay_case *p, char *desc)
+{
+ strscpy(desc, p->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(spi_mux_delay, spi_mux_delay_cases, spi_mux_delay_desc);
+
+static void spi_mux_error_test(struct kunit *test)
+{
+ const struct spi_mux_err_case *p = test->param_value;
+ struct spi_mux_test_ctx *ctx = test->priv;
+ int child = (p->type == ERR_SELECT || p->type == ERR_SETUP);
+ int len = (p->type == ERR_ODD_LEN) ? 3 : 4;
+ u8 bpw = (p->type == ERR_ODD_LEN) ? 16 : 0;
+ int ret;
+
+ /*
+ * spi-mux skips mux_control_select() and spi_setup(priv->spi) when
+ * priv->current_cs == spi_get_chipselect(spi, 0). Prime
+ * priv->current_cs = 0 on child[0] so targeting child[1] (child = 1)
+ * forces a channel switch that executes both mux_control_select()
+ * (ERR_SELECT) and spi_setup(priv->spi) (ERR_SETUP).
+ */
+ if (p->type >= ERR_SELECT) {
+ send_4b(test, ctx->child[0]);
+ ctx->mux_err = (p->type == ERR_SELECT) ? -EIO : 0;
+ ctx->setup_err = (p->type == ERR_SETUP) ? -EIO : 0;
+ }
+
+ if (p->type == ERR_LOCKED)
+ ret = spi_mux_test_send_locked(test, ctx->child[child], ctx->parent, len, bpw);
+ else
+ ret = spi_mux_test_send(test, ctx->child[child], len, bpw);
+
+ ctx->mux_err = 0;
+ ctx->setup_err = 0;
+ if (ret == -ETIMEDOUT) {
+ kthread_flush_worker(ctx->child[child]->controller->kworker);
+ kthread_flush_worker(ctx->parent->kworker);
+ }
+ /*
+ * Good situation (patched): spi_mux_transfer_one_message() finalizes
+ * ctx->msg with p->exp (-EBUSY, -EINVAL, or -EIO).
+ * Bad situation (unpatched): spi_finalize_current_message() is skipped
+ * on all 4 error paths so spi_mux_test_send() times out (-ETIMEDOUT).
+ */
+ KUNIT_ASSERT_EQ(test, ret, p->exp);
+ check_msg_restored(test, ctx, ctx->child[child]);
+ /*
+ * In ERR_SETUP, spi_mux_select(child[1]) overwrites
+ * priv->spi->max_speed_hz with child[1]'s 2000000 Hz before
+ * spi_setup(priv->spi) fails:
+ * - Good situation (patched): spi_mux_select() invalidates
+ * priv->current_cs = SPI_MUX_NO_CS on error, so transferring on
+ * child[0] re-applies 1000000 Hz.
+ * - Bad situation (if priv->current_cs remained 0):
+ * spi_mux_select(child[0]) would see current_cs == 0 and return
+ * early, running child[0] at child[1]'s 2000000 Hz.
+ */
+ if (p->type == ERR_SETUP)
+ check_sync_resend(test, ctx->child[0], 1000000);
+ check_sync_resend(test, ctx->child[child], child ? 2000000 : 1000000);
+}
+
+static void spi_mux_cs_delay_test(struct kunit *test)
+{
+ const struct spi_mux_delay_case *p = test->param_value;
+ struct spi_mux_test_ctx *ctx = test->priv;
+ struct spi_device *spi = ctx->child[p->child];
+
+ /*
+ * Transfer on child[1] with cs_hold = NS(500) first to overwrite
+ * priv->spi->cs_hold with 500 ns, so when switching to child[0] whose
+ * cs_hold.value == 0 (!child->value), the test verifies
+ * spi_mux_merge_cs_delay() actively writes *dst = *mux (300 ns)
+ * instead of leaving child[1]'s 500 ns stale.
+ */
+ if (!p->child) {
+ ctx->child[1]->cs_hold = (struct spi_delay)NS(500);
+ send_4b(test, ctx->child[1]);
+ }
+ spi->cs_setup = p->set_s.value ? p->set_s : spi->cs_setup;
+ spi->cs_hold = p->set_h.value ? p->set_h : spi->cs_hold;
+ spi->cs_inactive = p->set_in.value ? p->set_in : spi->cs_inactive;
+ send_4b(test, spi);
+ check_d(test, ctx->last_s, p->exp_s);
+ check_d(test, ctx->last_h, p->exp_h);
+ check_d(test, ctx->last_in, p->exp_in);
+}
+
+static void spi_mux_settings_lifecycle_test(struct kunit *test)
+{
+ struct spi_mux_test_ctx *ctx = test->priv;
+ struct device *dev = &ctx->mux_spi->dev;
+ struct spi_device *spi = ctx->child[0];
+
+ /*
+ * Phase 1 & 2: Select child[0] (priv->current_cs = 0), then mutate
+ * child[0]'s mode (SPI_CS_HIGH), max_speed_hz (500000), bits_per_word
+ * (16), and cs_setup (1200 ns), call spi_setup(child[0]), and transfer
+ * on child[0] again without switching channels:
+ * - Good situation (patched): spi_mux_setup() sets
+ * priv->current_cs = SPI_MUX_NO_CS, forcing the next transfer on
+ * child[0] to re-apply (500000 Hz, 16-bit, SPI_CS_HIGH, 1200 ns) to
+ * the parent.
+ * - Bad situation (unpatched): spi_mux_setup() leaves
+ * priv->current_cs == 0, so spi_mux_select(child[0]) returns early
+ * and the transfer runs with stale (1000000 Hz, 8-bit, 1000 ns)
+ * settings.
+ */
+ send_4b(test, spi);
+ KUNIT_EXPECT_EQ(test, ctx->opt_cnt, 1);
+ KUNIT_EXPECT_EQ(test, ctx->unopt_cnt, 1);
+ spi->mode ^= SPI_CS_HIGH;
+ spi->max_speed_hz = 500000;
+ spi->bits_per_word = 16;
+ spi->cs_setup = (struct spi_delay)NS(1200);
+ KUNIT_ASSERT_EQ(test, 0, spi_setup(spi));
+ send_4b(test, spi);
+ KUNIT_EXPECT_EQ(test, ctx->last_speed_hz, 500000);
+ KUNIT_EXPECT_EQ(test, ctx->last_bpw, 16);
+ KUNIT_EXPECT_TRUE(test, !!(ctx->last_mode & SPI_CS_HIGH));
+ KUNIT_EXPECT_EQ(test, ctx->last_s.value, 1200);
+ spi->cs_setup = (struct spi_delay){};
+ KUNIT_ASSERT_EQ(test, 0, spi_setup(spi));
+ /*
+ * Phase 3: Switch child[1] -> child[0] -> child[1]:
+ * - Good situation (patched): switching to child[1] applies child[1]'s
+ * merged CS delays (1500, 500, 700) and word_delay (2 us); switching
+ * back to child[0] (delays cleared to 0) restores the mux baseline
+ * (1000, 300, 0, 0); switching to child[1] again re-applies
+ * (1500, 500, 700, 2).
+ * - Bad situation (unpatched): child[1]'s CS and word delays are never
+ * copied to priv->spi, so check_delays(test, ctx, 1500, 500, 700, 2)
+ * fails with (1000, 300, 0, 0).
+ */
+ ctx->child[1]->cs_hold = (struct spi_delay)NS(500);
+ ctx->child[1]->word_delay = (struct spi_delay)US(2);
+ ctx->child[1]->cs_inactive = (struct spi_delay)NS(700);
+ send_4b(test, ctx->child[1]);
+ KUNIT_EXPECT_EQ(test, ctx->last_bpw, 8);
+ check_delays(test, ctx, 1500, 500, 700, 2);
+ send_4b(test, ctx->child[0]);
+ check_delays(test, ctx, 1000, 300, 0, 0);
+ send_4b(test, ctx->child[1]);
+ check_delays(test, ctx, 1500, 500, 700, 2);
+ kthread_flush_worker(spi->controller->kworker);
+ kthread_flush_worker(ctx->parent->kworker);
+ ctx->child[0] = NULL;
+ ctx->child[1] = NULL;
+ /*
+ * Phase 4: Unbind spi-mux while child[1]'s (1500, 500, 700, 2) delays
+ * are active on ctx->mux_spi, then re-attach spi-mux and transfer on
+ * child[0]:
+ * - Good situation (patched): the devm cleanup action
+ * spi_mux_restore_delays() restores ctx->mux_spi's original DT
+ * delays (1000, 300, 0, 0) on unbind so re-probing does not inherit
+ * child[1]'s delays as the new baseline.
+ * - Bad situation (without spi_mux_restore_delays()): ctx->mux_spi
+ * retains (1500, 500, 700, 2) across unbind, poisoning
+ * priv->mux_cs_* on re-probe.
+ */
+ device_release_driver(dev);
+ KUNIT_EXPECT_EQ(test, ctx->mux_spi->cs_setup.value, 1000);
+ KUNIT_EXPECT_EQ(test, ctx->mux_spi->cs_hold.value, 300);
+ KUNIT_EXPECT_EQ(test, ctx->mux_spi->cs_inactive.value, 0);
+ KUNIT_EXPECT_EQ(test, ctx->mux_spi->word_delay.value, 0);
+ KUNIT_ASSERT_EQ(test, 1, device_attach(dev));
+ ctx->child[0] = spi_mux_test_find_dev(test, PFX "/device@0");
+ ctx->child[1] = spi_mux_test_find_dev(test, PFX "/device@1");
+ send_4b(test, ctx->child[0]);
+ check_delays(test, ctx, 1000, 300, 0, 0);
+}
+
+static struct kunit_case spi_mux_test_cases[] = {
+ KUNIT_CASE_PARAM(spi_mux_error_test, spi_mux_err_gen_params),
+ KUNIT_CASE_PARAM(spi_mux_cs_delay_test, spi_mux_delay_gen_params),
+ KUNIT_CASE(spi_mux_settings_lifecycle_test),
+ {}
+};
+
+static struct kunit_suite spi_mux_test_suite = {
+ .name = "spi_mux",
+ .init = spi_mux_test_init,
+ .exit = spi_mux_test_exit,
+ .test_cases = spi_mux_test_cases,
+};
+
+kunit_test_suite(spi_mux_test_suite);
+
+MODULE_DESCRIPTION("KUnit tests for spi-mux");
+MODULE_LICENSE("GPL");
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 10/14] spi: Split spi_write_then_read()
2026-11-02 16:30 [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations Joseph CT Chan
` (8 preceding siblings ...)
2026-11-02 16:30 ` [PATCH 09/14] spi: spi-mux: add KUnit tests for forwarding errors and settings Joseph CT Chan
@ 2026-11-02 16:30 ` Joseph CT Chan
2026-11-02 16:30 ` [PATCH 11/14] spi: Add lock context annotations for bus_lock_mutex Joseph CT Chan
` (4 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Joseph CT Chan @ 2026-11-02 16:30 UTC (permalink / raw)
To: Mark Brown
Cc: Bart Van Assche, David Lechner, Conor Dooley, linux-spi,
Joseph CT Chan
From: Bart Van Assche <bvanassche@acm.org>
Extract spi_write_then_read_buf() from spi_write_then_read(), move the
buffer mutex to file scope as spi_buf_lock, and annotate buf with
__guarded_by(&spi_buf_lock). Eliminate the correlated conditional unlock
in spi_write_then_read() to enable Clang's static context analysis to
verify lock balance and buffer access safety at compile time.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Joseph CT Chan <josephctchan@google.com>
---
drivers/spi/spi.c | 95 ++++++++++++++++++++++++++---------------------
1 file changed, 53 insertions(+), 42 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 0d36460e2c87..85089b0ebf4b 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -5135,7 +5135,39 @@ EXPORT_SYMBOL_GPL(spi_bus_unlock);
/* Portable code must never pass more than 32 bytes */
#define SPI_BUFSIZ max(32, SMP_CACHE_BYTES)
-static u8 *buf;
+static DEFINE_MUTEX(spi_buf_lock);
+static u8 *buf __guarded_by(&spi_buf_lock);
+
+static int spi_write_then_read_buf(struct spi_device *spi,
+ const void *txbuf, unsigned int n_tx,
+ void *rxbuf, unsigned int n_rx, u8 *local_buf)
+{
+ struct spi_message message;
+ struct spi_transfer x[2];
+ int status;
+
+ spi_message_init(&message);
+ memset(x, 0, sizeof(x));
+ if (n_tx) {
+ x[0].len = n_tx;
+ spi_message_add_tail(&x[0], &message);
+ }
+ if (n_rx) {
+ x[1].len = n_rx;
+ spi_message_add_tail(&x[1], &message);
+ }
+
+ memcpy(local_buf, txbuf, n_tx);
+ x[0].tx_buf = local_buf;
+ x[1].rx_buf = local_buf + n_tx;
+
+ /* Do the I/O */
+ status = spi_sync(spi, &message);
+ if (status == 0)
+ memcpy(rxbuf, x[1].rx_buf, n_rx);
+
+ return status;
+}
/**
* spi_write_then_read - SPI synchronous write followed by read
@@ -5161,11 +5193,7 @@ int spi_write_then_read(struct spi_device *spi,
const void *txbuf, unsigned n_tx,
void *rxbuf, unsigned n_rx)
{
- static DEFINE_MUTEX(lock);
-
int status;
- struct spi_message message;
- struct spi_transfer x[2];
u8 *local_buf;
/*
@@ -5174,39 +5202,21 @@ int spi_write_then_read(struct spi_device *spi,
* keep heap costs out of the hot path unless someone else is
* using the pre-allocated buffer or the transfer is too large.
*/
- if ((n_tx + n_rx) > SPI_BUFSIZ || !mutex_trylock(&lock)) {
- local_buf = kmalloc(max((unsigned)SPI_BUFSIZ, n_tx + n_rx),
- GFP_KERNEL | GFP_DMA);
- if (!local_buf)
- return -ENOMEM;
- } else {
- local_buf = buf;
- }
-
- spi_message_init(&message);
- memset(x, 0, sizeof(x));
- if (n_tx) {
- x[0].len = n_tx;
- spi_message_add_tail(&x[0], &message);
- }
- if (n_rx) {
- x[1].len = n_rx;
- spi_message_add_tail(&x[1], &message);
+ if ((n_tx + n_rx) <= SPI_BUFSIZ && mutex_trylock(&spi_buf_lock)) {
+ status = spi_write_then_read_buf(spi, txbuf, n_tx, rxbuf, n_rx,
+ buf);
+ mutex_unlock(&spi_buf_lock);
+ return status;
}
- memcpy(local_buf, txbuf, n_tx);
- x[0].tx_buf = local_buf;
- x[1].rx_buf = local_buf + n_tx;
-
- /* Do the I/O */
- status = spi_sync(spi, &message);
- if (status == 0)
- memcpy(rxbuf, x[1].rx_buf, n_rx);
+ local_buf = kmalloc(max((unsigned int)SPI_BUFSIZ, n_tx + n_rx),
+ GFP_KERNEL | GFP_DMA);
+ if (!local_buf)
+ return -ENOMEM;
- if (x[0].tx_buf == buf)
- mutex_unlock(&lock);
- else
- kfree(local_buf);
+ status = spi_write_then_read_buf(spi, txbuf, n_tx, rxbuf, n_rx,
+ local_buf);
+ kfree(local_buf);
return status;
}
@@ -5371,10 +5381,10 @@ static int __init spi_init(void)
{
int status;
- buf = kmalloc(SPI_BUFSIZ, GFP_KERNEL);
- if (!buf) {
- status = -ENOMEM;
- goto err0;
+ scoped_guard(mutex, &spi_buf_lock) {
+ buf = kmalloc(SPI_BUFSIZ, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
}
status = bus_register(&spi_bus_type);
@@ -5403,9 +5413,10 @@ static int __init spi_init(void)
err2:
bus_unregister(&spi_bus_type);
err1:
- kfree(buf);
- buf = NULL;
-err0:
+ scoped_guard(mutex, &spi_buf_lock) {
+ kfree(buf);
+ buf = NULL;
+ }
return status;
}
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 11/14] spi: Add lock context annotations for bus_lock_mutex
2026-11-02 16:30 [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations Joseph CT Chan
` (9 preceding siblings ...)
2026-11-02 16:30 ` [PATCH 10/14] spi: Split spi_write_then_read() Joseph CT Chan
@ 2026-11-02 16:30 ` Joseph CT Chan
2026-11-02 16:30 ` [PATCH 12/14] spi: spi-mem: Add lock context annotations Joseph CT Chan
` (3 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Joseph CT Chan @ 2026-11-02 16:30 UTC (permalink / raw)
To: Mark Brown
Cc: Bart Van Assche, David Lechner, Conor Dooley, linux-spi,
Joseph CT Chan
From: Bart Van Assche <bvanassche@acm.org>
Annotate spi_bus_lock() with __acquires(), spi_bus_unlock() with
__releases(), spi_sync_locked() and __spi_sync() with __must_hold(), and
spi_sync() with __must_not_hold() for bus_lock_mutex in
include/linux/spi/spi.h and drivers/spi/spi.c. Document exclusive SPI
bus lock ownership and enable static context analysis to verify locking
invariants across callers and the SPI core.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Joseph CT Chan <josephctchan@google.com>
---
drivers/spi/spi.c | 5 +++++
include/linux/spi/spi.h | 12 ++++++++----
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 85089b0ebf4b..01dfc25c1376 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -4959,6 +4959,7 @@ static void spi_complete(void *arg)
}
static int __spi_sync(struct spi_device *spi, struct spi_message *message)
+ __must_hold(&spi->controller->bus_lock_mutex)
{
DECLARE_COMPLETION_ONSTACK(done);
unsigned long flags;
@@ -5045,6 +5046,7 @@ static int __spi_sync(struct spi_device *spi, struct spi_message *message)
* Return: zero on success, else a negative error code.
*/
int spi_sync(struct spi_device *spi, struct spi_message *message)
+ __must_not_hold(&spi->controller->bus_lock_mutex)
{
int ret;
@@ -5073,6 +5075,7 @@ EXPORT_SYMBOL_GPL(spi_sync);
* Return: zero on success, else a negative error code.
*/
int spi_sync_locked(struct spi_device *spi, struct spi_message *message)
+ __must_hold(&spi->controller->bus_lock_mutex)
{
return __spi_sync(spi, message);
}
@@ -5094,6 +5097,7 @@ EXPORT_SYMBOL_GPL(spi_sync_locked);
* Return: always zero.
*/
int spi_bus_lock(struct spi_controller *ctlr)
+ __acquires(&ctlr->bus_lock_mutex)
{
unsigned long flags;
@@ -5123,6 +5127,7 @@ EXPORT_SYMBOL_GPL(spi_bus_lock);
* Return: always zero.
*/
int spi_bus_unlock(struct spi_controller *ctlr)
+ __releases(&ctlr->bus_lock_mutex)
{
ctlr->bus_lock_flag = 0;
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index fb4baa0d3398..553c32987c97 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -1497,10 +1497,14 @@ extern int spi_split_transfers_maxwords(struct spi_controller *ctlr,
* they will sleep uninterruptibly until the async transfer completes.
*/
-extern int spi_sync(struct spi_device *spi, struct spi_message *message);
-extern int spi_sync_locked(struct spi_device *spi, struct spi_message *message);
-extern int spi_bus_lock(struct spi_controller *ctlr);
-extern int spi_bus_unlock(struct spi_controller *ctlr);
+extern int spi_sync(struct spi_device *spi, struct spi_message *message)
+ __must_not_hold(&spi->controller->bus_lock_mutex);
+extern int spi_sync_locked(struct spi_device *spi, struct spi_message *message)
+ __must_hold(&spi->controller->bus_lock_mutex);
+extern int spi_bus_lock(struct spi_controller *ctlr)
+ __acquires(&ctlr->bus_lock_mutex);
+extern int spi_bus_unlock(struct spi_controller *ctlr)
+ __releases(&ctlr->bus_lock_mutex);
/**
* spi_sync_transfer - synchronous SPI data transfer
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 12/14] spi: spi-mem: Add lock context annotations
2026-11-02 16:30 [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations Joseph CT Chan
` (10 preceding siblings ...)
2026-11-02 16:30 ` [PATCH 11/14] spi: Add lock context annotations for bus_lock_mutex Joseph CT Chan
@ 2026-11-02 16:30 ` Joseph CT Chan
2026-11-02 16:30 ` [PATCH 13/14] spi: microchip-core-qspi: " Joseph CT Chan
` (2 subsequent siblings)
14 siblings, 0 replies; 20+ messages in thread
From: Joseph CT Chan @ 2026-11-02 16:30 UTC (permalink / raw)
To: Mark Brown
Cc: Bart Van Assche, David Lechner, Conor Dooley, linux-spi,
Joseph CT Chan
From: Bart Van Assche <bvanassche@acm.org>
Annotate spi_mem_access_start() with __cond_acquires() and
spi_mem_access_end() with __releases() for both bus_lock_mutex and
io_mutex in drivers/spi/spi-mem.c. Enable static context analysis to
track controller lock ownership across SPI memory operations without
false positive lock imbalance warnings.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Joseph CT Chan <josephctchan@google.com>
---
drivers/spi/spi-mem.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index b94680b60af9..f115ef6bf53b 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -297,6 +297,8 @@ bool spi_mem_supports_op(struct spi_mem *mem, const struct spi_mem_op *op)
EXPORT_SYMBOL_GPL(spi_mem_supports_op);
static int spi_mem_access_start(struct spi_mem *mem)
+ __cond_acquires(0, &mem->spi->controller->bus_lock_mutex)
+ __cond_acquires(0, &mem->spi->controller->io_mutex)
{
struct spi_controller *ctlr = mem->spi->controller;
@@ -324,6 +326,8 @@ static int spi_mem_access_start(struct spi_mem *mem)
}
static void spi_mem_access_end(struct spi_mem *mem)
+ __releases(&mem->spi->controller->io_mutex)
+ __releases(&mem->spi->controller->bus_lock_mutex)
{
struct spi_controller *ctlr = mem->spi->controller;
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 13/14] spi: microchip-core-qspi: Add lock context annotations
2026-11-02 16:30 [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations Joseph CT Chan
` (11 preceding siblings ...)
2026-11-02 16:30 ` [PATCH 12/14] spi: spi-mem: Add lock context annotations Joseph CT Chan
@ 2026-11-02 16:30 ` Joseph CT Chan
2026-11-02 16:30 ` [PATCH 14/14] spi: Enable lock context analysis Joseph CT Chan
2026-11-02 17:07 ` [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations David Lechner
14 siblings, 0 replies; 20+ messages in thread
From: Joseph CT Chan @ 2026-11-02 16:30 UTC (permalink / raw)
To: Mark Brown
Cc: Bart Van Assche, David Lechner, Conor Dooley, linux-spi,
Joseph CT Chan
From: Bart Van Assche <bvanassche@acm.org>
Annotate mchp_coreqspi_prepare_message() with __cond_acquires() and
mchp_coreqspi_unprepare_message() with __releases() for op_lock in
drivers/spi/spi-microchip-core-qspi.c. Document message-scoped ownership
of op_lock across SPI core message preparation callbacks and enable
static context analysis to verify error-path unlocking.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Joseph CT Chan <josephctchan@google.com>
---
drivers/spi/spi-microchip-core-qspi.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/spi/spi-microchip-core-qspi.c b/drivers/spi/spi-microchip-core-qspi.c
index c0ea65014d2e..a3143d966538 100644
--- a/drivers/spi/spi-microchip-core-qspi.c
+++ b/drivers/spi/spi-microchip-core-qspi.c
@@ -603,7 +603,10 @@ static const struct spi_controller_mem_caps mchp_coreqspi_mem_caps = {
.per_op_freq = true,
};
+#define TO_MCHP_COREQSPI(ctlr) ((struct mchp_coreqspi *)spi_controller_get_devdata(ctlr))
+
static int mchp_coreqspi_unprepare_message(struct spi_controller *ctlr, struct spi_message *m)
+ __releases(&TO_MCHP_COREQSPI(ctlr)->op_lock)
{
struct mchp_coreqspi *qspi = spi_controller_get_devdata(ctlr);
@@ -619,6 +622,7 @@ static int mchp_coreqspi_unprepare_message(struct spi_controller *ctlr, struct s
}
static int mchp_coreqspi_prepare_message(struct spi_controller *ctlr, struct spi_message *m)
+ __cond_acquires(0, &TO_MCHP_COREQSPI(ctlr)->op_lock)
{
struct mchp_coreqspi *qspi = spi_controller_get_devdata(ctlr);
struct spi_transfer *t = NULL;
@@ -685,6 +689,8 @@ static int mchp_coreqspi_prepare_message(struct spi_controller *ctlr, struct spi
return 0;
};
+#undef TO_MCHP_COREQSPI
+
static int mchp_coreqspi_transfer_one(struct spi_controller *ctlr, struct spi_device *spi,
struct spi_transfer *t)
{
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 14/14] spi: Enable lock context analysis
2026-11-02 16:30 [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations Joseph CT Chan
` (12 preceding siblings ...)
2026-11-02 16:30 ` [PATCH 13/14] spi: microchip-core-qspi: " Joseph CT Chan
@ 2026-11-02 16:30 ` Joseph CT Chan
2026-11-02 17:07 ` [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations David Lechner
14 siblings, 0 replies; 20+ messages in thread
From: Joseph CT Chan @ 2026-11-02 16:30 UTC (permalink / raw)
To: Mark Brown
Cc: Bart Van Assche, David Lechner, Conor Dooley, linux-spi,
Joseph CT Chan
From: Bart Van Assche <bvanassche@acm.org>
Set CONTEXT_ANALYSIS := y in drivers/spi/Makefile and
drivers/spi/tests/Makefile to enable compile-time lock context checking
for the SPI core, controller drivers, and KUnit tests when building with
CONFIG_WARN_CONTEXT_ANALYSIS=y.
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Joseph CT Chan <josephctchan@google.com>
---
drivers/spi/Makefile | 2 ++
drivers/spi/tests/Makefile | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 9826626995a9..5a4a4f3f77df 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -5,6 +5,8 @@
ccflags-$(CONFIG_SPI_DEBUG) := -DDEBUG
+CONTEXT_ANALYSIS := y
+
# small core, mostly translating board-specific
# config declarations into driver model code
obj-$(CONFIG_SPI_MASTER) += spi.o
diff --git a/drivers/spi/tests/Makefile b/drivers/spi/tests/Makefile
index 0ec04ecd4d55..a9b804f99b4a 100644
--- a/drivers/spi/tests/Makefile
+++ b/drivers/spi/tests/Makefile
@@ -1,5 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
+CONTEXT_ANALYSIS := y
+
obj-$(CONFIG_SPI_DMA_KUNIT_TEST) += spi-dma-kunit.o
obj-$(CONFIG_SPI_CORE_KUNIT_TEST) += spi-core-kunit.o
obj-$(CONFIG_SPI_TI_QSPI_KUNIT_TEST) += spi-ti-qspi-kunit.o
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations
2026-11-02 16:30 [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations Joseph CT Chan
` (13 preceding siblings ...)
2026-11-02 16:30 ` [PATCH 14/14] spi: Enable lock context analysis Joseph CT Chan
@ 2026-11-02 17:07 ` David Lechner
2026-11-03 22:41 ` Joseph CT Chan
14 siblings, 1 reply; 20+ messages in thread
From: David Lechner @ 2026-11-02 17:07 UTC (permalink / raw)
To: Joseph CT Chan, Mark Brown
Cc: Bart Van Assche, Conor Dooley, linux-spi, Joseph CT Chan
On 11/2/26 10:30 AM, Joseph CT Chan wrote:
> From: Joseph CT Chan <josephctchan@google.com>
>
> Hi Mark,
>
> This series fixes four categories of error-handling and configuration
For future reference, it is easier on maintainers/reviewers if you split
up independent patches into separate series.
For example, patches 1 and 7 could have been a series on it's own. 7
depends on 1, but none of the other patches are related AFAICT.
> propagation bugs across the SPI core, spi-mux, and spi-ti-qspi
> (Patches 1-6), adds three KUnit test suites on ARCH=um with detailed
> inline comments explaining the expected vs. broken states at each
> assertion (Patches 7-9), and enables Clang compile-time lock context
> analysis across the SPI subsystem (Patches 10-14).
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations
2026-11-02 17:07 ` [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations David Lechner
@ 2026-11-03 22:41 ` Joseph CT Chan
0 siblings, 0 replies; 20+ messages in thread
From: Joseph CT Chan @ 2026-11-03 22:41 UTC (permalink / raw)
To: linux-spi
On 11/2/26 11:07 AM, David Lechner wrote:
> For future reference, it is easier on maintainers/reviewers if you split
> up independent patches into separate series.
>
> For example, patches 1 and 7 could have been a series on it's own. 7
> depends on 1, but none of the other patches are related AFAICT.
Thanks, that makes sense. I will send v2 as four series, with patches 1
and 7 on their own. One thing I found while splitting: the spi-mux test
for a locked parent bus also needs patch 1, because spi-mux forwards the
message with spi_async() on the parent and that is the path patch 1
fixes. The spi-mux and spi-ti-qspi fixes themselves apply on their own.
Thanks,
Joseph
^ permalink raw reply [flat|nested] 20+ messages in thread