[PATCH AUTOSEL 6.19-6.1] spi-geni-qcom: initialize mode related registers to 0

[PATCH AUTOSEL 6.19-6.1] spi-geni-qcom: initialize mode related registers to 0

[Sasha Levin]

From: Jonathan Marek <jonathan@marek.ca>

[ Upstream commit 739062a9f1e9a77a9687c8fd30f8e5dd12ec70be ]

setup_fifo_params assumes these will be zero, it won't write these
registers if the initial mode is zero.

Signed-off-by: Jonathan Marek <jonathan@marek.ca>
Link: https://patch.msgid.link/20251120211204.24078-4-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:

Now I have a complete understanding of the bug and the fix. Let me
compile the analysis.

---

## Detailed Analysis

### 1. COMMIT MESSAGE ANALYSIS

The commit subject "spi-geni-qcom: initialize mode related registers to
0" is straightforward. The commit body explains: "`setup_fifo_params`
assumes these will be zero, it won't write these registers if the
initial mode is zero." This is a clear description of a latent
initialization bug.

### 2. CODE CHANGE ANALYSIS - The Bug Mechanism

The bug centers on the interaction between two functions in `spi-geni-
qcom.c`:

**`spi_geni_init()`** (called once during probe at line 1147)
initializes the GENI SPI controller. In the `case 0:` (FIFO mode) branch
at lines 724-728, it sets the transfer mode but does NOT initialize the
SPI mode registers.

**`setup_fifo_params()`** (called for each SPI message at line 593 via
`spi_geni_prepare_message`) has an optimization at line 405:

```397:431:drivers/spi/spi-geni-qcom.c
static int setup_fifo_params(struct spi_device *spi_slv,
                                        struct spi_controller *spi)
{
        struct spi_geni_master *mas = spi_controller_get_devdata(spi);
        struct geni_se *se = &mas->se;
        u32 loopback_cfg = 0, cpol = 0, cpha = 0, demux_output_inv = 0;
        u32 demux_sel;

        if (mas->last_mode != spi_slv->mode) {
                // ... only writes registers when mode changes ...
                writel(loopback_cfg, se->base + SE_SPI_LOOPBACK);
                writel(demux_sel, se->base + SE_SPI_DEMUX_SEL);
                writel(cpha, se->base + SE_SPI_CPHA);
                writel(cpol, se->base + SE_SPI_CPOL);
                writel(demux_output_inv, se->base +
SE_SPI_DEMUX_OUTPUT_INV);

                mas->last_mode = spi_slv->mode;
        }
        // ...
}
```

**The critical chain of assumptions:**

1. `spi_geni_master` is allocated with `kzalloc` (confirmed in
   `__spi_alloc_controller` at spi.c line 3056), so `mas->last_mode`
   starts at **0**.
2. `SPI_MODE_0` = `(0|0)` = **0** (from `include/uapi/linux/spi/spi.h`
   line 10). This is the most common SPI mode.
3. When the first SPI device uses mode 0, `spi_slv->mode` is 0, so
   `mas->last_mode != spi_slv->mode` is **false** (0 != 0 → false).
4. **The register writes are skipped entirely** for the first SPI
   transaction.
5. This assumes the hardware registers already contain 0.

**When the assumption breaks:**

- **Bootloader contamination**: If the bootloader used SPI (very common
  on Qualcomm SoCs for reading from SPI flash/NOR), it may have
  configured these registers with non-zero values (e.g., CPOL=1, CPHA=1
  for SPI mode 3, or specific chip select muxing).
- **Firmware loading path**: The `geni_load_se_firmware()` path (line
  674-679) loads QUP firmware when `proto == GENI_SE_INVALID_PROTO`.
  After firmware loading, register state may be undefined.

**The consequence:**

If hardware registers retain stale non-zero values from the bootloader
and the first SPI device uses mode 0:
- **SE_SPI_CPOL** could be set to `BIT(2)` → wrong clock polarity
- **SE_SPI_CPHA** could be set to `BIT(0)` → wrong clock phase
- **SE_SPI_LOOPBACK** could be enabled → data loops back instead of
  going to device
- **SE_SPI_DEMUX_SEL** could select wrong chip → wrong device addressed
- **SE_SPI_DEMUX_OUTPUT_INV** could invert CS → chip select logic
  inverted

Any of these would cause **SPI communication failure or data
corruption**.

### 3. THE FIX

The fix is 5 `writel(0, ...)` calls added to the FIFO mode
initialization path in `spi_geni_init()`:

```c
writel(0, se->base + SE_SPI_LOOPBACK);
writel(0, se->base + SE_SPI_DEMUX_SEL);
writel(0, se->base + SE_SPI_CPHA);
writel(0, se->base + SE_SPI_CPOL);
writel(0, se->base + SE_SPI_DEMUX_OUTPUT_INV);
```

This ensures the hardware state matches the software assumption
(`last_mode = 0`), making `setup_fifo_params()`'s optimization correct.

### 4. CLASSIFICATION

- **Type**: Initialization bug fix (uninitialized hardware register
  state)
- **NOT a feature**: No new functionality, just ensuring correct
  initialization
- **Category**: Data corruption / device communication failure fix

### 5. SCOPE AND RISK ASSESSMENT

- **Lines changed**: 6 (5 writel + 1 comment)
- **Files touched**: 1 (`drivers/spi/spi-geni-qcom.c`)
- **Complexity**: Extremely low - simple register zeroing
- **Risk of regression**: Near zero. In the common case where registers
  are already 0, the writel calls are a harmless no-op. In the bug case,
  they fix incorrect register values.
- **Self-contained**: Yes - all register defines (`SE_SPI_LOOPBACK`,
  etc.) and the `spi_geni_init()` function exist in all stable trees
  that have this driver.

### 6. USER IMPACT

- **Qualcomm GENI SPI** is used on many Qualcomm SoCs (SDM845, SM8150,
  SM8250, SC7180, SC7280, SM8350, SM8450, X1E80100, etc.)
- These SoCs power phones, laptops (Qualcomm-based Chromebooks, Windows
  on ARM devices), IoT, and embedded systems
- SPI is used for touchscreen controllers, sensor hubs, TPM modules, and
  other critical peripherals
- If SPI communication fails, the affected device simply doesn't work

### 7. DEPENDENCY CHECK

The patch is **entirely self-contained**. It uses:
- `se->base` - available since the driver was created
- `SE_SPI_LOOPBACK`, `SE_SPI_DEMUX_SEL`, `SE_SPI_CPHA`, `SE_SPI_CPOL`,
  `SE_SPI_DEMUX_OUTPUT_INV` - all defined in the same file since
  original driver creation
- `writel()` - standard kernel API

The patch was "4/N" in a series (from the Link URL), but this specific
change is independent. It does not depend on any prior patch in the
series - it simply adds register zeroing to an existing function.

### 8. STABILITY INDICATORS

- Merged by Mark Brown (SPI subsystem maintainer) with `Signed-off-by`
- The fix is logically obvious and defensive
- The pattern (writing 0 to ensure known state during init) is standard
  practice throughout the kernel

### CONCLUSION

This commit fixes a real initialization bug where SPI mode-related
hardware registers may retain stale non-zero values from
bootloader/firmware, causing the driver's `setup_fifo_params()`
optimization to incorrectly skip the first register write when the SPI
device uses mode 0 (the most common mode). The consequence is SPI
communication failure or data corruption. The fix is a trivial 5-line
addition of register zeroing during init, with essentially zero
regression risk, affecting a widely-deployed Qualcomm SPI driver. It is
self-contained with no dependencies and applies cleanly to stable trees.

**YES**

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

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index a0d8d3425c6c6..9e9953469b3a0 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -724,6 +724,12 @@ static int spi_geni_init(struct spi_geni_master *mas)
 	case 0:
 		mas->cur_xfer_mode = GENI_SE_FIFO;
 		geni_se_select_mode(se, GENI_SE_FIFO);
+		/* setup_fifo_params assumes that these registers start with a zero value */
+		writel(0, se->base + SE_SPI_LOOPBACK);
+		writel(0, se->base + SE_SPI_DEMUX_SEL);
+		writel(0, se->base + SE_SPI_CPHA);
+		writel(0, se->base + SE_SPI_CPOL);
+		writel(0, se->base + SE_SPI_DEMUX_OUTPUT_INV);
 		ret = 0;
 		break;
 	}
-- 
2.51.0

[PATCH AUTOSEL 6.19-6.18] spi: cadence-qspi: Try hard to disable the clocks

[Sasha Levin]

From: "Miquel Raynal (Schneider Electric)" <miquel.raynal@bootlin.com>

[ Upstream commit 612227b392eed94a3398dc03334a84a699a82276 ]

In the remove path, we should try hard to perform all steps as we simply
cannot fail.

The "no runtime PM" quirk must only alter the state of the RPM core, but
the clocks should still be disabled if that is possible. Move the
disable call outside of the RPM quirk.

Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
Tested-by: Santhosh Kumar K <s-k6@ti.com>
Link: https://patch.msgid.link/20260122-schneider-6-19-rc1-qspi-v4-9-f9c21419a3e6@bootlin.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

So the `CQSPI_DISABLE_RUNTIME_PM` quirk is used by `socfpga_qspi` (Intel
SoCFPGA platforms). These are real, deployed embedded systems.

### Stability Indicators

- **Tested-by:** Two testers: Wolfram Sang and Santhosh Kumar K — both
  are known kernel contributors
- **Reviewed through proper channels:** Has a Link: to the mailing list
  patch series
- **Part of a series:** The commit is "v4-9" suggesting it's patch 9 in
  a series. This is a concern for dependency.

Let me check if this patch is self-contained or requires other patches
from the series.

The change itself is self-contained — it only modifies the logic of the
`cqspi_remove()` function. The `ret` variable is newly added and
initialized in this function. No new functions, types, or macros are
introduced. The change doesn't depend on other patches.

### Assessment

**What it fixes:** Clock resource leak on driver removal for platforms
with the `CQSPI_DISABLE_RUNTIME_PM` quirk (specifically SoCFPGA).
Without this fix, the QSPI clock remains enabled after driver removal,
which wastes power and can prevent proper power state transitions.

**Meets stable rules:**
- Obviously correct: Yes — the logic change is clear and straightforward
- Fixes a real bug: Yes — resource leak (clock not disabled on remove)
- Small and contained: Yes — ~6 lines in one file
- No new features: Correct — this is purely a bug fix

**Risk:** Very low. The change is minimal, the logic is straightforward,
and it has been tested by two people.

**Concern:** This is part of a larger series (patch 9 of N), but the
change itself appears self-contained. The only question is whether the
code it modifies exists in stable trees — the cadence-qspi driver and
the `CQSPI_DISABLE_RUNTIME_PM` quirk have been around for a while, so
this should apply.

This is a clear resource leak fix in the driver removal path, it's
small, tested, and low risk.

**YES**

 drivers/spi/spi-cadence-quadspi.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index b1c95b1c343fc..1ce433a8d64a5 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -2042,6 +2042,7 @@ static void cqspi_remove(struct platform_device *pdev)
 	const struct cqspi_driver_platdata *ddata;
 	struct cqspi_st *cqspi = platform_get_drvdata(pdev);
 	struct device *dev = &pdev->dev;
+	int ret = 0;
 
 	ddata = of_device_get_match_data(dev);
 
@@ -2061,8 +2062,10 @@ static void cqspi_remove(struct platform_device *pdev)
 		cqspi_jh7110_disable_clk(pdev, cqspi);
 
 	if (!(ddata && (ddata->quirks & CQSPI_DISABLE_RUNTIME_PM)))
-		if (pm_runtime_get_sync(&pdev->dev) >= 0)
-			clk_disable(cqspi->clk);
+		ret = pm_runtime_get_sync(&pdev->dev);
+
+	if (ret >= 0)
+		clk_disable(cqspi->clk);
 
 	if (!(ddata && (ddata->quirks & CQSPI_DISABLE_RUNTIME_PM))) {
 		pm_runtime_put_sync(&pdev->dev);
-- 
2.51.0

[PATCH AUTOSEL 6.19-6.6] spi: spi-mem: Limit octal DTR constraints to octal DTR situations

[Sasha Levin]

From: Miquel Raynal <miquel.raynal@bootlin.com>

[ Upstream commit 8618271887ca10ac5108fe7e1d82ba8f1b152cf9 ]

In this helper, any operation with a single DTR cycle (like 1S-1S-8D) is
considered requiring a duplicated command opcode. This is wrong as this
constraint only applies to octal DTR operations (8D-8D-8D).

Narrow the application of this constraint to the concerned bus
interface.

Note: none of the possible XD-XD-XD pattern, with X being one of {1, 2,
4} would benefit from this check either as there is only in octal DTR
mode that a single clock edge would be enough to transmit the full
opcode.

Make sure the constraint of expecting two bytes for the command is
applied to the relevant bus interface.

Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Link: https://patch.msgid.link/20260109-winbond-v6-17-rc1-oddr-v2-3-1fff6a2ddb80@bootlin.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

The core of the fix — moving the `op->cmd.nbytes != 2` check inside a
tighter condition — is self-contained. It doesn't depend on the `swap16`
feature (that's a separate, pre-existing check). The new even-byte
checks for addr/dummy/data in 8D-8D-8D mode are a tightening of
validation that could also be backported.

However, the presence of the `swap16` check in the surrounding context
means the patch won't apply cleanly to older stable trees that lack
commit `030ace430afcf` (from 6.12 cycle). The fix would need minor
adaptation for older stable trees, but the logic change itself is
independent.

### 7. Assessment Summary

**What the bug is:** `spi_mem_default_supports_op()` incorrectly rejects
valid non-8D-8D-8D DTR operations (like 1S-1S-8D) by requiring
`cmd.nbytes == 2` for ALL DTR operations, not just 8D-8D-8D ones.

**Impact:** Real functional bug — any SPI memory device using mixed DTR
modes (where only the data phase is DTR but not the command phase) would
have its operations rejected. This prevents legitimate device
operations.

**Scope of the fix:** The change is confined to a single function in one
file. It narrows a condition check and adds additional validation only
for the specific 8D-8D-8D case.

**Risk:** Low. The fix is logically correct — it restricts a previously
over-broad check. It can't introduce new failures for 8D-8D-8D mode
(those checks are preserved and even strengthened), and it unblocks
mixed-DTR modes that were incorrectly blocked.

**Who benefits:** Users with SPI memory devices that use non-8D-8D-8D
DTR modes (e.g., some flash chips using 1S-1S-8D for certain
operations).

**Concerns:**
- Part of a larger patch series (v2-3), but the fix to
  `spi_mem_default_supports_op()` is self-contained
- May need minor context adjustment for older stable trees (due to
  `swap16` context)
- The even-byte-count validation is technically a new constraint for
  8D-8D-8D mode, but it enforces existing hardware requirements that
  were previously unchecked

This is a clear correctness fix for a real bug that has existed since
kernel 5.18. It's small, contained, well-reviewed (Reviewed-by from
Tudor Ambarus, a key MTD maintainer), and fixes incorrect rejection of
valid SPI operations.

**YES**

 drivers/spi/spi-mem.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index c8b2add2640e5..6c7921469b90b 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -178,8 +178,19 @@ bool spi_mem_default_supports_op(struct spi_mem *mem,
 		if (op->data.swap16 && !spi_mem_controller_is_capable(ctlr, swap16))
 			return false;
 
-		if (op->cmd.nbytes != 2)
-			return false;
+		/* Extra 8D-8D-8D limitations */
+		if (op->cmd.dtr && op->cmd.buswidth == 8) {
+			if (op->cmd.nbytes != 2)
+				return false;
+
+			if ((op->addr.nbytes % 2) ||
+			    (op->dummy.nbytes % 2) ||
+			    (op->data.nbytes % 2)) {
+				dev_err(&ctlr->dev,
+					"Even byte numbers not allowed in octal DTR operations\n");
+				return false;
+			}
+		}
 	} else {
 		if (op->cmd.nbytes != 1)
 			return false;
-- 
2.51.0

[PATCH AUTOSEL 6.19-5.15] spi: stm32: fix Overrun issue at < 8bpw

[Sasha Levin]

From: Deepak Kumar <deepak.kumar01@st.com>

[ Upstream commit 1ac3be217c01d5df55ec5052f81e4f1708f46552 ]

When SPI communication is suspended by hardware automatically, it could
happen that few bits of next frame are already clocked out due to
internal synchronization delay.

To achieve a safe suspension, we need to ensure that each word must be
at least 8 SPI clock cycles long. That's why, if bpw is less than 8
bits, we need to use midi to reach 8 SPI clock cycles at least.

This will ensure that each word achieve safe suspension and prevent
overrun condition.

Signed-off-by: Deepak Kumar <deepak.kumar01@st.com>
Signed-off-by: Alain Volmat <alain.volmat@foss.st.com>
Link: https://patch.msgid.link/20251218-stm32-spi-enhancements-v2-2-3b69901ca9fe@foss.st.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have all the information needed for a comprehensive analysis.

## Comprehensive Analysis

### 1. COMMIT MESSAGE ANALYSIS

The subject line is **"spi: stm32: fix Overrun issue at < 8bpw"** -- a
clear bug fix. The body explains a hardware-level problem on the STM32H7
SPI controller: when SPI communication is suspended by the hardware
automatically, a few bits of the next frame can already be clocked out
due to internal synchronization delay. To achieve safe suspension, each
word must be at least 8 SPI clock cycles long. When `bits_per_word`
(bpw) is less than 8, the MIDI (Minimum Inter-Data Idleness) delay must
be increased to pad the total word time to at least 8 SPI clock cycles.

The commit is authored by an ST Microelectronics engineer (Deepak Kumar
at `st.com`), co-authored/signed by Alain Volmat (STM32 SPI maintainer
at `foss.st.com`), and merged by Mark Brown (SPI subsystem maintainer).
This lends high credibility.

### 2. CODE CHANGE ANALYSIS

The diff modifies `stm32h7_spi_data_idleness()` in `drivers/spi/spi-
stm32.c`:

**Old code (lines 1909-1912):**
```c
u32 midi = min_t(u32,
                 DIV_ROUND_UP(spi->cur_midi, sck_period_ns),
                 FIELD_GET(STM32H7_SPI_CFG2_MIDI,
                 STM32H7_SPI_CFG2_MIDI));
```

**New code:**
```c
u32 midi = DIV_ROUND_UP(spi->cur_midi, sck_period_ns);

if ((spi->cur_bpw + midi) < 8)
    midi = 8 - spi->cur_bpw;

midi = min_t(u32, midi, FIELD_MAX(STM32H7_SPI_CFG2_MIDI));
```

The changes are:
1. **Core fix (3 lines):** After computing the initial midi value, check
   if `bpw + midi < 8`. If so, increase midi to `8 - bpw`, ensuring a
   minimum total of 8 SPI clock cycles per word. This prevents the
   overrun condition.
2. **Style cleanup:** Replace `FIELD_GET(STM32H7_SPI_CFG2_MIDI,
   STM32H7_SPI_CFG2_MIDI)` with `FIELD_MAX(STM32H7_SPI_CFG2_MIDI)`. Both
   evaluate to 15 (the field is GENMASK(7,4), a 4-bit field), but
   `FIELD_MAX` is the semantically correct macro for getting the maximum
   field value.
3. **Reorder:** The clamping to maximum is now done AFTER the overrun
   adjustment, ensuring the adjustment isn't clamped away prematurely.

**Example scenario:** With bpw=4 and midi=1 (from a 100ns delay at 10MHz
clock):
- **Old:** Total = 4+1 = 5 cycles/word. Hardware suspension can corrupt
  next frame. → Overrun
- **New:** Since 4+1 < 8, midi is set to 4. Total = 4+4 = 8 cycles/word.
  → Safe suspension

### 3. THE BUG AND ITS IMPACT

When the STM32H7 SPI controller's overrun condition fires
(`STM32H7_SPI_SR_OVR`), the interrupt handler at line 1111-1113 logs
`"Overrun: RX data lost"` and sets `end = true`, which terminates the
transfer. This means:
- **Data loss** - received SPI data is discarded
- **Transfer failure** - the SPI communication is aborted
- Any device using bpw < 8 with MIDI configured experiences
  corrupted/failed SPI transfers

This is a real hardware bug affecting real STM32H7/STM32MP25 users with
SPI devices that use fewer than 8 bits per word.

### 4. SCOPE AND RISK ASSESSMENT

- **Lines changed:** Net +3 lines of logic. Very small.
- **Files touched:** 1 file (`drivers/spi/spi-stm32.c`).
- **Risk:** Very low. The new logic only activates when `spi->cur_bpw <
  8` AND `cur_midi > 0`. For all transfers with bpw >= 8 (the vast
  majority), behavior is completely unchanged. Even for bpw < 8 cases,
  the fix just increases the inter-data delay slightly, which is correct
  per the hardware spec.
- **Side effects:** Slightly longer word timing for bpw < 8. This is the
  desired behavior to prevent overrun.

### 5. DEPENDENCY ANALYSIS

This is the most significant concern. The function
`stm32h7_spi_data_idleness()` was significantly modified by:
- **`4956bf4452439`** ("spi: stm32: deprecate `st,spi-midi-ns`
  property", June 2025, v6.17-rc1): Changed the function signature from
  `(struct stm32_spi *spi, u32 len)` to `(struct stm32_spi *spi, struct
  spi_transfer *xfer)` and added the `spi_delay_ns` word_delay logic.
- **`244bc18e5f187`** ("spi: stm32: delete stray tabs", June 2025,
  v6.17-rc1+): Fixed indentation.

Both of these are post-6.16. Stable trees (6.12.y, 6.6.y, 6.1.y, 5.15.y)
have the **old** version of this function with the simpler `(spi, len)`
signature. However, the critical MIDI calculation block exists in ALL
versions since the driver's inception:

```c
if ((len > 1) && (spi->cur_midi > 0)) {
    u32 sck_period_ns = DIV_ROUND_UP(NSEC_PER_SEC, spi->cur_speed);
    u32 midi = min_t(u32,
                     DIV_ROUND_UP(spi->cur_midi, sck_period_ns),
                     FIELD_GET(STM32H7_SPI_CFG2_MIDI,
STM32H7_SPI_CFG2_MIDI));
```

The core 3-line overrun fix can be applied to the old function with
trivial context adjustment. The fix does not depend on the new function
signature -- it only uses `spi->cur_bpw` which has existed since the
driver was added.

### 6. CLASSIFICATION

This is a **hardware bug workaround / driver bug fix**. It's analogous
to a hardware quirk -- the STM32H7 SPI hardware requires at least 8
clock cycles per word for safe suspension, and the driver was not
enforcing this. The fix adds the enforcement.

### 7. VERDICT

**Positive factors:**
- Fixes a real, documented hardware issue (overrun = data loss +
  transfer failure)
- Extremely small and surgical (3 lines of logic)
- Very low regression risk (only affects bpw < 8 with midi configured)
- Authored by the hardware vendor (ST Microelectronics) who understands
  the hardware
- Reviewed and merged by the SPI subsystem maintainer
- The affected code exists in all stable trees since the driver was
  added (2017)

**Concerns:**
- The patch as-is won't apply cleanly to stable trees due to the
  function signature change from `4956bf4452439`, but the fix logic is
  self-contained and can be trivially adapted
- The fix only applies when `cur_midi > 0` (some inter-data delay is
  configured), so overrun with zero midi is not addressed (but this
  appears intentional)

The fix is small, surgical, and meets all stable kernel criteria. It
fixes a real hardware bug causing data loss for STM32H7 SPI users using
bpw < 8.

**YES**

 drivers/spi/spi-stm32.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-stm32.c b/drivers/spi/spi-stm32.c
index 2c804c1aef989..80986bd251d29 100644
--- a/drivers/spi/spi-stm32.c
+++ b/drivers/spi/spi-stm32.c
@@ -1906,11 +1906,12 @@ static void stm32h7_spi_data_idleness(struct stm32_spi *spi, struct spi_transfer
 	cfg2_clrb |= STM32H7_SPI_CFG2_MIDI;
 	if ((len > 1) && (spi->cur_midi > 0)) {
 		u32 sck_period_ns = DIV_ROUND_UP(NSEC_PER_SEC, spi->cur_speed);
-		u32 midi = min_t(u32,
-				 DIV_ROUND_UP(spi->cur_midi, sck_period_ns),
-				 FIELD_GET(STM32H7_SPI_CFG2_MIDI,
-				 STM32H7_SPI_CFG2_MIDI));
+		u32 midi = DIV_ROUND_UP(spi->cur_midi, sck_period_ns);
 
+		if ((spi->cur_bpw + midi) < 8)
+			midi = 8 - spi->cur_bpw;
+
+		midi = min_t(u32, midi, FIELD_MAX(STM32H7_SPI_CFG2_MIDI));
 
 		dev_dbg(spi->dev, "period=%dns, midi=%d(=%dns)\n",
 			sck_period_ns, midi, midi * sck_period_ns);
-- 
2.51.0

[PATCH AUTOSEL 6.19-6.6] spi-geni-qcom: use xfer->bits_per_word for can_dma()

[Sasha Levin]

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

[PATCH AUTOSEL 6.19-6.18] spi: cadence-quadspi: Parse DT for flashes with the rest of the DT parsing

[Sasha Levin]

From: Mark Brown <broonie@kernel.org>

[ Upstream commit 9f0736a4e136a6eb61e0cf530ddc18ab6d816ba3 ]

The recent refactoring of where runtime PM is enabled done in commit
f1eb4e792bb1 ("spi: spi-cadence-quadspi: Enable pm runtime earlier to
avoid imbalance") made the fact that when we do a pm_runtime_disable()
in the error paths of probe() we can trigger a runtime disable which in
turn results in duplicate clock disables.  This is particularly likely
to happen when there is missing or broken DT description for the flashes
attached to the controller.

Early on in the probe function we do a pm_runtime_get_noresume() since
the probe function leaves the device in a powered up state but in the
error path we can't assume that PM is enabled so we also manually
disable everything, including clocks. This means that when runtime PM is
active both it and the probe function release the same reference to the
main clock for the IP, triggering warnings from the clock subsystem:

[    8.693719] clk:75:7 already disabled
[    8.693791] WARNING: CPU: 1 PID: 185 at /usr/src/kernel/drivers/clk/clk.c:1188 clk_core_disable+0xa0/0xb
...
[    8.694261]  clk_core_disable+0xa0/0xb4 (P)
[    8.694272]  clk_disable+0x38/0x60
[    8.694283]  cqspi_probe+0x7c8/0xc5c [spi_cadence_quadspi]
[    8.694309]  platform_probe+0x5c/0xa4

Dealing with this issue properly is complicated by the fact that we
don't know if runtime PM is active so can't tell if it will disable the
clocks or not.  We can, however, sidestep the issue for the flash
descriptions by moving their parsing to when we parse the controller
properties which also save us doing a bunch of setup which can never be
used so let's do that.

Reported-by: Francesco Dolcini <francesco@dolcini.it>
Closes: https://lore.kernel.org/r/20251201072844.GA6785@francesco-nb
Signed-off-by: Mark Brown <broonie@kernel.org>
Link: https://patch.msgid.link/20251204-spi-cadence-qspi-runtime-pm-imbalance-v2-1-10af9115d531@kernel.org
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have all the information I need. Let me compile a comprehensive
analysis.

---

## Comprehensive Analysis

### 1. COMMIT MESSAGE ANALYSIS

The commit by Mark Brown (SPI subsystem maintainer) addresses a double
clock disable triggered during `cqspi_probe()` error handling,
specifically when device tree flash descriptions are missing or broken.
The commit references:

- **Root cause commit**: `f1eb4e792bb1` ("spi: spi-cadence-quadspi:
  Enable pm runtime earlier to avoid imbalance") - merged in v6.18
- **Reporter**: Francesco Dolcini (Toradex engineer) with a specific bug
  link
- **Symptom**: Kernel WARNING "clk:75:7 already disabled" plus a full
  stack trace from `clk_core_disable` -> `clk_disable` -> `cqspi_probe`

### 2. CODE CHANGE ANALYSIS

The change is very simple: it moves the `cqspi_setup_flash()` call from
**after** PM runtime is configured to **before** any resource
acquisition:

**Before (current v6.19 code)** - `cqspi_setup_flash()` at line 1990,
after pm_runtime_enable + pm_runtime_get_noresume:

```1983:1994:/home/sasha/linux-autosel/drivers/spi/spi-cadence-quadspi.c
        if (!(ddata && (ddata->quirks & CQSPI_DISABLE_RUNTIME_PM))) {
                pm_runtime_enable(dev);
                pm_runtime_set_autosuspend_delay(dev,
CQSPI_AUTOSUSPEND_TIMEOUT);
                pm_runtime_use_autosuspend(dev);
                pm_runtime_get_noresume(dev);
        }

        ret = cqspi_setup_flash(cqspi);
        if (ret) {
                dev_err(dev, "failed to setup flash parameters %d\n",
ret);
                goto probe_setup_failed;
        }
```

**After (with patch applied)** - `cqspi_setup_flash()` right after
`cqspi_of_get_pdata()`, before clock/resource acquisition:

```c
        ret = cqspi_of_get_pdata(cqspi);
        if (ret) { ... return -ENODEV; }

        ret = cqspi_setup_flash(cqspi);
        if (ret) {
                dev_err(dev, "failed to setup flash parameters %d\n",
ret);
                return ret;
        }

        /* Obtain QSPI clock. */
        cqspi->clk = devm_clk_get(dev, NULL);
```

**Why this is safe**: Looking at `cqspi_setup_flash()` (lines
1727-1765), it:
- Iterates device tree child nodes with
  `for_each_available_child_of_node_scoped`
- Reads DT properties (`reg`, `cdns,read-delay`, `cdns,tshsl-ns`, etc.)
- Populates `f_pdata` structures and `cqspi->num_chipselect`
- Has **zero** hardware dependencies (no clock access, no IO access, no
  IRQ, no PM runtime)

Its only dependency is `cqspi->num_chipselect` which is set by
`cqspi_of_get_pdata()` (called just before in both old and new code).

### 3. THE BUG MECHANISM

The error path when `cqspi_setup_flash()` fails in the current code:

```2021:2033:/home/sasha/linux-autosel/drivers/spi/spi-cadence-quadspi.c
probe_setup_failed:
        if (!(ddata && (ddata->quirks & CQSPI_DISABLE_RUNTIME_PM)))
                pm_runtime_disable(dev);
        cqspi_controller_enable(cqspi, 0);
probe_reset_failed:
        if (cqspi->is_jh7110)
                cqspi_jh7110_disable_clk(pdev, cqspi);

        if (pm_runtime_get_sync(&pdev->dev) >= 0)
                clk_disable_unprepare(cqspi->clk);
probe_clk_failed:
        return ret;
```

The problem: `pm_runtime_disable()` can interact with the PM runtime
state in a way that the runtime suspend callback
(`cqspi_runtime_suspend`) gets invoked, which calls
`clk_disable_unprepare(cqspi->clk)`. Then the error path falls through
to `probe_reset_failed` which also calls
`clk_disable_unprepare(cqspi->clk)` via `pm_runtime_get_sync()` +
explicit disable, causing the double disable WARNING.

### 4. FIX APPROACH COMPARISON

This commit represents the subsystem maintainer's preferred approach.
Instead of patching the complex error path interactions (as done in the
earlier attempts: `295fe8406a357`, `1889dd2081975`), it eliminates the
problem entirely by moving the DT parsing before any resource that needs
cleanup. If flash DT is broken, probe fails with a simple `return` - no
PM runtime, no clocks, nothing to clean up.

### 5. CLASSIFICATION AND IMPACT

- **Bug type**: Resource management error (double clock disable) causing
  kernel WARNING
- **Severity**: Medium - kernel WARNING, potential clock framework
  confusion
- **Trigger**: Missing or broken DT flash descriptions for Cadence QSPI
  controller
- **Real-world impact**: Confirmed by Francesco Dolcini (Toradex), a
  major embedded Linux vendor
- **Affected versions**: v6.18+ (where `f1eb4e792bb1` was merged)

### 6. SCOPE AND RISK

- **Lines changed**: ~12 lines moved (6 lines added in new location, 6
  lines removed from old location)
- **Files changed**: 1 (`drivers/spi/spi-cadence-quadspi.c`)
- **Risk**: Very low - pure DT parsing function moved earlier with no
  functional side effects
- **Author**: Mark Brown, the SPI subsystem maintainer

### 7. DEPENDENCIES

- Requires `f1eb4e792bb1` in the tree (v6.18+) since that's the commit
  that introduced the regression
- Compatible with `295fe8406a357` and `1889dd2081975` which are also
  present
- Self-contained; no other patches needed

### 8. STABLE CRITERIA CHECK

- **Obviously correct and tested**: Yes - authored by subsystem
  maintainer, function only does DT parsing
- **Fixes a real bug**: Yes - double clock disable with kernel WARNING
- **Important issue**: Yes - regression from v6.18 on real hardware
- **Small and contained**: Yes - ~12 lines in 1 file
- **No new features**: Correct - pure bug fix
- **Reported-by**: Francesco Dolcini with bug link

**YES**

 drivers/spi/spi-cadence-quadspi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 965b4cea3388a..597a04dbbe2e7 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1844,6 +1844,12 @@ static int cqspi_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	ret = cqspi_setup_flash(cqspi);
+	if (ret) {
+		dev_err(dev, "failed to setup flash parameters %d\n", ret);
+		return ret;
+	}
+
 	/* Obtain QSPI clock. */
 	cqspi->clk = devm_clk_get(dev, NULL);
 	if (IS_ERR(cqspi->clk)) {
@@ -1987,12 +1993,6 @@ static int cqspi_probe(struct platform_device *pdev)
 		pm_runtime_get_noresume(dev);
 	}
 
-	ret = cqspi_setup_flash(cqspi);
-	if (ret) {
-		dev_err(dev, "failed to setup flash parameters %d\n", ret);
-		goto probe_setup_failed;
-	}
-
 	host->num_chipselect = cqspi->num_chipselect;
 
 	if (ddata && (ddata->quirks & CQSPI_SUPPORT_DEVICE_RESET))
-- 
2.51.0

[PATCH AUTOSEL 6.19] spi: cadence-qspi: Fix probe error path and remove

[Sasha Levin]

From: "Miquel Raynal (Schneider Electric)" <miquel.raynal@bootlin.com>

[ Upstream commit f18c8cfa4f1af2cf7d68d86989a7d6109acfa1bb ]

The probe has been modified by many different users, it is hard to track
history, but for sure its current state is partially broken. One easy
rule to follow is to drop/free/release the resources in the opposite
order they have been queried.

Fix the labels, the order for freeing the resources, and add the
missing DMA channel step. Replicate these changes in the remove path as
well.

Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Miquel Raynal (Schneider Electric) <miquel.raynal@bootlin.com>
Tested-by: Santhosh Kumar K <s-k6@ti.com>
Link: https://patch.msgid.link/20260122-schneider-6-19-rc1-qspi-v4-8-f9c21419a3e6@bootlin.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Analysis of spi: cadence-qspi: Fix probe error path and remove

### 1. COMMIT MESSAGE ANALYSIS

The commit message explicitly says "Fix probe error path and remove" —
this is a bug fix. The author clearly states:
- "its current state is partially broken"
- Resources were not being freed in the correct order
- DMA channel release was missing from error paths
- The remove path also had ordering issues

The commit has strong trust indicators:
- **Two "Tested-by:" tags** (Wolfram Sang and Santhosh Kumar K) — both
  are well-known kernel contributors
- Accepted by Mark Brown (SPI subsystem maintainer)
- Part of a versioned series (v4), indicating careful review iterations

### 2. CODE CHANGE ANALYSIS

Let me trace through the specific fixes:

#### A. Probe Error Path Fixes

**Missing DMA channel release:** The old code had `probe_setup_failed`
label that jumped directly to disabling the controller and runtime PM,
but **never released `cqspi->rx_chan`**. The new code adds a dedicated
`release_dma_chan` label:
```c
release_dma_chan:
    if (cqspi->rx_chan)
        dma_release_channel(cqspi->rx_chan);
```
This fixes a **resource leak** — if `spi_register_controller()` fails
after DMA channels are requested, they would never be freed.

**Incorrect error label targets:** Several error paths were jumping to
`probe_reset_failed` which would attempt to disable clocks/reset that
hadn't been enabled yet, or skip cleanup that was needed. The new labels
(`disable_rpm`, `disable_clk`, `disable_clks`, `disable_controller`,
`release_dma_chan`) provide proper granularity matching the actual
resources acquired at each point.

**Reordering of runtime PM disable:** The old code disabled runtime PM
in `probe_setup_failed` (early in cleanup), but the new code moves
`pm_runtime_disable()` to `disable_rpm` (the last cleanup step before
return). This is correct because PM was enabled relatively late in the
probe sequence and should be disabled early in the reverse-order
cleanup.

#### B. Remove Path Fixes

**DMA channel release added before controller disable:** The old remove
path had:
1. Disable controller
2. Release DMA channel

The new order:
1. Release DMA channel
2. Disable controller

This is the correct reverse order — DMA was requested after controller
init, so it should be released before controller disable.

**Clock disable ordering fixed:** The old code disabled clocks before
JH7110-specific clock cleanup. The new code reverses this to match probe
order.

### 3. BUG CLASSIFICATION

This fixes multiple real bugs:
1. **Resource leak**: Missing `dma_release_channel()` in probe error
   path — DMA channels leaked on probe failure
2. **Incorrect cleanup ordering**: Resources freed in wrong order could
   cause use-after-free or access to already-disabled hardware
3. **Wrong goto targets**: Error paths jumping to labels that free
   resources not yet acquired (wasteful but potentially harmful) or that
   skip freeing resources that were acquired (leak)

### 4. SCOPE AND RISK ASSESSMENT

- **Files changed**: 1 file (`drivers/spi/spi-cadence-quadspi.c`)
- **Nature of changes**: Primarily label renaming and reordering of
  cleanup steps — no logic changes to the normal (non-error) path
- **Risk**: LOW. The normal successful probe/remove paths are minimally
  affected. The changes only affect error handling and teardown
  ordering.
- **Subsystem**: SPI driver (Cadence QSPI) — widely used on ARM/embedded
  platforms (TI, Intel, StarFive, Renesas, etc.)

### 5. USER IMPACT

The Cadence QSPI controller is used on many SoCs. Users affected
include:
- Anyone whose probe fails for any reason (missing firmware, DT
  misconfiguration, resource conflicts) — they'd leak DMA channels
- Anyone unloading/reloading the module — remove path had ordering bugs
- Embedded systems that are sensitive to resource leaks

### 6. STABLE SUITABILITY

- **Obviously correct**: Yes — reverse-order cleanup is a well-
  established pattern, and the missing DMA release is clearly a bug
- **Fixes a real bug**: Yes — resource leaks and incorrect cleanup
  ordering
- **Small and contained**: Yes — single file, ~80 lines changed, mostly
  label renaming
- **No new features**: Correct — purely error handling fixes
- **Tested**: Yes — two Tested-by tags from reputable contributors
- **Clean application**: The patch modifies only one file with
  straightforward changes. May need minor adjustment depending on the
  stable version but should be relatively clean.

### 7. DEPENDENCY CHECK

This appears to be self-contained. The cleanup labels and DMA channel
handling are local to this file. The `cqspi_jh7110_disable_clk()`
function and `cqspi_request_mmap_dma()` need to exist in the stable
tree, which they should for any kernel version that has this driver with
JH7110 support and DMA support.

### Conclusion

This commit fixes genuine resource leaks (DMA channels) and incorrect
cleanup ordering in both the probe error path and remove function of a
widely-used SPI controller driver. It's small, well-tested, obviously
correct (follows reverse-order cleanup pattern), and has zero risk to
the normal operation path. It meets all stable kernel criteria.

**YES**

 drivers/spi/spi-cadence-quadspi.c | 44 ++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index 597a04dbbe2e7..b1c95b1c343fc 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1891,7 +1891,7 @@ static int cqspi_probe(struct platform_device *pdev)
 	ret = clk_prepare_enable(cqspi->clk);
 	if (ret) {
 		dev_err(dev, "Cannot enable QSPI clock.\n");
-		goto probe_clk_failed;
+		goto disable_rpm;
 	}
 
 	/* Obtain QSPI reset control */
@@ -1899,14 +1899,14 @@ static int cqspi_probe(struct platform_device *pdev)
 	if (IS_ERR(rstc)) {
 		ret = PTR_ERR(rstc);
 		dev_err(dev, "Cannot get QSPI reset.\n");
-		goto probe_reset_failed;
+		goto disable_clk;
 	}
 
 	rstc_ocp = devm_reset_control_get_optional_exclusive(dev, "qspi-ocp");
 	if (IS_ERR(rstc_ocp)) {
 		ret = PTR_ERR(rstc_ocp);
 		dev_err(dev, "Cannot get QSPI OCP reset.\n");
-		goto probe_reset_failed;
+		goto disable_clk;
 	}
 
 	if (of_device_is_compatible(pdev->dev.of_node, "starfive,jh7110-qspi")) {
@@ -1914,7 +1914,7 @@ static int cqspi_probe(struct platform_device *pdev)
 		if (IS_ERR(rstc_ref)) {
 			ret = PTR_ERR(rstc_ref);
 			dev_err(dev, "Cannot get QSPI REF reset.\n");
-			goto probe_reset_failed;
+			goto disable_clk;
 		}
 		reset_control_assert(rstc_ref);
 		reset_control_deassert(rstc_ref);
@@ -1956,7 +1956,7 @@ static int cqspi_probe(struct platform_device *pdev)
 		if (ddata->jh7110_clk_init) {
 			ret = cqspi_jh7110_clk_init(pdev, cqspi);
 			if (ret)
-				goto probe_reset_failed;
+				goto disable_clk;
 		}
 		if (ddata->quirks & CQSPI_DISABLE_STIG_MODE)
 			cqspi->disable_stig_mode = true;
@@ -1964,7 +1964,7 @@ static int cqspi_probe(struct platform_device *pdev)
 		if (ddata->quirks & CQSPI_DMA_SET_MASK) {
 			ret = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
 			if (ret)
-				goto probe_reset_failed;
+				goto disable_clks;
 		}
 	}
 
@@ -1975,7 +1975,7 @@ static int cqspi_probe(struct platform_device *pdev)
 			       pdev->name, cqspi);
 	if (ret) {
 		dev_err(dev, "Cannot request IRQ.\n");
-		goto probe_reset_failed;
+		goto disable_clks;
 	}
 
 	cqspi_wait_idle(cqspi);
@@ -2002,14 +2002,14 @@ static int cqspi_probe(struct platform_device *pdev)
 		ret = cqspi_request_mmap_dma(cqspi);
 		if (ret == -EPROBE_DEFER) {
 			dev_err_probe(&pdev->dev, ret, "Failed to request mmap DMA\n");
-			goto probe_setup_failed;
+			goto disable_controller;
 		}
 	}
 
 	ret = spi_register_controller(host);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register SPI ctlr %d\n", ret);
-		goto probe_setup_failed;
+		goto release_dma_chan;
 	}
 
 	if (!(ddata && (ddata->quirks & CQSPI_DISABLE_RUNTIME_PM))) {
@@ -2018,17 +2018,22 @@ static int cqspi_probe(struct platform_device *pdev)
 	}
 
 	return 0;
-probe_setup_failed:
-	if (!(ddata && (ddata->quirks & CQSPI_DISABLE_RUNTIME_PM)))
-		pm_runtime_disable(dev);
+
+release_dma_chan:
+	if (cqspi->rx_chan)
+		dma_release_channel(cqspi->rx_chan);
+disable_controller:
 	cqspi_controller_enable(cqspi, 0);
-probe_reset_failed:
+disable_clks:
 	if (cqspi->is_jh7110)
 		cqspi_jh7110_disable_clk(pdev, cqspi);
-
+disable_clk:
 	if (pm_runtime_get_sync(&pdev->dev) >= 0)
 		clk_disable_unprepare(cqspi->clk);
-probe_clk_failed:
+disable_rpm:
+	if (!(ddata && (ddata->quirks & CQSPI_DISABLE_RUNTIME_PM)))
+		pm_runtime_disable(dev);
+
 	return ret;
 }
 
@@ -2046,18 +2051,19 @@ static void cqspi_remove(struct platform_device *pdev)
 		cqspi_wait_idle(cqspi);
 
 	spi_unregister_controller(cqspi->host);
-	cqspi_controller_enable(cqspi, 0);
 
 	if (cqspi->rx_chan)
 		dma_release_channel(cqspi->rx_chan);
 
-	if (!(ddata && (ddata->quirks & CQSPI_DISABLE_RUNTIME_PM)))
-		if (pm_runtime_get_sync(&pdev->dev) >= 0)
-			clk_disable(cqspi->clk);
+	cqspi_controller_enable(cqspi, 0);
 
 	if (cqspi->is_jh7110)
 		cqspi_jh7110_disable_clk(pdev, cqspi);
 
+	if (!(ddata && (ddata->quirks & CQSPI_DISABLE_RUNTIME_PM)))
+		if (pm_runtime_get_sync(&pdev->dev) >= 0)
+			clk_disable(cqspi->clk);
+
 	if (!(ddata && (ddata->quirks & CQSPI_DISABLE_RUNTIME_PM))) {
 		pm_runtime_put_sync(&pdev->dev);
 		pm_runtime_disable(&pdev->dev);
-- 
2.51.0

[PATCH AUTOSEL 6.19-6.12] spi: geni-qcom: Fix abort sequence execution for serial engine errors

[Sasha Levin]

From: Praveen Talari <praveen.talari@oss.qualcomm.com>

[ Upstream commit 96e041647bb0f9d92f95df1d69cb7442d7408b79 ]

The driver currently skips the abort sequence for target mode when serial
engine errors occur. This leads to improper error recovery as the serial
engine may remain in an undefined state without proper cleanup, potentially
causing subsequent operations to fail or behave unpredictably.

Fix this by ensuring the abort sequence and DMA reset always execute during
error recovery, as both are required for proper serial engine error
handling.

Co-developed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Link: https://patch.msgid.link/20260204162854.1206323-3-praveen.talari@oss.qualcomm.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I can see the **current code** (before the patch). The issue is
clear:

**Before the fix (buggy code):**
- When `spi->target` is true, the code does `goto reset_if_dma`, which
  **skips both**:
  1. The cancel command (correct - target mode doesn't support cancel)
  2. The abort command (INCORRECT - abort is still needed for proper
     error recovery)

**After the fix:**
- When `spi->target` is true, only the cancel command is skipped
- The abort command (`geni_se_abort_m_cmd()`) and subsequent DMA reset
  **always execute** regardless of target/controller mode

The logic restructuring:
- The old code: `if (spi->target) { goto reset_if_dma; }` - skips
  everything (cancel AND abort)
- The new code: `if (!spi->target) { do cancel; if cancel succeeded,
  goto reset_if_dma; }` then **always** falls through to the abort
  sequence

This is a critical error recovery path fix. Without the abort, the
serial engine can remain stuck in an error state.

### 3. CLASSIFICATION

This is clearly a **bug fix** for improper error recovery in the SPI
GENI Qualcomm driver:
- Fixes a functional bug where the abort sequence is incorrectly skipped
- The serial engine can get stuck in an undefined state
- Subsequent SPI transactions can fail

### 4. SCOPE AND RISK ASSESSMENT

- **Lines changed:** Small - approximately 10-15 lines of logic
  restructuring within a single function
- **Files touched:** 1 file (`drivers/spi/spi-geni-qcom.c`)
- **Subsystem:** SPI driver, specifically Qualcomm GENI SPI
- **Risk:** LOW - The change is well-contained within
  `handle_se_timeout()`, only affects error recovery logic, and the
  abort command is the standard recovery mechanism for the serial
  engine. The cancel path for controller mode is preserved unchanged.
- **The change is purely a control flow fix** - no new functions, no new
  variables, no new APIs

### 5. USER IMPACT

- **Who is affected:** Users of Qualcomm platforms (Snapdragon-based
  devices, many ARM boards/phones) using SPI in target mode
- **Severity:** When the bug triggers, the SPI bus can become stuck,
  requiring a reboot or causing cascading failures for SPI peripherals
- **Real-world impact:** This affects error recovery - when things go
  wrong (timeouts, SE errors), the system can't recover properly. This
  is a significant reliability issue.

### 6. STABILITY INDICATORS

- **Reviewed-by:** Konrad Dybcio (Qualcomm maintainer)
- **Signed-off-by:** Mark Brown (SPI subsystem maintainer)
- The commit is clearly described and the logic is straightforward

### 7. DEPENDENCY CHECK

The fix is self-contained - it restructures control flow within a single
function using existing APIs (`geni_se_abort_m_cmd`,
`geni_se_cancel_m_cmd`). No dependencies on other commits. The affected
code has been in the kernel for a while (the `spi->target` check and the
abort/cancel framework).

### Summary

This is a clear, small, well-reviewed bug fix that corrects an error
recovery path in the Qualcomm GENI SPI driver. Without this fix, when
serial engine errors occur in target mode, the abort sequence is
skipped, leaving the hardware in an undefined state. The fix is surgical
(single function, single file), low risk, and addresses a real
reliability problem on Qualcomm platforms. It meets all stable kernel
criteria.

**YES**

 drivers/spi/spi-geni-qcom.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/spi/spi-geni-qcom.c b/drivers/spi/spi-geni-qcom.c
index 5ab20d7955121..acfcf870efd84 100644
--- a/drivers/spi/spi-geni-qcom.c
+++ b/drivers/spi/spi-geni-qcom.c
@@ -160,24 +160,20 @@ static void handle_se_timeout(struct spi_controller *spi,
 	xfer = mas->cur_xfer;
 	mas->cur_xfer = NULL;
 
-	if (spi->target) {
-		/*
-		 * skip CMD Cancel sequnece since spi target
-		 * doesn`t support CMD Cancel sequnece
-		 */
+	/* The controller doesn't support the Cancel commnand in target mode */
+	if (!spi->target) {
+		reinit_completion(&mas->cancel_done);
+		geni_se_cancel_m_cmd(se);
+
 		spin_unlock_irq(&mas->lock);
-		goto reset_if_dma;
-	}
 
-	reinit_completion(&mas->cancel_done);
-	geni_se_cancel_m_cmd(se);
-	spin_unlock_irq(&mas->lock);
+		time_left = wait_for_completion_timeout(&mas->cancel_done, HZ);
+		if (time_left)
+			goto reset_if_dma;
 
-	time_left = wait_for_completion_timeout(&mas->cancel_done, HZ);
-	if (time_left)
-		goto reset_if_dma;
+		spin_lock_irq(&mas->lock);
+	}
 
-	spin_lock_irq(&mas->lock);
 	reinit_completion(&mas->abort_done);
 	geni_se_abort_m_cmd(se);
 	spin_unlock_irq(&mas->lock);
-- 
2.51.0

[PATCH AUTOSEL 6.19-5.10] spi: spi-mem: Protect dirmap_create() with spi_mem_access_start/end

[Sasha Levin]

From: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>

[ Upstream commit 53f826ff5e0e3ecb279862ca7cce1491b94bb017 ]

spi_mem_dirmap_create() may reconfigure controller-wide settings,
which can interfere with concurrent transfers to other devices
sharing the same SPI controller but using different chip selects.

Wrap the ->dirmap_create() callback with spi_mem_access_start() and
spi_mem_access_end() to serialize access and prevent cross-CS
interference during dirmap creation.

This patch has been verified on a setup where a SPI TPM is connected
to CS0 of a SPI controller, while a SPI NOR flash is connected to CS1
of the same controller. Without this patch, spi_mem_dirmap_create()
for the SPI NOR flash interferes with ongoing SPI TPM data transfers,
resulting in failure to create the TPM device. This was tested on an
ASPEED AST2700 EVB.

Signed-off-by: Chin-Ting Kuo <chin-ting_kuo@aspeedtech.com>
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Link: https://patch.msgid.link/20260120123005.1392071-2-chin-ting_kuo@aspeedtech.com
Signed-off-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

### 3. Pattern Consistency

The pattern is **completely consistent** with existing code. Looking at
the existing usages:
- `exec_op` (line 405-413): wrapped with start/end
- `dirmap_read` (line 849-855): wrapped with start/end
- `dirmap_write` (line 895-901): wrapped with start/end
- `poll_status` (line 967-975): wrapped with start/end
- **`dirmap_create`**: was **NOT** wrapped — this is the bug being fixed

The `dirmap_create` callback was the only mem_ops callback **missing**
this serialization. This is clearly an oversight/bug.

### 4. What the Serialization Does

`spi_mem_access_start()`:
1. Flushes the SPI message queue to prevent preemption of regular
   transfers
2. Handles runtime PM
3. Acquires `bus_lock_mutex` and `io_mutex`

This is the standard mechanism for serializing SPI memory operations
against each other and against regular SPI transfers. Without it,
`dirmap_create` can race with:
- Normal SPI transfers to other devices (like the TPM in the test case)
- Other spi-mem operations

### 5. Bug Severity and Impact

- **Concrete failure**: TPM device creation fails, meaning the TPM is
  completely unusable
- **Real hardware**: Tested on ASPEED AST2700 EVB, but this affects any
  multi-CS SPI controller setup
- **TPM implications**: TPM is security-critical hardware (used for
  secure boot, disk encryption, attestation)
- **Multi-CS SPI is common**: Many embedded systems have multiple SPI
  devices on one controller

### 6. Risk Assessment

**Very low risk:**
- The change adds exactly the same serialization pattern used by all
  other spi-mem callbacks
- It's a small, surgical change (~12 lines of actual change)
- The error handling is correct (kfree on failure, proper return)
- Reviewed by Paul Menzel, merged by Mark Brown (SPI maintainer)
- Existing code in the file already demonstrates this exact pattern 4
  times

**No dependency concerns:**
- `spi_mem_access_start()` and `spi_mem_access_end()` have existed since
  the spi-mem subsystem was created
- No new functions or APIs introduced
- Will apply cleanly to any stable tree that has spi-mem support

### 7. Classification

This is a **race condition fix** — specifically, missing serialization
for a controller-wide operation. The `dirmap_create` callback can modify
controller-wide settings (like register configurations for direct
mapping) without holding the bus lock, allowing concurrent transfers on
other chip selects to see inconsistent state or interfere.

### 8. Stable Criteria Checklist

- **Obviously correct**: Yes — follows the exact pattern of every other
  callback in the same file
- **Fixes a real bug**: Yes — verified failure on real hardware (TPM
  device creation failure)
- **Important issue**: Yes — prevents device creation failure, affects
  security-critical hardware (TPM)
- **Small and contained**: Yes — single file, ~12 lines of change
- **No new features**: Correct — only adds missing serialization
- **No new APIs**: Correct

### Conclusion

This is a textbook stable backport candidate. It fixes a real race
condition that causes device creation failure on multi-CS SPI
controllers. The fix is small, follows existing patterns exactly, is
well-tested on real hardware, and has been reviewed by maintainers. The
risk of regression is minimal since it's simply adding the same locking
used by all other spi-mem operations.

**YES**

 drivers/spi/spi-mem.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index 6c7921469b90b..965673bac98b9 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -719,9 +719,18 @@ spi_mem_dirmap_create(struct spi_mem *mem,
 
 	desc->mem = mem;
 	desc->info = *info;
-	if (ctlr->mem_ops && ctlr->mem_ops->dirmap_create)
+	if (ctlr->mem_ops && ctlr->mem_ops->dirmap_create) {
+		ret = spi_mem_access_start(mem);
+		if (ret) {
+			kfree(desc);
+			return ERR_PTR(ret);
+		}
+
 		ret = ctlr->mem_ops->dirmap_create(desc);
 
+		spi_mem_access_end(mem);
+	}
+
 	if (ret) {
 		desc->nodirmap = true;
 		if (!spi_mem_supports_op(desc->mem, &desc->info.op_tmpl))
-- 
2.51.0