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@xxxxxxxxxxx>
Signed-off-by: Kyunghwan Seo <khwan.seo@xxxxxxxxxxx>
---
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);
