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
next prev 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