* [PATCH 0/4] Add support for Kernel WDT
@ 2026-05-07 15:45 Kartik Rajput
2026-05-07 15:45 ` [PATCH 1/4] clocksource/drivers/timer-tegra186: Fix support for multiple watchdog instances Kartik Rajput
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Kartik Rajput @ 2026-05-07 15:45 UTC (permalink / raw)
To: daniel.lezcano, tglx, wim, linux, thierry.reding, jonathanh,
kkartik, linux-watchdog, linux-kernel, linux-tegra
Tegra186 and later SoCs support multiple watchdog timers in the TKE
block, the driver so far only uses WDT0. This series extends it to
make use of the additional instances and reserve the first available WDT
for Kernel.
Patch 1 fixes a read-modify-write bug in tegra186_wdt_enable()
that would otherwise make registering more than one watchdog unsafe.
Patch 2 corrects num_wdts for Tegra186 and Tegra234. WDT2 is connected
to the Audio Processing Engine (APE) and cannot be accessed from Linux,
so only WDT0 and WDT1 are available.
Patch 3 iterates over all WDTs and registers the ones whose TKE SCR
firewall grants OS access.
Patch 4 reserves the first accessible watchdog as a kernel-only
watchdog, arms it at probe with a 120 s timeout, and pets it from
the driver's IRQ handler, it is not exposed to userspace, so the
system can still reset if the kernel hangs before userspace opens a
watchdog.
Kartik Rajput (4):
clocksource/drivers/timer-tegra186: Fix support for multiple watchdog
instances
clocksource/drivers/timer-tegra186: Correct num_wdts for Tegra186 and
Tegra234
clocksource/drivers/timer-tegra186: Register all accessible watchdog
timers
clocksource/drivers/timer-tegra186: Reserve and service a kernel
watchdog
drivers/clocksource/timer-tegra186.c | 122 +++++++++++++++++++++++----
1 file changed, 104 insertions(+), 18 deletions(-)
--
2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/4] clocksource/drivers/timer-tegra186: Fix support for multiple watchdog instances
2026-05-07 15:45 [PATCH 0/4] Add support for Kernel WDT Kartik Rajput
@ 2026-05-07 15:45 ` Kartik Rajput
2026-05-07 22:58 ` sashiko-bot
2026-05-07 15:45 ` [PATCH 2/4] clocksource/drivers/timer-tegra186: Correct num_wdts for Tegra186 and Tegra234 Kartik Rajput
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Kartik Rajput @ 2026-05-07 15:45 UTC (permalink / raw)
To: daniel.lezcano, tglx, wim, linux, thierry.reding, jonathanh,
kkartik, linux-watchdog, linux-kernel, linux-tegra
Cc: stable
Tegra SoCs support multiple watchdogs; currently only one (WDT0) is
used. When multiple watchdogs are registered, tegra186_wdt_enable()
overwrites the TKEIE(x) register, discarding any existing watchdog
interrupt enable bits. As a result, enabling one watchdog inadvertently
disables interrupts for the others.
Fix this by preserving the existing TKEIE(x) value and updating it
using a read-modify-write sequence.
Fixes: 42cee19a9f83 ("clocksource: Add Tegra186 timers support")
Cc: stable@vger.kernel.org
Signed-off-by: Kartik Rajput <kkartik@nvidia.com>
---
drivers/clocksource/timer-tegra186.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
index 355558893e5f..bfe16d2d5104 100644
--- a/drivers/clocksource/timer-tegra186.c
+++ b/drivers/clocksource/timer-tegra186.c
@@ -149,7 +149,8 @@ static void tegra186_wdt_enable(struct tegra186_wdt *wdt)
u32 value;
/* unmask hardware IRQ, this may have been lost across powergate */
- value = TKEIE_WDT_MASK(wdt->index, 1);
+ value = readl(tegra->regs + TKEIE(wdt->tmr->hwirq));
+ value |= TKEIE_WDT_MASK(wdt->index, 1);
writel(value, tegra->regs + TKEIE(wdt->tmr->hwirq));
/* clear interrupt */
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/4] clocksource/drivers/timer-tegra186: Correct num_wdts for Tegra186 and Tegra234
2026-05-07 15:45 [PATCH 0/4] Add support for Kernel WDT Kartik Rajput
2026-05-07 15:45 ` [PATCH 1/4] clocksource/drivers/timer-tegra186: Fix support for multiple watchdog instances Kartik Rajput
@ 2026-05-07 15:45 ` Kartik Rajput
2026-05-07 15:45 ` [PATCH 3/4] clocksource/drivers/timer-tegra186: Register all accessible watchdog timers Kartik Rajput
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Kartik Rajput @ 2026-05-07 15:45 UTC (permalink / raw)
To: daniel.lezcano, tglx, wim, linux, thierry.reding, jonathanh,
kkartik, linux-watchdog, linux-kernel, linux-tegra
On Tegra186 and Tegra234, WDT2 is connected to the Audio Processing
Engine (APE) and cannot be accessed from Linux. Only WDT0 and WDT1
are accessible to Linux.
Update num_wdts from 3 to 2 for both Tegra186 and Tegra234 to reflect
the actual number of watchdogs available to Linux.
Signed-off-by: Kartik Rajput <kkartik@nvidia.com>
---
drivers/clocksource/timer-tegra186.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
index bfe16d2d5104..fd82a73ab2d2 100644
--- a/drivers/clocksource/timer-tegra186.c
+++ b/drivers/clocksource/timer-tegra186.c
@@ -511,12 +511,12 @@ static SIMPLE_DEV_PM_OPS(tegra186_timer_pm_ops, tegra186_timer_suspend,
static const struct tegra186_timer_soc tegra186_timer = {
.num_timers = 10,
- .num_wdts = 3,
+ .num_wdts = 2,
};
static const struct tegra186_timer_soc tegra234_timer = {
.num_timers = 16,
- .num_wdts = 3,
+ .num_wdts = 2,
};
static const struct of_device_id tegra186_timer_of_match[] = {
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/4] clocksource/drivers/timer-tegra186: Register all accessible watchdog timers
2026-05-07 15:45 [PATCH 0/4] Add support for Kernel WDT Kartik Rajput
2026-05-07 15:45 ` [PATCH 1/4] clocksource/drivers/timer-tegra186: Fix support for multiple watchdog instances Kartik Rajput
2026-05-07 15:45 ` [PATCH 2/4] clocksource/drivers/timer-tegra186: Correct num_wdts for Tegra186 and Tegra234 Kartik Rajput
@ 2026-05-07 15:45 ` Kartik Rajput
2026-05-07 23:38 ` sashiko-bot
2026-05-07 15:45 ` [PATCH 4/4] clocksource/drivers/timer-tegra186: Reserve and service a kernel watchdog Kartik Rajput
2026-05-12 13:56 ` [PATCH 0/4] Add support for Kernel WDT Jon Hunter
4 siblings, 1 reply; 9+ messages in thread
From: Kartik Rajput @ 2026-05-07 15:45 UTC (permalink / raw)
To: daniel.lezcano, tglx, wim, linux, thierry.reding, jonathanh,
kkartik, linux-watchdog, linux-kernel, linux-tegra
Tegra186+ SoCs expose multiple watchdog timers, but the driver only
registers WDT(0).
Iterate over num_wdts and, for each WDT, check the SCR (firewall) registers
in the TKE block to determine whether Linux has read and write access.
Register the watchdogs that are accessible.
Signed-off-by: Kartik Rajput <kkartik@nvidia.com>
---
drivers/clocksource/timer-tegra186.c | 61 +++++++++++++++++++++++-----
1 file changed, 50 insertions(+), 11 deletions(-)
diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
index fd82a73ab2d2..dd1d1a0dd63e 100644
--- a/drivers/clocksource/timer-tegra186.c
+++ b/drivers/clocksource/timer-tegra186.c
@@ -57,6 +57,13 @@
#define WDTUR 0x00c
#define WDTUR_UNLOCK_PATTERN 0x0000c45a
+/* WDT security configuration registers */
+#define WDTSCR(x) (0xf02c + (x) * 4)
+#define WDTSCR_SEC_WEN BIT(28)
+#define WDTSCR_SEC_REN BIT(27)
+#define WDTSCR_SEC_G1W BIT(9)
+#define WDTSCR_SEC_G1R BIT(1)
+
struct tegra186_timer_soc {
unsigned int num_timers;
unsigned int num_wdts;
@@ -89,7 +96,7 @@ struct tegra186_timer {
struct device *dev;
void __iomem *regs;
- struct tegra186_wdt *wdt;
+ struct tegra186_wdt **wdts;
struct clocksource usec;
struct clocksource tsc;
struct clocksource osc;
@@ -298,6 +305,23 @@ static const struct watchdog_ops tegra186_wdt_ops = {
.get_timeleft = tegra186_wdt_get_timeleft,
};
+static bool tegra186_wdt_is_accessible(struct tegra186_timer *tegra, unsigned int index)
+{
+ u32 value;
+
+ value = readl_relaxed(tegra->regs + WDTSCR(index));
+
+ /* Check OS write access if write blocking is enabled. */
+ if ((value & WDTSCR_SEC_WEN) && !(value & WDTSCR_SEC_G1W))
+ return false;
+
+ /* Check OS read access if read blocking is enabled. */
+ if ((value & WDTSCR_SEC_REN) && !(value & WDTSCR_SEC_G1R))
+ return false;
+
+ return true;
+}
+
static struct tegra186_wdt *tegra186_wdt_create(struct tegra186_timer *tegra,
unsigned int index)
{
@@ -424,6 +448,7 @@ static int tegra186_timer_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct tegra186_timer *tegra;
+ unsigned int i;
int err;
tegra = devm_kzalloc(dev, sizeof(*tegra), GFP_KERNEL);
@@ -442,12 +467,20 @@ static int tegra186_timer_probe(struct platform_device *pdev)
if (err < 0)
return err;
- /* create a watchdog using a preconfigured timer */
- tegra->wdt = tegra186_wdt_create(tegra, 0);
- if (IS_ERR(tegra->wdt)) {
- err = PTR_ERR(tegra->wdt);
- dev_err(dev, "failed to create WDT: %d\n", err);
- return err;
+ tegra->wdts = devm_kcalloc(dev, tegra->soc->num_wdts, sizeof(*tegra->wdts), GFP_KERNEL);
+ if (!tegra->wdts)
+ return -ENOMEM;
+
+ for (i = 0; i < tegra->soc->num_wdts; i++) {
+ if (!tegra186_wdt_is_accessible(tegra, i)) {
+ dev_warn(dev, "WDT%u is not accessible\n", i);
+ continue;
+ }
+
+ tegra->wdts[i] = tegra186_wdt_create(tegra, i);
+ if (IS_ERR(tegra->wdts[i]))
+ return dev_err_probe(dev, PTR_ERR(tegra->wdts[i]),
+ "failed to create WDT%u\n", i);
}
err = tegra186_timer_tsc_init(tegra);
@@ -489,9 +522,12 @@ static void tegra186_timer_remove(struct platform_device *pdev)
static int __maybe_unused tegra186_timer_suspend(struct device *dev)
{
struct tegra186_timer *tegra = dev_get_drvdata(dev);
+ unsigned int i;
- if (watchdog_active(&tegra->wdt->base))
- tegra186_wdt_disable(tegra->wdt);
+ for (i = 0; i < tegra->soc->num_wdts; i++) {
+ if (tegra->wdts[i] && watchdog_active(&tegra->wdts[i]->base))
+ tegra186_wdt_disable(tegra->wdts[i]);
+ }
return 0;
}
@@ -499,9 +535,12 @@ static int __maybe_unused tegra186_timer_suspend(struct device *dev)
static int __maybe_unused tegra186_timer_resume(struct device *dev)
{
struct tegra186_timer *tegra = dev_get_drvdata(dev);
+ unsigned int i;
- if (watchdog_active(&tegra->wdt->base))
- tegra186_wdt_enable(tegra->wdt);
+ for (i = 0; i < tegra->soc->num_wdts; i++) {
+ if (tegra->wdts[i] && watchdog_active(&tegra->wdts[i]->base))
+ tegra186_wdt_enable(tegra->wdts[i]);
+ }
return 0;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/4] clocksource/drivers/timer-tegra186: Reserve and service a kernel watchdog
2026-05-07 15:45 [PATCH 0/4] Add support for Kernel WDT Kartik Rajput
` (2 preceding siblings ...)
2026-05-07 15:45 ` [PATCH 3/4] clocksource/drivers/timer-tegra186: Register all accessible watchdog timers Kartik Rajput
@ 2026-05-07 15:45 ` Kartik Rajput
2026-05-08 0:23 ` sashiko-bot
2026-05-12 13:56 ` [PATCH 0/4] Add support for Kernel WDT Jon Hunter
4 siblings, 1 reply; 9+ messages in thread
From: Kartik Rajput @ 2026-05-07 15:45 UTC (permalink / raw)
To: daniel.lezcano, tglx, wim, linux, thierry.reding, jonathanh,
kkartik, linux-watchdog, linux-kernel, linux-tegra
Tegra SoCs supports multiple watchdog timers. If the kernel crashes or
hangs before userspace enables a watchdog, the system cannot recover and
may remain bricked, e.g. after a failed OTA update. The driver currently
leaves all watchdogs disabled until userspace configures them.
Reserve first available watchdog as a kernel-only watchdog for Tegra186
and Tegra234. Arm it during probe (120s timeout) and keep it alive in
the driver IRQ handler. Do not register it to userspace. Other available
watchdogs remain exposed to userspace. This guarantees the system can
reset itself in case of a hang or crash even when userspace never starts.
Signed-off-by: Kartik Rajput <kkartik@nvidia.com>
---
drivers/clocksource/timer-tegra186.c | 62 ++++++++++++++++++++++++----
1 file changed, 54 insertions(+), 8 deletions(-)
diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
index dd1d1a0dd63e..78600ddeb1c6 100644
--- a/drivers/clocksource/timer-tegra186.c
+++ b/drivers/clocksource/timer-tegra186.c
@@ -57,6 +57,8 @@
#define WDTUR 0x00c
#define WDTUR_UNLOCK_PATTERN 0x0000c45a
+#define TEGRA186_KERNEL_WDT_TIMEOUT 120
+
/* WDT security configuration registers */
#define WDTSCR(x) (0xf02c + (x) * 4)
#define WDTSCR_SEC_WEN BIT(28)
@@ -82,6 +84,7 @@ struct tegra186_wdt {
void __iomem *regs;
unsigned int index;
bool locked;
+ bool is_kernel_wdt;
struct tegra186_tmr *tmr;
};
@@ -182,6 +185,10 @@ static void tegra186_wdt_enable(struct tegra186_wdt *wdt)
value &= ~WDTCR_PERIOD_MASK;
value |= WDTCR_PERIOD(1);
+ /* enable local interrupt for kernel watchdog */
+ if (wdt->is_kernel_wdt)
+ value |= WDTCR_LOCAL_INT_ENABLE;
+
/* enable system POR reset */
value |= WDTCR_SYSTEM_POR_RESET_ENABLE;
@@ -219,6 +226,16 @@ static int tegra186_wdt_ping(struct watchdog_device *wdd)
return 0;
}
+static irqreturn_t tegra186_wdt_irq(int irq, void *data)
+{
+ struct tegra186_wdt *wdt = data;
+
+ tegra186_wdt_disable(wdt);
+ tegra186_wdt_enable(wdt);
+
+ return IRQ_HANDLED;
+}
+
static int tegra186_wdt_set_timeout(struct watchdog_device *wdd,
unsigned int timeout)
{
@@ -361,10 +378,6 @@ static struct tegra186_wdt *tegra186_wdt_create(struct tegra186_timer *tegra,
if (err < 0)
return ERR_PTR(err);
- err = devm_watchdog_register_device(tegra->dev, &wdt->base);
- if (err < 0)
- return ERR_PTR(err);
-
return wdt;
}
@@ -446,9 +459,11 @@ static int tegra186_timer_usec_init(struct tegra186_timer *tegra)
static int tegra186_timer_probe(struct platform_device *pdev)
{
+ struct tegra186_wdt *kernel_wdt = NULL;
struct device *dev = &pdev->dev;
struct tegra186_timer *tegra;
unsigned int i;
+ int irq;
int err;
tegra = devm_kzalloc(dev, sizeof(*tegra), GFP_KERNEL);
@@ -467,6 +482,8 @@ static int tegra186_timer_probe(struct platform_device *pdev)
if (err < 0)
return err;
+ irq = err;
+
tegra->wdts = devm_kcalloc(dev, tegra->soc->num_wdts, sizeof(*tegra->wdts), GFP_KERNEL);
if (!tegra->wdts)
return -ENOMEM;
@@ -481,6 +498,17 @@ static int tegra186_timer_probe(struct platform_device *pdev)
if (IS_ERR(tegra->wdts[i]))
return dev_err_probe(dev, PTR_ERR(tegra->wdts[i]),
"failed to create WDT%u\n", i);
+
+ /* Reserve the first accessible WDT for the Kernel. */
+ if (!kernel_wdt) {
+ kernel_wdt = tegra->wdts[i];
+ kernel_wdt->is_kernel_wdt = true;
+ } else {
+ err = devm_watchdog_register_device(dev, &tegra->wdts[i]->base);
+ if (err < 0)
+ return dev_err_probe(dev, err,
+ "failed to register WDT%u\n", i);
+ }
}
err = tegra186_timer_tsc_init(tegra);
@@ -501,8 +529,22 @@ static int tegra186_timer_probe(struct platform_device *pdev)
goto unregister_osc;
}
+ if (kernel_wdt) {
+ err = devm_request_irq(dev, irq, tegra186_wdt_irq, 0,
+ dev_name(dev), kernel_wdt);
+ if (err < 0) {
+ dev_err(dev, "failed to request kernel WDT IRQ: %d\n", err);
+ goto unregister_usec;
+ }
+
+ tegra186_wdt_set_timeout(&kernel_wdt->base, TEGRA186_KERNEL_WDT_TIMEOUT);
+ tegra186_wdt_enable(kernel_wdt);
+ }
+
return 0;
+unregister_usec:
+ clocksource_unregister(&tegra->usec);
unregister_osc:
clocksource_unregister(&tegra->osc);
unregister_tsc:
@@ -525,8 +567,10 @@ static int __maybe_unused tegra186_timer_suspend(struct device *dev)
unsigned int i;
for (i = 0; i < tegra->soc->num_wdts; i++) {
- if (tegra->wdts[i] && watchdog_active(&tegra->wdts[i]->base))
- tegra186_wdt_disable(tegra->wdts[i]);
+ struct tegra186_wdt *wdt = tegra->wdts[i];
+
+ if (wdt && (wdt->is_kernel_wdt || watchdog_active(&wdt->base)))
+ tegra186_wdt_disable(wdt);
}
return 0;
@@ -538,8 +582,10 @@ static int __maybe_unused tegra186_timer_resume(struct device *dev)
unsigned int i;
for (i = 0; i < tegra->soc->num_wdts; i++) {
- if (tegra->wdts[i] && watchdog_active(&tegra->wdts[i]->base))
- tegra186_wdt_enable(tegra->wdts[i]);
+ struct tegra186_wdt *wdt = tegra->wdts[i];
+
+ if (wdt && (wdt->is_kernel_wdt || watchdog_active(&wdt->base)))
+ tegra186_wdt_enable(wdt);
}
return 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] clocksource/drivers/timer-tegra186: Fix support for multiple watchdog instances
2026-05-07 15:45 ` [PATCH 1/4] clocksource/drivers/timer-tegra186: Fix support for multiple watchdog instances Kartik Rajput
@ 2026-05-07 22:58 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-05-07 22:58 UTC (permalink / raw)
To: Kartik Rajput; +Cc: linux-watchdog
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The read-modify-write sequence on the shared TKEIE register lacks synchronization, causing a data race when multiple watchdogs are enabled concurrently.
--
commit b62fcb75a56d43aa9fdaf906df5a4e961258346d
Author: Kartik Rajput <kkartik@nvidia.com>
clocksource/drivers/timer-tegra186: Fix support for multiple watchdog instances
This commit fixes support for multiple watchdog instances on Tegra186 by
changing the interrupt enable register update to a read-modify-write sequence.
This preserves the interrupt enable bits for previously configured watchdogs.
> diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
> index 355558893e5f3..bfe16d2d5104e 100644
> --- a/drivers/clocksource/timer-tegra186.c
> +++ b/drivers/clocksource/timer-tegra186.c
> @@ -149,7 +149,8 @@ static void tegra186_wdt_enable(struct tegra186_wdt *wdt)
> u32 value;
>
> /* unmask hardware IRQ, this may have been lost across powergate */
> - value = TKEIE_WDT_MASK(wdt->index, 1);
> + value = readl(tegra->regs + TKEIE(wdt->tmr->hwirq));
> + value |= TKEIE_WDT_MASK(wdt->index, 1);
> + writel(value, tegra->regs + TKEIE(wdt->tmr->hwirq));
Does this read-modify-write sequence need synchronization?
All watchdogs share the same TKEIE register since wdt->tmr->hwirq is
hardcoded to 0. Operations across different watchdog devices are not
serialized by the watchdog core, which only uses a per-device mutex.
If a userspace watchdog is started concurrently while a kernel watchdog is
being enabled, the read-modify-write sequences can overlap. If this happens,
one watchdog's interrupt enable bit can be silently overwritten and cleared.
Without its interrupt, the kernel watchdog could fail to ping itself,
resulting in an unintended system POR reset.
Could this shared MMIO register update be protected with a driver-level
spinlock in struct tegra186_timer?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260507154557.2082697-1-kkartik@nvidia.com?part=1
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/4] clocksource/drivers/timer-tegra186: Register all accessible watchdog timers
2026-05-07 15:45 ` [PATCH 3/4] clocksource/drivers/timer-tegra186: Register all accessible watchdog timers Kartik Rajput
@ 2026-05-07 23:38 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-05-07 23:38 UTC (permalink / raw)
To: Kartik Rajput; +Cc: linux-watchdog
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Data race on the shared TKEIE register during watchdog enable.
- [High] Hardware timer collision allows multiple watchdogs to clobber the same physical timer registers.
- [High] Missing bounds check on the timer source index allows out-of-bounds MMIO accesses.
--
commit 173c64a9e63a9e27e2f2b9544cfaa4b12f9d5f9c
Author: Kartik Rajput <kkartik@nvidia.com>
clocksource/drivers/timer-tegra186: Register all accessible watchdog timers
Tegra186+ SoCs expose multiple watchdog timers, but the driver only
registers WDT(0). Iterate over num_wdts and, for each WDT, check the SCR
(firewall) registers in the TKE block to determine whether Linux has read
and write access. Register the watchdogs that are accessible.
> diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
> index fd82a73ab2d27..dd1d1a0dd63eb 100644
> --- a/drivers/clocksource/timer-tegra186.c
> +++ b/drivers/clocksource/timer-tegra186.c
[ ... ]
> @@ -442,12 +467,20 @@ static int tegra186_timer_probe(struct platform_device *pdev)
> if (err < 0)
> return err;
>
> - /* create a watchdog using a preconfigured timer */
> - tegra->wdt = tegra186_wdt_create(tegra, 0);
> - if (IS_ERR(tegra->wdt)) {
> - err = PTR_ERR(tegra->wdt);
> - dev_err(dev, "failed to create WDT: %d\n", err);
> - return err;
> + tegra->wdts = devm_kcalloc(dev, tegra->soc->num_wdts, sizeof(*tegra->wdts), GFP_KERNEL);
> + if (!tegra->wdts)
> + return -ENOMEM;
> +
> + for (i = 0; i < tegra->soc->num_wdts; i++) {
> + if (!tegra186_wdt_is_accessible(tegra, i)) {
> + dev_warn(dev, "WDT%u is not accessible\n", i);
> + continue;
> + }
> +
> + tegra->wdts[i] = tegra186_wdt_create(tegra, i);
If the bootloader hasn't configured the watchdogs, could multiple watchdogs
extract the exact same source index and clobber each other's timer registers?
Looking at tegra186_wdt_create():
value = wdt_readl(wdt, WDTCR);
...
source = value & WDTCR_TIMER_SOURCE_MASK;
wdt->tmr = tegra186_tmr_create(tegra, source);
If WDTCR contains its default reset value, multiple watchdogs could be
assigned the identical physical timer MMIO region, allowing userspace to
concurrently overwrite the same TMRCR and TMRCSSR registers.
Is a bounds check needed to prevent out-of-bounds MMIO accesses?
The WDTCR_TIMER_SOURCE_MASK allows values up to 15, but
tegra->soc->num_timers is only 10 for Tegra186. If a value like 14 or 15
is provided, the computed MMIO offset in tegra186_tmr_create() could fall
entirely outside the valid mapped memory region.
Finally, can concurrent watchdog operations race on the shared TKEIE register?
Looking at tegra186_wdt_enable():
value = readl(tegra->regs + TKEIE(wdt->tmr->hwirq));
value |= TKEIE_WDT_MASK(wdt->index, 1);
writel(value, tegra->regs + TKEIE(wdt->tmr->hwirq));
Since hwirq is 0, all watchdogs access the exact same physical register. The
watchdog core's wdd->lock mutex only serializes operations on a per-device
basis. Concurrent start or ping operations to different watchdog devices
could cause one CPU's read-modify-write sequence to overwrite another's,
losing interrupt unmasks.
> + if (IS_ERR(tegra->wdts[i]))
> + return dev_err_probe(dev, PTR_ERR(tegra->wdts[i]),
> + "failed to create WDT%u\n", i);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260507154557.2082697-1-kkartik@nvidia.com?part=3
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 4/4] clocksource/drivers/timer-tegra186: Reserve and service a kernel watchdog
2026-05-07 15:45 ` [PATCH 4/4] clocksource/drivers/timer-tegra186: Reserve and service a kernel watchdog Kartik Rajput
@ 2026-05-08 0:23 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-05-08 0:23 UTC (permalink / raw)
To: Kartik Rajput; +Cc: linux-watchdog
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] The interrupt handler fails to clear shared interrupts from userspace watchdogs, causing an infinite IRQ storm.
- [High] Pinging the kernel watchdog from a hardware interrupt handler defeats the purpose of detecting software hangs.
- [High] Missing teardown of the kernel watchdog on driver unbind causes an unintended system reset.
- [High] Race condition between system suspend and the watchdog IRQ handler can leave the watchdog incorrectly armed during sleep.
--
commit 8065750aadbeb77247fd706790ed9368ba7e0064
Author: Kartik Rajput <kkartik@nvidia.com>
clocksource/drivers/timer-tegra186: Reserve and service a kernel watchdog
Tegra SoCs supports multiple watchdog timers. Reserve first available watchdog
as a kernel-only watchdog for Tegra186 and Tegra234. Arm it during probe (120s
timeout) and keep it alive in the driver IRQ handler.
> diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
> index dd1d1a0dd63eb..78600ddeb1c61 100644
> --- a/drivers/clocksource/timer-tegra186.c
> +++ b/drivers/clocksource/timer-tegra186.c
[ ... ]
> @@ -219,6 +226,16 @@ static int tegra186_wdt_ping(struct watchdog_device *wdd)
> return 0;
> }
>
> +static irqreturn_t tegra186_wdt_irq(int irq, void *data)
> +{
> + struct tegra186_wdt *wdt = data;
> +
> + tegra186_wdt_disable(wdt);
> + tegra186_wdt_enable(wdt);
> +
> + return IRQ_HANDLED;
> +}
The commit message states the kernel watchdog is intended to recover the system
if "the kernel crashes or hangs before userspace enables a watchdog" and
handles cases where "userspace never starts".
Because this handler runs as a hardirq, won't it continue to execute and feed
the watchdog even if the kernel deadlocks in task context, enters an infinite
loop, or userspace completely freezes? As long as local interrupts are enabled,
it seems the watchdog will silently ping itself indefinitely without resetting
the system.
Also, if userspace enables a different watchdog on the system, will this
handler cause an infinite IRQ storm?
Looking at tegra186_timer_probe(), the single shared hardware interrupt is
requested but only passed the kernel_wdt as data:
err = platform_get_irq(pdev, 0);
irq = err;
...
err = devm_request_irq(dev, irq, tegra186_wdt_irq, 0,
dev_name(dev), kernel_wdt);
When a userspace watchdog is enabled, its interrupt is unmasked. If it expires
and asserts the shared IRQ line, this handler will only clear the interrupt
status for kernel_wdt. Since the userspace watchdog's interrupt remains
asserted, won't the CPU immediately re-enter this handler after it returns
IRQ_HANDLED?
[ ... ]
> @@ -501,8 +529,22 @@ static int tegra186_timer_probe(struct platform_device *pdev)
> goto unregister_osc;
> }
>
> + if (kernel_wdt) {
> + err = devm_request_irq(dev, irq, tegra186_wdt_irq, 0,
> + dev_name(dev), kernel_wdt);
> + if (err < 0) {
> + dev_err(dev, "failed to request kernel WDT IRQ: %d\n", err);
> + goto unregister_usec;
> + }
> +
> + tegra186_wdt_set_timeout(&kernel_wdt->base, TEGRA186_KERNEL_WDT_TIMEOUT);
> + tegra186_wdt_enable(kernel_wdt);
> + }
> +
When the driver is unbound (e.g., via sysfs or module unloading), devres
automatically frees the IRQ handler, but there doesn't appear to be any
teardown of the hardware timer itself.
Will the hardware timer continue to tick unserviced and forcefully hard-reset
the system within 120 seconds of driver removal?
[ ... ]
> @@ -525,8 +567,10 @@ static int __maybe_unused tegra186_timer_suspend(struct device *dev)
> unsigned int i;
>
> for (i = 0; i < tegra->soc->num_wdts; i++) {
> - if (tegra->wdts[i] && watchdog_active(&tegra->wdts[i]->base))
> - tegra186_wdt_disable(tegra->wdts[i]);
> + struct tegra186_wdt *wdt = tegra->wdts[i];
> +
> + if (wdt && (wdt->is_kernel_wdt || watchdog_active(&wdt->base)))
> + tegra186_wdt_disable(wdt);
> }
>
> return 0;
If the watchdog's timer expires concurrently with suspend, can the
tegra186_wdt_irq handler execute on another CPU or immediately after suspend
unmasks interrupts?
Since the IRQ handler unconditionally calls tegra186_wdt_enable() without
checking any suspend state flags, it seems it could re-arm the watchdog after
this suspend callback has paused it, leading to a spurious hardware reset
during suspend.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260507154557.2082697-1-kkartik@nvidia.com?part=4
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/4] Add support for Kernel WDT
2026-05-07 15:45 [PATCH 0/4] Add support for Kernel WDT Kartik Rajput
` (3 preceding siblings ...)
2026-05-07 15:45 ` [PATCH 4/4] clocksource/drivers/timer-tegra186: Reserve and service a kernel watchdog Kartik Rajput
@ 2026-05-12 13:56 ` Jon Hunter
4 siblings, 0 replies; 9+ messages in thread
From: Jon Hunter @ 2026-05-12 13:56 UTC (permalink / raw)
To: Kartik Rajput, daniel.lezcano, tglx, wim, linux, thierry.reding,
linux-watchdog, linux-kernel, linux-tegra
On 07/05/2026 16:45, Kartik Rajput wrote:
> Tegra186 and later SoCs support multiple watchdog timers in the TKE
> block, the driver so far only uses WDT0. This series extends it to
> make use of the additional instances and reserve the first available WDT
> for Kernel.
>
> Patch 1 fixes a read-modify-write bug in tegra186_wdt_enable()
> that would otherwise make registering more than one watchdog unsafe.
>
> Patch 2 corrects num_wdts for Tegra186 and Tegra234. WDT2 is connected
> to the Audio Processing Engine (APE) and cannot be accessed from Linux,
> so only WDT0 and WDT1 are available.
>
> Patch 3 iterates over all WDTs and registers the ones whose TKE SCR
> firewall grants OS access.
>
> Patch 4 reserves the first accessible watchdog as a kernel-only
> watchdog, arms it at probe with a 120 s timeout, and pets it from
> the driver's IRQ handler, it is not exposed to userspace, so the
> system can still reset if the kernel hangs before userspace opens a
> watchdog.
>
> Kartik Rajput (4):
> clocksource/drivers/timer-tegra186: Fix support for multiple watchdog
> instances
> clocksource/drivers/timer-tegra186: Correct num_wdts for Tegra186 and
> Tegra234
> clocksource/drivers/timer-tegra186: Register all accessible watchdog
> timers
> clocksource/drivers/timer-tegra186: Reserve and service a kernel
> watchdog
>
> drivers/clocksource/timer-tegra186.c | 122 +++++++++++++++++++++++----
> 1 file changed, 104 insertions(+), 18 deletions(-)
>
For the series ...
Reviewed-by: Jon Hunter <jonathanh@nvidia.com>
Thanks!
Jon
--
nvpublic
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-05-12 13:56 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-07 15:45 [PATCH 0/4] Add support for Kernel WDT Kartik Rajput
2026-05-07 15:45 ` [PATCH 1/4] clocksource/drivers/timer-tegra186: Fix support for multiple watchdog instances Kartik Rajput
2026-05-07 22:58 ` sashiko-bot
2026-05-07 15:45 ` [PATCH 2/4] clocksource/drivers/timer-tegra186: Correct num_wdts for Tegra186 and Tegra234 Kartik Rajput
2026-05-07 15:45 ` [PATCH 3/4] clocksource/drivers/timer-tegra186: Register all accessible watchdog timers Kartik Rajput
2026-05-07 23:38 ` sashiko-bot
2026-05-07 15:45 ` [PATCH 4/4] clocksource/drivers/timer-tegra186: Reserve and service a kernel watchdog Kartik Rajput
2026-05-08 0:23 ` sashiko-bot
2026-05-12 13:56 ` [PATCH 0/4] Add support for Kernel WDT Jon Hunter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox