* [PATCH 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support
@ 2025-04-17 9:28 Robert Lin
2025-04-17 9:28 ` [PATCH 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging Robert Lin
` (6 more replies)
0 siblings, 7 replies; 13+ messages in thread
From: Robert Lin @ 2025-04-17 9:28 UTC (permalink / raw)
To: thierry.reding, daniel.lezcano, jonathanh, tglx, pohsuns
Cc: linux-kernel, linux-tegra, sumitg, Robert Lin
From: Pohsun Su <pohsuns@nvidia.com>
This change adds support for WDIOC_GETTIMELEFT so userspace
programs can get the number of seconds before system reset by
the watchdog timer via ioctl.
Signed-off-by: Pohsun Su <pohsuns@nvidia.com>
Signed-off-by: Robert Lin <robelin@nvidia.com>
---
drivers/clocksource/timer-tegra186.c | 56 +++++++++++++++++++++++++++-
1 file changed, 55 insertions(+), 1 deletion(-)
diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
index ea742889ee06..201b24ca59f4 100644
--- a/drivers/clocksource/timer-tegra186.c
+++ b/drivers/clocksource/timer-tegra186.c
@@ -1,8 +1,9 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * Copyright (c) 2019-2020 NVIDIA Corporation. All rights reserved.
+ * Copyright (c) 2019-2025 NVIDIA Corporation. All rights reserved.
*/
+#include <linux/bitfield.h>
#include <linux/clocksource.h>
#include <linux/module.h>
#include <linux/interrupt.h>
@@ -30,6 +31,7 @@
#define TMRSR 0x004
#define TMRSR_INTR_CLR BIT(30)
+#define TMRSR_PCV GENMASK(28, 0)
#define TMRCSSR 0x008
#define TMRCSSR_SRC_USEC (0 << 0)
@@ -46,6 +48,9 @@
#define WDTCR_TIMER_SOURCE_MASK 0xf
#define WDTCR_TIMER_SOURCE(x) ((x) & 0xf)
+#define WDTSR 0x004
+#define WDTSR_CURRENT_EXPIRATION_COUNT GENMASK(14, 12)
+
#define WDTCMDR 0x008
#define WDTCMDR_DISABLE_COUNTER BIT(1)
#define WDTCMDR_START_COUNTER BIT(0)
@@ -235,12 +240,61 @@ static int tegra186_wdt_set_timeout(struct watchdog_device *wdd,
return 0;
}
+static unsigned int tegra186_wdt_get_timeleft(struct watchdog_device *wdd)
+{
+ struct tegra186_wdt *wdt = to_tegra186_wdt(wdd);
+ u32 timeleft, expiration, val;
+
+ if (!watchdog_active(&wdt->base)) {
+ /* return zero if the watchdog timer is not activated. */
+ return 0;
+ }
+
+ /*
+ * Reset occurs on the fifth expiration of the
+ * watchdog timer and so when the watchdog timer is configured,
+ * the actual value programmed into the counter is 1/5 of the
+ * timeout value. Once the counter reaches 0, expiration count
+ * will be increased by 1 and the down counter restarts.
+ * Hence to get the time left before system reset we must
+ * combine 2 parts:
+ * 1. value of the current down counter
+ * 2. (number of counter expirations remaining) * (timeout/5)
+ */
+
+ /* Get the current number of counter expirations. Should be a
+ * value between 0 and 4
+ */
+ val = readl_relaxed(wdt->regs + WDTSR);
+ expiration = FIELD_GET(WDTSR_CURRENT_EXPIRATION_COUNT, val);
+
+ /* Get the current counter value in microsecond.
+ */
+ val = readl_relaxed(wdt->tmr->regs + TMRSR);
+ timeleft = FIELD_GET(TMRSR_PCV, val);
+
+ /*
+ * Calculate the time remaining by adding the time for the
+ * counter value to the time of the counter expirations that
+ * remain. Do the multiplication first on purpose just to keep
+ * the precision due to the integer division.
+ */
+ timeleft += wdt->base.timeout * (4 - expiration) / 5;
+ /*
+ * Convert the current counter value to seconds,
+ * rounding up to the nearest second.
+ */
+ timeleft = (timeleft + USEC_PER_SEC / 2) / USEC_PER_SEC;
+ return timeleft;
+}
+
static const struct watchdog_ops tegra186_wdt_ops = {
.owner = THIS_MODULE,
.start = tegra186_wdt_start,
.stop = tegra186_wdt_stop,
.ping = tegra186_wdt_ping,
.set_timeout = tegra186_wdt_set_timeout,
+ .get_timeleft = tegra186_wdt_get_timeleft,
};
static struct tegra186_wdt *tegra186_wdt_create(struct tegra186_timer *tegra,
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging 2025-04-17 9:28 [PATCH 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin @ 2025-04-17 9:28 ` Robert Lin 2025-04-17 9:28 ` [PATCH 3/3] clocksource/drivers/timer-tegra186: Remove unused bits Robert Lin ` (5 subsequent siblings) 6 siblings, 0 replies; 13+ messages in thread From: Robert Lin @ 2025-04-17 9:28 UTC (permalink / raw) To: thierry.reding, daniel.lezcano, jonathanh, tglx, pohsuns Cc: linux-kernel, linux-tegra, sumitg, Robert Lin From: Pohsun Su <pohsuns@nvidia.com> This change removes watchdog self-pinging behavior. The timer irq handler is triggered due to the 1st expiration, the handler disables and enables watchdog but also implicitly clears the expiration count so the count can only be 0 or 1. Since this watchdog supports opened, configured, or pinged by systemd, We remove this behavior or the watchdog may not bark when systemd crashes since the 5th expiration never comes. Signed-off-by: Pohsun Su <pohsuns@nvidia.com> Signed-off-by: Robert Lin <robelin@nvidia.com> --- drivers/clocksource/timer-tegra186.c | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c index 201b24ca59f4..708d9f8682ea 100644 --- a/drivers/clocksource/timer-tegra186.c +++ b/drivers/clocksource/timer-tegra186.c @@ -175,9 +175,6 @@ static void tegra186_wdt_enable(struct tegra186_wdt *wdt) value &= ~WDTCR_PERIOD_MASK; value |= WDTCR_PERIOD(1); - /* enable local interrupt for WDT petting */ - value |= WDTCR_LOCAL_INT_ENABLE; - /* enable local FIQ and remote interrupt for debug dump */ if (0) value |= WDTCR_REMOTE_INT_ENABLE | @@ -420,23 +417,10 @@ 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; - - if (watchdog_active(&tegra->wdt->base)) { - 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); @@ -455,8 +439,6 @@ static int tegra186_timer_probe(struct platform_device *pdev) if (err < 0) return err; - irq = err; - /* create a watchdog using a preconfigured timer */ tegra->wdt = tegra186_wdt_create(tegra, 0); if (IS_ERR(tegra->wdt)) { @@ -483,17 +465,8 @@ static int tegra186_timer_probe(struct platform_device *pdev) goto unregister_osc; } - 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); - goto unregister_usec; - } - return 0; -unregister_usec: - clocksource_unregister(&tegra->usec); unregister_osc: clocksource_unregister(&tegra->osc); unregister_tsc: -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] clocksource/drivers/timer-tegra186: Remove unused bits 2025-04-17 9:28 [PATCH 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin 2025-04-17 9:28 ` [PATCH 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging Robert Lin @ 2025-04-17 9:28 ` Robert Lin 2025-04-17 9:28 ` [PATCH v4 0/3] clocksource: fix Tegra234 SoC Watchdog Timer Robert Lin ` (4 subsequent siblings) 6 siblings, 0 replies; 13+ messages in thread From: Robert Lin @ 2025-04-17 9:28 UTC (permalink / raw) To: thierry.reding, daniel.lezcano, jonathanh, tglx, pohsuns Cc: linux-kernel, linux-tegra, sumitg, robelin From: robelin <robelin@nvidia.com> The intention to keep the unsed if(0) block is gone now. Remove them for clean codes. Signed-off-by: robelin <robelin@nvidia.com> --- drivers/clocksource/timer-tegra186.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c index 708d9f8682ea..a52b11b05934 100644 --- a/drivers/clocksource/timer-tegra186.c +++ b/drivers/clocksource/timer-tegra186.c @@ -175,15 +175,6 @@ static void tegra186_wdt_enable(struct tegra186_wdt *wdt) value &= ~WDTCR_PERIOD_MASK; value |= WDTCR_PERIOD(1); - /* enable local FIQ and remote interrupt for debug dump */ - if (0) - value |= WDTCR_REMOTE_INT_ENABLE | - WDTCR_LOCAL_FIQ_ENABLE; - - /* enable system debug reset (doesn't properly reboot) */ - if (0) - value |= WDTCR_SYSTEM_DEBUG_RESET_ENABLE; - /* enable system POR reset */ value |= WDTCR_SYSTEM_POR_RESET_ENABLE; -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 0/3] clocksource: fix Tegra234 SoC Watchdog Timer. 2025-04-17 9:28 [PATCH 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin 2025-04-17 9:28 ` [PATCH 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging Robert Lin 2025-04-17 9:28 ` [PATCH 3/3] clocksource/drivers/timer-tegra186: Remove unused bits Robert Lin @ 2025-04-17 9:28 ` Robert Lin 2025-04-17 9:37 ` Robert Lin 2025-04-17 9:28 ` [PATCH v4 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin ` (3 subsequent siblings) 6 siblings, 1 reply; 13+ messages in thread From: Robert Lin @ 2025-04-17 9:28 UTC (permalink / raw) To: thierry.reding, daniel.lezcano, jonathanh, tglx, pohsuns Cc: linux-kernel, linux-tegra, sumitg, robelin From: robelin <robelin@nvidia.com> This set of patches includes a fix for watchdog for it may not bark due to self-pinging and adds WDIOC_GETTIMELEFT support. -- V4: - Improve the precision of timeleft value V3: - Improve comment description - Refactor to fit codeline within 80 columns - Remove unused if(0) blocks V2: - Fix a compilation error, a warning and updates copyright -- Pohsun Su (2): clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support clocksource/drivers/timer-tegra186: fix watchdog self-pinging robelin (1): clocksource/drivers/timer-tegra186: Remove unused bits drivers/clocksource/timer-tegra186.c | 92 +++++++++++++++++----------- 1 file changed, 55 insertions(+), 37 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v4 0/3] clocksource: fix Tegra234 SoC Watchdog Timer. 2025-04-17 9:28 ` [PATCH v4 0/3] clocksource: fix Tegra234 SoC Watchdog Timer Robert Lin @ 2025-04-17 9:37 ` Robert Lin 0 siblings, 0 replies; 13+ messages in thread From: Robert Lin @ 2025-04-17 9:37 UTC (permalink / raw) To: thierry.reding@gmail.com, daniel.lezcano@linaro.org, Jon Hunter, tglx@linutronix.de, Pohsun Su Cc: linux-kernel@vger.kernel.org, linux-tegra@vger.kernel.org, Sumit Gupta > -----Original Message----- > From: Robert Lin <robelin@nvidia.com> > Sent: Thursday, April 17, 2025 5:28 PM > To: thierry.reding@gmail.com; daniel.lezcano@linaro.org; Jon Hunter > <jonathanh@nvidia.com>; tglx@linutronix.de; Pohsun Su > <pohsuns@nvidia.com> > Cc: linux-kernel@vger.kernel.org; linux-tegra@vger.kernel.org; Sumit Gupta > <sumitg@nvidia.com>; Robert Lin <robelin@nvidia.com> > Subject: [PATCH v4 0/3] clocksource: fix Tegra234 SoC Watchdog Timer. > > From: robelin <robelin@nvidia.com> > > This set of patches includes a fix for watchdog for it may not bark due to self- > pinging and adds WDIOC_GETTIMELEFT support. > > -- > V4: > - Improve the precision of timeleft value > > V3: > - Improve comment description > - Refactor to fit codeline within 80 columns > - Remove unused if(0) blocks > > > V2: > - Fix a compilation error, a warning and updates copyright > -- > > Pohsun Su (2): > clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support > clocksource/drivers/timer-tegra186: fix watchdog self-pinging > > robelin (1): > clocksource/drivers/timer-tegra186: Remove unused bits > > drivers/clocksource/timer-tegra186.c | 92 +++++++++++++++++----------- > 1 file changed, 55 insertions(+), 37 deletions(-) > > -- > 2.34.1 Sorry for the spam. My automation script has some issue to submit the wrong patch series. Please ignore these emails and move on to the next patch v4 series. All the best, Robert Lin ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support 2025-04-17 9:28 [PATCH 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin ` (2 preceding siblings ...) 2025-04-17 9:28 ` [PATCH v4 0/3] clocksource: fix Tegra234 SoC Watchdog Timer Robert Lin @ 2025-04-17 9:28 ` Robert Lin 2025-04-17 9:28 ` [PATCH v4 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging Robert Lin ` (2 subsequent siblings) 6 siblings, 0 replies; 13+ messages in thread From: Robert Lin @ 2025-04-17 9:28 UTC (permalink / raw) To: thierry.reding, daniel.lezcano, jonathanh, tglx, pohsuns Cc: linux-kernel, linux-tegra, sumitg, Robert Lin From: Pohsun Su <pohsuns@nvidia.com> This change adds support for WDIOC_GETTIMELEFT so userspace programs can get the number of seconds before system reset by the watchdog timer via ioctl. Signed-off-by: Pohsun Su <pohsuns@nvidia.com> Signed-off-by: Robert Lin <robelin@nvidia.com> --- drivers/clocksource/timer-tegra186.c | 56 +++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c index ea742889ee06..201b24ca59f4 100644 --- a/drivers/clocksource/timer-tegra186.c +++ b/drivers/clocksource/timer-tegra186.c @@ -1,8 +1,9 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2019-2020 NVIDIA Corporation. All rights reserved. + * Copyright (c) 2019-2025 NVIDIA Corporation. All rights reserved. */ +#include <linux/bitfield.h> #include <linux/clocksource.h> #include <linux/module.h> #include <linux/interrupt.h> @@ -30,6 +31,7 @@ #define TMRSR 0x004 #define TMRSR_INTR_CLR BIT(30) +#define TMRSR_PCV GENMASK(28, 0) #define TMRCSSR 0x008 #define TMRCSSR_SRC_USEC (0 << 0) @@ -46,6 +48,9 @@ #define WDTCR_TIMER_SOURCE_MASK 0xf #define WDTCR_TIMER_SOURCE(x) ((x) & 0xf) +#define WDTSR 0x004 +#define WDTSR_CURRENT_EXPIRATION_COUNT GENMASK(14, 12) + #define WDTCMDR 0x008 #define WDTCMDR_DISABLE_COUNTER BIT(1) #define WDTCMDR_START_COUNTER BIT(0) @@ -235,12 +240,61 @@ static int tegra186_wdt_set_timeout(struct watchdog_device *wdd, return 0; } +static unsigned int tegra186_wdt_get_timeleft(struct watchdog_device *wdd) +{ + struct tegra186_wdt *wdt = to_tegra186_wdt(wdd); + u32 timeleft, expiration, val; + + if (!watchdog_active(&wdt->base)) { + /* return zero if the watchdog timer is not activated. */ + return 0; + } + + /* + * Reset occurs on the fifth expiration of the + * watchdog timer and so when the watchdog timer is configured, + * the actual value programmed into the counter is 1/5 of the + * timeout value. Once the counter reaches 0, expiration count + * will be increased by 1 and the down counter restarts. + * Hence to get the time left before system reset we must + * combine 2 parts: + * 1. value of the current down counter + * 2. (number of counter expirations remaining) * (timeout/5) + */ + + /* Get the current number of counter expirations. Should be a + * value between 0 and 4 + */ + val = readl_relaxed(wdt->regs + WDTSR); + expiration = FIELD_GET(WDTSR_CURRENT_EXPIRATION_COUNT, val); + + /* Get the current counter value in microsecond. + */ + val = readl_relaxed(wdt->tmr->regs + TMRSR); + timeleft = FIELD_GET(TMRSR_PCV, val); + + /* + * Calculate the time remaining by adding the time for the + * counter value to the time of the counter expirations that + * remain. Do the multiplication first on purpose just to keep + * the precision due to the integer division. + */ + timeleft += wdt->base.timeout * (4 - expiration) / 5; + /* + * Convert the current counter value to seconds, + * rounding up to the nearest second. + */ + timeleft = (timeleft + USEC_PER_SEC / 2) / USEC_PER_SEC; + return timeleft; +} + static const struct watchdog_ops tegra186_wdt_ops = { .owner = THIS_MODULE, .start = tegra186_wdt_start, .stop = tegra186_wdt_stop, .ping = tegra186_wdt_ping, .set_timeout = tegra186_wdt_set_timeout, + .get_timeleft = tegra186_wdt_get_timeleft, }; static struct tegra186_wdt *tegra186_wdt_create(struct tegra186_timer *tegra, -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging 2025-04-17 9:28 [PATCH 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin ` (3 preceding siblings ...) 2025-04-17 9:28 ` [PATCH v4 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin @ 2025-04-17 9:28 ` Robert Lin 2025-04-17 9:28 ` [PATCH v4 3/3] clocksource/drivers/timer-tegra186: Remove unused bits Robert Lin 2025-04-17 9:43 ` [PATCH 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin 6 siblings, 0 replies; 13+ messages in thread From: Robert Lin @ 2025-04-17 9:28 UTC (permalink / raw) To: thierry.reding, daniel.lezcano, jonathanh, tglx, pohsuns Cc: linux-kernel, linux-tegra, sumitg, Robert Lin From: Pohsun Su <pohsuns@nvidia.com> This change removes watchdog self-pinging behavior. The timer irq handler is triggered due to the 1st expiration, the handler disables and enables watchdog but also implicitly clears the expiration count so the count can only be 0 or 1. Since this watchdog supports opened, configured, or pinged by systemd, We remove this behavior or the watchdog may not bark when systemd crashes since the 5th expiration never comes. Signed-off-by: Pohsun Su <pohsuns@nvidia.com> Signed-off-by: Robert Lin <robelin@nvidia.com> --- drivers/clocksource/timer-tegra186.c | 27 --------------------------- 1 file changed, 27 deletions(-) diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c index 201b24ca59f4..708d9f8682ea 100644 --- a/drivers/clocksource/timer-tegra186.c +++ b/drivers/clocksource/timer-tegra186.c @@ -175,9 +175,6 @@ static void tegra186_wdt_enable(struct tegra186_wdt *wdt) value &= ~WDTCR_PERIOD_MASK; value |= WDTCR_PERIOD(1); - /* enable local interrupt for WDT petting */ - value |= WDTCR_LOCAL_INT_ENABLE; - /* enable local FIQ and remote interrupt for debug dump */ if (0) value |= WDTCR_REMOTE_INT_ENABLE | @@ -420,23 +417,10 @@ 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; - - if (watchdog_active(&tegra->wdt->base)) { - 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); @@ -455,8 +439,6 @@ static int tegra186_timer_probe(struct platform_device *pdev) if (err < 0) return err; - irq = err; - /* create a watchdog using a preconfigured timer */ tegra->wdt = tegra186_wdt_create(tegra, 0); if (IS_ERR(tegra->wdt)) { @@ -483,17 +465,8 @@ static int tegra186_timer_probe(struct platform_device *pdev) goto unregister_osc; } - 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); - goto unregister_usec; - } - return 0; -unregister_usec: - clocksource_unregister(&tegra->usec); unregister_osc: clocksource_unregister(&tegra->osc); unregister_tsc: -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v4 3/3] clocksource/drivers/timer-tegra186: Remove unused bits 2025-04-17 9:28 [PATCH 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin ` (4 preceding siblings ...) 2025-04-17 9:28 ` [PATCH v4 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging Robert Lin @ 2025-04-17 9:28 ` Robert Lin 2025-04-17 9:43 ` [PATCH 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin 6 siblings, 0 replies; 13+ messages in thread From: Robert Lin @ 2025-04-17 9:28 UTC (permalink / raw) To: thierry.reding, daniel.lezcano, jonathanh, tglx, pohsuns Cc: linux-kernel, linux-tegra, sumitg, robelin From: robelin <robelin@nvidia.com> The intention to keep the unsed if(0) block is gone now. Remove them for clean codes. Signed-off-by: robelin <robelin@nvidia.com> --- drivers/clocksource/timer-tegra186.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c index 708d9f8682ea..a52b11b05934 100644 --- a/drivers/clocksource/timer-tegra186.c +++ b/drivers/clocksource/timer-tegra186.c @@ -175,15 +175,6 @@ static void tegra186_wdt_enable(struct tegra186_wdt *wdt) value &= ~WDTCR_PERIOD_MASK; value |= WDTCR_PERIOD(1); - /* enable local FIQ and remote interrupt for debug dump */ - if (0) - value |= WDTCR_REMOTE_INT_ENABLE | - WDTCR_LOCAL_FIQ_ENABLE; - - /* enable system debug reset (doesn't properly reboot) */ - if (0) - value |= WDTCR_SYSTEM_DEBUG_RESET_ENABLE; - /* enable system POR reset */ value |= WDTCR_SYSTEM_POR_RESET_ENABLE; -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* RE: [PATCH 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support 2025-04-17 9:28 [PATCH 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin ` (5 preceding siblings ...) 2025-04-17 9:28 ` [PATCH v4 3/3] clocksource/drivers/timer-tegra186: Remove unused bits Robert Lin @ 2025-04-17 9:43 ` Robert Lin 6 siblings, 0 replies; 13+ messages in thread From: Robert Lin @ 2025-04-17 9:43 UTC (permalink / raw) To: thierry.reding@gmail.com, daniel.lezcano@linaro.org, Jon Hunter, tglx@linutronix.de, Pohsun Su Cc: linux-kernel@vger.kernel.org, linux-tegra@vger.kernel.org, Sumit Gupta > -----Original Message----- > From: Robert Lin <robelin@nvidia.com> > Sent: Thursday, April 17, 2025 5:28 PM > To: thierry.reding@gmail.com; daniel.lezcano@linaro.org; Jon Hunter > <jonathanh@nvidia.com>; tglx@linutronix.de; Pohsun Su > <pohsuns@nvidia.com> > Cc: linux-kernel@vger.kernel.org; linux-tegra@vger.kernel.org; Sumit Gupta > <sumitg@nvidia.com>; Robert Lin <robelin@nvidia.com> > Subject: [PATCH 1/3] clocksource/drivers/timer-tegra186: add > WDIOC_GETTIMELEFT support > > From: Pohsun Su <pohsuns@nvidia.com> > > This change adds support for WDIOC_GETTIMELEFT so userspace programs > can get the number of seconds before system reset by the watchdog timer via > ioctl. > > Signed-off-by: Pohsun Su <pohsuns@nvidia.com> > Signed-off-by: Robert Lin <robelin@nvidia.com> > --- > drivers/clocksource/timer-tegra186.c | 56 > +++++++++++++++++++++++++++- > 1 file changed, 55 insertions(+), 1 deletion(-) > > diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer- > tegra186.c > index ea742889ee06..201b24ca59f4 100644 > --- a/drivers/clocksource/timer-tegra186.c > +++ b/drivers/clocksource/timer-tegra186.c > @@ -1,8 +1,9 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* > - * Copyright (c) 2019-2020 NVIDIA Corporation. All rights reserved. > + * Copyright (c) 2019-2025 NVIDIA Corporation. All rights reserved. > */ > > +#include <linux/bitfield.h> > #include <linux/clocksource.h> > #include <linux/module.h> > #include <linux/interrupt.h> > @@ -30,6 +31,7 @@ > > #define TMRSR 0x004 > #define TMRSR_INTR_CLR BIT(30) > +#define TMRSR_PCV GENMASK(28, 0) > > #define TMRCSSR 0x008 > #define TMRCSSR_SRC_USEC (0 << 0) > @@ -46,6 +48,9 @@ > #define WDTCR_TIMER_SOURCE_MASK 0xf > #define WDTCR_TIMER_SOURCE(x) ((x) & 0xf) > > +#define WDTSR 0x004 > +#define WDTSR_CURRENT_EXPIRATION_COUNT GENMASK(14, 12) > + > #define WDTCMDR 0x008 > #define WDTCMDR_DISABLE_COUNTER BIT(1) #define > WDTCMDR_START_COUNTER BIT(0) @@ -235,12 +240,61 @@ static int > tegra186_wdt_set_timeout(struct watchdog_device *wdd, > return 0; > } > > +static unsigned int tegra186_wdt_get_timeleft(struct watchdog_device > +*wdd) { > + struct tegra186_wdt *wdt = to_tegra186_wdt(wdd); > + u32 timeleft, expiration, val; > + > + if (!watchdog_active(&wdt->base)) { > + /* return zero if the watchdog timer is not activated. */ > + return 0; > + } > + > + /* > + * Reset occurs on the fifth expiration of the > + * watchdog timer and so when the watchdog timer is configured, > + * the actual value programmed into the counter is 1/5 of the > + * timeout value. Once the counter reaches 0, expiration count > + * will be increased by 1 and the down counter restarts. > + * Hence to get the time left before system reset we must > + * combine 2 parts: > + * 1. value of the current down counter > + * 2. (number of counter expirations remaining) * (timeout/5) > + */ > + > + /* Get the current number of counter expirations. Should be a > + * value between 0 and 4 > + */ > + val = readl_relaxed(wdt->regs + WDTSR); > + expiration = FIELD_GET(WDTSR_CURRENT_EXPIRATION_COUNT, val); > + > + /* Get the current counter value in microsecond. > + */ > + val = readl_relaxed(wdt->tmr->regs + TMRSR); > + timeleft = FIELD_GET(TMRSR_PCV, val); > + > + /* > + * Calculate the time remaining by adding the time for the > + * counter value to the time of the counter expirations that > + * remain. Do the multiplication first on purpose just to keep > + * the precision due to the integer division. > + */ > + timeleft += wdt->base.timeout * (4 - expiration) / 5; > + /* > + * Convert the current counter value to seconds, > + * rounding up to the nearest second. > + */ > + timeleft = (timeleft + USEC_PER_SEC / 2) / USEC_PER_SEC; > + return timeleft; > +} > + > static const struct watchdog_ops tegra186_wdt_ops = { > .owner = THIS_MODULE, > .start = tegra186_wdt_start, > .stop = tegra186_wdt_stop, > .ping = tegra186_wdt_ping, > .set_timeout = tegra186_wdt_set_timeout, > + .get_timeleft = tegra186_wdt_get_timeleft, > }; > > static struct tegra186_wdt *tegra186_wdt_create(struct tegra186_timer > *tegra, > -- > 2.34.1 Sorry for the spam. My automation script has some issue to submit the wrong patch series. Please ignore these emails and move on to the next patch v4 series: https://lore.kernel.org/lkml/20250417093110.2751877-1-robelin@nvidia.com/T/#t Let me know if this cause trouble and I'll need to submit a V5 patch instead, I apology for the mess. All the best, Robert Lin ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 0/3] clocksource: fix Tegra234 SoC Watchdog Timer. @ 2025-04-17 9:31 Robert Lin 2025-04-17 9:31 ` [PATCH v4 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin 0 siblings, 1 reply; 13+ messages in thread From: Robert Lin @ 2025-04-17 9:31 UTC (permalink / raw) To: thierry.reding, daniel.lezcano, jonathanh, tglx, pohsuns Cc: linux-kernel, linux-tegra, sumitg, robelin From: robelin <robelin@nvidia.com> This set of patches includes a fix for watchdog for it may not bark due to self-pinging and adds WDIOC_GETTIMELEFT support. -- V4: - Improve the precision of timeleft value V3: - Improve comment description - Refactor to fit codeline within 80 columns - Remove unused if(0) blocks V2: - Fix a compilation error, a warning and updates copyright -- Pohsun Su (2): clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support clocksource/drivers/timer-tegra186: fix watchdog self-pinging robelin (1): clocksource/drivers/timer-tegra186: Remove unused bits drivers/clocksource/timer-tegra186.c | 92 +++++++++++++++++----------- 1 file changed, 55 insertions(+), 37 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support 2025-04-17 9:31 [PATCH v4 0/3] clocksource: fix Tegra234 SoC Watchdog Timer Robert Lin @ 2025-04-17 9:31 ` Robert Lin 2025-04-17 11:09 ` Jon Hunter 0 siblings, 1 reply; 13+ messages in thread From: Robert Lin @ 2025-04-17 9:31 UTC (permalink / raw) To: thierry.reding, daniel.lezcano, jonathanh, tglx, pohsuns Cc: linux-kernel, linux-tegra, sumitg, Robert Lin From: Pohsun Su <pohsuns@nvidia.com> This change adds support for WDIOC_GETTIMELEFT so userspace programs can get the number of seconds before system reset by the watchdog timer via ioctl. Signed-off-by: Pohsun Su <pohsuns@nvidia.com> Signed-off-by: Robert Lin <robelin@nvidia.com> --- drivers/clocksource/timer-tegra186.c | 56 +++++++++++++++++++++++++++- 1 file changed, 55 insertions(+), 1 deletion(-) diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c index ea742889ee06..201b24ca59f4 100644 --- a/drivers/clocksource/timer-tegra186.c +++ b/drivers/clocksource/timer-tegra186.c @@ -1,8 +1,9 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2019-2020 NVIDIA Corporation. All rights reserved. + * Copyright (c) 2019-2025 NVIDIA Corporation. All rights reserved. */ +#include <linux/bitfield.h> #include <linux/clocksource.h> #include <linux/module.h> #include <linux/interrupt.h> @@ -30,6 +31,7 @@ #define TMRSR 0x004 #define TMRSR_INTR_CLR BIT(30) +#define TMRSR_PCV GENMASK(28, 0) #define TMRCSSR 0x008 #define TMRCSSR_SRC_USEC (0 << 0) @@ -46,6 +48,9 @@ #define WDTCR_TIMER_SOURCE_MASK 0xf #define WDTCR_TIMER_SOURCE(x) ((x) & 0xf) +#define WDTSR 0x004 +#define WDTSR_CURRENT_EXPIRATION_COUNT GENMASK(14, 12) + #define WDTCMDR 0x008 #define WDTCMDR_DISABLE_COUNTER BIT(1) #define WDTCMDR_START_COUNTER BIT(0) @@ -235,12 +240,61 @@ static int tegra186_wdt_set_timeout(struct watchdog_device *wdd, return 0; } +static unsigned int tegra186_wdt_get_timeleft(struct watchdog_device *wdd) +{ + struct tegra186_wdt *wdt = to_tegra186_wdt(wdd); + u32 timeleft, expiration, val; + + if (!watchdog_active(&wdt->base)) { + /* return zero if the watchdog timer is not activated. */ + return 0; + } + + /* + * Reset occurs on the fifth expiration of the + * watchdog timer and so when the watchdog timer is configured, + * the actual value programmed into the counter is 1/5 of the + * timeout value. Once the counter reaches 0, expiration count + * will be increased by 1 and the down counter restarts. + * Hence to get the time left before system reset we must + * combine 2 parts: + * 1. value of the current down counter + * 2. (number of counter expirations remaining) * (timeout/5) + */ + + /* Get the current number of counter expirations. Should be a + * value between 0 and 4 + */ + val = readl_relaxed(wdt->regs + WDTSR); + expiration = FIELD_GET(WDTSR_CURRENT_EXPIRATION_COUNT, val); + + /* Get the current counter value in microsecond. + */ + val = readl_relaxed(wdt->tmr->regs + TMRSR); + timeleft = FIELD_GET(TMRSR_PCV, val); + + /* + * Calculate the time remaining by adding the time for the + * counter value to the time of the counter expirations that + * remain. Do the multiplication first on purpose just to keep + * the precision due to the integer division. + */ + timeleft += wdt->base.timeout * (4 - expiration) / 5; + /* + * Convert the current counter value to seconds, + * rounding up to the nearest second. + */ + timeleft = (timeleft + USEC_PER_SEC / 2) / USEC_PER_SEC; + return timeleft; +} + static const struct watchdog_ops tegra186_wdt_ops = { .owner = THIS_MODULE, .start = tegra186_wdt_start, .stop = tegra186_wdt_stop, .ping = tegra186_wdt_ping, .set_timeout = tegra186_wdt_set_timeout, + .get_timeleft = tegra186_wdt_get_timeleft, }; static struct tegra186_wdt *tegra186_wdt_create(struct tegra186_timer *tegra, -- 2.34.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support 2025-04-17 9:31 ` [PATCH v4 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin @ 2025-04-17 11:09 ` Jon Hunter 2025-04-17 11:34 ` Robert Lin 0 siblings, 1 reply; 13+ messages in thread From: Jon Hunter @ 2025-04-17 11:09 UTC (permalink / raw) To: Robert Lin, thierry.reding, daniel.lezcano, tglx, pohsuns Cc: linux-kernel, linux-tegra, sumitg On 17/04/2025 10:31, Robert Lin wrote: > From: Pohsun Su <pohsuns@nvidia.com> > > This change adds support for WDIOC_GETTIMELEFT so userspace > programs can get the number of seconds before system reset by > the watchdog timer via ioctl. > > Signed-off-by: Pohsun Su <pohsuns@nvidia.com> > Signed-off-by: Robert Lin <robelin@nvidia.com> > --- > drivers/clocksource/timer-tegra186.c | 56 +++++++++++++++++++++++++++- > 1 file changed, 55 insertions(+), 1 deletion(-) > > diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c > index ea742889ee06..201b24ca59f4 100644 > --- a/drivers/clocksource/timer-tegra186.c > +++ b/drivers/clocksource/timer-tegra186.c > @@ -1,8 +1,9 @@ > // SPDX-License-Identifier: GPL-2.0-only > /* > - * Copyright (c) 2019-2020 NVIDIA Corporation. All rights reserved. > + * Copyright (c) 2019-2025 NVIDIA Corporation. All rights reserved. > */ > > +#include <linux/bitfield.h> > #include <linux/clocksource.h> > #include <linux/module.h> > #include <linux/interrupt.h> > @@ -30,6 +31,7 @@ > > #define TMRSR 0x004 > #define TMRSR_INTR_CLR BIT(30) > +#define TMRSR_PCV GENMASK(28, 0) > > #define TMRCSSR 0x008 > #define TMRCSSR_SRC_USEC (0 << 0) > @@ -46,6 +48,9 @@ > #define WDTCR_TIMER_SOURCE_MASK 0xf > #define WDTCR_TIMER_SOURCE(x) ((x) & 0xf) > > +#define WDTSR 0x004 > +#define WDTSR_CURRENT_EXPIRATION_COUNT GENMASK(14, 12) > + > #define WDTCMDR 0x008 > #define WDTCMDR_DISABLE_COUNTER BIT(1) > #define WDTCMDR_START_COUNTER BIT(0) > @@ -235,12 +240,61 @@ static int tegra186_wdt_set_timeout(struct watchdog_device *wdd, > return 0; > } > > +static unsigned int tegra186_wdt_get_timeleft(struct watchdog_device *wdd) > +{ > + struct tegra186_wdt *wdt = to_tegra186_wdt(wdd); > + u32 timeleft, expiration, val; > + > + if (!watchdog_active(&wdt->base)) { > + /* return zero if the watchdog timer is not activated. */ > + return 0; > + } > + > + /* > + * Reset occurs on the fifth expiration of the > + * watchdog timer and so when the watchdog timer is configured, > + * the actual value programmed into the counter is 1/5 of the > + * timeout value. Once the counter reaches 0, expiration count > + * will be increased by 1 and the down counter restarts. > + * Hence to get the time left before system reset we must > + * combine 2 parts: > + * 1. value of the current down counter > + * 2. (number of counter expirations remaining) * (timeout/5) > + */ > + > + /* Get the current number of counter expirations. Should be a > + * value between 0 and 4 > + */ > + val = readl_relaxed(wdt->regs + WDTSR); > + expiration = FIELD_GET(WDTSR_CURRENT_EXPIRATION_COUNT, val); The above says 'should be between 0 and 4', however, we never check. I am wondering if we should ... if (WARN_ON(expiration > 4) expiration = 4; To avoid any overflow later on. > + > + /* Get the current counter value in microsecond. > + */ > + val = readl_relaxed(wdt->tmr->regs + TMRSR); > + timeleft = FIELD_GET(TMRSR_PCV, val); > + > + /* > + * Calculate the time remaining by adding the time for the > + * counter value to the time of the counter expirations that > + * remain. Do the multiplication first on purpose just to keep > + * the precision due to the integer division. > + */ > + timeleft += wdt->base.timeout * (4 - expiration) / 5; > + /* > + * Convert the current counter value to seconds, > + * rounding up to the nearest second. > + */ > + timeleft = (timeleft + USEC_PER_SEC / 2) / USEC_PER_SEC; > + return timeleft; > +} > + > static const struct watchdog_ops tegra186_wdt_ops = { > .owner = THIS_MODULE, > .start = tegra186_wdt_start, > .stop = tegra186_wdt_stop, > .ping = tegra186_wdt_ping, > .set_timeout = tegra186_wdt_set_timeout, > + .get_timeleft = tegra186_wdt_get_timeleft, > }; > > static struct tegra186_wdt *tegra186_wdt_create(struct tegra186_timer *tegra, -- nvpublic ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v4 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support 2025-04-17 11:09 ` Jon Hunter @ 2025-04-17 11:34 ` Robert Lin 2025-04-17 13:13 ` Jon Hunter 0 siblings, 1 reply; 13+ messages in thread From: Robert Lin @ 2025-04-17 11:34 UTC (permalink / raw) To: Jon Hunter, thierry.reding@gmail.com, daniel.lezcano@linaro.org, tglx@linutronix.de, Pohsun Su Cc: linux-kernel@vger.kernel.org, linux-tegra@vger.kernel.org, Sumit Gupta > -----Original Message----- > From: Jon Hunter <jonathanh@nvidia.com> > Sent: Thursday, April 17, 2025 7:10 PM > To: Robert Lin <robelin@nvidia.com>; thierry.reding@gmail.com; > daniel.lezcano@linaro.org; tglx@linutronix.de; Pohsun Su > <pohsuns@nvidia.com> > Cc: linux-kernel@vger.kernel.org; linux-tegra@vger.kernel.org; Sumit Gupta > <sumitg@nvidia.com> > Subject: Re: [PATCH v4 1/3] clocksource/drivers/timer-tegra186: add > WDIOC_GETTIMELEFT support > > > On 17/04/2025 10:31, Robert Lin wrote: > > From: Pohsun Su <pohsuns@nvidia.com> > > > > This change adds support for WDIOC_GETTIMELEFT so userspace programs > > can get the number of seconds before system reset by the watchdog > > timer via ioctl. > > > > Signed-off-by: Pohsun Su <pohsuns@nvidia.com> > > Signed-off-by: Robert Lin <robelin@nvidia.com> > > --- > > drivers/clocksource/timer-tegra186.c | 56 > +++++++++++++++++++++++++++- > > 1 file changed, 55 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/clocksource/timer-tegra186.c > > b/drivers/clocksource/timer-tegra186.c > > index ea742889ee06..201b24ca59f4 100644 > > --- a/drivers/clocksource/timer-tegra186.c > > +++ b/drivers/clocksource/timer-tegra186.c > > @@ -1,8 +1,9 @@ > > // SPDX-License-Identifier: GPL-2.0-only > > /* > > - * Copyright (c) 2019-2020 NVIDIA Corporation. All rights reserved. > > + * Copyright (c) 2019-2025 NVIDIA Corporation. All rights reserved. > > */ > > > > +#include <linux/bitfield.h> > > #include <linux/clocksource.h> > > #include <linux/module.h> > > #include <linux/interrupt.h> > > @@ -30,6 +31,7 @@ > > > > #define TMRSR 0x004 > > #define TMRSR_INTR_CLR BIT(30) > > +#define TMRSR_PCV GENMASK(28, 0) > > > > #define TMRCSSR 0x008 > > #define TMRCSSR_SRC_USEC (0 << 0) > > @@ -46,6 +48,9 @@ > > #define WDTCR_TIMER_SOURCE_MASK 0xf > > #define WDTCR_TIMER_SOURCE(x) ((x) & 0xf) > > > > +#define WDTSR 0x004 > > +#define WDTSR_CURRENT_EXPIRATION_COUNT GENMASK(14, 12) > > + > > #define WDTCMDR 0x008 > > #define WDTCMDR_DISABLE_COUNTER BIT(1) > > #define WDTCMDR_START_COUNTER BIT(0) @@ -235,12 +240,61 @@ > static > > int tegra186_wdt_set_timeout(struct watchdog_device *wdd, > > return 0; > > } > > > > +static unsigned int tegra186_wdt_get_timeleft(struct watchdog_device > > +*wdd) { > > + struct tegra186_wdt *wdt = to_tegra186_wdt(wdd); > > + u32 timeleft, expiration, val; > > + > > + if (!watchdog_active(&wdt->base)) { > > + /* return zero if the watchdog timer is not activated. */ > > + return 0; > > + } > > + > > + /* > > + * Reset occurs on the fifth expiration of the > > + * watchdog timer and so when the watchdog timer is configured, > > + * the actual value programmed into the counter is 1/5 of the > > + * timeout value. Once the counter reaches 0, expiration count > > + * will be increased by 1 and the down counter restarts. > > + * Hence to get the time left before system reset we must > > + * combine 2 parts: > > + * 1. value of the current down counter > > + * 2. (number of counter expirations remaining) * (timeout/5) > > + */ > > + > > + /* Get the current number of counter expirations. Should be a > > + * value between 0 and 4 > > + */ > > + val = readl_relaxed(wdt->regs + WDTSR); > > + expiration = FIELD_GET(WDTSR_CURRENT_EXPIRATION_COUNT, val); > > The above says 'should be between 0 and 4', however, we never check. > > I am wondering if we should ... > > if (WARN_ON(expiration > 4) > expiration = 4; > > To avoid any overflow later on. > Warning for the bad value seems to be good. But for the part to forcibly bound the value to 4, I'm not sure if this makes sense. Using the bad value from WDTSR or 4 both lead to wrong timeleft value at the end. > > -- > nvpublic ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support 2025-04-17 11:34 ` Robert Lin @ 2025-04-17 13:13 ` Jon Hunter 0 siblings, 0 replies; 13+ messages in thread From: Jon Hunter @ 2025-04-17 13:13 UTC (permalink / raw) To: Robert Lin, thierry.reding@gmail.com, daniel.lezcano@linaro.org, tglx@linutronix.de, Pohsun Su Cc: linux-kernel@vger.kernel.org, linux-tegra@vger.kernel.org, Sumit Gupta On 17/04/2025 12:34, Robert Lin wrote: > > >> -----Original Message----- >> From: Jon Hunter <jonathanh@nvidia.com> >> Sent: Thursday, April 17, 2025 7:10 PM >> To: Robert Lin <robelin@nvidia.com>; thierry.reding@gmail.com; >> daniel.lezcano@linaro.org; tglx@linutronix.de; Pohsun Su >> <pohsuns@nvidia.com> >> Cc: linux-kernel@vger.kernel.org; linux-tegra@vger.kernel.org; Sumit Gupta >> <sumitg@nvidia.com> >> Subject: Re: [PATCH v4 1/3] clocksource/drivers/timer-tegra186: add >> WDIOC_GETTIMELEFT support >> >> >> On 17/04/2025 10:31, Robert Lin wrote: >>> From: Pohsun Su <pohsuns@nvidia.com> >>> >>> This change adds support for WDIOC_GETTIMELEFT so userspace programs >>> can get the number of seconds before system reset by the watchdog >>> timer via ioctl. >>> >>> Signed-off-by: Pohsun Su <pohsuns@nvidia.com> >>> Signed-off-by: Robert Lin <robelin@nvidia.com> >>> --- >>> drivers/clocksource/timer-tegra186.c | 56 >> +++++++++++++++++++++++++++- >>> 1 file changed, 55 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/clocksource/timer-tegra186.c >>> b/drivers/clocksource/timer-tegra186.c >>> index ea742889ee06..201b24ca59f4 100644 >>> --- a/drivers/clocksource/timer-tegra186.c >>> +++ b/drivers/clocksource/timer-tegra186.c >>> @@ -1,8 +1,9 @@ >>> // SPDX-License-Identifier: GPL-2.0-only >>> /* >>> - * Copyright (c) 2019-2020 NVIDIA Corporation. All rights reserved. >>> + * Copyright (c) 2019-2025 NVIDIA Corporation. All rights reserved. >>> */ >>> >>> +#include <linux/bitfield.h> >>> #include <linux/clocksource.h> >>> #include <linux/module.h> >>> #include <linux/interrupt.h> >>> @@ -30,6 +31,7 @@ >>> >>> #define TMRSR 0x004 >>> #define TMRSR_INTR_CLR BIT(30) >>> +#define TMRSR_PCV GENMASK(28, 0) >>> >>> #define TMRCSSR 0x008 >>> #define TMRCSSR_SRC_USEC (0 << 0) >>> @@ -46,6 +48,9 @@ >>> #define WDTCR_TIMER_SOURCE_MASK 0xf >>> #define WDTCR_TIMER_SOURCE(x) ((x) & 0xf) >>> >>> +#define WDTSR 0x004 >>> +#define WDTSR_CURRENT_EXPIRATION_COUNT GENMASK(14, 12) >>> + >>> #define WDTCMDR 0x008 >>> #define WDTCMDR_DISABLE_COUNTER BIT(1) >>> #define WDTCMDR_START_COUNTER BIT(0) @@ -235,12 +240,61 @@ >> static >>> int tegra186_wdt_set_timeout(struct watchdog_device *wdd, >>> return 0; >>> } >>> >>> +static unsigned int tegra186_wdt_get_timeleft(struct watchdog_device >>> +*wdd) { >>> + struct tegra186_wdt *wdt = to_tegra186_wdt(wdd); >>> + u32 timeleft, expiration, val; >>> + >>> + if (!watchdog_active(&wdt->base)) { >>> + /* return zero if the watchdog timer is not activated. */ >>> + return 0; >>> + } >>> + >>> + /* >>> + * Reset occurs on the fifth expiration of the >>> + * watchdog timer and so when the watchdog timer is configured, >>> + * the actual value programmed into the counter is 1/5 of the >>> + * timeout value. Once the counter reaches 0, expiration count >>> + * will be increased by 1 and the down counter restarts. >>> + * Hence to get the time left before system reset we must >>> + * combine 2 parts: >>> + * 1. value of the current down counter >>> + * 2. (number of counter expirations remaining) * (timeout/5) >>> + */ >>> + >>> + /* Get the current number of counter expirations. Should be a >>> + * value between 0 and 4 >>> + */ >>> + val = readl_relaxed(wdt->regs + WDTSR); >>> + expiration = FIELD_GET(WDTSR_CURRENT_EXPIRATION_COUNT, val); >> >> The above says 'should be between 0 and 4', however, we never check. >> >> I am wondering if we should ... >> >> if (WARN_ON(expiration > 4) >> expiration = 4; >> >> To avoid any overflow later on. >> > > Warning for the bad value seems to be good. But for the part to forcibly bound the value to 4, I'm not sure if this makes sense. Using the bad value from WDTSR or 4 both lead to wrong timeleft value at the end. I was looking at the code, and if it is 4, then the following ... timeleft += wdt->base.timeout * (4 - expiration) / 5; ... becomes 0. However, given that in this case something very bad has happened, then the other alternative may be to just ... if (WARN_ON(expiration > 4) return 0; Jon -- nvpublic ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-04-17 13:13 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-04-17 9:28 [PATCH 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin 2025-04-17 9:28 ` [PATCH 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging Robert Lin 2025-04-17 9:28 ` [PATCH 3/3] clocksource/drivers/timer-tegra186: Remove unused bits Robert Lin 2025-04-17 9:28 ` [PATCH v4 0/3] clocksource: fix Tegra234 SoC Watchdog Timer Robert Lin 2025-04-17 9:37 ` Robert Lin 2025-04-17 9:28 ` [PATCH v4 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin 2025-04-17 9:28 ` [PATCH v4 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging Robert Lin 2025-04-17 9:28 ` [PATCH v4 3/3] clocksource/drivers/timer-tegra186: Remove unused bits Robert Lin 2025-04-17 9:43 ` [PATCH 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin -- strict thread matches above, loose matches on Subject: below -- 2025-04-17 9:31 [PATCH v4 0/3] clocksource: fix Tegra234 SoC Watchdog Timer Robert Lin 2025-04-17 9:31 ` [PATCH v4 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin 2025-04-17 11:09 ` Jon Hunter 2025-04-17 11:34 ` Robert Lin 2025-04-17 13:13 ` Jon Hunter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox