Repository navigation
[26.04_linux-nvidia] NVIDIA: SAUCE: watchdog: sbsa_gwdt: park the watchdog around system sleep on MediaTek implementations - #627
Closed
dcemin-nv wants to merge 1 commit into
Conversation
Collaborator
BaseOS Kernel ReviewNote 🔄 Review in progressBoro is reviewing this pull request. Results will appear here when ready. 🔍 Review artifacts
Review metadata
This comment is maintained by BaseOS Reviewer and updated when the GitHub watcher publishes a newer review. |
Contributor
PR Validation ReportPatchscan ✅ No Missing FixesAll cherry-picked commits checked — no missing upstream fixes found. PR Lint ✅ All checks passedDetailsChecking 1 commits... Cherry-pick digest: ┌──────────────┬──────────────────────────────────────────────────────────────────┬────────────┬─────────┬───────────────────────────┐ │ Local │ Referenced upstream / Patch subject │ Patch-ID │ Subject │ SoB chain │ ├──────────────┼──────────────────────────────────────────────────────────────────┼────────────┼─────────┼───────────────────────────┤ │ d2de08beaab0 │ [SAUCE] watchdog: sbsa_gwdt: park the watchdog around system sle │ N/A │ N/A │ kbutala, dcemin │ └──────────────┴──────────────────────────────────────────────────────────────────┴────────────┴─────────┴───────────────────────────┘ Lint: all checks passed. |
Collaborator
|
Please see the review comments on #628. |
dcemin-nv
force-pushed
the
wdog-park-26.04
branch
3 times, most recently
from
October 1, 2026 03:27
81c08ea to
8be4fe9
Compare
Author
|
New head 8be4fe9, same content as 628 head 42f9ca3. Details in #628 (comment) |
dcemin-nv
force-pushed
the
wdog-park-26.04
branch
from
October 1, 2026 06:43
8be4fe9 to
63d7f3f
Compare
Author
|
New head 63d7f3f, same content as 628 head 7570293. Details in #628 (comment) |
dcemin-nv
force-pushed
the
wdog-park-26.04
branch
from
October 1, 2026 17:03
63d7f3f to
8a322fb
Compare
Author
|
New head 8a322fb, same content as 628 head 333499c. Details in #628 (comment) |
…leep on MediaTek implementations BugLink: https://bugs.launchpad.net/bugs/2169002 Laptops built on the MT8901 ACPI platform reset intermittently around system suspend, both S3 and s2idle, with the SBSA generic watchdog enabled. Register-level measurements on the reference laptop show three properties of the MediaTek watchdog implementation that the driver has to work with: - the compare logic ignores WCS.EN: a watchdog that was disabled but refreshed a few seconds earlier still fires when the period elapses, while one whose last reload is long past never does; - a WOR write is only latched into the countdown while WCS.EN is set, although the register reads back the written value either way; - WCV reads back the counter value of the last reload, not the deadline. Around a system sleep the driver clears WCS.EN, but the watchdog core keeps refreshing the hardware until the CPUs go offline, so the last reload is a few seconds old when the platform enters the sleep state and the watchdog fires while nobody can refresh it. Whether the wake beats the second stage decides between a spurious wake, a reset a few seconds after resume, and a reset during the sleep, which is the intermittent pattern seen across the fleet. The platform firmware also disables the watchdog before WFI and re-enables it on the way out of the sleep, which latches whatever period the register holds onto the reload from before the sleep and produces the resume-side resets. The MediaTek implementation therefore cannot be stopped, and the driver stops pretending that it can: such an instance registers no stop op, so WDIOS_DISABLECARD and a magic close leave the watchdog running with the core refreshing it (WDOG_HW_RUNNING), the same path the driver already takes for a watchdog found armed at probe. The reboot notifier, which calls the stop op directly, is not registered for it. A running watchdog must not lose its driver either: the core already pins the module while the hardware runs, and the driver now suppresses the sysfs bind attributes, so only a probe failure can leave an armed instance behind, parked with a warning. Around a system sleep nobody can refresh the watchdog, so park it there: program WOR to the 48-bit maximum while WCS.EN is set and refresh, so the pending reload is days away, and leave WCS.EN set. A parked, enabled watchdog cannot fire for days, every later refresh (the core's keepalives until the CPUs go offline) reloads with the parked period again, and should a park ever fail the first stage wakes the system instead of the second stage resetting it. On resume, set WCS.EN, refresh so the reload moves to now while the parked period is still in effect, then restore the configured timeout (the WOR write is only taken while enabled) and refresh again. On a first start, with the block disabled, the order is enable, program, refresh instead: the period firmware left behind may be zero or expired and must not be used for a reload. Other implementations keep the architectural disable, on stop and around a sleep. A version 0 MediaTek block, should one exist, gets no stop op either, and since its 32-bit WOR cannot hold a park period the driver refuses a system sleep while such a watchdog is armed rather than let it fire unrefreshed. The block commits WOR on its own clock, tens of kHz, and a refresh issued right after the write reloads with whatever it holds at that moment. Two measures make the park robust against that: the two 32-bit halves are written high half first when parking (a mixed value is then new-high:old-low, at least 2^32 ticks) and low half first when restoring (a mixed value is the parked period again, fixed by the next keepalive), and the park waits for the commit before refreshing and refreshes twice. Without this the park failed intermittently: with the default 10 s timeout the mixed value the other way round is 8.6 s, which showed up as a reset about 17 s after any sleep entry on one unit while another unit of the same model was mostly unaffected. The timeout is not programmed into WOR while a sleep transition is in progress, so a probe that finds a firmware-armed watchdog during the transition cannot replace the parked period with a short one; the restart programs it. Every WOR write waits for the block to commit the new period before returning, so a refresh issued right after it (the core pings after WDIOC_SETTIMEOUT) reloads with the new period. wdd->timeout keeps the requested value, as the core expects; only the hardware programming is clamped. An instance joins the list the sleep notifier works on only once its timeout and mode are final, so a restart racing the probe programs the configured period. Known limitation: a parked watchdog stays enabled through the sleep with a finite period, about 78 hours at 1 GHz. A sleep longer than that asserts WS0 before the resume path restores the timeout, which panics with action=1 or resets about 78 hours later with action=0. The property that causes this is in the hardware; multi-day sleeps were not tested. Validated on two laptops of the platform, including the unit that failed every earlier variant: repeated S3 and s2idle cycles resume without a reset, WOR reads the configured value after resume and WS0 asserts at the configured period again, so the watchdog is live after the sleep rather than left parked. A magic close and WDIOS_DISABLECARD leave it running under the core, where the old stop path reset the unit about two periods later. Signed-off-by: Kaushal Rajeev Butala <kbutala@nvidia.com> Co-developed-by: David Cemin <dcemin@nvidia.com> Signed-off-by: David Cemin <dcemin@nvidia.com>
dcemin-nv
force-pushed
the
wdog-park-26.04
branch
from
October 1, 2026 19:37
8a322fb to
d2de08b
Compare
Author
|
New head d2de08b, same content as 628 head b86b428. Details in #628 (comment) |
Collaborator
|
|
Collaborator
|
|
Collaborator
|
Merged, closing PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intermittent resets around system suspend (S3 and s2idle) on the MT8901 ACPI laptop platform with the SBSA generic watchdog enabled.
Root cause, measured at register level on the reference laptop: the MediaTek watchdog implementation keeps comparing against the last reload after WCS.EN is cleared, latches a WOR write into the countdown only while WCS.EN is set (the register reads back the written value either way), and commits the two WOR halves on a slow clock. The driver disables the watchdog at suspend prepare, but the core keeps refreshing it until the CPUs go offline, so the platform enters the sleep with a fresh 5 s deadline that nobody can feed; the firmware re-enable after WFI then latches the short period onto the pre-sleep reload and produces the resume-side resets.
Fix: park instead of disable. On stop, program WOR to the 48-bit maximum (high half first) while enabled, wait for the commit and refresh twice, and leave WCS.EN set; on start, enable, refresh, restore the timeout (low half first) and refresh again. Version 0 watchdogs keep the architectural disable.
Validated on two laptops of the platform, including the unit that failed every earlier variant: repeated S3 and s2idle cycles (5 + 5 on that unit) resume without a reset, WOR reads the configured value after resume and WS0 asserts at the configured period, so the watchdog is live after the sleep. Same change under review as the downstream distribution MR.
Internal tracking: NVBugs 6813557 and 6783303.
BugLink: https://bugs.launchpad.net/bugs/2169002