linux-spi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joseph CT Chan <jctchan@gmail.com>
To: Mark Brown <broonie@kernel.org>
Cc: Bart Van Assche <bvanassche@acm.org>,
	David Lechner <dlechner@baylibre.com>,
	Conor Dooley <conor.dooley@microchip.com>,
	linux-spi@vger.kernel.org,
	Joseph CT Chan <josephctchan@google.com>
Subject: [PATCH 00/14] spi: spi-mux, core, ti-qspi: error handling, delay fixes, KUnit tests, and lock context annotations
Date: Mon,  2 Nov 2026 16:30:09 +0000	[thread overview]
Message-ID: <20261102163023.2574632-1-jctchan@gmail.com> (raw)

From: Joseph CT Chan <josephctchan@google.com>

Hi Mark,

This series fixes four categories of error-handling and configuration
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).

Detailed line-by-line code walkthroughs of the four bug mechanisms and
the three KUnit test suites are also available at:
https://gist.github.com/joectchan/9a14b5e6facb75058ebc5a6ff1757131

Overview of the Bugs Fixed (Patches 1-6)
========================================

1. SPI core leaks message optimize state when queuing fails (Patch 1):
   spi_async() and __spi_sync() call spi_maybe_optimize_message() before
   attempting to queue the message. Optimizing a message can allocate
   split transfers on msg->resources (via spi_split_transfers_maxsize())
   and controller state in msg->opt_state (such as compiled command
   programs in spi-axi-spi-engine), and sets msg->optimized = true.
   Normally spi_finalize_current_message() unoptimizes the message after
   it runs, but when queuing fails (-EBUSY when another driver such as
   mmc_spi holds spi_bus_lock(), -ESHUTDOWN when the queue is stopped,
   or -ENOTSUPP when ctlr->transfer is NULL), the message was never
   queued and is never finalized. Patch 1 calls
   spi_maybe_unoptimize_message() on the queue failure paths in
   spi_async() and __spi_sync(), while preserving caller pre-optimized
   messages (msg->pre_optimized == true).

2. spi-mux leaks the mux semaphore on parent spi_setup() failure and
   ignores live spi_setup() updates from the active child (Patches 2-3):
   - In spi_mux_select(), mux_control_select() acquires the mux
     semaphore before calling spi_setup(priv->spi). If parent
     spi_setup() fails, spi_mux_select() returned the error with the mux
     semaphore still held and priv->current_cs already set to the failed
     child's chip select. Patch 2 resets priv->current_cs =
     SPI_MUX_NO_CS and calls mux_control_deselect(priv->mux) when
     spi_setup(priv->spi) fails.
   - When the currently selected child updates its mode (for example
     mmc_spi flipping SPI_CS_HIGH), max_speed_hz, bits_per_word, or
     delays and calls spi_setup(), spi_mux_setup() left priv->current_cs
     unchanged, so the next transfer on that child hit the
     priv->current_cs == spi_get_chipselect(spi, 0) early return in
     spi_mux_select() and ran with stale parent settings. Patch 3 resets
     priv->current_cs = SPI_MUX_NO_CS in spi_mux_setup().

3. Early errors in spi-mux and spi-ti-qspi skip
   spi_finalize_current_message() and hang callers (Patches 4-5):
   - spi-mux sets ctlr->must_async = true and
     ctlr->defer_optimize_message = true, so every child spi_sync()
     sleeps in wait_for_completion(&done) while validation is deferred
     to the parent spi_async(priv->spi, m) call inside
     spi_mux_transfer_one_message(). When spi_mux_select() failed (e.g.
     an I2C mux NACK or parent spi_setup() error) or parent spi_async()
     failed synchronously (e.g. -EBUSY while the parent bus is locked,
     or -EINVAL when parent __spi_validate() rejects an odd-length
     16-bit transfer), spi_mux_transfer_one_message() returned the error
     without finalizing the message. Because spi_mux_complete_cb() never
     ran, the child caller slept forever holding mux_ctlr->bus_lock_mutex
     and, on parent spi_async() failure, leaked the mux semaphore and
     left msg->spi, msg->complete, and msg->context overwritten. Patch 4
     finalizes the message on spi_mux_select() failure and invokes
     spi_mux_complete_cb(priv) on parent spi_async() failure.
   - In spi-ti-qspi, when qspi_transfer_msg() failed (such as
     ti_qspi_poll_wc() timing out with -ETIMEDOUT),
     ti_qspi_start_transfer_one() unlocked list_lock and returned early
     without writing QSPI_INVAL, updating m->status, or calling
     spi_finalize_current_message(host). That left noqueue spi_sync()
     returning stale -EINPROGRESS with msg->optimized leaked and caused
     queued spi_async() callers to hang. Patch 5 breaks out of the loop
     on error so the common epilogue writes QSPI_INVAL, records status,
     and finalizes the message.

4. spi-mux drops child chip-select and word delays (Patch 6):
   spi_mux_select() copied max_speed_hz, mode, and bits_per_word to
   priv->spi, but dropped cs_setup, cs_hold, cs_inactive, and
   word_delay. Because the SPI core and parent controller read all four
   delays from priv->spi after m->spi is rewritten, child device delays
   were ignored. Patch 6 saves the mux device's own delays at probe,
   merges each child CS delay with the mux's delay (taking the longer of
   the two so both timing requirements are met), forwards word_delay,
   and restores the mux's original delays via devm on unbind.

Overview of the KUnit Suites (Patches 7-9)
==========================================

All three KUnit suites run on User-Mode Linux (ARCH=um) without physical
hardware and include detailed inline comments at each assertion
documenting both the expected patched behavior and the broken unpatched
behavior:

  python3 tools/testing/kunit/kunit.py run \
    --arch=um --kunitconfig=drivers/spi/tests

- Patch 7 (spi_core_optimize, drivers/spi/tests/spi-core-kunit.c):
  7 parameterized cases testing spi_async() and spi_sync() queue errors
  (-EBUSY on a locked bus, -ESHUTDOWN on a suspended controller, and
  -ENOTSUPP on a mem_ops-only controller) with max_transfer_size = 2
  splitting a 4-byte transfer into two 2-byte transfers on
  msg.resources. Verifies that core-optimized messages are unoptimized
  on queue failure, caller pre-optimized messages are preserved, and
  messages can be reused after transient errors clear.

- Patch 8 (spi_ti_qspi, drivers/spi/tests/spi-ti-qspi-kunit.c):
  Uses logic_iomem emulated MMIO at 0x30000000 on ARCH=um to withhold
  QSPI_WC on QSPI_SPI_STATUS_REG (0x4c), forcing ti_qspi_poll_wc() to
  time out with -ETIMEDOUT. Verifies that both noqueue spi_sync() and
  queued spi_async() return -ETIMEDOUT, unoptimize the message, write
  QSPI_INVAL to QSPI_SPI_CMD_REG (0x48), and recover cleanly on the next
  transfer.

- Patch 9 (spi_mux, drivers/spi/tests/spi-mux-kunit.c +
  kunit_spi_mux.dtso):
  Applies a Device Tree overlay registering a fake mux_chip, a fake
  parent spi_controller, the real spi-mux driver, and two child SPI
  devices across 9 test executions: 4 forwarding error cases
  (bus_locked, rejects_odd_len, select_fails, parent_setup_fails), 4 CS
  delay merge cases (longer, tie, sck_zero, sck_set), and a 4-phase
  settings lifecycle test covering live child spi_setup(), channel
  switching, and driver unbind/re-attach delay restoration.

On unmodified upstream master (c339f4010dea), running the 3 suites
produces 13 failures and 4 passing baseline guard cases (with bounded
timeouts so none of the unfinalized message bugs hang the test runner).
With Patches 1-6 applied, all 17 test cases pass.

Lock Context Analysis (Patches 10-14)
=====================================

Patches 10-14 (by Bart Van Assche) split spi_write_then_read(), add
Clang thread-safety and lock context annotations (__acquires,
__releases, __cond_acquires, __must_hold, __must_not_hold, and
__guarded_by) for bus_lock_mutex and spi-mem, and enable
CONTEXT_ANALYSIS := y in drivers/spi/Makefile and
drivers/spi/tests/Makefile with zero warnings under Clang 23
(CONFIG_WARN_CONTEXT_ANALYSIS=y).

