[26.04_linux-nvidia-bos] NVIDIA: SAUCE: watchdog: sbsa_gwdt: stop ping worker during system sleep

[bdintakurti-nv]

On MediaTek SBSA watchdogs in action=1 mode, the sleep notifier parks the hardware during system sleep, but the watchdog-core keepalive timer can remain armed. Its SYS_TIMER interrupt can wake MT8901 from S3 even though the hardware watchdog is parked.

This change opts only that mode into the existing watchdog-core suspend hook, canceling the ping timer and work at PM prepare and restoring the worker on resume. action=0 remains unchanged because its WS0 heartbeat workaround needs separate resume-time accounting. The watchdog core's existing prepare-to-freeze rearm window is outside this PR's scope.

Validation: On MT8901 with action=1, early_enable=1, and a 10-second timeout, three 60-second deep S3 cycles with TAD wake resumed without an early SYS_TIMER wake or watchdog reset. The boot ID was unchanged.

Follow-up to #628.
bos twin of #632.

BugLink: https://bugs.launchpad.net/bugs/2169002

[nirmoy]

BaseOS Kernel Review

Tip

✅ Review passed

No issues found across the reviewed commits.

Findings: none

🔍 Review artifacts

• 🟢 Boro review: no findings
• 🔵 Test plan: ready for validation
• 🔴 KTS Report: 1 regression

📦 Build checks — 🟢 4/4 passed

• 🟢 amd64: build report and 4 debs
• 🟢 arm64: build report and 8 debs
• 🟢 amd64: NVIDIA driver and DOCA build report
• 🟢 arm64: NVIDIA driver and DOCA build report

Note

Build reports and debs are retained for 10 days after the PR closes.

• ⚪ PR explanation: inactive

Review metadata

• Reviewed head: 011bf55bddb1
• Overall status: kernel validation regression
• Build checks: 4/4 passed

This comment is maintained by BaseOS Reviewer and updated when the GitHub watcher publishes a newer review.

[github-actions]

PR Validation Report

Patchscan ✅ No Missing Fixes

All cherry-picked commits checked — no missing upstream fixes found.

PR Lint ✅ All checks passed

Details

Checking 1 commits...

Cherry-pick digest:
┌──────────────┬──────────────────────────────────────────────────────────────────┬────────────┬─────────┬───────────────────────────┐
│ Local        │ Referenced upstream / Patch subject                              │ Patch-ID   │ Subject │ SoB chain                 │
├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤
│ 011bf55bddb1 │ [SAUCE] watchdog: sbsa_gwdt: stop mt8901 ping worker during susp │ N/A        │ N/A     │ bdintaku                  │
└──────────────┴──────────────────────────────────────────────────────────────────┴────────────┴─────────┴───────────────────────────┘

Lint: all checks passed.

[clsotog]

I am not sure if this is the same medium finding from Boro but this is what I see from Codex:
Medium drivers/watchdog/sbsa_gwdt.c:834: enabling watchdog_stop_ping_on_suspend() cancels the core ping worker, but the driver’s PM notifier still restarts and refreshes the hardware itself in sbsa_gwdt_sleep_restart() at line 611. The watchdog core does not learn about that refresh, so last_hw_keepalive can remain from before suspend. On MediaTek/
action=0, where min_hw_heartbeat_ms is deliberately set to avoid pinging at the WS0 boundary, the first post-resume core ping can be scheduled relative to the stale pre-suspend timestamp and land around timeout / 2 after the driver’s resume refresh. With the default 10s timeout, a suspend entered ~1s after the previous core ping can resume and
schedule the next worker ping ~5s after sbsa_gwdt_hw_start(), which is exactly the race window the driver is trying to avoid. The fix needs to keep the core’s keepalive timestamp aligned with driver-initiated resume/start refreshes, or otherwise force the first post-resume core ping to be delayed from the actual resume refresh.

[nvmochs]

@bdintakurti-nv

My review had this finding, however it called it out as an existing coverage gap (which I think Carol's finding also falls under that category). So please review, but IMO these could be pursued in a separate follow-on PR (or added to this PR as a series).

---

[P2] A watchdog registered during suspend entry can miss core timer cancellation.

If an SBSA probe completes after PM_SUSPEND_PREPARE, the driver can correctly park the hardware while watchdog registration starts the core’s keepalive timer for a firmware-enabled or early_enable watchdog. watchdog_stop_ping_on_suspend() registers a PM notifier, but the completed prepare event is not replayed. The driver’s later .prepare() callback only handles the hardware, leaving the software timer armed.

Consequently, that suspend attempt can still suffer an unwanted timer-triggered wakeup. A complete fix would also quiesce the core timer when registering a watchdog during an ongoing sleep transition.

This is an existing coverage gap, not a new regression. It requires probing to overlap suspend entry and does not affect the usual case where the watchdog was registered at boot.

[bdintakurti-nv]

@nvmochs Thanks, agreed. This is an existing coverage gap limited to watchdog registration overlapping suspend entry, and it does not affect the normal boot-registered path addressed here. We will track the late-registration/core-timer coordination separately rather than expanding this fix.

[nvmochs]

No further issues from me.

Acked-by: Matthew R. Ochs <mochs@nvidia.com>

[bdintakurti-nv]

@clsotog Thanks, fixed in the latest revision. After the SBSA resume path refreshes the hardware, it now updates last_hw_keepalive while the core ping worker is quiesced, so the next ping is scheduled from the actual resume refresh rather than the stale pre-suspend timestamp.

[clsotog]

@clsotog Thanks, fixed in the latest revision. After the SBSA resume path refreshes the hardware, it now updates last_hw_keepalive while the core ping worker is quiesced, so the next ping is scheduled from the actual resume refresh rather than the stale pre-suspend timestamp.

Thanks for the fix. Codex found another one.
drivers/watchdog/sbsa_gwdt.c:617: the previous stale-last_hw_keepalive issue is addressed in intent, but the new call to watchdog_set_last_hw_keepalive() is not actually serialized against watchdog core operations. PM_POST_SUSPEND runs after suspend_thaw_processes() in kernel/power/suspend.c, so userspace can concurrently write/ioctl /dev/watchdog while the SBSA notifier is holding only sbsa_gwdt_lock. The helper updates wd_data->last_hw_keepalive and can call __watchdog_ping() without wd_data->lock, racing the normal core paths that do hold that mutex.

[clsotog]

Findings:
[P1] The stale last_hw_keepalive race is not fully resolved.
The direct sbsa_gwdt.c call to watchdog_set_last_hw_keepalive() is gone, and the new update happens under wd_data->lock in watchdog_dev_resume(). But PM_POST_SUSPEND still runs after suspend_thaw_processes() (kernel/power/suspend.c:562-564). SBSA refreshes hardware first in its PM notifier (drivers/watchdog/sbsa_gwdt.c:606-612), and only later does the watchdog-core PM notifier update last_hw_keepalive (drivers/watchdog/watchdog_dev.c:1306-1310). A thawed userspace watchdog daemon can write/ioctl /dev/watchdog between those two notifiers, acquire wd_data->lock first, and ping using the stale pre-suspend timestamp. So the lock is correct, but the ordering is still wrong.

[P1] New finding: suspend-side timer cancellation can be undone before tasks freeze.
PM_SUSPEND_PREPARE is called before process freezing (kernel/power/suspend.c:381-387). watchdog_dev_suspend() cancels the timer/work (drivers/watchdog/watchdog_dev.c:1288-1289), but it does not set any state preventing a later userspace write/ioctl, still before freeze, from calling watchdog_ping() and rearming the hrtimer. That can recreate the SYS_TIMER wakeup this PR is trying to prevent.

[bdintakurti-nv]

Thanks, Carol. To avoid a broader watchdog-core redesign, I narrowed this change to the MediaTek SBSA action=1 configuration used on MT8901. This PR now only enables the existing suspend-time ping cancellation hook for action=1; action=0 remains unchanged. The core re-arm and late-registration windows remain separate issues.

[clsotog]

The first finding is resolved.
The second finding is still not resolved. watchdog_dev_suspend() still only cancels the timer/work after PM_SUSPEND_PREPARE, and there is still no core “suspended” state preventing userspace from rearming the hrtimer before tasks freeze:

• PM_SUSPEND_PREPARE before freeze: kernel/power/suspend.c:381-387
• cancel only: drivers/watchdog/watchdog_dev.c:1288-1289
• normal ping can still arm timer: drivers/watchdog/watchdog_dev.c:154-157, 129-138

I maybe have this question about this PR:
If the PR’s goal is now only “use the existing watchdog core suspend hook for MT8901 action=1,” then the patch is defensible as a narrow incremental fix.
or
If the PR is being presented as “prevents the watchdog core timer from waking S3,” then it is incomplete because userspace can still rearm the timer after PM_SUSPEND_PREPARE and before freeze.

[bdintakurti-nv]

@clsotog Thank you. The prepare-to-freeze rearm window is an existing limitation of the watchdog-core hook, not introduced by this PR. This change enables that hook for the MediaTek SBSA watchdog in action=1 mode to address the reproduced SYS_TIMER wake. It is not intended to prevent a userspace ping in that window. I’ll discuss the core race with the team as a separate follow-up

[clsotog]

@clsotog Thank you. The prepare-to-freeze rearm window is an existing limitation of the watchdog-core hook, not introduced by this PR. This change enables that hook for the MediaTek SBSA watchdog in action=1 mode to address the reproduced SYS_TIMER wake. It is not intended to prevent a userspace ping in that window. I’ll discuss the core race with the team as a separate follow-up

Based on this I can ack.
There is a small nit: Can you fix the commit message
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
No need to put buglink at commit message. that will be added later.
Same for PR 632.

[nirmoy]

Please add the following Fixes: trailer:

Fixes: b0953e81348c22881042957f98eff84bf1db75b7 ("NVIDIA: SAUCE: watchdog: sbsa_gwdt: park the watchdog around system sleep on MediaTek implementations")

Please also wrap the commit-message body at 72 columns, keeping the Fixes: trailer on one line, and update the PR description to match the current action=1 scope.

Otherwise, this change looks fine to me.

[nirmoy]

Acked-by: Nirmoy Das <nirmoyd@nvidia.com>

[clsotog]

Acked-by: Carol L Soto <csoto@nvidia.com>

[nvmochs]

Merged, closing PR.

43f2507f37ed (nresolute/nvidia-bos-next) NVIDIA: SAUCE: watchdog: sbsa_gwdt: stop MT8901 ping worker during suspend