patches.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Cristian Marussi <cristian.marussi@arm.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Sasha Levin <sashal@kernel.org>,
	arm-scmi@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: [PATCH AUTOSEL 6.17-6.6] firmware: arm_scmi: Skip RAW initialization on failure
Date: Tue, 28 Oct 2025 08:48:07 -0400	[thread overview]
Message-ID: <20251028124815.1058740-7-sashal@kernel.org> (raw)
In-Reply-To: <20251028124815.1058740-1-sashal@kernel.org>

From: Cristian Marussi <cristian.marussi@arm.com>

[ Upstream commit 092b9e2ce6dd63d2f36822751a51957412706986 ]

Avoid attempting to initialize RAW mode when the debug subsystem itself
has failed to initialize, since doing so is pointless and emits
misleading error messages.

Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
Message-Id: <20251014115346.2391418-3-cristian.marussi@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have enough information to make my determination. Let me create a
comprehensive analysis:

## BACKPORT ANALYSIS DECISION

**Backport Status: YES**

---

## COMPREHENSIVE ANALYSIS

### 1. CODE CHANGE ANALYSIS

The commit makes two related changes:

**Change 1** (drivers/firmware/arm_scmi/driver.c:3047-3048):
```c
- if (!info->dbg)
- return -EINVAL;
```
Removes the NULL check from inside `scmi_debugfs_raw_mode_setup()`.

**Change 2** (drivers/firmware/arm_scmi/driver.c:3221):
```c
- if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT)) {
+               if (info->dbg &&
IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT)) {
```
Adds guard condition to prevent calling the function when `info->dbg` is
NULL.

### 2. SEMANTIC ANALYSIS PERFORMED

**Tools Used:**
- `git log` and `git show` for commit history analysis
- `Grep` for code pattern searches across SCMI subsystem
- `Read` for examining driver code and Kconfig
- Task agent (Explore) for understanding SCMI ecosystem context
- Git blame and tag analysis for version tracking

**Key Findings from Analysis:**

**Impact Scope (from code examination):**
- Function `scmi_debugfs_raw_mode_setup()` is called from only ONE
  location: `scmi_probe()` at line 3222
- Function accesses `info->dbg->top_dentry` at line 3068, which would be
  a **NULL pointer dereference** if the removed check wasn't there
- The calling site now has proper guard, so function is never called
  with NULL dbg

**Caller Analysis:**
- `scmi_probe()` is the driver probe function - called during device
  initialization
- Prior commits in series (2290ab43b9d8e, 289ce7e9a5e1a) fixed NULL
  handling in debug helpers
- This commit completes the debug failure handling by preventing
  pointless RAW init attempt

**Dependency Analysis:**
- RAW mode requires debugfs (CONFIG dependency in Kconfig line 35)
- `scmi_raw_mode_init()` expects valid `top_dentry` parameter (line
  3068)
- If debug setup fails, `info->dbg` is NULL, making RAW mode init
  impossible

### 3. BUG IMPACT ANALYSIS

**Affected Kernel Versions:**
- Introduced: v6.3-rc1 (commit 7063887b53860, January 2023)
- Fixed: v6.18-rc3 (commit 092b9e2ce6dd6, October 2025)
- **Affected range: v6.3 through v6.17**

**Failure Scenario:**
```c
// Before fix - in scmi_probe():
if (scmi_top_dentry) {
    info->dbg = scmi_debugfs_common_setup(info);  // Returns NULL on
failure
    if (!info->dbg)
        dev_warn(dev, "Failed to setup SCMI debugfs.\n");

    if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT)) {
        ret = scmi_debugfs_raw_mode_setup(info);  // Called with NULL
dbg!
        if (!coex) {  // Default config
            if (ret)  // ret = -EINVAL
                goto clear_dev_req_notifier;  // PROBE FAILS!
            return 0;
        }
    }
}
```

**Real-World Impact:**
- **Who is affected**: Development/testing systems with
  CONFIG_ARM_SCMI_RAW_MODE_SUPPORT=y
- **When it triggers**: When debugfs initialization fails (no debugfs
  support, memory failure, mount issues)
- **What breaks**: **Entire SCMI driver probe fails** → system may not
  boot
- **Criticality**: SCMI manages power domains, clocks, DVFS, thermal
  sensors - critical for ARM platforms

From Explore agent findings:
- SCMI is used on STM32MP, ARM Juno, NXP i.MX95 platforms
- Controls essential services: CPU frequency, power states, system
  shutdown/reboot
- Kconfig notes it "may be needed early in boot" for power control

**Severity Assessment:**
- **Medium-High for affected configurations**
- Not production issue (RAW mode is debug/testing feature per Kconfig
  line 42)
- But complete driver failure is **unacceptable** for development
  systems
- Misleading error messages make debugging difficult

### 4. ARCHITECTURAL & SIDE EFFECTS

**Architectural Impact:** None
- No data structure changes
- No API modifications
- No changes to core SCMI protocol handling

**Side Effects:** Positive only
- Prevents probe failure when debugfs fails
- Eliminates misleading error messages
- Improves graceful degradation
- No functional changes to success path

### 5. STABLE TREE COMPLIANCE

**Compliance Check:**
✅ **Bug fix**: Yes - fixes probe failure
✅ **Small and contained**: 2 simple changes in 1 file
✅ **Obviously correct**: Guard check before function call
✅ **No new features**: Pure bug fix
✅ **No architectural changes**: Minimal scope
✅ **Low regression risk**: Only affects error path

**Stable Tree Rules Alignment:**
- Fixes important bug (probe failure)
- Self-contained change
- Easy to review and verify
- Part of coordinated fix series (related commits already backported)

### 6. BACKPORT INDICATORS

**Evidence for backporting:**
- **Related commits already backported**: Commit 2290ab43b9d8e (with
  Fixes tag) has been backported to multiple stable trees
  (7056e61a28a83, 4c0f9a50f2e98, fae8405cfddb0, 58d3e2680bea9,
  30c89140a4ddc, 75446183128d2)
- **Part of fix series**: This is patch 3/3 in a coordinated series
  addressing debug initialization failures
- **Fixes merged in mainline**: Present in v6.18-rc3, merged via
  arm/fixes tree (71a5970259c3f)
- **No Cc: stable tag**: But related patches were backported, suggesting
  subsystem maintainers want the series in stable

**Evidence against backporting:**
- No explicit "Fixes:" tag (but commit message clearly states it fixes a
  problem)
- No "Cc: stable@vger.kernel.org" tag
- Only affects debug/testing configuration

### 7. DEPENDENCY CHECK

**Depends on:**
- None - independent fix
- Works better with commits 2290ab43b9d8e and 289ce7e9a5e1a (already
  backported)

**Required by:**
- None identified

---

## FINAL RECOMMENDATION

**YES - This commit SHOULD be backported to stable kernel trees.**

**Justification:**

1. **Fixes probe failure**: When debugfs fails + RAW mode enabled
   (without coex), entire SCMI driver fails to load, breaking critical
   platform services

2. **Completes backported series**: Related fixes (2290ab43b9d8e,
   289ce7e9a5e1a) already backported to stable. This commit completes
   the debug failure handling

3. **Low risk, high correctness**: Minimal change, obviously correct
   logic (guard before function call)

4. **Stable tree compliant**: Small, contained, bug-fix-only change with
   no architectural impact

5. **Affects supported feature**: CONFIG_ARM_SCMI_RAW_MODE_SUPPORT
   exists in stable kernels since v6.3

6. **Graceful degradation**: Enables proper fallback behavior when debug
   subsystem fails

**Target stable trees:** v6.3+ (where RAW mode was introduced)

**Priority:** Medium - affects development/testing systems, not
production, but fixes complete driver failure

 drivers/firmware/arm_scmi/driver.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index bd56a877fdfc8..85392228d1739 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -3044,9 +3044,6 @@ static int scmi_debugfs_raw_mode_setup(struct scmi_info *info)
 	u8 channels[SCMI_MAX_CHANNELS] = {};
 	DECLARE_BITMAP(protos, SCMI_MAX_CHANNELS) = {};
 
-	if (!info->dbg)
-		return -EINVAL;
-
 	/* Enumerate all channels to collect their ids */
 	idr_for_each_entry(&info->tx_idr, cinfo, id) {
 		/*
@@ -3218,7 +3215,7 @@ static int scmi_probe(struct platform_device *pdev)
 		if (!info->dbg)
 			dev_warn(dev, "Failed to setup SCMI debugfs.\n");
 
-		if (IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT)) {
+		if (info->dbg && IS_ENABLED(CONFIG_ARM_SCMI_RAW_MODE_SUPPORT)) {
 			ret = scmi_debugfs_raw_mode_setup(info);
 			if (!coex) {
 				if (ret)
-- 
2.51.0


  parent reply	other threads:[~2025-10-28 12:48 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-28 12:48 [PATCH AUTOSEL 6.17-6.1] spi: intel-pci: Add support for Intel Wildcat Lake SPI serial flash Sasha Levin
2025-10-28 12:48 ` [PATCH AUTOSEL 6.17] erofs: consolidate z_erofs_extent_lookback() Sasha Levin
2025-10-28 12:48 ` [PATCH AUTOSEL 6.17-6.12] drm/panic: Fix overlap between qr code and logo Sasha Levin
2025-10-28 12:48 ` [PATCH AUTOSEL 6.17-6.1] net: datagram: introduce datagram_poll_queue for custom receive queues Sasha Levin
2025-10-28 12:48 ` [PATCH AUTOSEL 6.17-5.4] of/irq: Fix OF node refcount in of_msi_get_domain() Sasha Levin
2025-10-28 12:48 ` [PATCH AUTOSEL 6.17-6.1] riscv: mm: Define MAX_POSSIBLE_PHYSMEM_BITS for zsmalloc Sasha Levin
2025-10-28 12:48 ` Sasha Levin [this message]
2025-10-28 12:48 ` [PATCH AUTOSEL 6.17-6.1] spi: intel-pci: Add support for Arrow Lake-H SPI serial flash Sasha Levin
2025-10-28 12:48 ` [PATCH AUTOSEL 6.17-6.12] drm/panic: Fix divide by 0 if the screen width < font width 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=20251028124815.1058740-7-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=arm-scmi@vger.kernel.org \
    --cc=cristian.marussi@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=patches@lists.linux.dev \
    --cc=stable@vger.kernel.org \
    --cc=sudeep.holla@arm.com \
    /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).