Netdev List
 help / color / mirror / Atom feed
* [PATCH net] net: wwan: iosm: Re-bind the driver around system suspend too
@ 2026-11-05 10:20 Zev Paneth
  2026-11-09 10:33 ` netdev-bot+sashiko
  2026-11-09 22:33 ` Maciej S. Szmigiero
  0 siblings, 2 replies; 3+ messages in thread
From: Zev Paneth @ 2026-11-05 10:20 UTC (permalink / raw)
  To: Loic Poulain, Sergey Ryazanov
  Cc: Johannes Berg, Maciej S . Szmigiero, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, netdev,
	Zev Paneth

On some platforms the modem loses power during suspend-to-idle even
though the driver keeps the device in D0 and its s2idle resume path
only sends a wake-up command. After a long enough s2idle cycle (about
five minutes or more on the test platform) the device comes back in
its boot ROM stage: resume logs "msg timeout", ModemManager then fails
to open the ports ("PORT open refused, phase A-ROM", "Failed to find
primary AT port"), and once userspace has touched the ports the modem
stays dead until a full reboot. This is the same failure mode that
commit 0b6f6593aa8c ("net: wwan: iosm: Fix hibernation by re-binding
the driver around it") addressed for hibernation.

Extend that approach to system suspend: unbind the driver on
PM_SUSPEND_PREPARE and re-bind it on PM_POST_SUSPEND, so the device is
re-initialized from scratch on resume, exactly as it is at boot.

The cost is a full modem re-enumeration after every suspend (about
50 seconds until reconnected on the test platform), also on platforms
where the modem does survive s2idle. If that is considered too heavy,
this could instead be gated by a DMI quirk for the affected platforms.

Tested on a Fibocom L860-GL-16 (Intel XMM7560, 8086:7560) in an
HP ProBook 450 G10 (BIOS V72 01.13.01) with kernel 7.0: with this
change the modem survives an 11-minute lid-closed s2idle and
reconnects automatically; without it, any s2idle longer than roughly
five minutes left the modem in A-ROM until reboot.

Signed-off-by: Zev Paneth <zevpaneth@gmail.com>
---
 drivers/net/wwan/iosm/iosm_ipc_pcie.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wwan/iosm/iosm_ipc_pcie.c b/drivers/net/wwan/iosm/iosm_ipc_pcie.c
index 49052eb51..ebd4c45e1 100644
--- a/drivers/net/wwan/iosm/iosm_ipc_pcie.c
+++ b/drivers/net/wwan/iosm/iosm_ipc_pcie.c
@@ -534,12 +534,14 @@ void ipc_pcie_kfree_skb(struct iosm_pcie *ipc_pcie, struct sk_buff *skb)
 
 static int pm_notify(struct notifier_block *nb, unsigned long mode, void *_unused)
 {
-	if (mode == PM_HIBERNATION_PREPARE || mode == PM_RESTORE_PREPARE) {
+	if (mode == PM_HIBERNATION_PREPARE || mode == PM_RESTORE_PREPARE ||
+	    mode == PM_SUSPEND_PREPARE) {
 		if (pci_registered) {
 			pci_unregister_driver(&iosm_ipc_driver);
 			pci_registered = false;
 		}
-	} else if (mode == PM_POST_HIBERNATION || mode == PM_POST_RESTORE) {
+	} else if (mode == PM_POST_HIBERNATION || mode == PM_POST_RESTORE ||
+		   mode == PM_POST_SUSPEND) {
 		if (!pci_registered) {
 			int ret;
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: wwan: iosm: Re-bind the driver around system suspend too
  2026-11-05 10:20 [PATCH net] net: wwan: iosm: Re-bind the driver around system suspend too Zev Paneth
@ 2026-11-09 10:33 ` netdev-bot+sashiko
  2026-11-09 22:33 ` Maciej S. Szmigiero
  1 sibling, 0 replies; 3+ messages in thread
From: netdev-bot+sashiko @ 2026-11-09 10:33 UTC (permalink / raw)
  To: zevpaneth
  Cc: loic.poulain, ryazanov.s.a, johannes, mail, davem, edumazet, kuba,
	pabeni, horms, netdev

Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider.

Critical: 0 · High: 0 (1) · Medium: 4 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message rests on a premise the code contradicts:
  that "the driver keeps the device in D0" during s2idle.
- [Medium] pm_notify() now unbinds and re-binds the driver on
  PM_SUSPEND_PREPARE/PM_POST_SUSPEND with no conditions.
- [Medium] Because the driver is unbound at PM_SUSPEND_PREPARE, which
  suspend_prepare() sends before devices suspend and before processes…
- [Medium] The commit message says the device is "re-initialized from
  scratch on resume, exactly as it is at boot", but nothing resets the…
- [Low] After this patch the driver is unbound before every system-sleep
  transition: suspend (via PM_SUSPEND_PREPARE, sent for every state from…

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch
  makes it reachable on every suspend.

--

net: wwan: iosm: Re-bind the driver around system suspend too

This commit extends pm_notify() in the iosm driver. The PCI driver is now
unregistered on PM_SUSPEND_PREPARE and re-registered on PM_POST_SUSPEND,
in addition to the existing hibernation and restore events. The goal is to
recover a modem that comes back in its boot ROM stage after a long s2idle
cycle by re-probing it from scratch on resume.

> On some platforms the modem loses power during suspend-to-idle even
> though the driver keeps the device in D0 and its s2idle resume path
> only sends a wake-up command.

[Severity: Medium]
Is the "keeps the device in D0" part accurate?

With the driver bound and the D0L1.2 BIOS setting, ipc_pcie_suspend_cb()
calls ipc_pcie_suspend_s2idle(), which only does this:

drivers/net/wwan/iosm/iosm_ipc_pcie.c:ipc_pcie_suspend_s2idle() {
	ipc_cp_irq_sleep_control(ipc_pcie, IPC_MEM_DEV_PM_FORCE_SLEEP);
	...
	set_bit(0, &ipc_pcie->suspend);
	...
	ipc_imem_pm_s2idle_sleep(ipc_pcie->imem, true);
}

Nothing in drivers/net/wwan/iosm/ calls pci_save_state(), and
SIMPLE_DEV_PM_OPS() provides no .suspend_noirq. So pci_pm_suspend_noirq()
falls through to pci_pm_suspend_noirq_common() with state_saved clear:

drivers/pci/pci-driver.c:pci_pm_suspend_noirq_common() {
	if (!pci_dev->state_saved) {
		pci_save_state(pci_dev);
		...
		if (!pci_dev->skip_bus_pm && pci_power_manageable(pci_dev))
			pci_prepare_to_sleep(pci_dev);
	}
	...
}

pci_prepare_to_sleep() then picks D3hot or a platform state for the
modem, possibly D3cold through ACPI.

If the PCI core already moves the modem out of D0 on the D0L12 s2idle
path, could that be what cuts modem power on the test platform?

If so, would it be a more targeted fix to keep the device in D0 for the
D0L12 case (for example by saving PCI state in the suspend callback to
opt out of bus PM) instead of unbinding the driver on every suspend?

> diff --git a/drivers/net/wwan/iosm/iosm_ipc_pcie.c b/drivers/net/wwan/iosm/iosm_ipc_pcie.c
> index 49052eb511baa..ebd4c45e1f4e5 100644
> --- a/drivers/net/wwan/iosm/iosm_ipc_pcie.c
> +++ b/drivers/net/wwan/iosm/iosm_ipc_pcie.c
> @@ -534,12 +534,14 @@ void ipc_pcie_kfree_skb(struct iosm_pcie *ipc_pcie, struct sk_buff *skb)
>  
>  static int pm_notify(struct notifier_block *nb, unsigned long mode, void *_unused)
>  {
> -	if (mode == PM_HIBERNATION_PREPARE || mode == PM_RESTORE_PREPARE) {
> +	if (mode == PM_HIBERNATION_PREPARE || mode == PM_RESTORE_PREPARE ||
> +	    mode == PM_SUSPEND_PREPARE) {
>  		if (pci_registered) {
>  			pci_unregister_driver(&iosm_ipc_driver);
>  			pci_registered = false;
>  		}

[Severity: Medium]
Should this be limited to the affected platforms or suspend states?

suspend_prepare() sends PM_SUSPEND_PREPARE for every suspend state:

kernel/power/suspend.c:suspend_prepare() {
	...
	error = pm_notifier_call_chain_robust(PM_SUSPEND_PREPARE, PM_POST_SUSPEND);
	...
}

Nothing here checks pm_suspend_target_state, ipc_pcie->d3l2_support or
the platform. So every iosm device (7360 and 7560) on every platform now
goes through pci_unregister_driver()->ipc_pcie_remove()->
ipc_imem_cleanup() on each s2idle, S3 or standby entry:

	if (test_and_clear_bit(FULLY_FUNCTIONAL, &ipc_imem->flag)) {
		ipc_mux_deinit(ipc_imem->mux);
		ipc_debugfs_deinit(ipc_imem);
		ipc_wwan_deinit(ipc_imem->wwan);
		ipc_port_deinit(ipc_imem->ipc_port);
	}

That destroys the wwan netdevs, hangs up open AT/MBIM port fds, tears
down devlink and drops the data session. The roughly 50 second reconnect
follows.

Commit 0b6f6593aa8c, which this patch builds on, says it was "Tested on
XMM7360 in HP EliteBook 855 G7 both with s2idle (which uses the existing
suspend / resume handlers)". So at least that configuration worked with
s2idle before this change.

Would the DMI quirk mentioned in the commit message be the safer default?

[Severity: Medium]
What happens to the modem during system suspend once it has no driver?

The unbind happens at PM_SUSPEND_PREPARE, before dpm_suspend(). So
dev->driver is NULL when the PCI core suspends the device, and
pci_pm_suspend_noirq() takes this branch:

drivers/pci/pci-driver.c:pci_pm_suspend_noirq() {
	...
	if (!pm) {
		pci_save_state(pci_dev);
		goto set_unknown;
	}
	...
}

This skips pci_pm_suspend_noirq_common() entirely. As far as I can tell,
that means:

- The driver's own sleep handshake no longer runs. Neither the D0L12
  IPC_MEM_DEV_PM_FORCE_SLEEP doorbell plus ipc_imem_pm_s2idle_sleep()
  nor the D3L2 ipc_pcie_suspend() is called. The only thing sent to the
  modem is the IPC_MEM_DEVICE_IPC_UNINIT doorbell from
  ipc_imem_device_ipc_uninit().

- The endpoint is never put into D3 and wake/PME is never armed, because
  pci_prepare_to_sleep() is where device_may_wakeup() is honored. So
  modem-initiated wakeup cannot work.

- The endpoint no longer sets bus->self->skip_bus_pm. The root port may
  then enter D3hot/D3cold while the endpoint is still in D0 and its CP
  has not been force-slept.

Could this raise suspend power or hurt S0ix residency on platforms where
the modem used to survive s2idle?

[Severity: Low]
This isn't a bug, but with PM_SUSPEND_PREPARE added the driver is now
unbound before every system sleep transition. As a result the
iosm_ipc_pm table can no longer be reached:

static SIMPLE_DEV_PM_OPS(iosm_ipc_pm, ipc_pcie_suspend_cb, ipc_pcie_resume_cb);

The same goes for ipc_pcie_suspend_s2idle()/ipc_pcie_resume_s2idle(),
ipc_pcie_suspend()/ipc_pcie_resume(), the suspend bit checked in the IRQ
path, and the d3l2_support value that ipc_pcie_read_bios_cfg() reads at
probe. The BIOS D0L1.2/D3L2/Disabled setting no longer changes anything.

Should these callbacks be removed, or at least documented as unused?

The kernel-doc for ipc_pcie_suspend()/ipc_pcie_resume() in
iosm_ipc_pcie.h calls them runtime PM callbacks. That was already wrong,
and now it describes functions that nothing can call.

[Severity: High]
This isn't a bug introduced by this patch, but the patch makes an existing
use-after-free in the teardown path reachable on every suspend.

In MUX_AGGREGATION mode (7560), the UL encode paths in
ipc_imem_handle_irq() and ipc_mux_tq_ul_trigger_encode() arm
ipc_imem->adb_timer with a 500 us timeout. Nothing on the teardown path
cancels it. ipc_imem_cleanup() only does:

	hrtimer_cancel(&ipc_imem->td_alloc_timer);
	hrtimer_cancel(&ipc_imem->tdupdate_timer);
	hrtimer_cancel(&ipc_imem->fast_update_timer);
	hrtimer_cancel(&ipc_imem->startup_timer);

It then calls ipc_mux_deinit(), which frees the mux without clearing
ipc_imem->mux. The imem_config_fail error path in ipc_imem_init() also
leaves adb_timer pending.

If adb_timer is still pending at that point:

ipc_imem_adb_timer_cb()
  ipc_task_queue_send_task()
    ipc_imem_tq_adb_timer_cb()
      ipc_mux_ul_adb_finish(ipc_imem->mux)   <- freed mux

ipc_mux_ul_adb_finish() reads ul_adb from the freed mux and writes the
sequence and accounting fields. It can also call
ipc_imem_td_update_timer_start(), which re-arms tdupdate_timer after it
was cancelled. That timer can then fire after ipc_pcie_deinit() has done
kfree(ipc_pcie->imem).

If adb_timer fires after ipc_task_deinit() and kfree(ipc_imem->ipc_task),
ipc_task_queue_send_task() touches the freed task as well.

Before this patch the teardown ran only on rmmod or hibernation. Now
pm_notify() runs it from suspend_prepare() on every s2idle/S3 entry,
before processes are frozen, so UL traffic can still be arming adb_timer
while the driver is unbound.

Should ipc_imem_cleanup() also cancel adb_timer before ipc_mux_deinit()?

> -	} else if (mode == PM_POST_HIBERNATION || mode == PM_POST_RESTORE) {
> +	} else if (mode == PM_POST_HIBERNATION || mode == PM_POST_RESTORE ||
> +		   mode == PM_POST_SUSPEND) {
>  		if (!pci_registered) {
>  			int ret;

[Severity: Medium]
The commit message says the device is "re-initialized from scratch on
resume, exactly as it is at boot". Does that hold when the modem keeps
power?

That covers platforms where the modem survives s2idle, short s2idle
cycles on the affected platform, and PM_POST_SUSPEND after a suspend
that was aborted before platform sleep.

Nothing on this path resets the modem: no FLR, PERST, _RST or cold
reset. Re-probe then finds the CP still in RUN and depends on the UNINIT
handshake done at unbind time in ipc_imem_device_ipc_uninit():

	if (ipc_pcie_check_data_link_active(ipc_imem->pcie)) {
		ipc_doorbell_fire(ipc_imem->pcie, IPC_DOORBELL_IRQ_IPC,
				  IPC_MEM_DEVICE_IPC_UNINIT);
		...
		while ((ipc_state <= IPC_MEM_DEVICE_IPC_DONT_CARE) &&
		       (ipc_state != IPC_MEM_DEVICE_IPC_UNINIT) &&
		       (timeout > 0)) {

This handshake is skipped if the link is not active. It waits at most
about 30 ms and returns void, so an incomplete handshake goes unnoticed.

On re-bind, ipc_imem_config() requires the UNINIT state for the
PSI/EBL/RUN phases:

		if (ipc_imem->ipc_requested_state ==
		    ipc_mmio_get_ipc_state(ipc_imem->mmio)) {
			ipc_imem_ipc_init_check(ipc_imem);

			return 0;
		}
		dev_err(ipc_imem->dev,
			"ipc_status(%d) != IPC_MEM_DEVICE_IPC_UNINIT",
		...
	return -EIO;

If the state is anything else, probe fails. pci_register_driver() still
returns 0 in that case, so pm_notify() sets pci_registered = true with
no device bound.

Could this leave the modem unusable after resume until a manual rebind
or a reboot?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261105102009.34534-1-zevpaneth%40gmail.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: wwan: iosm: Re-bind the driver around system suspend too
  2026-11-05 10:20 [PATCH net] net: wwan: iosm: Re-bind the driver around system suspend too Zev Paneth
  2026-11-09 10:33 ` netdev-bot+sashiko
@ 2026-11-09 22:33 ` Maciej S. Szmigiero
  1 sibling, 0 replies; 3+ messages in thread
From: Maciej S. Szmigiero @ 2026-11-09 22:33 UTC (permalink / raw)
  To: Zev Paneth
  Cc: Johannes Berg, Sergey Ryazanov, Loic Poulain, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, netdev

On 5.11.2026 11:20, Zev Paneth wrote:
> On some platforms the modem loses power during suspend-to-idle even
> though the driver keeps the device in D0 and its s2idle resume path
> only sends a wake-up command. After a long enough s2idle cycle (about
> five minutes or more on the test platform) the device comes back in
> its boot ROM stage: resume logs "msg timeout", ModemManager then fails
> to open the ports ("PORT open refused, phase A-ROM", "Failed to find
> primary AT port"), and once userspace has touched the ports the modem
> stays dead until a full reboot. This is the same failure mode that
> commit 0b6f6593aa8c ("net: wwan: iosm: Fix hibernation by re-binding
> the driver around it") addressed for hibernation.
> 
> Extend that approach to system suspend: unbind the driver on
> PM_SUSPEND_PREPARE and re-bind it on PM_POST_SUSPEND, so the device is
> re-initialized from scratch on resume, exactly as it is at boot.
> 
> The cost is a full modem re-enumeration after every suspend (about
> 50 seconds until reconnected on the test platform), also on platforms
> where the modem does survive s2idle. If that is considered too heavy,
> this could instead be gated by a DMI quirk for the affected platforms.
> 
> Tested on a Fibocom L860-GL-16 (Intel XMM7560, 8086:7560) in an
> HP ProBook 450 G10 (BIOS V72 01.13.01) with kernel 7.0: with this
> change the modem survives an 11-minute lid-closed s2idle and
> reconnects automatically; without it, any s2idle longer than roughly
> five minutes left the modem in A-ROM until reboot.
> 
> Signed-off-by: Zev Paneth <zevpaneth@gmail.com>
> ---
>   drivers/net/wwan/iosm/iosm_ipc_pcie.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
To be honest, I'm not sure such "big hammer" is indeed the right
solution.
Maybe the DMI quirk you describe would work as the last resort.

Could you instead try this iosm s2idle patch at [1]?
This already helped one user.
You should see "forced D3 sleep" at the modem device probe time.

[1]:
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/992#note_2938256

Thanks,
Maciej


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-09 22:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-05 10:20 [PATCH net] net: wwan: iosm: Re-bind the driver around system suspend too Zev Paneth
2026-11-09 10:33 ` netdev-bot+sashiko
2026-11-09 22:33 ` Maciej S. Szmigiero

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox