Re: [PATCH v3 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support

Thierry Reding
2025-04-10T10:09:14Z

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

On Mon, Apr 07, 2025 at 06:23:21PM +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>
> ---
> drivers/clocksource/timer-tegra186.c | 53 +++++++++++++++++++++++++++-
> 1 file changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
> index ea742889ee06..fc2ca86c1436 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,58 @@ 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);
> +
> + /* Convert the current counter value to seconds, rounding up
> + * to the nearest second.
> + */
> + val = readl_relaxed(wdt->tmr->regs + TMRSR);
> + timeleft = FIELD_GET(TMRSR_PCV, val);
> + timeleft = (timeleft + USEC_PER_SEC / 2) / USEC_PER_SEC;
> +
> + /*
> + * 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;

I'm a little worried that we might be getting a very skewed value here
due to the multiple rounds of rounding. Can we not compute timeleft in
microseconds until the very end and only convert to seconds then?

Thierry

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