* [PATCH v2 2/2] spi: add KUnit tests for message optimize state on queue errors
2026-11-03 22:47 [PATCH v2 0/2] spi: unoptimize message when queuing it fails Joseph CT Chan
2026-11-03 22:47 ` [PATCH v2 1/2] " Joseph CT Chan
@ 2026-11-03 22:47 ` Joseph CT Chan
2026-11-09 12:21 ` Mark Brown
1 sibling, 1 reply; 4+ messages in thread
From: Joseph CT Chan @ 2026-11-03 22:47 UTC (permalink / raw)
To: Mark Brown; +Cc: linux-spi, David Lechner, Bart Van Assche, 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 the controller's transfer() callback returns
an error, 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.
Developed with help from the Claude and Gemini AI assistants.
Signed-off-by: Joseph CT Chan <josephctchan@google.com>
---
v2: Inject errors through a transfer() callback on the fake controller
instead of mem_ops, use designated initializers, and trim the comments.
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 | 262 +++++++++++++++++++++++++++++
5 files changed, 281 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..5ddd6c574170
--- /dev/null
+++ b/drivers/spi/tests/spi-core-kunit.c
@@ -0,0 +1,262 @@
+// 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>
+
+struct spi_core_test_ctx {
+ struct spi_controller *ctlr;
+ struct spi_device *spi;
+ int (*queued_transfer)(struct spi_device *spi, struct spi_message *msg);
+ int optimize_count, unoptimize_count;
+ int transfer_err;
+ 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_transfer(struct spi_device *spi,
+ struct spi_message *msg)
+{
+ struct spi_core_test_ctx *ctx =
+ spi_controller_get_devdata(spi->controller);
+
+ if (ctx->transfer_err)
+ return ctx->transfer_err;
+
+ return ctx->queued_transfer(spi, msg);
+}
+
+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)
+{
+ 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->transfer_one_message = spi_core_test_xfer_one;
+ KUNIT_ASSERT_EQ(test, 0, devm_spi_register_controller(dev, ctlr));
+ ctx->queued_transfer = ctlr->transfer;
+ ctlr->transfer = spi_core_test_transfer;
+ ctx->spi = spi_new_device(ctlr, &info);
+ KUNIT_ASSERT_NOT_NULL(test, ctx->spi);
+ init_completion(&ctx->done);
+ /* 4-byte TX buffer with max_transfer_size = 2 splits into two 2-byte transfers. */
+ 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_TRANSFER,
+ 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[] = {
+ {
+ .name = "bus_locked_async",
+ .mode = ERR_BUS_LOCKED,
+ .exp = -EBUSY,
+ },
+ {
+ .name = "suspend_async",
+ .mode = ERR_SUSPEND,
+ .exp = -ESHUTDOWN,
+ },
+ {
+ .name = "xfer_err_sync",
+ .mode = ERR_TRANSFER,
+ .exp = -EIO,
+ .use_sync = true,
+ .must_async = true,
+ },
+ {
+ .name = "pre_opt_locked",
+ .mode = ERR_BUS_LOCKED,
+ .exp = -EBUSY,
+ .pre_opt = true,
+ },
+ {
+ .name = "pre_opt_xfer_err_sync",
+ .mode = ERR_TRANSFER,
+ .exp = -EIO,
+ .use_sync = true,
+ .pre_opt = true,
+ .must_async = true,
+ },
+ {
+ .name = "sync_success",
+ .mode = ERR_NONE,
+ .exp = 0,
+ .use_sync = true,
+ },
+ {
+ .name = "sync_success_queued",
+ .mode = ERR_NONE,
+ .exp = 0,
+ .use_sync = true,
+ .must_async = 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);
+
+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);
+ 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_TRANSFER)
+ ctx->transfer_err = -EIO;
+
+ 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);
+
+ ctx->transfer_err = 0;
+ 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));
+ check_state(test, ctx, 1, 1, false);
+ return;
+ }
+
+ /* Core-optimized messages are unoptimized on queue error; pre-optimized stay optimized. */
+ check_state(test, ctx, 1, p->pre_opt ? 0 : 1, p->pre_opt);
+ if (p->pre_opt)
+ spi_unoptimize_message(&ctx->msg);
+
+ reinit_completion(&ctx->done);
+ ctx->msg.complete = kunit_done;
+ ctx->msg.context = &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);
+ /* Resubmitting the message after the transient error clears succeeds cleanly. */
+ 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.385.gac671c819a-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread