[PATCH v2] clocksource: timer-tegra186: Enable WDT at probe

[PATCH v2] clocksource: timer-tegra186: Enable WDT at probe

[Kartik Rajput]

Currently, if the system crashes or hangs during kernel boot before
userspace initializes and configures the watchdog timer, then the
watchdog won’t be able to recover the system as it’s not running. This
becomes crucial during an over-the-air update, where if the newly
updated kernel crashes on boot, the watchdog is needed to reset the
device and boot into an alternative system partition. If the watchdog
is disabled in such scenarios, it can lead to the system getting
bricked.

Enable the WDT during driver probe to allow recovery from any crash/hang
seen during early kernel boot. Also, disable interrupts once userspace
starts pinging the watchdog.

Signed-off-by: Kartik Rajput <kkartik@nvidia.com>
---
 drivers/clocksource/timer-tegra186.c | 46 ++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
index e5394f98a02e..bb16a119d55f 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 WDT_DEFAULT_TIMEOUT 120
+
 struct tegra186_timer_soc {
 	unsigned int num_timers;
 	unsigned int num_wdts;
@@ -74,6 +76,7 @@ struct tegra186_wdt {
 
 	void __iomem *regs;
 	unsigned int index;
+	bool enable_irq;
 	bool locked;
 
 	struct tegra186_tmr *tmr;
@@ -174,6 +177,16 @@ static void tegra186_wdt_enable(struct tegra186_wdt *wdt)
 		value &= ~WDTCR_PERIOD_MASK;
 		value |= WDTCR_PERIOD(1);
 
+		/*
+		 * If enable_irq is set then enable the watchdog IRQ for kernel
+		 * petting, otherwise userspace is responsible for petting the
+		 * watchdog.
+		 */
+		if (wdt->enable_irq)
+			value |= WDTCR_LOCAL_INT_ENABLE;
+		else
+			value &= ~WDTCR_LOCAL_INT_ENABLE;
+
 		/* enable system POR reset */
 		value |= WDTCR_SYSTEM_POR_RESET_ENABLE;
 
@@ -205,6 +218,10 @@ static int tegra186_wdt_ping(struct watchdog_device *wdd)
 {
 	struct tegra186_wdt *wdt = to_tegra186_wdt(wdd);
 
+	/* Disable the watchdog IRQ now userspace is taking over. */
+	if (wdt->enable_irq)
+		wdt->enable_irq = false;
+
 	tegra186_wdt_disable(wdt);
 	tegra186_wdt_enable(wdt);
 
@@ -315,6 +332,8 @@ static struct tegra186_wdt *tegra186_wdt_create(struct tegra186_timer *tegra,
 	if (value & WDTCR_LOCAL_INT_ENABLE)
 		wdt->locked = true;
 
+	wdt->enable_irq = true;
+
 	source = value & WDTCR_TIMER_SOURCE_MASK;
 
 	wdt->tmr = tegra186_tmr_create(tegra, source);
@@ -339,6 +358,13 @@ static struct tegra186_wdt *tegra186_wdt_create(struct tegra186_timer *tegra,
 		return ERR_PTR(err);
 	}
 
+	/*
+	 * Start the watchdog to recover the system if it crashes before
+	 * userspace initialize the WDT.
+	 */
+	tegra186_wdt_set_timeout(&wdt->base, WDT_DEFAULT_TIMEOUT);
+	tegra186_wdt_start(&wdt->base);
+
 	return wdt;
 }
 
@@ -415,10 +441,21 @@ static int tegra186_timer_usec_init(struct tegra186_timer *tegra)
 	return clocksource_register_hz(&tegra->usec, USEC_PER_SEC);
 }
 
+static irqreturn_t tegra186_timer_irq(int irq, void *data)
+{
+	struct tegra186_timer *tegra = data;
+
+	tegra186_wdt_disable(tegra->wdt);
+	tegra186_wdt_enable(tegra->wdt);
+
+	return IRQ_HANDLED;
+}
+
 static int tegra186_timer_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct tegra186_timer *tegra;
+	unsigned int irq;
 	int err;
 
 	tegra = devm_kzalloc(dev, sizeof(*tegra), GFP_KERNEL);
@@ -437,6 +474,15 @@ static int tegra186_timer_probe(struct platform_device *pdev)
 	if (err < 0)
 		return err;
 
+	irq = err;
+
+	err = devm_request_irq(dev, irq, tegra186_timer_irq, 0,
+			       "tegra186-timer", tegra);
+	if (err < 0) {
+		dev_err(dev, "failed to request IRQ#%u: %d\n", irq, err);
+		return err;
+	}
+
 	/* create a watchdog using a preconfigured timer */
 	tegra->wdt = tegra186_wdt_create(tegra, 0);
 	if (IS_ERR(tegra->wdt)) {
-- 
2.43.0

Re: [PATCH v2] clocksource: timer-tegra186: Enable WDT at probe

[Daniel Lezcano]

On Thu, Jul 03, 2025 at 04:34:15PM +0530, Kartik Rajput wrote:
> Currently, if the system crashes or hangs during kernel boot before
> userspace initializes and configures the watchdog timer, then the
> watchdog won’t be able to recover the system as it’s not running. This
> becomes crucial during an over-the-air update, where if the newly
> updated kernel crashes on boot, the watchdog is needed to reset the
> device and boot into an alternative system partition. If the watchdog
> is disabled in such scenarios, it can lead to the system getting
> bricked.
> 
> Enable the WDT during driver probe to allow recovery from any crash/hang
> seen during early kernel boot. Also, disable interrupts once userspace
> starts pinging the watchdog.

Please resend with proper recipients (linux-watchdog@, Wim Van
Sebroeck, Guenter Roeck) and the changelog.

Can someone take the opportunity to split this watchdog code and move
it in the proper watchdog drivers directory ?

As previously discussed, the auxiliary device seems the appropriate
approach [1][2].

Thanks
  -- Daniel

[1] https://lore.kernel.org/all/CABjd4YyXwznntcLVcYL6qx16YEwv4_VWzrXrE7_QHmQxiE0pXQ@mail.gmail.com/
[2] https://lore.kernel.org/all/20250521-vt8500-timer-updates-v4-3-2d4306a16ae4@gmail.com/


 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v2] clocksource: timer-tegra186: Enable WDT at probe

[Jon Hunter]

On 07/07/2025 10:51, Daniel Lezcano wrote:
> On Thu, Jul 03, 2025 at 04:34:15PM +0530, Kartik Rajput wrote:
>> Currently, if the system crashes or hangs during kernel boot before
>> userspace initializes and configures the watchdog timer, then the
>> watchdog won’t be able to recover the system as it’s not running. This
>> becomes crucial during an over-the-air update, where if the newly
>> updated kernel crashes on boot, the watchdog is needed to reset the
>> device and boot into an alternative system partition. If the watchdog
>> is disabled in such scenarios, it can lead to the system getting
>> bricked.
>>
>> Enable the WDT during driver probe to allow recovery from any crash/hang
>> seen during early kernel boot. Also, disable interrupts once userspace
>> starts pinging the watchdog.
> 
> Please resend with proper recipients (linux-watchdog@, Wim Van
> Sebroeck, Guenter Roeck) and the changelog.

ACK.

> Can someone take the opportunity to split this watchdog code and move
> it in the proper watchdog drivers directory ?

I understand that this was mentioned before, but Thierry previously 
objected to this for this particular driver [0].

Cheers,
Jon

[0] 
https://lore.kernel.org/linux-tegra/4ks74upuufmt2ibh5ur5zpazvfj66ak4gyq7v4rtz2zi2u5wsi@rls64ws3rukp/

-- 
nvpublic

Re: [PATCH v2] clocksource: timer-tegra186: Enable WDT at probe

[Daniel Lezcano]

On 07/07/2025 23:19, Jon Hunter wrote:
> 
> On 07/07/2025 10:51, Daniel Lezcano wrote:
>> On Thu, Jul 03, 2025 at 04:34:15PM +0530, Kartik Rajput wrote:
>>> Currently, if the system crashes or hangs during kernel boot before
>>> userspace initializes and configures the watchdog timer, then the
>>> watchdog won’t be able to recover the system as it’s not running. This
>>> becomes crucial during an over-the-air update, where if the newly
>>> updated kernel crashes on boot, the watchdog is needed to reset the
>>> device and boot into an alternative system partition. If the watchdog
>>> is disabled in such scenarios, it can lead to the system getting
>>> bricked.
>>>
>>> Enable the WDT during driver probe to allow recovery from any crash/hang
>>> seen during early kernel boot. Also, disable interrupts once userspace
>>> starts pinging the watchdog.
>>
>> Please resend with proper recipients (linux-watchdog@, Wim Van
>> Sebroeck, Guenter Roeck) and the changelog.
> 
> ACK.
> 
>> Can someone take the opportunity to split this watchdog code and move
>> it in the proper watchdog drivers directory ?
> 
> I understand that this was mentioned before, but Thierry previously 
> objected to this for this particular driver [0].

Yes but meanwhile we found that the auxiliary device is designed for 
this situation.

> [0] https://lore.kernel.org/linux- 
> tegra/4ks74upuufmt2ibh5ur5zpazvfj66ak4gyq7v4rtz2zi2u5wsi@rls64ws3rukp/
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v2] clocksource: timer-tegra186: Enable WDT at probe

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1792 bytes --]

On Tue, Jul 08, 2025 at 12:03:52AM +0200, Daniel Lezcano wrote:
> On 07/07/2025 23:19, Jon Hunter wrote:
> > 
> > On 07/07/2025 10:51, Daniel Lezcano wrote:
> > > On Thu, Jul 03, 2025 at 04:34:15PM +0530, Kartik Rajput wrote:
> > > > Currently, if the system crashes or hangs during kernel boot before
> > > > userspace initializes and configures the watchdog timer, then the
> > > > watchdog won’t be able to recover the system as it’s not running. This
> > > > becomes crucial during an over-the-air update, where if the newly
> > > > updated kernel crashes on boot, the watchdog is needed to reset the
> > > > device and boot into an alternative system partition. If the watchdog
> > > > is disabled in such scenarios, it can lead to the system getting
> > > > bricked.
> > > > 
> > > > Enable the WDT during driver probe to allow recovery from any crash/hang
> > > > seen during early kernel boot. Also, disable interrupts once userspace
> > > > starts pinging the watchdog.
> > > 
> > > Please resend with proper recipients (linux-watchdog@, Wim Van
> > > Sebroeck, Guenter Roeck) and the changelog.
> > 
> > ACK.
> > 
> > > Can someone take the opportunity to split this watchdog code and move
> > > it in the proper watchdog drivers directory ?
> > 
> > I understand that this was mentioned before, but Thierry previously
> > objected to this for this particular driver [0].
> 
> Yes but meanwhile we found that the auxiliary device is designed for this
> situation.

