public inbox for linux-spi@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Jonathan Marek <jonathan@marek.ca>,
	Mark Brown <broonie@kernel.org>, Sasha Levin <sashal@kernel.org>,
	linux-arm-msm@vger.kernel.org, linux-spi@vger.kernel.org
Subject: [PATCH AUTOSEL 6.19-6.6] spi-geni-qcom: use xfer->bits_per_word for can_dma()
Date: Fri, 13 Feb 2026 19:58:30 -0500	[thread overview]
Message-ID: <20260214010245.3671907-30-sashal@kernel.org> (raw)
In-Reply-To: <20260214010245.3671907-1-sashal@kernel.org>

From: Jonathan Marek <jonathan@marek.ca>

[ Upstream commit fb2bbe3838728f572485706677590e4fc41eec5c ]

mas->cur_bits_per_word may not reflect the value of xfer->bits_per_word
when can_dma() is called. Use the right value instead.

Signed-off-by: Jonathan Marek <jonathan@marek.ca>
Link: https://patch.msgid.link/20251120211204.24078-3-jonathan@marek.ca
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Here is my complete analysis:

---

## Commit Analysis: spi-geni-qcom: use xfer->bits_per_word for can_dma()

### 1. Problem Description

The `geni_can_dma()` function and its helper `get_xfer_len_in_words()`
use `mas->cur_bits_per_word` (a cached state variable on the driver
struct) to compute the transfer length in SPI words and the effective
FIFO size. However, **`mas->cur_bits_per_word` may not reflect the
actual `bits_per_word` of the current transfer** when `can_dma()` is
called by the SPI core.

### 2. Root Cause - Call Order Mismatch

The SPI core's `__spi_pump_transfer_message()` calls operations in this
order:

```1726:1825:drivers/spi/spi.c
// Step 1: prepare_message
if (ctlr->prepare_message) {
    ret = ctlr->prepare_message(ctlr, msg);
    // ...
}

// Step 2: spi_map_msg → calls can_dma() for EACH transfer
ret = spi_map_msg(ctlr, msg);

// Step 3: transfer_one_message → calls transfer_one per-xfer
ret = ctlr->transfer_one_message(ctlr, msg);
```

Looking at Step 1, `spi_geni_prepare_message` calls `setup_fifo_params`,
which sets:

```419:419:drivers/spi/spi-geni-qcom.c
mas->cur_bits_per_word = spi_slv->bits_per_word;
```

But **only if mode changed** (`mas->last_mode != spi_slv->mode` at line
405). If the mode hasn't changed between messages,
`mas->cur_bits_per_word` retains whatever value was set during the
**last transfer** of the **previous message**.

In Step 2, `spi_map_msg` → `__spi_map_msg` iterates over **all
transfers** in the message and calls `can_dma()` for each:

```1246:1251:drivers/spi/spi.c
list_for_each_entry(xfer, &msg->transfers, transfer_list) {
    unsigned long attrs = DMA_ATTR_SKIP_CPU_SYNC;
    if (!ctlr->can_dma(ctlr, msg->spi, xfer))
        continue;
```

At this point, `mas->cur_bits_per_word` could be **wrong** for any
transfer whose `bits_per_word` differs from the stale cached value.

Step 3 is where `setup_se_xfer()` (line 863-865) finally updates
`mas->cur_bits_per_word = xfer->bits_per_word` — but this is **too
late** for the `can_dma()` decision, which already happened in Step 2.

### 3. Bug Impact

The buggy `geni_can_dma()` function uses the wrong `bits_per_word` in
two calculations:

**a) Transfer length in words (`get_xfer_len_in_words`):**

```551:554:drivers/spi/spi-geni-qcom.c
if (!(mas->cur_bits_per_word % MIN_WORD_LEN))
    len = xfer->len * BITS_PER_BYTE / mas->cur_bits_per_word;
else
    len = xfer->len / (mas->cur_bits_per_word / BITS_PER_BYTE + 1);
```

**b) FIFO size calculation:**

```574:574:drivers/spi/spi-geni-qcom.c
fifo_size = mas->tx_fifo_depth * mas->fifo_width_bits /
mas->cur_bits_per_word;
```

If `mas->cur_bits_per_word` is wrong, both `len` and `fifo_size` are
incorrect. This causes the wrong DMA vs FIFO mode selection:
- If the stale value is **smaller** than actual, `fifo_size` is inflated
  and `len` deflated → **FIFO chosen when DMA should be used** →
  potential FIFO overflow for large transfers
- If the stale value is **larger** than actual, the opposite happens →
  **unnecessary DMA for small transfers**

### 4. Fix Analysis

The fix is trivially correct: replace `mas->cur_bits_per_word` with
`xfer->bits_per_word` in both `get_xfer_len_in_words()` and
`geni_can_dma()`. The `xfer` parameter is already available, and the SPI
core guarantees `xfer->bits_per_word` is always populated via
`__spi_validate()`:

```4079:4080:drivers/spi/spi.c
if (!xfer->bits_per_word)
    xfer->bits_per_word = spi->bits_per_word;
```

This validation runs in the `spi_async()` path, well before `can_dma()`
is called.

The fix is also correct for the other caller of
`get_xfer_len_in_words()` — `setup_se_xfer()` at line 876. In that
context, `mas->cur_bits_per_word` was already set to
`xfer->bits_per_word` (line 865), so using `xfer->bits_per_word`
directly is equivalent and always correct.

### 5. Scope and Risk

- **Size**: Only 4 lines changed across 2 functions in a single file
- **Risk**: Very low — the change replaces a potentially-stale cached
  value with the authoritative source of truth that was always available
- **No behavioral change** for the common case where all transfers use
  the same `bits_per_word`
- **Fixes the bug** for devices using per-transfer `bits_per_word`
  overrides

### 6. Series Dependency

The message-id (`24078-3-jonathan@marek.ca`) indicates this is patch 3
of a series. However, this change is entirely **self-contained** — it
only swaps `mas->cur_bits_per_word` for `xfer->bits_per_word` in
functions that already receive the `xfer` parameter. No dependency on
other patches.

### 7. Affected Stable Versions

- `geni_can_dma` was introduced in commit b59c122484ecb ("spi: spi-geni-
  qcom: Add support for GPI dma") — around v5.16
- `get_xfer_len_in_words` was introduced in commit 3a76c7ca9e772 ("spi:
  spi-geni-qcom: Do not do DMA map/unmap inside driver, use framework
  instead") — v6.5-rc1
- The bug in `geni_can_dma` (using `mas->cur_bits_per_word` for
  fifo_size) has existed since `geni_can_dma` was added, but
  `get_xfer_len_in_words` was refactored in v6.5

### 8. Conclusion

This is a clear bug fix. The commit:
- Fixes a real bug: wrong value used for DMA/FIFO mode selection,
  potentially causing incorrect SPI transfers
- Is small and surgical: 4 line changes, self-contained
- Has very low regression risk: uses the correct data source that was
  always available
- Applies to stable trees v6.5+ (where `get_xfer_len_in_words` exists),
  with a simpler variant applicable to v5.16+
- Does not introduce new features or APIs

**YES**

 drivers/spi/spi-geni-qcom.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 9e9953469b3a0..5ab20d7955121 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -548,10 +548,10 @@ static u32 get_xfer_len_in_words(struct spi_transfer *xfer,
 {
 	u32 len;
 
-	if (!(mas->cur_bits_per_word % MIN_WORD_LEN))
-		len = xfer->len * BITS_PER_BYTE / mas->cur_bits_per_word;
+	if (!(xfer->bits_per_word % MIN_WORD_LEN))
+		len = xfer->len * BITS_PER_BYTE / xfer->bits_per_word;
 	else
-		len = xfer->len / (mas->cur_bits_per_word / BITS_PER_BYTE + 1);
+		len = xfer->len / (xfer->bits_per_word / BITS_PER_BYTE + 1);
 	len &= TRANS_LEN_MSK;
 
 	return len;
@@ -571,7 +571,7 @@ static bool geni_can_dma(struct spi_controller *ctlr,
 		return true;
 
 	len = get_xfer_len_in_words(xfer, mas);
-	fifo_size = mas->tx_fifo_depth * mas->fifo_width_bits / mas->cur_bits_per_word;
+	fifo_size = mas->tx_fifo_depth * mas->fifo_width_bits / xfer->bits_per_word;
 
 	if (len > fifo_size)
 		return true;
-- 
2.51.0


  parent reply	other threads:[~2026-02-14  1:03 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260214010245.3671907-1-sashal@kernel.org>
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-6.1] spi-geni-qcom: initialize mode related registers to 0 Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-6.18] spi: cadence-qspi: Try hard to disable the clocks Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-6.6] spi: spi-mem: Limit octal DTR constraints to octal DTR situations Sasha Levin
2026-02-14  0:58 ` [PATCH AUTOSEL 6.19-5.15] spi: stm32: fix Overrun issue at < 8bpw Sasha Levin
2026-02-14  0:58 ` Sasha Levin [this message]
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-6.18] spi: cadence-quadspi: Parse DT for flashes with the rest of the DT parsing Sasha Levin
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19] spi: cadence-qspi: Fix probe error path and remove Sasha Levin
2026-02-14  0:59 ` [PATCH AUTOSEL 6.19-6.12] spi: geni-qcom: Fix abort sequence execution for serial engine errors Sasha Levin
2026-02-14  1:00 ` [PATCH AUTOSEL 6.19-5.10] spi: spi-mem: Protect dirmap_create() with spi_mem_access_start/end Sasha Levin

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=20260214010245.3671907-30-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=broonie@kernel.org \
    --cc=jonathan@marek.ca \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=stable@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