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: Praveen Talari <praveen.talari@oss.qualcomm.com>,
	Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>,
	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.12] spi: geni-qcom: Fix abort sequence execution for serial engine errors
Date: Fri, 13 Feb 2026 19:59:59 -0500	[thread overview]
Message-ID: <20260214010245.3671907-119-sashal@kernel.org> (raw)
In-Reply-To: <20260214010245.3671907-1-sashal@kernel.org>

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


  parent reply	other threads:[~2026-02-14  1:07 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 ` [PATCH AUTOSEL 6.19-6.6] spi-geni-qcom: use xfer->bits_per_word for can_dma() Sasha Levin
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 ` Sasha Levin [this message]
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-119-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=broonie@kernel.org \
    --cc=konrad.dybcio@oss.qualcomm.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=praveen.talari@oss.qualcomm.com \
    --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