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