Please consider this patch series for the next merge window.

Thanks,
Joseph

Bart Van Assche (5):
  spi: Split spi_write_then_read()
  spi: Add lock context annotations for bus_lock_mutex
  spi: spi-mem: Add lock context annotations
  spi: microchip-core-qspi: Add lock context annotations
  spi: Enable lock context analysis

Joseph CT Chan (9):
  spi: unoptimize message when queuing it fails
  spi: spi-mux: release mux when parent setup fails
  spi: spi-mux: re-apply settings after child calls spi_setup()
  spi: spi-mux: finalize message when forwarding fails
  spi: spi-ti-qspi: finalize message on transfer error
  spi: spi-mux: pass child CS and word delays to the parent
  spi: add KUnit tests for message optimize state on queue errors
  spi: spi-ti-qspi: add KUnit tests for transfer error finalization
  spi: spi-mux: add KUnit tests for forwarding errors and settings

 drivers/spi/Kconfig                   |  39 ++
 drivers/spi/Makefile                  |   5 +
 drivers/spi/spi-mem.c                 |   4 +
 drivers/spi/spi-microchip-core-qspi.c |   6 +
 drivers/spi/spi-mux.c                 | 124 +++++-
 drivers/spi/spi-ti-qspi.c             |   4 +-
 drivers/spi/spi.c                     | 107 +++--
 drivers/spi/tests/.kunitconfig        |  12 +
 drivers/spi/tests/Makefile            |   6 +
 drivers/spi/tests/kunit_spi_mux.dtso  |  42 ++
 drivers/spi/tests/spi-core-kunit.c    | 308 ++++++++++++++
 drivers/spi/tests/spi-mux-kunit.c     | 581 ++++++++++++++++++++++++++
 drivers/spi/tests/spi-ti-qspi-kunit.c | 225 ++++++++++
 include/linux/spi/spi.h               |  12 +-
 14 files changed, 1423 insertions(+), 52 deletions(-)
 create mode 100644 drivers/spi/tests/.kunitconfig
 create mode 100644 drivers/spi/tests/kunit_spi_mux.dtso
 create mode 100644 drivers/spi/tests/spi-core-kunit.c
 create mode 100644 drivers/spi/tests/spi-mux-kunit.c
 create mode 100644 drivers/spi/tests/spi-ti-qspi-kunit.c


base-commit: c339f4010dea8b0d754f84ace70cd1b089878ef8
-- 
2.56.0.317.g454f8302e1-goog

             reply	other threads:[~2026-11-02 16:33 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-02 16:30 Joseph CT Chan [this message]
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
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 ` [PATCH 03/14] spi: spi-mux: re-apply settings after child calls spi_setup() Joseph CT Chan
2026-11-02 16:30 ` [PATCH 04/14] spi: spi-mux: finalize message when forwarding fails Joseph CT Chan
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 ` [PATCH 06/14] spi: spi-mux: pass child CS and word delays to the parent 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
2026-11-02 17:29   ` David Lechner
2026-11-03 22:41     ` Joseph CT Chan
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 ` [PATCH 09/14] spi: spi-mux: add KUnit tests for forwarding errors and settings Joseph CT Chan
2026-11-02 16:30 ` [PATCH 10/14] spi: Split spi_write_then_read() Joseph CT Chan
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 ` [PATCH 12/14] spi: spi-mem: Add lock context annotations Joseph CT Chan
2026-11-02 16:30 ` [PATCH 13/14] spi: microchip-core-qspi: " 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
2026-11-03 22:41   ` Joseph CT Chan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261102163023.2574632-1-jctchan@gmail.com \
    --to=jctchan@gmail.com \
    --cc=broonie@kernel.org \
    --cc=bvanassche@acm.org \
    --cc=conor.dooley@microchip.com \
    --cc=dlechner@baylibre.com \
    --cc=josephctchan@google.com \
    --cc=linux-spi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).