[PATCH 0/3] watchdog: omap: Preserve OMAP watchdog across boot to detect faulty boots

[PATCH 0/3] watchdog: omap: Preserve OMAP watchdog across boot to detect faulty boots

[Diogo Ivo]

Allow the OMAP watchdog to survive being stopped on kernel initialization
so it can detect a faulty boot in cases where the bootloader leaves it
running and the watchdog driver picks it up during kernel init.

- Patch 1 removes a duplicate omap_wdt_start() call left behind by
cd004d8299f1 ("watchdog: Fix OMAP watchdog early handling"). This is
unrelated to the main goal of the series and can be picked up
independently.

- Patch 2 adds support for reading the watchdog boot status. Probe now
checks whether the watchdog is already running and takes it over instead
of blindly stopping it based on early_enable alone. This introduces a
regression possibility, explained in detail in the patch's message.

- Patch 3 marks the OMAP4 watchdog node ti,no-reset-on-init so the
ti-sysc driver stops resetting it. A detailed explaination of why is
also provided in the commit message of the patch.

This series has been tested on a platform based on the VAR-SOM-OM44 from
Variscite, running a TI OMAP4460 SoC.

Signed-off-by: Diogo Ivo <diogo.ivo@bootlin.com>
---
Diogo Ivo (3):
      watchdog: omap: Remove duplicate start() with early_init
      watchdog: omap: Add support for reading boot status
      arm: dts: ti: omap: Prevent watchdog from being reset on kernel boot

 arch/arm/boot/dts/ti/omap/omap4-l4.dtsi |  1 +
 drivers/watchdog/omap_wdt.c             | 22 ++++++++++++++++++----
 2 files changed, 19 insertions(+), 4 deletions(-)
---
base-commit: 83d343e6246c83e694548fec49856819124d32c0
change-id: 20260910-omap4-wdt_boot-472b73d4af6f

Best regards,
--  
Diogo Ivo <diogo.ivo@bootlin.com>

[PATCH 1/3] watchdog: omap: Remove duplicate start() with early_init

[Diogo Ivo]

Commit cd004d8299f1 ("watchdog: Fix OMAP watchdog early handling")
reworked the handling of the early_enable parameter and added an
explicit call to omap_wdt_start() in case that parameter is true,
but left the call that was previously there, which became duplicate.

Fix this by removing the unnecessary duplicate call.

Signed-off-by: Diogo Ivo <diogo.ivo@bootlin.com>
---
 drivers/watchdog/omap_wdt.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index 2482a1982c8b..e6d869e36c43 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -286,9 +286,6 @@ static int omap_wdt_probe(struct platform_device *pdev)
 		readl_relaxed(wdev->base + OMAP_WATCHDOG_REV) & 0xFF,
 		wdev->wdog.timeout);
 
-	if (early_enable)
-		omap_wdt_start(&wdev->wdog);
-
 	pm_runtime_put(wdev->dev);
 
 	return 0;

-- 
2.55.0

[PATCH 2/3] watchdog: omap: Add support for reading boot status

[Diogo Ivo]

Add support for determining the boot status of the watchdog.

Signed-off-by: Diogo Ivo <diogo.ivo@bootlin.com>
---
One case worth mentioning explicitly here in terms of regressions:

Consider a machine where:
 - the bootloader turns on the wd
 - early_init = 0
 - omap_wdt is compiled into the kernel
 - CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED=n
 - userspace does not service the watchdog

The behaviour of such a machine up until this patch was that the
the system would not reboot as the watchdog would be unconditionally
stopped. However, after this patch such systems _will_ reboot since the
watchdog will be kept on and nothing will service it. In practice, with
just this patch this will not happen since the ti-sysc.c driver (that
probes prior to the watchdog driver) will anyway stop the watchdog, but
when adding the next patch in this series to stop that behaviour this
regression becomes a real scenario.
---
 drivers/watchdog/omap_wdt.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
index e6d869e36c43..7627d3c626c2 100644
--- a/drivers/watchdog/omap_wdt.c
+++ b/drivers/watchdog/omap_wdt.c
@@ -26,6 +26,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/kernel.h>
@@ -42,6 +43,8 @@
 
 #include "omap_wdt.h"
 
+#define RATE_32K 32768
+
 static bool nowayout = WATCHDOG_NOWAYOUT;
 module_param(nowayout, bool, 0);
 MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started "
@@ -225,6 +228,20 @@ static const struct watchdog_ops omap_wdt_ops = {
 	.get_timeleft	= omap_wdt_get_timeleft,
 };
 
+static bool omap_wdt_is_running(struct omap_wdt_dev *wdev)
+{
+	unsigned long period_us = USEC_PER_SEC / RATE_32K;
+	void __iomem *base = wdev->base;
+	u32 value;
+
+	value = readl_relaxed(base + OMAP_WATCHDOG_CRR);
+
+	/* Give the watchdog some time to count if it's on */
+	usleep_range(period_us * 10, period_us * 11);
+
+	return readl_relaxed(base + OMAP_WATCHDOG_CRR) != value;
+}
+
 static int omap_wdt_probe(struct platform_device *pdev)
 {
 	struct omap_wd_timer_platform_data *pdata = dev_get_platdata(&pdev->dev);
@@ -267,7 +284,7 @@ static int omap_wdt_probe(struct platform_device *pdev)
 			wdev->wdog.bootstatus = WDIOF_CARDRESET;
 	}
 
-	if (early_enable) {
+	if (omap_wdt_is_running(wdev) || early_enable) {
 		omap_wdt_start(&wdev->wdog);
 		set_bit(WDOG_HW_RUNNING, &wdev->wdog.status);
 	} else {

-- 
2.55.0

[PATCH 3/3] arm: dts: ti: omap: Prevent watchdog from being reset on kernel boot

[Diogo Ivo]

Currently the ti-sysc driver resets and idles the watchdog when it probes
its target-module node. In a system where the watchdog is being used to
detect a faulty boot this makes it impossible for the watchdog to do its
function, in an unexpected and time-consuming manner to debug.

Explicitly mark the watchdog node with ti,no-reset-on-init to prevent
the reset portion from happening. This allows the watchdog driver proper
to maintain the watchdog running.

Signed-off-by: Diogo Ivo <diogo.ivo@bootlin.com>
---
The ti,no-idle property is deliberately not added to minimize the
possibility of a regression on systems that were relying on the ti-sysc
driver disabling the watchdog and did not service it. With just
ti,no-reset-on-init we get a 3 second window for the watchdog driver to
probe and maintain the watchdog alive, otherwise the system will behave
almost in the same way as before. The only difference comes from introducing
the small 3 second window that could lead to a watchdog expiration on systems
where the watchdog was being stopped right next to its expiration, but
that risk is much smaller regression-wise than always keeping its boot
status. If this approach is deemed appropriate this patch should be
mimicked in other TI SoC's with this watchdog.
---
 arch/arm/boot/dts/ti/omap/omap4-l4.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/ti/omap/omap4-l4.dtsi b/arch/arm/boot/dts/ti/omap/omap4-l4.dtsi
index c1afc49f456c..eb0e10e51ba4 100644
--- a/arch/arm/boot/dts/ti/omap/omap4-l4.dtsi
+++ b/arch/arm/boot/dts/ti/omap/omap4-l4.dtsi
@@ -1126,6 +1126,7 @@ target-module@4000 {			/* 0x4a314000, ap 7 18.0 */
 					<SYSC_IDLE_SMART>,
 					<SYSC_IDLE_SMART_WKUP>;
 			ti,syss-mask = <1>;
+			ti,no-reset-on-init;
 			/* Domains (V, P, C): wakeup, wkup_pwrdm, l4_wkup_clkdm */
 			clocks = <&l4_wkup_clkctrl OMAP4_WD_TIMER2_CLKCTRL 0>;
 			clock-names = "fck";

-- 
2.55.0

Re: [PATCH 0/3] watchdog: omap: Preserve OMAP watchdog across boot to detect faulty boots

[Guenter Roeck]

On 9/11/26 02:17, Diogo Ivo wrote:
> Allow the OMAP watchdog to survive being stopped on kernel initialization
> so it can detect a faulty boot in cases where the bootloader leaves it
> running and the watchdog driver picks it up during kernel init.
> 
> - Patch 1 removes a duplicate omap_wdt_start() call left behind by
> cd004d8299f1 ("watchdog: Fix OMAP watchdog early handling"). This is
> unrelated to the main goal of the series and can be picked up
> independently.
> 
> - Patch 2 adds support for reading the watchdog boot status. Probe now
> checks whether the watchdog is already running and takes it over instead
> of blindly stopping it based on early_enable alone. This introduces a
> regression possibility, explained in detail in the patch's message.
> 
> - Patch 3 marks the OMAP4 watchdog node ti,no-reset-on-init so the
> ti-sysc driver stops resetting it. A detailed explaination of why is
> also provided in the commit message of the patch.
> 
> This series has been tested on a platform based on the VAR-SOM-OM44 from
> Variscite, running a TI OMAP4460 SoC.
> 
Please address the issues reported by Sashiko, or explain why they don't apply.

Thanks,
Guenter

Re: [PATCH 0/3] watchdog: omap: Preserve OMAP watchdog across boot to detect faulty boots

[Diogo Ivo]

Hi Guenter,

On 9/11/26 4:29 PM, Guenter Roeck wrote:
> On 9/11/26 02:17, Diogo Ivo wrote:
>> Allow the OMAP watchdog to survive being stopped on kernel initialization
>> so it can detect a faulty boot in cases where the bootloader leaves it
>> running and the watchdog driver picks it up during kernel init.
>>
>> - Patch 1 removes a duplicate omap_wdt_start() call left behind by
>> cd004d8299f1 ("watchdog: Fix OMAP watchdog early handling"). This is
>> unrelated to the main goal of the series and can be picked up
>> independently.
>>
>> - Patch 2 adds support for reading the watchdog boot status. Probe now
>> checks whether the watchdog is already running and takes it over instead
>> of blindly stopping it based on early_enable alone. This introduces a
>> regression possibility, explained in detail in the patch's message.
>>
>> - Patch 3 marks the OMAP4 watchdog node ti,no-reset-on-init so the
>> ti-sysc driver stops resetting it. A detailed explaination of why is
>> also provided in the commit message of the patch.
>>
>> This series has been tested on a platform based on the VAR-SOM-OM44 from
>> Variscite, running a TI OMAP4460 SoC.
>>
> Please address the issues reported by Sashiko, or explain why they don't 
> apply.

I have just replied to the Sashiko reviews but I'm not sure if you got
the replies as Sashiko did not include your e-mail in its review. If you
did not receive them please let me know and I can resend them. In
any case if you could give your opinion on the comments I left on the
patches about regressions that would be great as I think after the
Sashiko points are addressed that is the main blocker for this series.

Thanks,
Diogo

> Thanks,
> Guenter

Re: [PATCH 0/3] watchdog: omap: Preserve OMAP watchdog across boot to detect faulty boots

[Guenter Roeck]

On 9/11/26 08:17, Diogo Ivo wrote:
> Hi Guenter,
> 
> On 9/11/26 4:29 PM, Guenter Roeck wrote:
>> On 9/11/26 02:17, Diogo Ivo wrote:
>>> Allow the OMAP watchdog to survive being stopped on kernel initialization
>>> so it can detect a faulty boot in cases where the bootloader leaves it
>>> running and the watchdog driver picks it up during kernel init.
>>>
>>> - Patch 1 removes a duplicate omap_wdt_start() call left behind by
>>> cd004d8299f1 ("watchdog: Fix OMAP watchdog early handling"). This is
>>> unrelated to the main goal of the series and can be picked up
>>> independently.
>>>
>>> - Patch 2 adds support for reading the watchdog boot status. Probe now
>>> checks whether the watchdog is already running and takes it over instead
>>> of blindly stopping it based on early_enable alone. This introduces a
>>> regression possibility, explained in detail in the patch's message.
>>>
>>> - Patch 3 marks the OMAP4 watchdog node ti,no-reset-on-init so the
>>> ti-sysc driver stops resetting it. A detailed explaination of why is
>>> also provided in the commit message of the patch.
>>>
>>> This series has been tested on a platform based on the VAR-SOM-OM44 from
>>> Variscite, running a TI OMAP4460 SoC.
>>>
>> Please address the issues reported by Sashiko, or explain why they don't apply.
> 
> I have just replied to the Sashiko reviews but I'm not sure if you got
> the replies as Sashiko did not include your e-mail in its review. If you
> did not receive them please let me know and I can resend them. In
> any case if you could give your opinion on the comments I left on the
> patches about regressions that would be great as I think after the
> Sashiko points are addressed that is the main blocker for this series.
> 
I did. I just wonder if the effort is worth the pain / cost.

Is there an actual use case ? Is the problem you are trying to solve
a real problem, or a theoretic one ? For example, the patches impose
a hard boot delay of more than 30 ms in omap_wdt_is_running().
Even though that could be optimized (there is no reason to wait
that long; the value could change a microsecond after the first read),
it is nevertheless a mandatory boot delay.

Another concern is the impact and potential side effects of setting
ti,no-reset-on-init (and the possible boot loop cause by it due to the odd
30-second init delay). After this change, a running watchdog is no longer
stopped. What happens on systems which do not load the watchdog at all
(for example because the driver was not configured) ? Will that also cause
a boot loop on such systems ?

This is just a couple of problems introduced by this series. You better have
a very good reason for it to warrant having to deal with the potential fallout.

Thanks,
Guenter

Re: [PATCH 0/3] watchdog: omap: Preserve OMAP watchdog across boot to detect faulty boots

[Diogo Ivo]

On 9/11/26 7:25 PM, Guenter Roeck wrote:
> On 9/11/26 08:17, Diogo Ivo wrote:
>> Hi Guenter,
>>
>> On 9/11/26 4:29 PM, Guenter Roeck wrote:
>>> On 9/11/26 02:17, Diogo Ivo wrote:
>>>> Allow the OMAP watchdog to survive being stopped on kernel 
>>>> initialization
>>>> so it can detect a faulty boot in cases where the bootloader leaves it
>>>> running and the watchdog driver picks it up during kernel init.
>>>>
>>>> - Patch 1 removes a duplicate omap_wdt_start() call left behind by
>>>> cd004d8299f1 ("watchdog: Fix OMAP watchdog early handling"). This is
>>>> unrelated to the main goal of the series and can be picked up
>>>> independently.
>>>>
>>>> - Patch 2 adds support for reading the watchdog boot status. Probe now
>>>> checks whether the watchdog is already running and takes it over 
>>>> instead
>>>> of blindly stopping it based on early_enable alone. This introduces a
>>>> regression possibility, explained in detail in the patch's message.
>>>>
>>>> - Patch 3 marks the OMAP4 watchdog node ti,no-reset-on-init so the
>>>> ti-sysc driver stops resetting it. A detailed explaination of why is
>>>> also provided in the commit message of the patch.
>>>>
>>>> This series has been tested on a platform based on the VAR-SOM-OM44 
>>>> from
>>>> Variscite, running a TI OMAP4460 SoC.
>>>>
>>> Please address the issues reported by Sashiko, or explain why they 
>>> don't apply.
>>
>> I have just replied to the Sashiko reviews but I'm not sure if you got
>> the replies as Sashiko did not include your e-mail in its review. If you
>> did not receive them please let me know and I can resend them. In
>> any case if you could give your opinion on the comments I left on the
>> patches about regressions that would be great as I think after the
>> Sashiko points are addressed that is the main blocker for this series.
>>
> I did. I just wonder if the effort is worth the pain / cost.
> 
> Is there an actual use case ? Is the problem you are trying to solve
> a real problem, or a theoretic one ? For example, the patches impose
> a hard boot delay of more than 30 ms in omap_wdt_is_running().
> Even though that could be optimized (there is no reason to wait
> that long; the value could change a microsecond after the first read),
> it is nevertheless a mandatory boot delay.

Yes, I stumbled upon this problem while trying to do exactly what is on
the commit messages, so having a system that does A/B updates and uses
the watchdog to detect if the boot succeeded or failed. The current boot
delay is actually (1000000 / 32768) * 10 = 305us, so quite a bit smaller
than the 30ms you mention, even though from the Sashiko comments I need
to take into account the prescaler, which in the worst case brings the
value to the 30ms you mention, but if I adjust it I can bring it down to
3ms in the worst case of a 128 prescaler. However, for a system with
prescaler=1 the delay can be brought down to 30us.

> Another concern is the impact and potential side effects of setting
> ti,no-reset-on-init (and the possible boot loop cause by it due to the odd
> 30-second init delay). After this change, a running watchdog is no longer
> stopped. What happens on systems which do not load the watchdog at all
> (for example because the driver was not configured) ? Will that also cause
> a boot loop on such systems ?

Here with ti,no-reset-on-init the ti-sysc driver will shutdown the
watchdog driver after 30 seconds, which can indeed cause problems. When
I sent the patch I thought the timeout would be 3 seconds, which
considerable reduces the possibility of a bootloop. I will look into it
to see if it makes sense to change the timeout value and reduce the
possibility of the bootloop. My initial assumption of the 3 seconds was
what actually made me not add ti,no-idle-on-reset since that would
completely block the watchdog stopping from ti-sysc.

> This is just a couple of problems introduced by this series. You better 
> have
> a very good reason for it to warrant having to deal with the potential 
> fallout.

The reason is that in these systems the watchdog is not behaving as one
would expect: if the bootloader leaves the watchdog running it is a
reasonable expectation that unless the kernel or userspace services it
the system should shutdown. With this series I have tried to achieve
this while minimizing the risk of regressions, and from my point of
view the only sore point is indeed the 30s timeout in ti-sysc. If you
agree with my reasoning and think this is worth pursuing let me know and
I will change the 30s timeout for v2. In the meantime patch 1 is
completely regression free and can be picked up!

Thanks,
Diogo

> Thanks,
> Guenter