linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/5] Increase max timeout value of s3c2410 watchdog
       [not found] <CGME20250806070029epcas2p18a80cf9215e331012e0ed4890cdae700@epcas2p1.samsung.com>
@ 2025-08-06  6:55 ` Sangwook Shin
       [not found]   ` <CGME20250806070029epcas2p3a937682ba1bdad1317ef62aab8f826ae@epcas2p3.samsung.com>
                     ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Sangwook Shin @ 2025-08-06  6:55 UTC (permalink / raw)
  To: krzk, alim.akhtar, wim, linux, semen.protsenko, dongil01.park,
	khwan.seo
  Cc: linux-arm-kernel, linux-samsung-soc, linux-watchdog, linux-kernel,
	Sangwook Shin

The ExynosAutoV9 and ExynosAutoV920 SoCs have a 32-bit counter register,
but due to code constraints, only 16-bit values could be used.
This series enables these SoCs to use the 32-bit counter.
Additionally, it addresses the issue where the ExynosAutoV9 SoC supports
the DBGACK bit but it was not set.

V4->V5:
  - Update s3c2410wdt_max_timeout with Sam Protsenko and Guenter Roeck's sugestion.
  - Break [v4 3/4] into two [v5 3/5] and [v5 4/5].
  - Rename S3C2410_WTCNT_MAXCNT to S3C2410_WTCNT_MAXCNT_16.
  - Rename QUIRK_HAS_32BIT_MAXCNT to QUIRK_HAS_32BIT_CNT.
  - Minor Typographical Errors and Style Adjustments.
  - Link to v4:
    https://lore.kernel.org/linux-watchdog/20250724080854.3866566-1-sw617.shin@samsung.com/

V3->V4:
  - Merge patches [v3 3/5] and [v3 4/5] into one so that Quirk and its consumer
    are part of the same patch.
  - Link to v3:
    https://lore.kernel.org/linux-watchdog/20250714055440.3138135-1-sw617.shin@samsung.com/
    https://lore.kernel.org/linux-watchdog/20250515075350.3368635-1-sw617.shin@samsung.com/

V2->V3:
  - Correct the incorrect tag information.
  - Link to v2:
    https://lore.kernel.org/linux-watchdog/20250514094220.1561378-1-sw617.shin@samsung.com/

V1->V2:
  - Modify the max_timeout calculation considering overflow
  - Separate tha max_timeout calculation into a separate patch
  - Add max_cnt in struct s3c2410_wdt
  - Set max_cnt once in probe function
  - Add patch that uses S3C2410_WTCON_PRESCALE_MAX instead of hardcoded one
  - Remove unnecessary inner parentheses
  - Link to v1:
    https://lore.kernel.org/linux-watchdog/20250513094711.2691059-1-sw617.shin@samsung.com/

Sangwook Shin (5):
  watchdog: s3c2410_wdt: Replace hardcoded values with macro definitions
  watchdog: s3c2410_wdt: Fix max_timeout being calculated larger
  watchdog: s3c2410_wdt: Increase max timeout value of watchdog
  watchdog: s3c2410_wdt: exynosautov920: Enable QUIRK_HAS_32BIT_CNT
  watchdog: s3c2410_wdt: exynosautov9: Enable supported features

 drivers/watchdog/s3c2410_wdt.c | 45 ++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 13 deletions(-)

-- 
2.25.1


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

* [PATCH v5 1/5] watchdog: s3c2410_wdt: Replace hardcoded values with macro definitions
       [not found]   ` <CGME20250806070029epcas2p3a937682ba1bdad1317ef62aab8f826ae@epcas2p3.samsung.com>
@ 2025-08-06  6:55     ` Sangwook Shin
  0 siblings, 0 replies; 10+ messages in thread
From: Sangwook Shin @ 2025-08-06  6:55 UTC (permalink / raw)
  To: krzk, alim.akhtar, wim, linux, semen.protsenko, dongil01.park,
	khwan.seo
  Cc: linux-arm-kernel, linux-samsung-soc, linux-watchdog, linux-kernel,
	Sangwook Shin

