[PATCH net v2] net: jme: keep RX MAC clock enabled for WoL

[PATCH net v2] net: jme: keep RX MAC clock enabled for WoL

[Franck Baur via B4 Relay]

From: Franck Baur <franck.baur.67@gmail.com>

On the JMicron JMC250 revision 03, the MAC reset performed during
suspend leaves GPREG1_RXCLKOFF set. Although the driver enables Magic
Packet detection, the gated RX MAC clock prevents wake-up from ACPI S3.

Keep the RX MAC clock enabled while WoL is armed. Restore the gated
clock state immediately after disabling WoL during resume, preserving
the RX engine initialization ordering.

Prevent concurrent link-change work from gating the clock again after
WoL has been armed. Quiesce interrupt-driven link-change scheduling,
synchronize pending interrupts, and cancel outstanding link-change
work before programming WoL in suspend and shutdown paths.

Restore the deferred processing state before requesting link
reinitialization on resume.

Tested on an ASUS K42JE with JMC250 [197b:0250] rev 03 using a
Debian 6.12.111 kernel and an externally built module. ACPI S3 wake
with a Magic Packet completed in 19 seconds, while a control without
a Magic Packet resumed through the RTC alarm after 123 seconds.
Ethernet and SSH recovered after resume.

S4 and S5 wake behaviour has not been validated with this revision.

Fixes: 854a2e7c331a ("jme: Safer MAC processor reset sequence")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Franck Baur <franck.baur.67@gmail.com>
---
Changes in v2:
- Restore RX MAC clock gating when WoL is disabled on resume, before
  reinitializing the RX engine.
- Prevent new interrupt-driven link-change work during suspend and
  shutdown, and drain pending work before arming WoL.
- Restore deferred processing before requesting link reinitialization.
- Validate Magic Packet wake from ACPI S3 on a JMC250 rev 03,
  including an RTC-only control test.
- Link to v1:
  https://lore.kernel.org/r/20261011-jme-wol-submit-v1-1-9cc033e259e4@gmail.com/

The full kernel allyesconfig and allmodconfig builds have not been
completed. The JME driver object builds with W=1 under both
configurations. The full allyesconfig build was limited by host
memory during vmlinux.o processing. S4 has not been tested, and S5
wake remains unconfirmed with this revision.
---
 drivers/net/ethernet/jme.c | 37 ++++++++++++++++++++++++++++++++-----
 drivers/net/ethernet/jme.h |  1 +
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/jme.c b/drivers/net/ethernet/jme.c
index 25ec5f757c6828a5f5241340bf3fb01d8e0aa3e9..acb4d41f56e1002c0a34a253be20c780ad9b062f 100644
--- a/drivers/net/ethernet/jme.c
+++ b/drivers/net/ethernet/jme.c
@@ -1502,7 +1502,8 @@ jme_intr_msi(struct jme_adapter *jme, u32 intrstat)
 		 * all other events are ignored
 		 */
 		jwrite32(jme, JME_IEVE, intrstat);
-		schedule_work(&jme->linkch_task);
+		if (!test_bit(JME_FLAG_WOL_QUIESCE, &jme->flags))
+			schedule_work(&jme->linkch_task);
 		goto out_reenable;
 	}
 
@@ -1546,7 +1547,8 @@ jme_intr_msi(struct jme_adapter *jme, u32 intrstat)
 	/*
 	 * Re-enable interrupt
 	 */
-	jwrite32f(jme, JME_IENS, INTR_ENABLE);
+	if (!test_bit(JME_FLAG_WOL_QUIESCE, &jme->flags))
+		jwrite32f(jme, JME_IENS, INTR_ENABLE);
 }
 
 static irqreturn_t
@@ -1892,6 +1894,7 @@ jme_powersave_phy(struct jme_adapter *jme)
 		jme_set_100m_half(jme);
 		if (jme->reg_pmcs & (PMCS_LFEN | PMCS_LREN))
 			jme_wait_link(jme);
+		jme_mac_rxclk_on(jme);
 		jme_clear_pm_enable_wol(jme);
 	} else {
 		jme_phy_off(jme);
@@ -3158,6 +3161,21 @@ jme_shutdown(struct pci_dev *pdev)
 	struct net_device *netdev = pci_get_drvdata(pdev);
 	struct jme_adapter *jme = netdev_priv(netdev);
 
+	if (netif_running(netdev)) {
+		set_bit(JME_FLAG_WOL_QUIESCE, &jme->flags);
+		netif_device_detach(netdev);
+		netif_stop_queue(netdev);
+		jme_stop_irq(jme);
+		synchronize_irq(pdev->irq);
+		jme_stop_irq(jme);
+		cancel_work_sync(&jme->linkch_task);
+		tasklet_disable(&jme->pcc_task);
+		napi_disable(&jme->napi);
+		tasklet_disable(&jme->txclean_task);
+		tasklet_disable(&jme->rxclean_task);
+		tasklet_disable(&jme->rxempty_task);
+	}
+
 	jme_powersave_phy(jme);
 	pci_pme_active(pdev, true);
 }
@@ -3172,11 +3190,16 @@ jme_suspend(struct device *dev)
 	if (!netif_running(netdev))
 		return 0;
 
-	atomic_dec(&jme->link_changing);
-
+	set_bit(JME_FLAG_WOL_QUIESCE, &jme->flags);
 	netif_device_detach(netdev);
 	netif_stop_queue(netdev);
 	jme_stop_irq(jme);
+	synchronize_irq(jme->pdev->irq);
+	jme_stop_irq(jme);
+	cancel_work_sync(&jme->linkch_task);
+	tasklet_disable(&jme->pcc_task);
+	napi_disable(&jme->napi);
+	atomic_dec(&jme->link_changing);
 
 	tasklet_disable(&jme->txclean_task);
 	tasklet_disable(&jme->rxclean_task);
@@ -3215,6 +3238,7 @@ jme_resume(struct device *dev)
 		return 0;
 
 	jme_clear_pm_disable_wol(jme);
+	jme_mac_rxclk_off(jme);
 	jme_phy_on(jme);
 	if (test_bit(JME_FLAG_SSET, &jme->flags))
 		jme_set_link_ksettings(netdev, &jme->old_cmd);
@@ -3226,8 +3250,11 @@ jme_resume(struct device *dev)
 
 	atomic_inc(&jme->link_changing);
 
-	jme_reset_link(jme);
+	napi_enable(&jme->napi);
+	tasklet_enable(&jme->pcc_task);
+	clear_bit(JME_FLAG_WOL_QUIESCE, &jme->flags);
 
+	jme_reset_link(jme);
 	jme_start_irq(jme);
 
 	return 0;
diff --git a/drivers/net/ethernet/jme.h b/drivers/net/ethernet/jme.h
index 860494ff37144da9dc25d3105ecabb77072077b0..ec5dfddf7322730a316a4900026a88e479f8a8a1 100644
--- a/drivers/net/ethernet/jme.h
+++ b/drivers/net/ethernet/jme.h
@@ -450,6 +450,7 @@ enum jme_flags_bits {
 	JME_FLAG_SSET		= 2,
 	JME_FLAG_POLL		= 5,
 	JME_FLAG_SHUTDOWN	= 6,
+	JME_FLAG_WOL_QUIESCE	= 7,
 };
 
 #define TX_TIMEOUT		(5 * HZ)

---
base-commit: af32da41b0327b9c6a37856ba82b6760d6c8d10e
change-id: 20261024-jme-wol-v2-submit-0fe4db872853

Best regards,
-- 
Franck Baur <franck.baur.67@gmail.com>

Re: [PATCH net v2] net: jme: keep RX MAC clock enabled for WoL

[netdev-bot+sashiko]

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

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

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).

- [High] jme_suspend() now calls napi_disable() and
  tasklet_disable(pcc_task) and sets JME_FLAG_WOL_QUIESCE.
- [High] jme_shutdown() checks netif_running() and then calls
  napi_disable() and tasklet_disable() without holding rtnl_lock().
- [Medium] The patch keeps tasklets disabled for long periods while they
  can still be pending or newly scheduled.
- [Medium] Netpoll can queue link-change work after the new quiesce has
  run cancel_work_sync().

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but neither
  jme_close() nor jme_remove_one() calls tasklet_kill(&jme->pcc_task).
- [Medium] This problem wasn't introduced by this patch, but
  jme_shutdown() arms WoL while the RX/TX engines are still enabled and…

--

net: jme: keep RX MAC clock enabled for WoL

This commit keeps the RX MAC clock ungated while WoL is armed on JMC250
rev 03 so that Magic Packet wake from S3 works, and gates the clock again
on resume. It also adds a JME_FLAG_WOL_QUIESCE flag. jme_suspend() and
jme_shutdown() now quiesce IRQs, link-change work, NAPI and tasklets
before programming WoL, and jme_resume() undoes this.

> diff --git a/drivers/net/ethernet/jme.c b/drivers/net/ethernet/jme.c
> index 25ec5f757c682..acb4d41f56e10 100644
> --- a/drivers/net/ethernet/jme.c
> +++ b/drivers/net/ethernet/jme.c
> @@ -1502,7 +1502,8 @@ jme_intr_msi(struct jme_adapter *jme, u32 intrstat)
>  		 * all other events are ignored
>  		 */
>  		jwrite32(jme, JME_IEVE, intrstat);
> -		schedule_work(&jme->linkch_task);
> +		if (!test_bit(JME_FLAG_WOL_QUIESCE, &jme->flags))
> +			schedule_work(&jme->linkch_task);

[Severity: Medium]

Can netpoll still queue linkch_task after jme_suspend() or jme_shutdown()
has already run cancel_work_sync()?

With CONFIG_NET_POLL_CONTROLLER and netconsole bound to this device,
jme_netpoll() calls the interrupt handler directly:

drivers/net/ethernet/jme.c:jme_netpoll() {
	local_irq_save(flags);
	jme_intr(dev->irq, dev);
	local_irq_restore(flags);
}

synchronize_irq() doesn't wait for this caller. netpoll_poll_dev() only
checks netif_running(), and netif_device_detach() doesn't clear that.

Suppose the netpoll CPU reads the flag as clear and is then delayed
(NMI, SMI, vCPU preemption) before schedule_work():

CPU1 (netpoll)                      CPU2 (suspend/shutdown)
jme_intr_msi()
  test_bit(WOL_QUIESCE) == 0
                                    set_bit(JME_FLAG_WOL_QUIESCE)
                                    synchronize_irq()
                                    cancel_work_sync(&jme->linkch_task)
  schedule_work(&jme->linkch_task)

In jme_shutdown(), link_changing is still 1. That means
jme_link_change_work() can run all the way through and reach
jme_reset_mac_processor()->jme_mac_rxclk_off() after WoL is armed. The
commit message says this re-gating is prevented.

In jme_suspend(), the work can overlap the RX/TX ring teardown. If it
starts after atomic_dec(&jme->link_changing), it spins in its
link_changing wait loop until resume.

>  		goto out_reenable;
>  	}

[ ... ]

> @@ -3158,6 +3161,21 @@ jme_shutdown(struct pci_dev *pdev)
>  	struct net_device *netdev = pci_get_drvdata(pdev);
>  	struct jme_adapter *jme = netdev_priv(netdev);
>  
> +	if (netif_running(netdev)) {
> +		set_bit(JME_FLAG_WOL_QUIESCE, &jme->flags);
> +		netif_device_detach(netdev);
> +		netif_stop_queue(netdev);
> +		jme_stop_irq(jme);
> +		synchronize_irq(pdev->irq);
> +		jme_stop_irq(jme);
> +		cancel_work_sync(&jme->linkch_task);
> +		tasklet_disable(&jme->pcc_task);
> +		napi_disable(&jme->napi);

[Severity: High]

Is anything serializing this with jme_close()?

jme_shutdown() checks netif_running() and then calls napi_disable() and
tasklet_disable() without rtnl_lock(). Meanwhile jme_close() tears down
the same NAPI instance under rtnl. device_shutdown() can run during
reboot -f or kexec -e while userspace (for example NetworkManager) is
still able to close the interface.

After a completed disable, napi_disable_locked() leaves
NAPIF_STATE_SCHED and NAPIF_STATE_NPSVC set, so a second disable spins
here forever:

net/core/dev.c:napi_disable_locked() {
	...
		while (val & (NAPIF_STATE_SCHED | NAPIF_STATE_NPSVC)) {
			usleep_range(20, 200);
			val = READ_ONCE(n->state);
		}
	...
}

The napi_disable_pending() guard in JME_NAPI_DISABLE() doesn't detect an
earlier disable that has already completed.

If jme_close() runs JME_NAPI_DISABLE() between the netif_running() check
and this napi_disable(), wouldn't the reboot hang here?

If the order is reversed, jme_close() would hang in napi_disable() while
holding rtnl. Its tasklet_kill() calls would also wait forever on
tasklets that are disabled but still scheduled.

Before this patch, jme_shutdown() didn't touch NAPI or tasklet state.
Would wrapping this block in rtnl_lock()/rtnl_unlock(), as
__igb_shutdown() does, avoid the race?

> +		tasklet_disable(&jme->txclean_task);
> +		tasklet_disable(&jme->rxclean_task);
> +		tasklet_disable(&jme->rxempty_task);
> +	}

[Severity: Medium]

Can these disabled tasklets still be pending, and would that leave
ksoftirqd spinning?

tasklet_disable() waits for a running instance but not for a pending
one. jme_intr_msi() also still schedules the tasklets without checking
JME_FLAG_WOL_QUIESCE:

drivers/net/ethernet/jme.c:jme_intr_msi() {
	...
	if (intrstat & INTR_TMINTR) {
		jwrite32(jme, JME_IEVE, INTR_TMINTR);
		tasklet_schedule(&jme->pcc_task);
	}
	...
}

The handler can still run after the quiesce in three ways:

- through the shared INTx line (jme_request_irq() uses IRQF_SHARED)
  when an event bit is latched in JME_IEVE
- from a handler that raced past synchronize_irq()
- from netpoll

When a scheduled tasklet has a non-zero count, tasklet_action_common()
requeues it and raises the softirq again:

kernel/softirq.c:tasklet_action_common() {
	...
		local_irq_disable();
		t->next = NULL;
		*tl_head->tail = t;
		tl_head->tail = &t->next;
		__raise_softirq_irqoff(softirq_nr);
		local_irq_enable();
	...
}

jme_suspend() keeps pcc_task disabled until jme_resume(). The CPU would
then keep running softirqs for the whole suspend period and couldn't go
idle under s2idle.

In jme_shutdown(), pcc_task, txclean_task, rxclean_task and rxempty_task
are never re-enabled. The RX engine is also left running with the RX
clock forced on. Wouldn't the spin then last for the rest of the
shutdown sequence?

Before this patch, pcc_task was never disabled, and the other tasklets
were disabled only briefly in jme_suspend().

> +
>  	jme_powersave_phy(jme);
>  	pci_pme_active(pdev, true);
>  }

[Severity: Medium]

This is a pre-existing issue and wasn't introduced by this patch.
jme_shutdown() arms WoL while the RX/TX engines are still enabled and
the descriptor ring registers still point at host memory. jme_suspend()
quiesces the MAC first:

drivers/net/ethernet/jme.c:jme_suspend() {
	...
		jme_stop_pcc_timer(jme);
		jme_disable_rx_engine(jme);
		jme_disable_tx_engine(jme);
		jme_reset_mac_processor(jme);
	...
}

jme_shutdown() does none of this before jme_powersave_phy(). Then
jme_set_100m_half() writes JME_GHC and forces the PHY to 100M half
while the engines are live. Now that NAPI and the RX tasklets are
disabled in this path, nothing drains the live RX ring either.

Could this leave the MAC in an undefined state for S5 Magic Packet
wake? The commit message notes that S5 wake wasn't validated.

> @@ -3172,11 +3190,16 @@ jme_suspend(struct device *dev)
>  	if (!netif_running(netdev))
>  		return 0;
>  
> -	atomic_dec(&jme->link_changing);
> -
> +	set_bit(JME_FLAG_WOL_QUIESCE, &jme->flags);
>  	netif_device_detach(netdev);
>  	netif_stop_queue(netdev);
>  	jme_stop_irq(jme);
> +	synchronize_irq(jme->pdev->irq);
> +	jme_stop_irq(jme);
> +	cancel_work_sync(&jme->linkch_task);
> +	tasklet_disable(&jme->pcc_task);

[Severity: High]

This isn't a bug introduced by this patch. Since pcc_task is now managed
explicitly here, though: neither jme_close() nor jme_remove_one() calls
tasklet_kill(&jme->pcc_task).

jme_close() kills only the other deferred contexts:

drivers/net/ethernet/jme.c:jme_close() {
	...
	cancel_work_sync(&jme->linkch_task);
	tasklet_kill(&jme->txclean_task);
	tasklet_kill(&jme->rxclean_task);
	tasklet_kill(&jme->rxempty_task);
	...
}

free_irq() waits for running handlers, but it doesn't flush tasklets
those handlers have already scheduled. The last pcc_task scheduled on
INTR_TMINTR may still be pending (for example, ksoftirqd starved by a
real-time task) when jme_remove_one() does this:

	unregister_netdev(netdev);
	iounmap(jme->regs);
	free_netdev(netdev);

jme_pcc_tasklet() would then dereference the freed jme_adapter and do
MMIO through the unmapped jme->regs in
jme_stop_pcc_timer()/jme_start_pcc_timer().

Should jme_close() also call tasklet_kill() on pcc_task?

> +	napi_disable(&jme->napi);

[Severity: High]

What happens if the interface is closed after jme_suspend() and before
jme_resume()?

NAPI stays disabled until jme_resume(), and jme_resume() only undoes
that when netif_running() is true. A close in that window would reach
JME_NAPI_DISABLE() in jme_close() and call napi_disable() a second
time.

After a completed disable, napi_disable_locked() leaves
NAPIF_STATE_SCHED and NAPIF_STATE_NPSVC set. Wouldn't the second call
spin forever while holding rtnl_lock and the netdev instance lock?

One path that can run during suspend is netns cleanup on netns_wq,
which is not freezable. It moves physical devices back to init_net:

netns cleanup work (netns_wq)
  __dev_change_net_namespace()
    netif_close()
      jme_close()
        JME_NAPI_DISABLE()
          napi_disable()

Even if the close got past NAPI, jme_resume() returns early for a
closed device. Neither jme_close() nor jme_open() clears
JME_FLAG_WOL_QUIESCE or re-enables pcc_task. After that, jme_intr_msi()
would never schedule link-change work or re-enable JME_IENS.

jme_shutdown() leaves the same state behind, and nothing ever undoes
it there.

> +	atomic_dec(&jme->link_changing);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261024-jme-wol-v2-submit-v2-1-7a89ef6fb4c9%40gmail.com