Thread (17 messages) 17 messages, 3 authors, 2025-08-06

Re: [PATCH v4 2/4] watchdog: s3c2410_wdt: Fix max_timeout being calculated larger

From: Sam Protsenko <semen.protsenko@linaro.org>
Date: 2025-08-05 22:53:24
Also in: linux-samsung-soc, linux-watchdog, lkml

On Tue, Aug 5, 2025 at 2:26 AM [off-list ref] wrote:
On Tuesday, August 5, 2025 at 2:03 PM Sam Protsenko [off-list ref] wrote:
quoted
quoted
quoted
+       u64 t_max = n_max / freq;
Make sure this compiles on 32-bit builds.
Can you please elaborate what might be the possible problem -- just
curious? I admit I never though about 32-bit case when writing that code,
but don't see any immediate issues with that too.
In my opinion, it seems that Gunter Reck's explanation is correct.
I've found out that the error of "undefined reference to '__aeabi_uldivmod'" may occur when compiling new code on a 32-bit architecture.
Indeed. I was able to reproduce that behavior when building for ARM32 too.
quoted hunk ↗ jump to hunk
If you don't mind, I would like to proceed with maintaining the previous revision below.
From my perspective, this approach appears to be the most reasonable solution for supporting both 32-bit and 64-bit architectures.
@@ -411,8 +411,8 @@ static inline unsigned int s3c2410wdt_max_timeout(struct s3c2410_wdt *wdt)
 {
        const unsigned long freq = s3c2410wdt_get_freq(wdt);

-       return S3C2410_WTCNT_MAXCNT / (freq / (S3C2410_WTCON_PRESCALE_MAX + 1)
-                                      / S3C2410_WTCON_MAXDIV);
+       return S3C2410_WTCNT_MAXCNT / DIV_ROUND_UP(freq,
+               (S3C2410_WTCON_PRESCALE_MAX + 1) * S3C2410_WTCON_MAXDIV);
 }
I don't mind, although it's quite easy to fix the code I suggested by
replacing this line:

    u64 t_max = n_max / freq;

with this one:

    u64 t_max = div64_ul(n_max, freq);

from <math64.h>, as Guenter suggested. But I'm totally fine with your
implementation as well.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help