Honestly, adding auxiliary bus into the mix is total overkill here. I
always thought with all the tools we have today it'd be easy enough to
have drivers spread across subsystems if that's what's best.

But if y'all think this is the way, then sure, we'll do that.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] clocksource: timer-tegra186: Enable WDT at probe

[Jon Hunter]

On 08/07/2025 12:22, Thierry Reding wrote:
> On Tue, Jul 08, 2025 at 12:03:52AM +0200, Daniel Lezcano wrote:
>> On 07/07/2025 23:19, Jon Hunter wrote:
>>>
>>> On 07/07/2025 10:51, Daniel Lezcano wrote:
>>>> On Thu, Jul 03, 2025 at 04:34:15PM +0530, Kartik Rajput wrote:
>>>>> Currently, if the system crashes or hangs during kernel boot before
>>>>> userspace initializes and configures the watchdog timer, then the
>>>>> watchdog won’t be able to recover the system as it’s not running. This
>>>>> becomes crucial during an over-the-air update, where if the newly
>>>>> updated kernel crashes on boot, the watchdog is needed to reset the
>>>>> device and boot into an alternative system partition. If the watchdog
>>>>> is disabled in such scenarios, it can lead to the system getting
>>>>> bricked.
>>>>>
>>>>> Enable the WDT during driver probe to allow recovery from any crash/hang
>>>>> seen during early kernel boot. Also, disable interrupts once userspace
>>>>> starts pinging the watchdog.
>>>>
>>>> Please resend with proper recipients (linux-watchdog@, Wim Van
>>>> Sebroeck, Guenter Roeck) and the changelog.
>>>
>>> ACK.
>>>
>>>> Can someone take the opportunity to split this watchdog code and move
>>>> it in the proper watchdog drivers directory ?
>>>
>>> I understand that this was mentioned before, but Thierry previously
>>> objected to this for this particular driver [0].
>>
>> Yes but meanwhile we found that the auxiliary device is designed for this
>> situation.
> 
> Honestly, adding auxiliary bus into the mix is total overkill here. I
> always thought with all the tools we have today it'd be easy enough to
> have drivers spread across subsystems if that's what's best.
> 
> But if y'all think this is the way, then sure, we'll do that.


Yes we are happy to conform to whatever is preferred. However, this is 
really a fix and IMO outside the scope of any refactoring work. I have 
been pushing the necessary people at NVIDIA to get fixes upstream so 
that any known issues are fixed. Hence, I would prefer to handle the 
refactoring separately.

Thanks!
Jon

-- 
nvpublic

Re: [PATCH v2] clocksource: timer-tegra186: Enable WDT at probe

[Daniel Lezcano]

On 08/07/2025 13:42, Jon Hunter wrote:
> 
> On 08/07/2025 12:22, Thierry Reding wrote:
>> On Tue, Jul 08, 2025 at 12:03:52AM +0200, Daniel Lezcano wrote:
>>> On 07/07/2025 23:19, Jon Hunter wrote:
>>>>
>>>> On 07/07/2025 10:51, Daniel Lezcano wrote:
>>>>> On Thu, Jul 03, 2025 at 04:34:15PM +0530, Kartik Rajput wrote:
>>>>>> Currently, if the system crashes or hangs during kernel boot before
>>>>>> userspace initializes and configures the watchdog timer, then the
>>>>>> watchdog won’t be able to recover the system as it’s not running. 
>>>>>> This
>>>>>> becomes crucial during an over-the-air update, where if the newly
>>>>>> updated kernel crashes on boot, the watchdog is needed to reset the
>>>>>> device and boot into an alternative system partition. If the watchdog
>>>>>> is disabled in such scenarios, it can lead to the system getting
>>>>>> bricked.
>>>>>>
>>>>>> Enable the WDT during driver probe to allow recovery from any 
>>>>>> crash/hang
>>>>>> seen during early kernel boot. Also, disable interrupts once 
>>>>>> userspace
>>>>>> starts pinging the watchdog.
>>>>>
>>>>> Please resend with proper recipients (linux-watchdog@, Wim Van
>>>>> Sebroeck, Guenter Roeck) and the changelog.
>>>>
>>>> ACK.
>>>>
>>>>> Can someone take the opportunity to split this watchdog code and move
>>>>> it in the proper watchdog drivers directory ?
>>>>
>>>> I understand that this was mentioned before, but Thierry previously
>>>> objected to this for this particular driver [0].
>>>
>>> Yes but meanwhile we found that the auxiliary device is designed for 
>>> this
>>> situation.
>>
>> Honestly, adding auxiliary bus into the mix is total overkill here. I
>> always thought with all the tools we have today it'd be easy enough to
>> have drivers spread across subsystems if that's what's best.
>>
>> But if y'all think this is the way, then sure, we'll do that.
> 
> 
> Yes we are happy to conform to whatever is preferred. However, this is 
> really a fix and IMO outside the scope of any refactoring work. I have 
> been pushing the necessary people at NVIDIA to get fixes upstream so 
> that any known issues are fixed. Hence, I would prefer to handle the 
> refactoring separately.


Yes, absolutely


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog