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

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

[Franck BAUR via B4 Relay]

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

The suspend path disables the RX engine and resets the MAC before
jme_powersave_phy() programs PMCS for wake-on-LAN. The MAC reset leaves
GPREG1_RXCLKOFF set, so the RX MAC clock remains gated when WoL is
armed.

On a JMicron JMC250 [197b:0250] revision 03, PMCS_MFEN is programmed
correctly but Magic Packet detection does not work while the RX MAC
clock is gated. Instrumenting JME_PMCS showed that MFDET remained clear
after receiving Magic Packets.

Re-enabling the RX MAC clock immediately before enabling WoL makes
MFDET assert and restores wake from ACPI S3. An A/B test without a
Magic Packet remained suspended until the RTC wake alarm. The wakeup
was reproduced repeatedly with the patched driver.

Keep the RX MAC clock running while WoL is armed.

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>
---
 drivers/net/ethernet/jme.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/jme.c b/drivers/net/ethernet/jme.c
index 25ec5f757c6828a5f5241340bf3fb01d8e0aa3e9..27ac7405f76fed169d9f1fa84d9875611dcf119c 100644
--- a/drivers/net/ethernet/jme.c
+++ b/drivers/net/ethernet/jme.c
@@ -1892,6 +1892,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);

---
base-commit: af32da41b0327b9c6a37856ba82b6760d6c8d10e
change-id: 20261011-jme-wol-submit-9df9accc5367

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

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

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 2 · 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).

- [Medium] The patch turns the RX MAC clock on before arming WoL, but
  resume never turns it back off.
- [Medium] The patch's guarantee ('Keep the RX MAC clock running while
  WoL is armed') can be undone by jme_link_change_work() running at the…

--

net: jme: keep RX MAC clock enabled for WoL

This patch turns the RX MAC clock back on in jme_powersave_phy() just
before jme_clear_pm_enable_wol() arms WoL. Earlier in the suspend path the
MAC reset leaves GPREG1_RXCLKOFF set. On a JMC250 rev 03, Magic Packet
detection does not work while the RX MAC clock is gated.

> diff --git a/drivers/net/ethernet/jme.c b/drivers/net/ethernet/jme.c
> index 25ec5f757c68..27ac7405f76f 100644
> --- a/drivers/net/ethernet/jme.c
> +++ b/drivers/net/ethernet/jme.c
> @@ -1892,6 +1892,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);

[Severity: Medium]
Does anything turn the RX MAC clock back off on resume?

jme_mac_rxclk_on() changes the cached copy as well as the register:

    jme->reg_gpreg1 &= ~GPREG1_RXCLKOFF;
    jwrite32f(jme, JME_GPREG1, jme->reg_gpreg1);

jme_resume() calls jme_clear_pm_disable_wol(), jme_phy_on(),
jme_set_link_ksettings() or jme_reset_phy_processor(),
jme_phy_calibration(), jme_phy_setEA(), jme_reset_link() and
jme_start_irq(). None of them calls jme_mac_rxclk_off() or
jme_reset_mac_processor().

jme_link_change_work() also skips its disable/reset block the first time
it runs after resume, because jme_suspend() already called
netif_carrier_off():

    if (netif_carrier_ok(netdev)) {
        jme_disable_rx_engine(jme);
        jme_disable_tx_engine(jme);
        jme_reset_mac_processor(jme);
        ...

After each WoL suspend/resume, if the link stays down, the RX MAC clock
keeps running while the RX engine is disabled. Before this patch it was
gated in that state.

If the link comes up, jme_check_link() writes the cached reg_gpreg1, which
no longer has RXCLKOFF set, to JME_GPREG1. jme_enable_rx_engine() then
programs RXCS, RXDBA, RXNDA and RXQDC while the clock is already running.
Its trailing jme_mac_rxclk_on() then has nothing to do.

Doesn't this skip the order that commit 854a2e7c331a ("jme: Safer MAC
processor reset sequence") introduced, where the RX engine is programmed
with the clock gated and the clock is started afterwards?

The commit message says "Keep the RX MAC clock running while WoL is
armed". However, the clock stays on after WoL is disarmed, until the next
link-down runs jme_disable_rx_engine() and jme_reset_mac_processor().

Would calling jme_mac_rxclk_off() in jme_resume(), for example right after
jme_clear_pm_disable_wol(), keep the WoL fix and restore the previous
state?

[Severity: Medium]
Can jme_link_change_work() running at the same time gate the clock again
after this call?

jme_link_change_work() calls jme_disable_rx_engine() and
jme_reset_mac_processor(). Both end in jme_mac_rxclk_off(). Neither
caller of jme_powersave_phy() waits for linkch_task to finish first.

On the suspend path, jme_suspend() only does:

    atomic_dec(&jme->link_changing);

The work uses that atomic as an ad-hoc lock:

    while (!atomic_dec_and_test(&jme->link_changing)) {
        atomic_inc(&jme->link_changing);
        ...
    }

If the work has already passed that gate, the counter just drops to -1 and
suspend continues without waiting. jme_stop_irq() masks the device
interrupt but does not flush or cancel linkch_task. schedule_work() puts
the work on system_wq, which is not freezable.

Suppose the work is preempted after its netif_carrier_ok() check.
jme_suspend() can then run the new jme_mac_rxclk_on() and arm PMCS. When
the work resumes, it resets the MAC and gates the clock. If the link is
down, the work never turns the clock back on.

On the shutdown path:

    jme_shutdown()
        jme_powersave_phy(jme);
        pci_pme_active(pdev, true);

there is no jme_stop_irq(), no link_changing gate and no
cancel_work_sync(). With the interface up, jme_set_100m_half() forces
100M/half. That drops the link, raises INTR_LINKCH and schedules
linkch_task. With the default reg_pmcs = PMCS_MFEN, jme_wait_link() is
skipped, so jme_shutdown() returns right after the clock is turned on.

If power-off happens before the link comes back up, could the work reset
the MAC and leave WoL armed with RXCLKOFF set?

jme_mac_rxclk_on(), jme_mac_rxclk_off() and jme_check_link() also do
unlocked read-modify-writes of jme->reg_gpreg1, so concurrent updates to
that cached value can be lost.

The missing synchronization with the link-change work and the atomic gate
are older than this patch. But this fix only works if nothing gates the
clock after it is turned on.

Should linkch_task be stopped from rescheduling and drained (for example
with cancel_work_sync()) before WoL is armed?

>  		jme_clear_pm_enable_wol(jme);
>  	} else {
>  		jme_phy_off(jme);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261011-jme-wol-submit-v1-1-9cc033e259e4%40gmail.com