Increase max_timeout value from 55s to 3665038s (1018h 3min 58s) with
38400000 frequency system if the system has 32-bit WTCNT register.

cat /sys/devices/platform/10060000.watchdog_cl0/watchdog/watchdog0/max_timeout
3665038

[    0.208763][    T1] s3c2410-wdt 10060000.watchdog_cl0: Heartbeat: count=1099511400000, timeout=3665038, freq=300000
[    0.208767][    T1] s3c2410-wdt 10060000.watchdog_cl0: Heartbeat: timeout=3665038, divisor=256, count=1099511400000 (fffffc87)
[    0.208799][    T1] s3c2410-wdt 10060000.watchdog_cl0: starting watchdog timer
[    0.208969][    T1] s3c2410-wdt 10060000.watchdog_cl0: watchdog active, reset enabled, irq disabled

If system has 32-bit WTCNT, add QUIRK_HAS_32BIT_MAXCNT to its quirk flags, then
it will operation with 32-bit counter. If not, with 16-bit counter like previous.

Signed-off-by: Sangwook Shin <sw617.shin@samsung.com>
Signed-off-by: Kyunghwan Seo <khwan.seo@samsung.com>
---
 drivers/watchdog/s3c2410_wdt.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index bdd81d8074b2..a13768a11f20 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -34,6 +34,7 @@
 #define S3C2410_WTCLRINT	0x0c
 
 #define S3C2410_WTCNT_MAXCNT	0xffff
+#define S3C2410_WTCNT_MAXCNT_32	0xffffffff
 
 #define S3C2410_WTCON_RSTEN		BIT(0)
 #define S3C2410_WTCON_INTEN		BIT(2)
@@ -119,6 +120,10 @@
  * %QUIRK_HAS_DBGACK_BIT: WTCON register has DBGACK_MASK bit. Setting the
  * DBGACK_MASK bit disables the watchdog outputs when the SoC is in debug mode.
  * Debug mode is determined by the DBGACK CPU signal.
+ *
+ * %QUIRK_HAS_32BIT_MAXCNT: WTDAT and WTCNT are 32-bit registers. With these
+ * 32-bit registers, larger values to be set, which means that larger timeouts
+ * value can be set.
  */
 #define QUIRK_HAS_WTCLRINT_REG			BIT(0)
 #define QUIRK_HAS_PMU_MASK_RESET		BIT(1)
@@ -126,6 +131,7 @@
 #define QUIRK_HAS_PMU_AUTO_DISABLE		BIT(3)
 #define QUIRK_HAS_PMU_CNT_EN			BIT(4)
 #define QUIRK_HAS_DBGACK_BIT			BIT(5)
+#define QUIRK_HAS_32BIT_MAXCNT			BIT(6)
 
 /* These quirks require that we have a PMU register map */
 #define QUIRKS_HAVE_PMUREG \
@@ -378,9 +384,13 @@ static inline unsigned long s3c2410wdt_get_freq(struct s3c2410_wdt *wdt)
 static inline unsigned int s3c2410wdt_max_timeout(struct s3c2410_wdt *wdt)
 {
 	const unsigned long freq = s3c2410wdt_get_freq(wdt);
+	unsigned long max_cnt = S3C2410_WTCNT_MAXCNT;
+
+	if ((wdt->drv_data->quirks & QUIRK_HAS_32BIT_MAXCNT))
+		max_cnt = S3C2410_WTCNT_MAXCNT_32;
 
-	return S3C2410_WTCNT_MAXCNT / (freq / (S3C2410_WTCON_PRESCALE_MAX + 1)
-				       / S3C2410_WTCON_MAXDIV);
+	return max_cnt * (S3C2410_WTCON_PRESCALE_MAX + 1)
+				* S3C2410_WTCON_MAXDIV / freq;
 }
 
 static int s3c2410wdt_disable_wdt_reset(struct s3c2410_wdt *wdt, bool mask)
@@ -534,9 +544,10 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd,
 {
 	struct s3c2410_wdt *wdt = watchdog_get_drvdata(wdd);
 	unsigned long freq = s3c2410wdt_get_freq(wdt);
-	unsigned int count;
+	unsigned long count;
 	unsigned int divisor = 1;
 	unsigned long wtcon;
+	unsigned int max_cnt = S3C2410_WTCNT_MAXCNT;
 
 	if (timeout < 1)
 		return -EINVAL;
@@ -544,7 +555,7 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd,
 	freq = DIV_ROUND_UP(freq, 128);
 	count = timeout * freq;
 
-	dev_dbg(wdt->dev, "Heartbeat: count=%d, timeout=%d, freq=%lu\n",
+	dev_dbg(wdt->dev, "Heartbeat: count=%lu, timeout=%d, freq=%lu\n",
 		count, timeout, freq);
 
 	/* if the count is bigger than the watchdog register,
@@ -552,16 +563,19 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd,
 	   actually make this value
 	*/
 
-	if (count >= 0x10000) {
-		divisor = DIV_ROUND_UP(count, 0xffff);
+	if ((wdt->drv_data->quirks & QUIRK_HAS_32BIT_MAXCNT))
+		max_cnt = S3C2410_WTCNT_MAXCNT_32;
+
+	if (count > max_cnt) {
+		divisor = DIV_ROUND_UP(count, max_cnt);
 
-		if (divisor > 0x100) {
+		if (divisor > (S3C2410_WTCON_PRESCALE_MAX + 1)) {
 			dev_err(wdt->dev, "timeout %d too big\n", timeout);
 			return -EINVAL;
 		}
 	}
 
-	dev_dbg(wdt->dev, "Heartbeat: timeout=%d, divisor=%d, count=%d (%08x)\n",
+	dev_dbg(wdt->dev, "Heartbeat: timeout=%d, divisor=%d, count=%lu (%08lx)\n",
 		timeout, divisor, count, DIV_ROUND_UP(count, divisor));
 
 	count = DIV_ROUND_UP(count, divisor);
-- 
2.40.1

On 5/13/25 02:47, Sangwook Shin wrote:
> Increase max_timeout value from 55s to 3665038s (1018h 3min 58s) with
> 38400000 frequency system if the system has 32-bit WTCNT register.
> 
> cat /sys/devices/platform/10060000.watchdog_cl0/watchdog/watchdog0/max_timeout
> 3665038
> 
> [    0.208763][    T1] s3c2410-wdt 10060000.watchdog_cl0: Heartbeat: count=1099511400000, timeout=3665038, freq=300000
> [    0.208767][    T1] s3c2410-wdt 10060000.watchdog_cl0: Heartbeat: timeout=3665038, divisor=256, count=1099511400000 (fffffc87)
> [    0.208799][    T1] s3c2410-wdt 10060000.watchdog_cl0: starting watchdog timer
> [    0.208969][    T1] s3c2410-wdt 10060000.watchdog_cl0: watchdog active, reset enabled, irq disabled
> 
> If system has 32-bit WTCNT, add QUIRK_HAS_32BIT_MAXCNT to its quirk flags, then
> it will operation with 32-bit counter. If not, with 16-bit counter like previous.
> 
> Signed-off-by: Sangwook Shin <sw617.shin@samsung.com>
> Signed-off-by: Kyunghwan Seo <khwan.seo@samsung.com>
> ---
>   drivers/watchdog/s3c2410_wdt.c | 30 ++++++++++++++++++++++--------
>   1 file changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index bdd81d8074b2..a13768a11f20 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -34,6 +34,7 @@
>   #define S3C2410_WTCLRINT	0x0c
>   
>   #define S3C2410_WTCNT_MAXCNT	0xffff
> +#define S3C2410_WTCNT_MAXCNT_32	0xffffffff
>   
>   #define S3C2410_WTCON_RSTEN		BIT(0)
>   #define S3C2410_WTCON_INTEN		BIT(2)
> @@ -119,6 +120,10 @@
>    * %QUIRK_HAS_DBGACK_BIT: WTCON register has DBGACK_MASK bit. Setting the
>    * DBGACK_MASK bit disables the watchdog outputs when the SoC is in debug mode.
>    * Debug mode is determined by the DBGACK CPU signal.
> + *
> + * %QUIRK_HAS_32BIT_MAXCNT: WTDAT and WTCNT are 32-bit registers. With these
> + * 32-bit registers, larger values to be set, which means that larger timeouts
> + * value can be set.
>    */
>   #define QUIRK_HAS_WTCLRINT_REG			BIT(0)
>   #define QUIRK_HAS_PMU_MASK_RESET		BIT(1)
> @@ -126,6 +131,7 @@
>   #define QUIRK_HAS_PMU_AUTO_DISABLE		BIT(3)
>   #define QUIRK_HAS_PMU_CNT_EN			BIT(4)
>   #define QUIRK_HAS_DBGACK_BIT			BIT(5)
> +#define QUIRK_HAS_32BIT_MAXCNT			BIT(6)
>   
>   /* These quirks require that we have a PMU register map */
>   #define QUIRKS_HAVE_PMUREG \
> @@ -378,9 +384,13 @@ static inline unsigned long s3c2410wdt_get_freq(struct s3c2410_wdt *wdt)
>   static inline unsigned int s3c2410wdt_max_timeout(struct s3c2410_wdt *wdt)
>   {
>   	const unsigned long freq = s3c2410wdt_get_freq(wdt);
> +	unsigned long max_cnt = S3C2410_WTCNT_MAXCNT;
> +
> +	if ((wdt->drv_data->quirks & QUIRK_HAS_32BIT_MAXCNT))
> +		max_cnt = S3C2410_WTCNT_MAXCNT_32;
>   
> -	return S3C2410_WTCNT_MAXCNT / (freq / (S3C2410_WTCON_PRESCALE_MAX + 1)
> -				       / S3C2410_WTCON_MAXDIV);
> +	return max_cnt * (S3C2410_WTCON_PRESCALE_MAX + 1)
> +				* S3C2410_WTCON_MAXDIV / freq;

This will overflow if the build is a 32-bit build with sizeof(long) == 4.
The previous calculation did not have that problem. If there was a secondary
reason to change the calculation, please submit a second patch with that change
and explanation on its own.

Thanks,
Guenter

>   }
>   
>   static int s3c2410wdt_disable_wdt_reset(struct s3c2410_wdt *wdt, bool mask)
> @@ -534,9 +544,10 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd,
>   {
>   	struct s3c2410_wdt *wdt = watchdog_get_drvdata(wdd);
>   	unsigned long freq = s3c2410wdt_get_freq(wdt);
> -	unsigned int count;
> +	unsigned long count;
>   	unsigned int divisor = 1;
>   	unsigned long wtcon;
> +	unsigned int max_cnt = S3C2410_WTCNT_MAXCNT;
>   
>   	if (timeout < 1)
>   		return -EINVAL;
> @@ -544,7 +555,7 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd,
>   	freq = DIV_ROUND_UP(freq, 128);
>   	count = timeout * freq;
>   
> -	dev_dbg(wdt->dev, "Heartbeat: count=%d, timeout=%d, freq=%lu\n",
> +	dev_dbg(wdt->dev, "Heartbeat: count=%lu, timeout=%d, freq=%lu\n",
>   		count, timeout, freq);
>   
>   	/* if the count is bigger than the watchdog register,
> @@ -552,16 +563,19 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd,
>   	   actually make this value
>   	*/
>   
> -	if (count >= 0x10000) {
> -		divisor = DIV_ROUND_UP(count, 0xffff);
> +	if ((wdt->drv_data->quirks & QUIRK_HAS_32BIT_MAXCNT))
> +		max_cnt = S3C2410_WTCNT_MAXCNT_32;
> +

I think it would make sense to store max_cnt in struct s3c2410_wdt instead
of recalculating it repeatedly during runtime.

> +	if (count > max_cnt) {
> +		divisor = DIV_ROUND_UP(count, max_cnt);
>   
> -		if (divisor > 0x100) {
> +		if (divisor > (S3C2410_WTCON_PRESCALE_MAX + 1)) {

That is another change that seems unrelated to this patch. Note that inner
the ( ) s unnecessary.

>   			dev_err(wdt->dev, "timeout %d too big\n", timeout);
>   			return -EINVAL;
>   		}
>   	}
>   
> -	dev_dbg(wdt->dev, "Heartbeat: timeout=%d, divisor=%d, count=%d (%08x)\n",
> +	dev_dbg(wdt->dev, "Heartbeat: timeout=%d, divisor=%d, count=%lu (%08lx)\n",
>   		timeout, divisor, count, DIV_ROUND_UP(count, divisor));
>   
>   	count = DIV_ROUND_UP(count, divisor);