public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Adrian Hunter <adrian.hunter@intel.com>,
	Frank Li <Frank.Li@nxp.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	Sasha Levin <sashal@kernel.org>,
	jarkko.nikula@linux.intel.com
Subject: [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci-pci: Add System Suspend support
Date: Tue, 10 Feb 2026 18:30:54 -0500	[thread overview]
Message-ID: <20260210233123.2905307-9-sashal@kernel.org> (raw)
In-Reply-To: <20260210233123.2905307-1-sashal@kernel.org>

From: Adrian Hunter <adrian.hunter@intel.com>

[ Upstream commit 4280197d154cae1d1d5acb54484da26da04eac32 ]

Assign the driver PM operations pointer, which is necessary for the PCI
subsystem to put the device into a low power state.  Refer to
pci_pm_suspend_noirq() which bails out if the pointer is NULL, before
it has the opportunity to call pci_prepare_to_sleep().

No other actions are necessary as the mipi-i3c-hci driver takes care of
controller state.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Link: https://patch.msgid.link/20260123063325.8210-4-adrian.hunter@intel.com
Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have a complete picture. Let me compile my detailed analysis.

---

## Comprehensive Analysis

### 1. COMMIT MESSAGE ANALYSIS

The commit subject "Add System Suspend support" makes this sound like a
new feature, but the body reveals the real story: it's fixing a bug
where the PCI subsystem **cannot put the device into a low power state**
during system suspend. The commit message specifically references
`pci_pm_suspend_noirq()` which "bails out if the pointer is NULL, before
it has the opportunity to call `pci_prepare_to_sleep()`."

### 2. CODE CHANGE ANALYSIS

The change is extremely minimal:
1. Adds an **empty** `struct dev_pm_ops` (no callbacks):
  ```c
  static const struct dev_pm_ops mipi_i3c_hci_pci_pm_ops = {
  };
  ```
2. Assigns it to the PCI driver structure:
  ```c
  .driver = {
  .pm = pm_ptr(&mipi_i3c_hci_pci_pm_ops)
  },
  ```

### 3. THE BUG MECHANISM

The critical code path is in `pci_pm_suspend_noirq()`:

```863:866:drivers/pci/pci-driver.c
        if (!pm) {
                pci_save_state(pci_dev);
                goto Fixup;
        }
```

When `dev->driver->pm` is NULL (as in the current driver),
`pci_pm_suspend_noirq()` takes the early exit at line 863-866: it saves
PCI state but jumps to `Fixup`, **completely skipping** the call to
`pci_prepare_to_sleep()` at line 895:

```886:896:drivers/pci/pci-driver.c
        if (!pci_dev->state_saved) {
                pci_save_state(pci_dev);

                /*
   - If the device is a bridge with a child in D0 below it,
   - it needs to stay in D0, so check skip_bus_pm to avoid
   - putting it into a low-power state in that case.
                 */
                if (!pci_dev->skip_bus_pm &&
pci_power_manageable(pci_dev))
                        pci_prepare_to_sleep(pci_dev);
        }
```

With the fix (non-NULL but empty `pm` ops), the code:
- Skips the NULL-pm early exit
- Skips the `pm->suspend_noirq` block (callback is NULL)
- Falls through to `pci_save_state()` AND `pci_prepare_to_sleep()`,
  which transitions the device to D3hot/D3cold

### 4. USER IMPACT

Without this fix, the Intel I3C HCI PCI controller stays in **D0 (full
power)** during system suspend. This has real consequences:
- **Higher power consumption during suspend** - device draws power when
  it shouldn't
- **May prevent platform from reaching deeper sleep states** (S0ix/S3)
  on Intel platforms
- Affects Intel Panther Lake, Wildcat Lake, and Nova Lake platforms

This is a known pattern - other PCI drivers like `surface_dtx` and
`aggregator/core.c` use the same empty `dev_pm_ops` trick for exactly
this reason.

### 5. DRIVER EXISTENCE IN STABLE TREES

- **v6.14**: Driver first added (148 lines). No `.driver.pm` pointer.
- **v6.15, v6.16**: No changes to this file.
- **v6.17**: 1 commit (Wildcat Lake-U device IDs). Still no PM.
- **v6.18**: No changes.
- **v6.19**: 13 commits (major refactoring). Still no PM.

The bug exists in **ALL versions** of this driver. However, the diff
from the analyzed commit targets a codebase significantly different from
any released version (references `intel_mi_1_info`, `intel_si_2_info`,
`mfd_remove_devices`, `mipi_i3c_hci_pci_rpm_forbid` - none of which
exist in v6.14-v6.19). This means:
- The exact diff **will NOT apply cleanly** to any stable tree
- A manual adaptation would be needed, but it's **trivially simple**
  (just add the empty struct and pointer to the existing `pci_driver`
  struct)

### 6. CLASSIFICATION

This is a **bug fix**, not a new feature. System suspend is expected to
work correctly for any PCI device. The driver was missing a required
piece (`dev_pm_ops` pointer) that the PCI subsystem needs to properly
power-manage the device. The comment in the code even says "PM ops must
exist for PCI to put a device to a low power state."

### 7. RISK ASSESSMENT

- **Risk: Essentially zero.** The `dev_pm_ops` structure is completely
  empty - no callbacks are defined. Its sole purpose is to not be NULL,
  so the PCI core doesn't take the early-exit path.
- **Lines changed**: ~7 lines added
- **Files changed**: 1
- **Reviewed-by**: Frank Li (NXP)

### 8. CONCERNS

- **Dependency**: The exact diff won't apply cleanly to stable trees
  (context mismatch). A trivial manual adaptation is needed.
- **Hardware newness**: The supported platforms (Panther Lake, etc.) are
  new Intel hardware with potentially limited user base.
- **Not a crash/security bug**: This is a power management regression,
  not a crash or data corruption.

### 9. VERDICT

The fix resolves a real power management bug that affects every user
with Intel I3C HCI hardware doing system suspend. The change is
trivially small, zero-risk, and follows an established pattern used by
other PCI drivers. While the hardware is new and the issue isn't a
crash, broken system suspend is a significant user-visible issue - users
with these platforms will see higher battery drain during suspend, and
the platform may not reach its desired sleep state.

The fix meets stable criteria: it's obviously correct, fixes a real bug,
is small and contained, and doesn't introduce new features. The empty PM
ops structure adds no new behavior - it simply allows the existing PCI
power management infrastructure to work as designed.

**YES**

 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
index dc8ede0f8ad8d..f8ac1ae371cc9 100644
--- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
+++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
@@ -265,6 +265,10 @@ static void mipi_i3c_hci_pci_remove(struct pci_dev *pci)
 	ida_free(&mipi_i3c_hci_pci_ida, dev_id);
 }
 
