All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tzung-Bi Shih <tzungbi@kernel.org>
To: Guenter Roeck <linux@roeck-us.net>
Cc: linux-watchdog@vger.kernel.org,
	Xingyu Wu <xingyu.wu@starfivetech.com>,
	Ziv Xu <ziv.xu@starfivetech.com>
Subject: Re: [PATCH v2 3/8] watchdog: starfive-wdt: Avoid unclocked register access in remove and PM paths
Date: Fri, 6 Nov 2026 14:21:50 +0000	[thread overview]
Message-ID: <au3ifkTtWPvGevf8@google.com> (raw)
In-Reply-To: <20261105190530.2371854-4-linux@roeck-us.net>

On Thu, Nov 05, 2026 at 11:05:24AM -0800, Guenter Roeck wrote:
> When runtime PM is enabled and the watchdog is inactive, the device enters
> runtime suspend and starfive_wdt_runtime_suspend() disables core_clk and
> apb_clk.
> 
> However, starfive_wdt_remove(), starfive_wdt_shutdown(),
> starfive_wdt_suspend(), and starfive_wdt_resume() unconditionally read and
> write watchdog MMIO registers without checking whether the watchdog is
> running or clocks are enabled, causing a bus fault when the device is
> runtime-suspended:
> 
> - In starfive_wdt_remove(), starfive_wdt_stop() is called before
>   watchdog_unregister_device() while clocks may be gated off. Unregister
>   the watchdog device first (which stops an active watchdog via
>   watchdog_stop_on_unregister while its runtime PM reference keeps clocks
>   enabled), and only call starfive_wdt_stop() if watchdog_hw_running() is
>   still set.
> - In starfive_wdt_pm_stop(), check pm_runtime_enabled() and ignore
>   non-zero idle return codes from pm_runtime_put_sync() so watchdog_stop()
>   does not falsely treat a stopped watchdog as still running.
> - Remove the redundant starfive_wdt_shutdown() callback. Because
>   starfive_wdt_probe() already sets watchdog_stop_on_reboot(),
>   watchdog_reboot_notifier() stops a running watchdog on reboot, halt, or
>   power-off (while respecting the watchdog.stop_on_reboot parameter)
>   before device_shutdown() runs; calling starfive_wdt_pm_stop() again in
>   .shutdown causes unclocked MMIO when suspended and an unbalanced
>   pm_runtime_put_sync() when inactive or already stopped.
> - In starfive_wdt_suspend() and starfive_wdt_resume(), only save/stop and
>   restore/restart the hardware watchdog when watchdog_hw_running() is set
>   (in which case an active runtime PM reference ensures clocks are enabled
>   before suspend and re-enabled by pm_runtime_force_resume()).
> 
> Fixes: db728ea9c7be ("drivers: watchdog: Add StarFive Watchdog driver")
> Cc: Xingyu Wu <xingyu.wu@starfivetech.com>
> Cc: Ziv Xu <ziv.xu@starfivetech.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>

  reply	other threads:[~2026-11-06 14:21 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-05 19:05 [PATCH v2 0/8] watchdog: starfive-wdt: Fix runtime PM and unclocked register access bugs Guenter Roeck
2026-11-05 19:05 ` [PATCH v2 1/8] watchdog: starfive-wdt: Drop runtime PM reference if starting watchdog fails Guenter Roeck
2026-11-05 19:05 ` [PATCH v2 2/8] watchdog: starfive-wdt: Balance pm_runtime_enable() on probe error paths Guenter Roeck
2026-11-05 19:05 ` [PATCH v2 3/8] watchdog: starfive-wdt: Avoid unclocked register access in remove and PM paths Guenter Roeck
2026-11-06 14:21   ` Tzung-Bi Shih [this message]
2026-11-05 19:05 ` [PATCH v2 4/8] watchdog: starfive-wdt: Fix runtime PM usage count leaks in probe and remove Guenter Roeck
2026-11-05 19:05 ` [PATCH v2 5/8] watchdog: starfive-wdt: Drop runtime PM reference before registering watchdog Guenter Roeck
2026-11-05 19:05 ` [PATCH v2 6/8] watchdog: starfive-wdt: Avoid unclocked MMIO read in starfive_wdt_get_timeleft() Guenter Roeck
2026-11-06 14:22   ` Tzung-Bi Shih
2026-11-05 19:05 ` [PATCH v2 7/8] watchdog: starfive-wdt: Stop ping worker on system suspend Guenter Roeck
2026-11-06 14:22   ` Tzung-Bi Shih
2026-11-05 19:05 ` [PATCH v2 8/8] watchdog: starfive-wdt: Avoid unclocked MMIO access in starfive_wdt_set_timeout() Guenter Roeck
2026-11-06 14:21   ` Tzung-Bi Shih
2026-11-06 15:20     ` Guenter Roeck
2026-11-06 15:51       ` Tzung-Bi Shih

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=au3ifkTtWPvGevf8@google.com \
    --to=tzungbi@kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=xingyu.wu@starfivetech.com \
    --cc=ziv.xu@starfivetech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.