Messages in this thread | | | From | Baolin Wang <> | Date | Fri, 4 May 2018 14:23:13 +0800 | Subject | Re: [PATCH 2/2] MIPS: Convert update_persistent_clock() to update_persistent_clock64() |
| |
On 4 May 2018 at 06:31, Arnd Bergmann <arnd@arndb.de> wrote: > On Wed, May 2, 2018 at 10:53 PM, Baolin Wang <baolin.wang@linaro.org> wrote: >> diff --git a/arch/mips/include/asm/time.h b/arch/mips/include/asm/time.h >> index 17d4cd2..c4e2a1a 100644 >> --- a/arch/mips/include/asm/time.h >> +++ b/arch/mips/include/asm/time.h >> @@ -27,8 +27,8 @@ >> * rtc_mips_set_mmss - similar to rtc_set_time, but only min and sec need >> * to be set. Used by RTC sync-up. >> */ >> -extern int rtc_mips_set_time(unsigned long); >> -extern int rtc_mips_set_mmss(unsigned long); >> +extern int rtc_mips_set_time(time64_t); >> +extern int rtc_mips_set_mmss(time64_t); >> > > I think these should just get removed, and each implementation replaced > with a direct update_persistent_clock64() function.
I thought this was one minor modification that will reduce the risk of introducing other issues, but as you suggested we can do some complete cleanup by removing set_mmss/set_time. OK, I will do that.
> >> -int update_persistent_clock(struct timespec now) >> +int update_persistent_clock64(struct timespec64 now) >> { >> return rtc_mips_set_mmss(now.tv_sec); >> } > > And this one also removed
Sure.
> >> @@ -69,7 +69,7 @@ int proc_dolasatrtc(struct ctl_table *table, int write, >> if (rtctmp < 0) >> rtctmp = 0; >> } >> - r = proc_dointvec(table, write, buffer, lenp, ppos); >> + r = proc_doulongvec_minmax(table, write, buffer, lenp, ppos); >> if (r) >> return r; >> >> @@ -224,7 +224,7 @@ int proc_lasat_prid(struct ctl_table *table, int write, >> { >> .procname = "rtc", >> .data = &rtctmp, >> - .maxlen = sizeof(int), >> + .maxlen = sizeof(time64_t), >> .mode = 0644, >> .proc_handler = proc_dolasatrtc, >> }, > > Something seems wrong here: time64_t is not the same as 'unsigned long', > and the 'rtctmp' variable is still 'unsigned int'. Not sure what the right fix > would be (we don't seem to have a sysctl helper for s64), but the change > here makes it worse.
After checking again, I agree with you. So I will keep the original code here.
-- Baolin.wang Best Regards
| |