Modify the code to utilize macro-defined values instead of hardcoded
values. The value 0x100 in the s3c2410wdt_set_heartbeat function represents
S3C2410_WTCON_PRESCALE_MAX + 1, but it is hardcoded, making its meaning
difficult to understand and reducing code readability.

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
Signed-off-by: Sangwook Shin <sw617.shin@samsung.com>
---
 drivers/watchdog/s3c2410_wdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 40901bdac426..95f7207e390a 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -587,7 +587,7 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd,
 	if (count >= 0x10000) {
 		divisor = DIV_ROUND_UP(count, 0xffff);
 
-		if (divisor > 0x100) {
+		if (divisor > S3C2410_WTCON_PRESCALE_MAX + 1) {
 			dev_err(wdt->dev, "timeout %d too big\n", timeout);
 			return -EINVAL;
 		}
-- 
2.25.1


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

* [PATCH v5 2/5] watchdog: s3c2410_wdt: Fix max_timeout being calculated larger
       [not found]   ` <CGME20250806070029epcas2p1f764301b115b766f88b422b0dbdd68f1@epcas2p1.samsung.com>
@ 2025-08-06  6:55     ` Sangwook Shin
  2025-08-06 17:17       ` Sam Protsenko
  0 siblings, 1 reply; 10+ messages in thread
From: Sangwook Shin @ 2025-08-06  6:55 UTC (permalink / raw)
  To: krzk, alim.akhtar, wim, linux, semen.protsenko, dongil01.park,
	khwan.seo
  Cc: linux-arm-kernel, linux-samsung-soc, linux-watchdog, linux-kernel,
	Sangwook Shin

Fix the issue of max_timeout being calculated larger than actual value.
The calculation result of freq / (S3C2410_WTCON_PRESCALE_MAX + 1) /
S3C2410_WTCON_MAXDIV is smaller than the actual value because the remainder
is discarded during the calculation process. This leads to a larger
calculated value for max_timeout compared to the actual settable value.
To resolve this issue, the order of calculations in the computation process
has been adjusted.

Signed-off-by: Sangwook Shin <sw617.shin@samsung.com>
---
 drivers/watchdog/s3c2410_wdt.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 95f7207e390a..0a4c0ab2a3d6 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -27,6 +27,7 @@
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
 #include <linux/delay.h>
+#include <linux/math64.h>
 
 #define S3C2410_WTCON		0x00
 #define S3C2410_WTDAT		0x04
@@ -410,9 +411,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);
+	//(S3C2410_WTCON_PRESCALE_MAX + 1) * S3C2410_WTCON_MAXDIV = 0x8000
+	u64 t_max = div64_ul((u64)S3C2410_WTCNT_MAXCNT * 0x8000, freq);
 
-	return S3C2410_WTCNT_MAXCNT / (freq / (S3C2410_WTCON_PRESCALE_MAX + 1)
-				       / S3C2410_WTCON_MAXDIV);
+	if (t_max > UINT_MAX)
+		t_max = UINT_MAX;
+
+	return t_max;
 }
 
 static int s3c2410wdt_disable_wdt_reset(struct s3c2410_wdt *wdt, bool mask)
-- 
2.25.1


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

* [PATCH v5 3/5] watchdog: s3c2410_wdt: Increase max timeout value of watchdog
       [not found]   ` <CGME20250806070030epcas2p24cc79e6dd3e505afd980fc05ee9e4f19@epcas2p2.samsung.com>
@ 2025-08-06  6:55     ` Sangwook Shin
  2025-08-06 18:06       ` Sam Protsenko
  0 siblings, 1 reply; 10+ messages in thread
From: Sangwook Shin @ 2025-08-06  6:55 UTC (permalink / raw)
  To: krzk, alim.akhtar, wim, linux, semen.protsenko, dongil01.park,
	khwan.seo
  Cc: linux-arm-kernel, linux-samsung-soc, linux-watchdog, linux-kernel,
	Sangwook Shin

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/class/watchdog/watchdog0/max_timeout
3665038

[    0.330082] s3c2410-wdt 10060000.watchdog_cl0: Heartbeat: count=1099511400000, timeout=3665038, freq=300000
[    0.330087] s3c2410-wdt 10060000.watchdog_cl0: Heartbeat: timeout=3665038, divisor=256, count=1099511400000 (fffffc87)
[    0.330127] s3c2410-wdt 10060000.watchdog_cl0: starting watchdog timer
[    0.330134] s3c2410-wdt 10060000.watchdog_cl0: Starting watchdog: count=0xfffffc87, wtcon=0001ff39
[    0.330319] s3c2410-wdt 10060000.watchdog_cl0: watchdog active, reset enabled, irq disabled

If the system has a 32-bit WTCNT, add QUIRK_HAS_32BIT_CNT to its quirk flags,
and it will operate with a 32-bit counter. If not, it will operate with a 16-bit
counter like in the previous version.

Signed-off-by: Sangwook Shin <sw617.shin@samsung.com>
---
 drivers/watchdog/s3c2410_wdt.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 0a4c0ab2a3d6..673ab6768688 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -34,7 +34,8 @@
 #define S3C2410_WTCNT		0x08
 #define S3C2410_WTCLRINT	0x0c
 
-#define S3C2410_WTCNT_MAXCNT	0xffff
+#define S3C2410_WTCNT_MAXCNT_16	0xffff
+#define S3C2410_WTCNT_MAXCNT_32	0xffffffff
 
 #define S3C2410_WTCON_RSTEN		BIT(0)
 #define S3C2410_WTCON_INTEN		BIT(2)
@@ -124,6 +125,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_CNT: WTDAT and WTCNT are 32-bit registers. With these
+ * 32-bit registers, larger values will 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)
@@ -131,6 +136,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_CNT			BIT(6)
 
 /* These quirks require that we have a PMU register map */
 #define QUIRKS_HAVE_PMUREG \
@@ -199,6 +205,7 @@ struct s3c2410_wdt {
 	struct notifier_block	freq_transition;
 	const struct s3c2410_wdt_variant *drv_data;
 	struct regmap *pmureg;
+	u32 max_cnt;
 };
 
 static const struct s3c2410_wdt_variant drv_data_s3c2410 = {
@@ -412,7 +419,7 @@ static inline unsigned int s3c2410wdt_max_timeout(struct s3c2410_wdt *wdt)
 {
 	const unsigned long freq = s3c2410wdt_get_freq(wdt);
 	//(S3C2410_WTCON_PRESCALE_MAX + 1) * S3C2410_WTCON_MAXDIV = 0x8000
-	u64 t_max = div64_ul((u64)S3C2410_WTCNT_MAXCNT * 0x8000, freq);
+	u64 t_max = div64_ul((u64)wdt->max_cnt * 0x8000, freq);
 
 	if (t_max > UINT_MAX)
 		t_max = UINT_MAX;
@@ -571,7 +578,7 @@ 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;
 
@@ -581,7 +588,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,
@@ -589,8 +596,8 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd,
 	   actually make this value
 	*/
 
-	if (count >= 0x10000) {
-		divisor = DIV_ROUND_UP(count, 0xffff);
+	if (count > wdt->max_cnt) {
+		divisor = DIV_ROUND_UP(count, wdt->max_cnt);
 
 		if (divisor > S3C2410_WTCON_PRESCALE_MAX + 1) {
 			dev_err(wdt->dev, "timeout %d too big\n", timeout);
@@ -598,7 +605,7 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd,
 		}
 	}
 
-	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);
@@ -806,6 +813,11 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
 	if (IS_ERR(wdt->src_clk))
 		return dev_err_probe(dev, PTR_ERR(wdt->src_clk), "failed to get source clock\n");
 
+	if (wdt->drv_data->quirks & QUIRK_HAS_32BIT_CNT)
+		wdt->max_cnt = S3C2410_WTCNT_MAXCNT_32;
+	else
+		wdt->max_cnt = S3C2410_WTCNT_MAXCNT_16;
+
 	wdt->wdt_device.min_timeout = 1;
 	wdt->wdt_device.max_timeout = s3c2410wdt_max_timeout(wdt);
 
-- 
2.25.1


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

* [PATCH v5 4/5] watchdog: s3c2410_wdt: exynosautov920: Enable QUIRK_HAS_32BIT_CNT
       [not found]   ` <CGME20250806070030epcas2p3f2b0c36b6c492629e50e76be469b2db3@epcas2p3.samsung.com>
@ 2025-08-06  6:55     ` Sangwook Shin
  2025-08-06 18:08       ` Sam Protsenko
  0 siblings, 1 reply; 10+ messages in thread
From: Sangwook Shin @ 2025-08-06  6:55 UTC (permalink / raw)
  To: krzk, alim.akhtar, wim, linux, semen.protsenko, dongil01.park,
	khwan.seo
  Cc: linux-arm-kernel, linux-samsung-soc, linux-watchdog, linux-kernel,
	Sangwook Shin

Enable QUIRK_HAS_32BIT_CNT to ExynosAutov920 SoC which has 32-bit WTCNT.

Signed-off-by: Sangwook Shin <sw617.shin@samsung.com>
---
 drivers/watchdog/s3c2410_wdt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 673ab6768688..541870b4d01a 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -357,7 +357,7 @@ static const struct s3c2410_wdt_variant drv_data_exynosautov920_cl0 = {
 	.cnt_en_bit = 8,
 	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET |
 		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN |
-		  QUIRK_HAS_DBGACK_BIT,
+		  QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_32BIT_CNT,
 };
 
 static const struct s3c2410_wdt_variant drv_data_exynosautov920_cl1 = {
@@ -370,7 +370,7 @@ static const struct s3c2410_wdt_variant drv_data_exynosautov920_cl1 = {
 	.cnt_en_bit = 8,
 	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET |
 		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN |
-		  QUIRK_HAS_DBGACK_BIT,
+		  QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_32BIT_CNT,
 };
 
 static const struct of_device_id s3c2410_wdt_match[] = {
-- 
2.25.1


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

* [PATCH v5 5/5] watchdog: s3c2410_wdt: exynosautov9: Enable supported features
       [not found]   ` <CGME20250806070030epcas2p4f9b779bd9fde57b4bfea958efdf62cf4@epcas2p4.samsung.com>
@ 2025-08-06  6:55     ` Sangwook Shin
  2025-08-06 18:07       ` Sam Protsenko
  0 siblings, 1 reply; 10+ messages in thread
From: Sangwook Shin @ 2025-08-06  6:55 UTC (permalink / raw)
  To: krzk, alim.akhtar, wim, linux, semen.protsenko, dongil01.park,
	khwan.seo
  Cc: linux-arm-kernel, linux-samsung-soc, linux-watchdog, linux-kernel,
	Sangwook Shin

Enable supported features for ExynosAutov9 SoC.
- QUIRK_HAS_DBGACK_BIT
- QUIRK_HAS_32BIT_CNT

Signed-off-by: Sangwook Shin <sw617.shin@samsung.com>
---
 drivers/watchdog/s3c2410_wdt.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
index 541870b4d01a..f25563585983 100644
--- a/drivers/watchdog/s3c2410_wdt.c
+++ b/drivers/watchdog/s3c2410_wdt.c
@@ -306,7 +306,8 @@ static const struct s3c2410_wdt_variant drv_data_exynosautov9_cl0 = {
 	.cnt_en_reg = EXYNOS850_CLUSTER0_NONCPU_OUT,
 	.cnt_en_bit = 7,
 	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET |
-		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN,
+		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN |
+		  QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_32BIT_CNT,
 };
 
 static const struct s3c2410_wdt_variant drv_data_exynosautov9_cl1 = {
@@ -318,7 +319,8 @@ static const struct s3c2410_wdt_variant drv_data_exynosautov9_cl1 = {
 	.cnt_en_reg = EXYNOSAUTOV9_CLUSTER1_NONCPU_OUT,
 	.cnt_en_bit = 7,
 	.quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET |
-		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN,
+		  QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN |
+		  QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_32BIT_CNT,
 };
 
 static const struct s3c2410_wdt_variant drv_data_gs101_cl0 = {
-- 
2.25.1


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

* Re: [PATCH v5 2/5] watchdog: s3c2410_wdt: Fix max_timeout being calculated larger
  2025-08-06  6:55     ` [PATCH v5 2/5] watchdog: s3c2410_wdt: Fix max_timeout being calculated larger Sangwook Shin
@ 2025-08-06 17:17       ` Sam Protsenko
  0 siblings, 0 replies; 10+ messages in thread
From: Sam Protsenko @ 2025-08-06 17:17 UTC (permalink / raw)
  To: Sangwook Shin
  Cc: krzk, alim.akhtar, wim, linux, dongil01.park, khwan.seo,
	linux-arm-kernel, linux-samsung-soc, linux-watchdog, linux-kernel

On Wed, Aug 6, 2025 at 2:00 AM Sangwook Shin <sw617.shin@samsung.com> wrote:
>
> Fix the issue of max_timeout being calculated larger than actual value.
> The calculation result of freq / (S3C2410_WTCON_PRESCALE_MAX + 1) /
> S3C2410_WTCON_MAXDIV is smaller than the actual value because the remainder
> is discarded during the calculation process. This leads to a larger
> calculated value for max_timeout compared to the actual settable value.
> To resolve this issue, the order of calculations in the computation process
> has been adjusted.
>
> Signed-off-by: Sangwook Shin <sw617.shin@samsung.com>
> ---
>  drivers/watchdog/s3c2410_wdt.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index 95f7207e390a..0a4c0ab2a3d6 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -27,6 +27,7 @@
>  #include <linux/mfd/syscon.h>
>  #include <linux/regmap.h>
>  #include <linux/delay.h>
> +#include <linux/math64.h>
>
>  #define S3C2410_WTCON          0x00
>  #define S3C2410_WTDAT          0x04
> @@ -410,9 +411,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);
> +       //(S3C2410_WTCON_PRESCALE_MAX + 1) * S3C2410_WTCON_MAXDIV = 0x8000

This single-line comment style (//) is usually not used in the kernel
code. Prefer multi-line comment style instead (/* */). More
importantly, this comment is not needed: using the raw 0x8000 value
below is still hard-coding it, and this comment is not the best way to
fix that. So declare either a proper constant or a define, and use it
further. I'd go with this:

    const u64 cnt_max = (S3C2410_WTCON_PRESCALE_MAX + 1) *
                        S3C2410_WTCON_MAXDIV * S3C2410_WTCNT_MAXCNT;

and then:

    u64 t_max = div64_ul(cnt_max, freq);

Sorry I didn't catch your related comment earlier in v4!

With that addressed, feel free to add:

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

> +       u64 t_max = div64_ul((u64)S3C2410_WTCNT_MAXCNT * 0x8000, freq);
>
> -       return S3C2410_WTCNT_MAXCNT / (freq / (S3C2410_WTCON_PRESCALE_MAX + 1)
> -                                      / S3C2410_WTCON_MAXDIV);
> +       if (t_max > UINT_MAX)
> +               t_max = UINT_MAX;
> +
> +       return t_max;
>  }
>
>  static int s3c2410wdt_disable_wdt_reset(struct s3c2410_wdt *wdt, bool mask)
> --
> 2.25.1
>

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

* Re: [PATCH v5 3/5] watchdog: s3c2410_wdt: Increase max timeout value of watchdog
  2025-08-06  6:55     ` [PATCH v5 3/5] watchdog: s3c2410_wdt: Increase max timeout value of watchdog Sangwook Shin
@ 2025-08-06 18:06       ` Sam Protsenko
  0 siblings, 0 replies; 10+ messages in thread
From: Sam Protsenko @ 2025-08-06 18:06 UTC (permalink / raw)
  To: Sangwook Shin
  Cc: krzk, alim.akhtar, wim, linux, dongil01.park, khwan.seo,
	linux-arm-kernel, linux-samsung-soc, linux-watchdog, linux-kernel

On Wed, Aug 6, 2025 at 2:00 AM Sangwook Shin <sw617.shin@samsung.com> 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/class/watchdog/watchdog0/max_timeout
> 3665038
>
> [    0.330082] s3c2410-wdt 10060000.watchdog_cl0: Heartbeat: count=1099511400000, timeout=3665038, freq=300000
> [    0.330087] s3c2410-wdt 10060000.watchdog_cl0: Heartbeat: timeout=3665038, divisor=256, count=1099511400000 (fffffc87)
> [    0.330127] s3c2410-wdt 10060000.watchdog_cl0: starting watchdog timer
> [    0.330134] s3c2410-wdt 10060000.watchdog_cl0: Starting watchdog: count=0xfffffc87, wtcon=0001ff39
> [    0.330319] s3c2410-wdt 10060000.watchdog_cl0: watchdog active, reset enabled, irq disabled
>
> If the system has a 32-bit WTCNT, add QUIRK_HAS_32BIT_CNT to its quirk flags,
> and it will operate with a 32-bit counter. If not, it will operate with a 16-bit
> counter like in the previous version.
>
> Signed-off-by: Sangwook Shin <sw617.shin@samsung.com>
> ---
>  drivers/watchdog/s3c2410_wdt.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index 0a4c0ab2a3d6..673ab6768688 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -34,7 +34,8 @@
>  #define S3C2410_WTCNT          0x08
>  #define S3C2410_WTCLRINT       0x0c
>
> -#define S3C2410_WTCNT_MAXCNT   0xffff
> +#define S3C2410_WTCNT_MAXCNT_16        0xffff
> +#define S3C2410_WTCNT_MAXCNT_32        0xffffffff
>
>  #define S3C2410_WTCON_RSTEN            BIT(0)
>  #define S3C2410_WTCON_INTEN            BIT(2)
> @@ -124,6 +125,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_CNT: WTDAT and WTCNT are 32-bit registers. With these
> + * 32-bit registers, larger values will 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)
> @@ -131,6 +136,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_CNT                    BIT(6)
>
>  /* These quirks require that we have a PMU register map */
>  #define QUIRKS_HAVE_PMUREG \
> @@ -199,6 +205,7 @@ struct s3c2410_wdt {
>         struct notifier_block   freq_transition;
>         const struct s3c2410_wdt_variant *drv_data;
>         struct regmap *pmureg;
> +       u32 max_cnt;
>  };
>
>  static const struct s3c2410_wdt_variant drv_data_s3c2410 = {
> @@ -412,7 +419,7 @@ static inline unsigned int s3c2410wdt_max_timeout(struct s3c2410_wdt *wdt)
>  {
>         const unsigned long freq = s3c2410wdt_get_freq(wdt);
>         //(S3C2410_WTCON_PRESCALE_MAX + 1) * S3C2410_WTCON_MAXDIV = 0x8000
> -       u64 t_max = div64_ul((u64)S3C2410_WTCNT_MAXCNT * 0x8000, freq);
> +       u64 t_max = div64_ul((u64)wdt->max_cnt * 0x8000, freq);
>

If you rework the hard-coded 0x8000 value w.r.t. my comments for the
previous patch, this should be changed accordingly too. So either:

    u32 div_max = (S3C2410_WTCON_PRESCALE_MAX + 1) * S3C2410_WTCON_MAXDIV;
    u64 t_max = div64_ul((u64)wdt->max_cnt * div_max, freq);

...or...

    u32 cnt_max = (u64)wdt->max_cnt * (S3C2410_WTCON_PRESCALE_MAX + 1) *
                  S3C2410_WTCON_MAXDIV;
    u64 t_max = div64_ul(cnt_max, freq);

Apart from that, looks good to me:

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

>         if (t_max > UINT_MAX)
>                 t_max = UINT_MAX;
> @@ -571,7 +578,7 @@ 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;
>
> @@ -581,7 +588,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,
> @@ -589,8 +596,8 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd,
>            actually make this value
>         */
>
> -       if (count >= 0x10000) {
> -               divisor = DIV_ROUND_UP(count, 0xffff);
> +       if (count > wdt->max_cnt) {
> +               divisor = DIV_ROUND_UP(count, wdt->max_cnt);
>
>                 if (divisor > S3C2410_WTCON_PRESCALE_MAX + 1) {
>                         dev_err(wdt->dev, "timeout %d too big\n", timeout);
> @@ -598,7 +605,7 @@ static int s3c2410wdt_set_heartbeat(struct watchdog_device *wdd,
>                 }
>         }
>
> -       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);
> @@ -806,6 +813,11 @@ static int s3c2410wdt_probe(struct platform_device *pdev)
>         if (IS_ERR(wdt->src_clk))
>                 return dev_err_probe(dev, PTR_ERR(wdt->src_clk), "failed to get source clock\n");
>
> +       if (wdt->drv_data->quirks & QUIRK_HAS_32BIT_CNT)
> +               wdt->max_cnt = S3C2410_WTCNT_MAXCNT_32;
> +       else
> +               wdt->max_cnt = S3C2410_WTCNT_MAXCNT_16;
> +
>         wdt->wdt_device.min_timeout = 1;
>         wdt->wdt_device.max_timeout = s3c2410wdt_max_timeout(wdt);
>
> --
> 2.25.1
>

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

* Re: [PATCH v5 5/5] watchdog: s3c2410_wdt: exynosautov9: Enable supported features
  2025-08-06  6:55     ` [PATCH v5 5/5] watchdog: s3c2410_wdt: exynosautov9: Enable supported features Sangwook Shin
@ 2025-08-06 18:07       ` Sam Protsenko
  0 siblings, 0 replies; 10+ messages in thread
From: Sam Protsenko @ 2025-08-06 18:07 UTC (permalink / raw)
  To: Sangwook Shin
  Cc: krzk, alim.akhtar, wim, linux, dongil01.park, khwan.seo,
	linux-arm-kernel, linux-samsung-soc, linux-watchdog, linux-kernel

On Wed, Aug 6, 2025 at 2:00 AM Sangwook Shin <sw617.shin@samsung.com> wrote:
>
> Enable supported features for ExynosAutov9 SoC.
> - QUIRK_HAS_DBGACK_BIT
> - QUIRK_HAS_32BIT_CNT
>
> Signed-off-by: Sangwook Shin <sw617.shin@samsung.com>
> ---

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

>  drivers/watchdog/s3c2410_wdt.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index 541870b4d01a..f25563585983 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -306,7 +306,8 @@ static const struct s3c2410_wdt_variant drv_data_exynosautov9_cl0 = {
>         .cnt_en_reg = EXYNOS850_CLUSTER0_NONCPU_OUT,
>         .cnt_en_bit = 7,
>         .quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET |
> -                 QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN,
> +                 QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN |
> +                 QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_32BIT_CNT,
>  };
>
>  static const struct s3c2410_wdt_variant drv_data_exynosautov9_cl1 = {
> @@ -318,7 +319,8 @@ static const struct s3c2410_wdt_variant drv_data_exynosautov9_cl1 = {
>         .cnt_en_reg = EXYNOSAUTOV9_CLUSTER1_NONCPU_OUT,
>         .cnt_en_bit = 7,
>         .quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET |
> -                 QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN,
> +                 QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN |
> +                 QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_32BIT_CNT,
>  };
>
>  static const struct s3c2410_wdt_variant drv_data_gs101_cl0 = {
> --
> 2.25.1
>

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

* Re: [PATCH v5 4/5] watchdog: s3c2410_wdt: exynosautov920: Enable QUIRK_HAS_32BIT_CNT
  2025-08-06  6:55     ` [PATCH v5 4/5] watchdog: s3c2410_wdt: exynosautov920: Enable QUIRK_HAS_32BIT_CNT Sangwook Shin
@ 2025-08-06 18:08       ` Sam Protsenko
  0 siblings, 0 replies; 10+ messages in thread
From: Sam Protsenko @ 2025-08-06 18:08 UTC (permalink / raw)
  To: Sangwook Shin
  Cc: krzk, alim.akhtar, wim, linux, dongil01.park, khwan.seo,
	linux-arm-kernel, linux-samsung-soc, linux-watchdog, linux-kernel

On Wed, Aug 6, 2025 at 2:00 AM Sangwook Shin <sw617.shin@samsung.com> wrote:
>
> Enable QUIRK_HAS_32BIT_CNT to ExynosAutov920 SoC which has 32-bit WTCNT.
>
> Signed-off-by: Sangwook Shin <sw617.shin@samsung.com>
> ---

Reviewed-by: Sam Protsenko <semen.protsenko@linaro.org>

>  drivers/watchdog/s3c2410_wdt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/watchdog/s3c2410_wdt.c b/drivers/watchdog/s3c2410_wdt.c
> index 673ab6768688..541870b4d01a 100644
> --- a/drivers/watchdog/s3c2410_wdt.c
> +++ b/drivers/watchdog/s3c2410_wdt.c
> @@ -357,7 +357,7 @@ static const struct s3c2410_wdt_variant drv_data_exynosautov920_cl0 = {
>         .cnt_en_bit = 8,
>         .quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET |
>                   QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN |
> -                 QUIRK_HAS_DBGACK_BIT,
> +                 QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_32BIT_CNT,
>  };
>
>  static const struct s3c2410_wdt_variant drv_data_exynosautov920_cl1 = {
> @@ -370,7 +370,7 @@ static const struct s3c2410_wdt_variant drv_data_exynosautov920_cl1 = {
>         .cnt_en_bit = 8,
>         .quirks = QUIRK_HAS_WTCLRINT_REG | QUIRK_HAS_PMU_MASK_RESET |
>                   QUIRK_HAS_PMU_RST_STAT | QUIRK_HAS_PMU_CNT_EN |
> -                 QUIRK_HAS_DBGACK_BIT,
> +                 QUIRK_HAS_DBGACK_BIT | QUIRK_HAS_32BIT_CNT,
>  };
>
>  static const struct of_device_id s3c2410_wdt_match[] = {
> --
> 2.25.1
>

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

end of thread, other threads:[~2025-08-06 18:08 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20250806070029epcas2p18a80cf9215e331012e0ed4890cdae700@epcas2p1.samsung.com>
2025-08-06  6:55 ` [PATCH v5 0/5] Increase max timeout value of s3c2410 watchdog Sangwook Shin
     [not found]   ` <CGME20250806070029epcas2p3a937682ba1bdad1317ef62aab8f826ae@epcas2p3.samsung.com>
2025-08-06  6:55     ` [PATCH v5 1/5] watchdog: s3c2410_wdt: Replace hardcoded values with macro definitions Sangwook Shin
     [not found]   ` <CGME20250806070029epcas2p1f764301b115b766f88b422b0dbdd68f1@epcas2p1.samsung.com>
2025-08-06  6:55     ` [PATCH v5 2/5] watchdog: s3c2410_wdt: Fix max_timeout being calculated larger Sangwook Shin
2025-08-06 17:17       ` Sam Protsenko
     [not found]   ` <CGME20250806070030epcas2p24cc79e6dd3e505afd980fc05ee9e4f19@epcas2p2.samsung.com>
2025-08-06  6:55     ` [PATCH v5 3/5] watchdog: s3c2410_wdt: Increase max timeout value of watchdog Sangwook Shin
2025-08-06 18:06       ` Sam Protsenko
     [not found]   ` <CGME20250806070030epcas2p3f2b0c36b6c492629e50e76be469b2db3@epcas2p3.samsung.com>
2025-08-06  6:55     ` [PATCH v5 4/5] watchdog: s3c2410_wdt: exynosautov920: Enable QUIRK_HAS_32BIT_CNT Sangwook Shin
2025-08-06 18:08       ` Sam Protsenko
     [not found]   ` <CGME20250806070030epcas2p4f9b779bd9fde57b4bfea958efdf62cf4@epcas2p4.samsung.com>
2025-08-06  6:55     ` [PATCH v5 5/5] watchdog: s3c2410_wdt: exynosautov9: Enable supported features Sangwook Shin
2025-08-06 18:07       ` Sam Protsenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).