+/* PM ops must exist for PCI to put a device to a low power state */
+static const struct dev_pm_ops mipi_i3c_hci_pci_pm_ops = {
+};
+
 static const struct pci_device_id mipi_i3c_hci_pci_devices[] = {
 	/* Wildcat Lake-U */
 	{ PCI_VDEVICE(INTEL, 0x4d7c), (kernel_ulong_t)&intel_info},
@@ -287,6 +291,9 @@ static struct pci_driver mipi_i3c_hci_pci_driver = {
 	.id_table = mipi_i3c_hci_pci_devices,
 	.probe = mipi_i3c_hci_pci_probe,
 	.remove = mipi_i3c_hci_pci_remove,
+	.driver = {
+		.pm = pm_ptr(&mipi_i3c_hci_pci_pm_ops)
+	},
 };
 
 module_pci_driver(mipi_i3c_hci_pci_driver);
-- 
2.51.0


  parent reply	other threads:[~2026-02-10 23:31 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-10 23:30 [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-5.15] gfs2: fiemap page fault fix Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.18] dlm: fix recovery pending middle conversion Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.6] smb: client: prevent races in ->query_interfaces() Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Ensure proper bus clean-up Sasha Levin
2026-02-11  7:56   ` Adrian Hunter
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-5.10] audit: add fchmodat2() to change attributes class Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.12] btrfs: fallback to buffered IO if the data profile has duplication Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19] btrfs: don't BUG() on unexpected delayed ref type in run_one_delayed_ref() Sasha Levin
2026-02-10 23:30 ` Sasha Levin [this message]
2026-02-11  7:57   ` [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci-pci: Add System Suspend support Adrian Hunter
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.18] hfsplus: fix volume corruption issue for generic/480 Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.18] kselftest/kublk: include message in _Static_assert for C11 compatibility Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.12] dlm: validate length in dlm_search_rsb_tree Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.18] i3c: mipi-i3c-hci: Stop reading Extended Capabilities if capability ID is 0 Sasha Levin
2026-02-10 23:30 ` [PATCH AUTOSEL 6.19-6.1] fs/buffer: add alert in try_to_free_buffers() for folios without buffers Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-5.15] i3c: master: svc: Initialize 'dev' to NULL in svc_i3c_master_ibi_isr() Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-6.12] statmount: permission check should return EPERM Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-5.10] audit: add missing syscalls to read class Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-5.10] hfsplus: pretend special inodes as regular files Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-5.10] hfsplus: fix volume corruption issue for generic/498 Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-6.18] netfs: when subreq is marked for retry, do not check if it faced an error Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19] hfs: Replace BUG_ON with error handling for CNID count checks Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-6.1] smb: client: add proper locking around ses->iface_last_update Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-6.6] btrfs: handle user interrupt properly in btrfs_trim_fs() Sasha Levin
2026-02-10 23:31 ` [PATCH AUTOSEL 6.19-5.10] minix: Add required sanity checking to minix_check_superblock() Sasha Levin
2026-02-11  7:56 ` [PATCH AUTOSEL 6.19-6.12] i3c: mipi-i3c-hci: Reset RING_OPERATION1 fields during init Adrian Hunter

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=20260210233123.2905307-9-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=Frank.Li@nxp.com \
    --cc=adrian.hunter@intel.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=jarkko.nikula@linux.intel.com \
    --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