public inbox for linux-tegra@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] clocksource: fix Tegra234 SoC Watchdog Timer.
@ 2025-04-07 10:23 Robert Lin
  2025-04-07 10:23 ` [PATCH v3 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Robert Lin @ 2025-04-07 10:23 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.

--
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 | 86 +++++++++++++++++-----------
 1 file changed, 52 insertions(+), 34 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support
  2025-04-07 10:23 [PATCH v3 0/3] clocksource: fix Tegra234 SoC Watchdog Timer Robert Lin
@ 2025-04-07 10:23 ` Robert Lin
  2025-04-10 10:09   ` Thierry Reding
  2025-04-07 10:23 ` [PATCH v3 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging Robert Lin
  2025-04-07 10:23 ` [PATCH v3 3/3] clocksource/drivers/timer-tegra186: Remove unused bits Robert Lin
  2 siblings, 1 reply; 9+ messages in thread
From: Robert Lin @ 2025-04-07 10:23 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 | 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;
+	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] 9+ messages in thread

* [PATCH v3 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging
  2025-04-07 10:23 [PATCH v3 0/3] clocksource: fix Tegra234 SoC Watchdog Timer Robert Lin
  2025-04-07 10:23 ` [PATCH v3 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin
@ 2025-04-07 10:23 ` Robert Lin
  2025-04-08  8:54   ` kernel test robot
  2025-04-10 10:15   ` Thierry Reding
  2025-04-07 10:23 ` [PATCH v3 3/3] clocksource/drivers/timer-tegra186: Remove unused bits Robert Lin
  2 siblings, 2 replies; 9+ messages in thread
From: Robert Lin @ 2025-04-07 10:23 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 | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/drivers/clocksource/timer-tegra186.c b/drivers/clocksource/timer-tegra186.c
index fc2ca86c1436..3967d023781b 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 |
@@ -417,18 +414,6 @@ 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;
@@ -480,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] 9+ messages in thread

* [PATCH v3 3/3] clocksource/drivers/timer-tegra186: Remove unused bits
  2025-04-07 10:23 [PATCH v3 0/3] clocksource: fix Tegra234 SoC Watchdog Timer Robert Lin
  2025-04-07 10:23 ` [PATCH v3 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin
  2025-04-07 10:23 ` [PATCH v3 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging Robert Lin
@ 2025-04-07 10:23 ` Robert Lin
  2025-04-10 10:16   ` Thierry Reding
  2 siblings, 1 reply; 9+ messages in thread
From: Robert Lin @ 2025-04-07 10:23 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 3967d023781b..2c1a30291226 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] 9+ messages in thread

* Re: [PATCH v3 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging
  2025-04-07 10:23 ` [PATCH v3 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging Robert Lin
@ 2025-04-08  8:54   ` kernel test robot
  2025-04-10 10:15   ` Thierry Reding
  1 sibling, 0 replies; 9+ messages in thread
From: kernel test robot @ 2025-04-08  8:54 UTC (permalink / raw)
  To: Robert Lin, thierry.reding, daniel.lezcano, jonathanh, tglx,
	pohsuns
  Cc: oe-kbuild-all, linux-kernel, linux-tegra, sumitg, Robert Lin

Hi Robert,

kernel test robot noticed the following build warnings:

[auto build test WARNING on tip/timers/core]
[also build test WARNING on linus/master v6.15-rc1 next-20250408]
[cannot apply to daniel-lezcano/clockevents/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Robert-Lin/clocksource-drivers-timer-tegra186-add-WDIOC_GETTIMELEFT-support/20250407-182954
base:   tip/timers/core
patch link:    https://lore.kernel.org/r/20250407102323.2690911-3-robelin%40nvidia.com
patch subject: [PATCH v3 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging
config: sh-randconfig-001-20250408 (https://download.01.org/0day-ci/archive/20250408/202504081506.0KaQZjFQ-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250408/202504081506.0KaQZjFQ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202504081506.0KaQZjFQ-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/clocksource/timer-tegra186.c: In function 'tegra186_timer_probe':
>> drivers/clocksource/timer-tegra186.c:420:22: warning: variable 'irq' set but not used [-Wunused-but-set-variable]
     420 |         unsigned int irq;
         |                      ^~~


vim +/irq +420 drivers/clocksource/timer-tegra186.c

42cee19a9f839f Thierry Reding 2022-07-04  415  
42cee19a9f839f Thierry Reding 2022-07-04  416  static int tegra186_timer_probe(struct platform_device *pdev)
42cee19a9f839f Thierry Reding 2022-07-04  417  {
42cee19a9f839f Thierry Reding 2022-07-04  418  	struct device *dev = &pdev->dev;
42cee19a9f839f Thierry Reding 2022-07-04  419  	struct tegra186_timer *tegra;
42cee19a9f839f Thierry Reding 2022-07-04 @420  	unsigned int irq;
42cee19a9f839f Thierry Reding 2022-07-04  421  	int err;
42cee19a9f839f Thierry Reding 2022-07-04  422  
42cee19a9f839f Thierry Reding 2022-07-04  423  	tegra = devm_kzalloc(dev, sizeof(*tegra), GFP_KERNEL);
42cee19a9f839f Thierry Reding 2022-07-04  424  	if (!tegra)
42cee19a9f839f Thierry Reding 2022-07-04  425  		return -ENOMEM;
42cee19a9f839f Thierry Reding 2022-07-04  426  
42cee19a9f839f Thierry Reding 2022-07-04  427  	tegra->soc = of_device_get_match_data(dev);
42cee19a9f839f Thierry Reding 2022-07-04  428  	dev_set_drvdata(dev, tegra);
42cee19a9f839f Thierry Reding 2022-07-04  429  	tegra->dev = dev;
42cee19a9f839f Thierry Reding 2022-07-04  430  
42cee19a9f839f Thierry Reding 2022-07-04  431  	tegra->regs = devm_platform_ioremap_resource(pdev, 0);
42cee19a9f839f Thierry Reding 2022-07-04  432  	if (IS_ERR(tegra->regs))
42cee19a9f839f Thierry Reding 2022-07-04  433  		return PTR_ERR(tegra->regs);
42cee19a9f839f Thierry Reding 2022-07-04  434  
42cee19a9f839f Thierry Reding 2022-07-04  435  	err = platform_get_irq(pdev, 0);
42cee19a9f839f Thierry Reding 2022-07-04  436  	if (err < 0)
42cee19a9f839f Thierry Reding 2022-07-04  437  		return err;
42cee19a9f839f Thierry Reding 2022-07-04  438  
42cee19a9f839f Thierry Reding 2022-07-04  439  	irq = err;
42cee19a9f839f Thierry Reding 2022-07-04  440  
42cee19a9f839f Thierry Reding 2022-07-04  441  	/* create a watchdog using a preconfigured timer */
42cee19a9f839f Thierry Reding 2022-07-04  442  	tegra->wdt = tegra186_wdt_create(tegra, 0);
42cee19a9f839f Thierry Reding 2022-07-04  443  	if (IS_ERR(tegra->wdt)) {
42cee19a9f839f Thierry Reding 2022-07-04  444  		err = PTR_ERR(tegra->wdt);
42cee19a9f839f Thierry Reding 2022-07-04  445  		dev_err(dev, "failed to create WDT: %d\n", err);
42cee19a9f839f Thierry Reding 2022-07-04  446  		return err;
42cee19a9f839f Thierry Reding 2022-07-04  447  	}
42cee19a9f839f Thierry Reding 2022-07-04  448  
42cee19a9f839f Thierry Reding 2022-07-04  449  	err = tegra186_timer_tsc_init(tegra);
42cee19a9f839f Thierry Reding 2022-07-04  450  	if (err < 0) {
42cee19a9f839f Thierry Reding 2022-07-04  451  		dev_err(dev, "failed to register TSC counter: %d\n", err);
42cee19a9f839f Thierry Reding 2022-07-04  452  		return err;
42cee19a9f839f Thierry Reding 2022-07-04  453  	}
42cee19a9f839f Thierry Reding 2022-07-04  454  
42cee19a9f839f Thierry Reding 2022-07-04  455  	err = tegra186_timer_osc_init(tegra);
42cee19a9f839f Thierry Reding 2022-07-04  456  	if (err < 0) {
42cee19a9f839f Thierry Reding 2022-07-04  457  		dev_err(dev, "failed to register OSC counter: %d\n", err);
42cee19a9f839f Thierry Reding 2022-07-04  458  		goto unregister_tsc;
42cee19a9f839f Thierry Reding 2022-07-04  459  	}
42cee19a9f839f Thierry Reding 2022-07-04  460  
42cee19a9f839f Thierry Reding 2022-07-04  461  	err = tegra186_timer_usec_init(tegra);
42cee19a9f839f Thierry Reding 2022-07-04  462  	if (err < 0) {
42cee19a9f839f Thierry Reding 2022-07-04  463  		dev_err(dev, "failed to register USEC counter: %d\n", err);
42cee19a9f839f Thierry Reding 2022-07-04  464  		goto unregister_osc;
42cee19a9f839f Thierry Reding 2022-07-04  465  	}
42cee19a9f839f Thierry Reding 2022-07-04  466  
42cee19a9f839f Thierry Reding 2022-07-04  467  	return 0;
42cee19a9f839f Thierry Reding 2022-07-04  468  
42cee19a9f839f Thierry Reding 2022-07-04  469  unregister_osc:
42cee19a9f839f Thierry Reding 2022-07-04  470  	clocksource_unregister(&tegra->osc);
42cee19a9f839f Thierry Reding 2022-07-04  471  unregister_tsc:
42cee19a9f839f Thierry Reding 2022-07-04  472  	clocksource_unregister(&tegra->tsc);
42cee19a9f839f Thierry Reding 2022-07-04  473  	return err;
42cee19a9f839f Thierry Reding 2022-07-04  474  }
42cee19a9f839f Thierry Reding 2022-07-04  475  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support
  2025-04-07 10:23 ` [PATCH v3 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin
@ 2025-04-10 10:09   ` Thierry Reding
  2025-04-15  2:57     ` Robert Lin
  0 siblings, 1 reply; 9+ messages in thread
From: Thierry Reding @ 2025-04-10 10:09 UTC (permalink / raw)
  To: Robert Lin
  Cc: daniel.lezcano, jonathanh, tglx, pohsuns, linux-kernel,
	linux-tegra, sumitg

[-- 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 --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging
  2025-04-07 10:23 ` [PATCH v3 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging Robert Lin
  2025-04-08  8:54   ` kernel test robot
@ 2025-04-10 10:15   ` Thierry Reding
  1 sibling, 0 replies; 9+ messages in thread
From: Thierry Reding @ 2025-04-10 10:15 UTC (permalink / raw)
  To: Robert Lin
  Cc: daniel.lezcano, jonathanh, tglx, pohsuns, linux-kernel,
	linux-tegra, sumitg

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

On Mon, Apr 07, 2025 at 06:23:22PM +0800, Robert Lin wrote:
> 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 | 24 ------------------------
>  1 file changed, 24 deletions(-)

Yeah, in retrospect this doesn't make sense:

Acked-by: Thierry Reding <treding@nvidia.com>

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 3/3] clocksource/drivers/timer-tegra186: Remove unused bits
  2025-04-07 10:23 ` [PATCH v3 3/3] clocksource/drivers/timer-tegra186: Remove unused bits Robert Lin
@ 2025-04-10 10:16   ` Thierry Reding
  0 siblings, 0 replies; 9+ messages in thread
From: Thierry Reding @ 2025-04-10 10:16 UTC (permalink / raw)
  To: Robert Lin
  Cc: daniel.lezcano, jonathanh, tglx, pohsuns, linux-kernel,
	linux-tegra, sumitg

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

On Mon, Apr 07, 2025 at 06:23:23PM +0800, Robert Lin wrote:
> 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(-)

Acked-by: Thierry Reding <treding@nvidia.com>

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH v3 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support
  2025-04-10 10:09   ` Thierry Reding
@ 2025-04-15  2:57     ` Robert Lin
  0 siblings, 0 replies; 9+ messages in thread
From: Robert Lin @ 2025-04-15  2:57 UTC (permalink / raw)
  To: Thierry Reding
  Cc: daniel.lezcano@linaro.org, Jon Hunter, tglx@linutronix.de,
	Pohsun Su, linux-kernel@vger.kernel.org,
	linux-tegra@vger.kernel.org, Sumit Gupta

> From: Thierry Reding <thierry.reding@gmail.com>
> Sent: Thursday, April 10, 2025 6:09 PM
> To: Robert Lin <robelin@nvidia.com>
> Cc: daniel.lezcano@linaro.org; Jon Hunter <jonathanh@nvidia.com>;
> tglx@linutronix.de; Pohsun Su <pohsuns@nvidia.com>; linux-
> kernel@vger.kernel.org; linux-tegra@vger.kernel.org; Sumit Gupta
> <sumitg@nvidia.com>
> Subject: Re: [PATCH v3 1/3] clocksource/drivers/timer-tegra186: add
> WDIOC_GETTIMELEFT support
>  
> 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?
> 

Will do in the next patch, thank you.

Robert Lin

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-04-15  2:57 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-07 10:23 [PATCH v3 0/3] clocksource: fix Tegra234 SoC Watchdog Timer Robert Lin
2025-04-07 10:23 ` [PATCH v3 1/3] clocksource/drivers/timer-tegra186: add WDIOC_GETTIMELEFT support Robert Lin
2025-04-10 10:09   ` Thierry Reding
2025-04-15  2:57     ` Robert Lin
2025-04-07 10:23 ` [PATCH v3 2/3] clocksource/drivers/timer-tegra186: fix watchdog self-pinging Robert Lin
2025-04-08  8:54   ` kernel test robot
2025-04-10 10:15   ` Thierry Reding
2025-04-07 10:23 ` [PATCH v3 3/3] clocksource/drivers/timer-tegra186: Remove unused bits Robert Lin
2025-04-10 10:16   ` Thierry Reding

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox