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 | 58 +++++++++++++++++++++++++++-
1 file changed, 57 insertions(+), 1 deletion(-)
diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
index ea742889ee06..56d08bf1b6b0 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,63 @@ 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);
+ if (WARN_ON(expiration > 4))
+ return 0;
+
+ /* 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.1On Mon, Apr 21, 2025 at 06:08:19PM +0800, 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>
> ---
Hi Robert,
I realize that this driver should be split in two and the watchdog part go
under drivers/watchdog.
> drivers/clocksource/timer-tegra186.c | 58 +++++++++++++++++++++++++++-
> 1 file changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
> index ea742889ee06..56d08bf1b6b0 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,63 @@ 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);
> + if (WARN_ON(expiration > 4))
> + return 0;
> +
> + /* 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
>
--
<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
Hi Daniel, On 29/04/2025 09:59, Daniel Lezcano wrote: > On Mon, Apr 21, 2025 at 06:08:19PM +0800, 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> >> --- > > Hi Robert, > > I realize that this driver should be split in two and the watchdog part go > under drivers/watchdog. Are there any other examples you know of where the timer is split in this way? It is not clear to me how you propose we do this? BTW, for this series, I just want to get these updates merged. Any other re-factoring we can handle later. Cheers! Jon -- nvpublic
On 29/04/2025 11:15, Jon Hunter wrote: > Hi Daniel, > > On 29/04/2025 09:59, Daniel Lezcano wrote: >> On Mon, Apr 21, 2025 at 06:08:19PM +0800, 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> >>> --- >> >> Hi Robert, >> >> I realize that this driver should be split in two and the watchdog >> part go >> under drivers/watchdog. > > Are there any other examples you know of where the timer is split in > this way? It is not clear to me how you propose we do this? Just keep the clocksource and move the watchdog code (everything related to the watchdog_ops) to a new driver under drivers/watchdog BTW, there are three clocksources with the same rating, what is the point of having them supported ? Is it not the architected clocksource enough ? May be the clocksource can be removed and the driver remains a pure watchdog driver ? > BTW, for this series, I just want to get these updates merged. Any other > re-factoring we can handle later. > > Cheers! > Jon > -- <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
On Tue, Apr 29, 2025 at 03:19:25PM +0200, Daniel Lezcano wrote: > On 29/04/2025 11:15, Jon Hunter wrote: > > Hi Daniel, > > > > On 29/04/2025 09:59, Daniel Lezcano wrote: > > > On Mon, Apr 21, 2025 at 06:08:19PM +0800, 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> > > > > --- > > > > > > Hi Robert, > > > > > > I realize that this driver should be split in two and the watchdog > > > part go > > > under drivers/watchdog. > > > > Are there any other examples you know of where the timer is split in > > this way? It is not clear to me how you propose we do this? > > Just keep the clocksource and move the watchdog code (everything related to > the watchdog_ops) to a new driver under drivers/watchdog That's a bad idea. This is all a single register space, so we can't have "proper" drivers (i.e. ones that exclusively request I/O memory regions) if we split them up. I understand that it's nice and easy to have things split up along subsystem boundaries, but sometimes hardware designs just aren't that cleanly separated. > BTW, there are three clocksources with the same rating, what is the point of > having them supported ? > > Is it not the architected clocksource enough ? The TSC clock source that this driver exposes is different from the architected timer. It's a SoC-wide clock that is routed to various IP blocks and used for timestamping events. This clocksource allows these events to be properly compared, etc. Thierry
On Tue, Apr 29, 2025 at 04:23:22PM +0200, Thierry Reding wrote: > On Tue, Apr 29, 2025 at 03:19:25PM +0200, Daniel Lezcano wrote: > > On 29/04/2025 11:15, Jon Hunter wrote: > > > Hi Daniel, > > > > > > On 29/04/2025 09:59, Daniel Lezcano wrote: > > > > On Mon, Apr 21, 2025 at 06:08:19PM +0800, 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> > > > > > --- > > > > > > > > Hi Robert, > > > > > > > > I realize that this driver should be split in two and the watchdog > > > > part go > > > > under drivers/watchdog. > > > > > > Are there any other examples you know of where the timer is split in > > > this way? It is not clear to me how you propose we do this? > > > > Just keep the clocksource and move the watchdog code (everything related to > > the watchdog_ops) to a new driver under drivers/watchdog > > That's a bad idea. This is all a single register space, so we can't have > "proper" drivers (i.e. ones that exclusively request I/O memory regions) > if we split them up. > > I understand that it's nice and easy to have things split up along > subsystem boundaries, but sometimes hardware designs just aren't that > cleanly separated. Yes, that's true. The driver has a lot of watchdog code inside and I think it is possible to move most part of it under drivers/watchdog. Perhaps by exporting tegra186_wdt_disable() / tegra186_wdt_enable(). Anyway, I understand that is an important change and I don't want to block this series for this reason. At the first glance, these changes seem to be fine for me, I'll just do a last review and comment/pick them. > > BTW, there are three clocksources with the same rating, what is the point of > > having them supported ? > > > > Is it not the architected clocksource enough ? > > The TSC clock source that this driver exposes is different from the > architected timer. It's a SoC-wide clock that is routed to various IP > blocks and used for timestamping events. This clocksource allows these > events to be properly compared, etc. I see, thanks for clarifying -- Daniel -- <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
Hi Robert,
On 21/04/2025 11:08, 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 | 58 +++++++++++++++++++++++++++-
> 1 file changed, 57 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
> index ea742889ee06..56d08bf1b6b0 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,63 @@ 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);
> + if (WARN_ON(expiration > 4))
> + return 0;
> +
> + /* Get the current counter value in microsecond.
> + */
> + val = readl_relaxed(wdt->tmr->regs + TMRSR);
> + timeleft = FIELD_GET(TMRSR_PCV, val);
So this value is in microseconds.
> +
> + /*
> + * 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;
However, wdt->base.timeout is in seconds. So I don't think we can simply
add this. Don't we need to ...
timeleft += (wdt->base.timeout * USEC_PER_SEC * (4 - expiration)) / 5;
Given that this could be quite a big number, we probably want to make
timeleft a 64-bit type too. So we may want to define a 'u64
timeleft_usecs' and 'u32 timeleft_secs' that we return.
Jon
--
nvpublic
© 2016 - 2026 Red Hat, Inc.