| Message ID | CAK8P3a3+86Me3H-EfVmpFGY2cpFv0zo-Q7+wTDmeOJ62PTfs9A@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Apr 20, 2018 at 05:19:43PM +0200, Arnd Bergmann wrote: > On Thu, Apr 19, 2018 at 8:06 AM, Baolin Wang <baolin.wang@linaro.org> wrote: > > The read_persistent_clock() uses a timespec, which is not year 2038 safe > > on 32bit systems. Moreover on sh platform, we have RTC drivers that can > > be used to compensate the system suspend time. Thus we can remove the > > read_persistent_clock() safely. > > > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > > Hi Baolin and sh maintainers. > > I have done a similar patch but never got around to posting it. Please > see my patch below. > > Note that the patch is whitespace broken, I can send a proper > version to the sh maintainers if they want to merge it, or I can > apply my patch to my y2038 tree myself. > > Arnd > > commit cade6829ca223d9761863e74595d677b3dc14ecd > Author: Arnd Bergmann <arnd@arndb.de> > Date: Wed Jan 24 16:18:50 2018 +0100 > > sh: remove unused rtc_sh_get/set_time infrastructure > > All platforms are now converted to RTC drivers, so this has become > obsolete. The board_time_init() callback still has one caller, but > could otherwise also get killed. > > This removes one more usage of the deprecated timespec structure, > which overflows in y2038. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> In principle this looks correct, but I don't think all the drivers have been converted. I did a quick grep for where board_time_init is set, and found mach-dreamcast and mach-sh03 boards have rtc drivers that set rtc_sh_[gs]et_time pointers. These need to be converted to proper rtc drivers, I think. The of-generic board also uses board_time_init, but only to call the arch-independent timer_probe which uses devicetree to get clocksource/clockevent devices. This probe should probably be moved to the place where board_timer_init is called, right? Rich > diff --git a/arch/sh/include/asm/rtc.h b/arch/sh/include/asm/rtc.h > index c63555ee1255..fe55fbb181aa 100644 > --- a/arch/sh/include/asm/rtc.h > +++ b/arch/sh/include/asm/rtc.h > @@ -4,8 +4,6 @@ > > void time_init(void); > extern void (*board_time_init)(void); > -extern void (*rtc_sh_get_time)(struct timespec *); > -extern int (*rtc_sh_set_time)(const time_t); > > #define RTC_CAP_4_DIGIT_YEAR (1 << 0) > > diff --git a/arch/sh/kernel/time.c b/arch/sh/kernel/time.c > index fcd5e41977d1..eb0a91270499 100644 > --- a/arch/sh/kernel/time.c > +++ b/arch/sh/kernel/time.c > @@ -22,75 +22,6 @@ > #include <asm/clock.h> > #include <asm/rtc.h> > > -/* Dummy RTC ops */ > -static void null_rtc_get_time(struct timespec *tv) > -{ > - tv->tv_sec = mktime(2000, 1, 1, 0, 0, 0); > - tv->tv_nsec = 0; > -} > - > -static int null_rtc_set_time(const time_t secs) > -{ > - return 0; > -} > - > -void (*rtc_sh_get_time)(struct timespec *) = null_rtc_get_time; > -int (*rtc_sh_set_time)(const time_t) = null_rtc_set_time; > - > -void read_persistent_clock(struct timespec *ts) > -{ > - rtc_sh_get_time(ts); > -} > - > -#ifdef CONFIG_GENERIC_CMOS_UPDATE > -int update_persistent_clock(struct timespec now) > -{ > - return rtc_sh_set_time(now.tv_sec); > -} > -#endif > - > -static int rtc_generic_get_time(struct device *dev, struct rtc_time *tm) > -{ > - struct timespec tv; > - > - rtc_sh_get_time(&tv); > - rtc_time_to_tm(tv.tv_sec, tm); > - return 0; > -} > - > - &rtc_generic_ops, > - sizeof(rtc_generic_ops)); > - > - > - return PTR_ERR_OR_ZERO(pdev); > -} > -device_initcall(rtc_generic_init); > - > void (*board_time_init)(void); > > static void __init sh_late_time_init(void) -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 20, 2018 at 5:32 PM, Rich Felker <dalias@libc.org> wrote: > On Fri, Apr 20, 2018 at 05:19:43PM +0200, Arnd Bergmann wrote: >> On Thu, Apr 19, 2018 at 8:06 AM, Baolin Wang <baolin.wang@linaro.org> wrote: >> > The read_persistent_clock() uses a timespec, which is not year 2038 safe >> > on 32bit systems. Moreover on sh platform, we have RTC drivers that can >> > be used to compensate the system suspend time. Thus we can remove the >> > read_persistent_clock() safely. >> > >> > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> >> >> Hi Baolin and sh maintainers. >> >> I have done a similar patch but never got around to posting it. Please >> see my patch below. >> >> Note that the patch is whitespace broken, I can send a proper >> version to the sh maintainers if they want to merge it, or I can >> apply my patch to my y2038 tree myself. >> >> Arnd >> >> commit cade6829ca223d9761863e74595d677b3dc14ecd >> Author: Arnd Bergmann <arnd@arndb.de> >> Date: Wed Jan 24 16:18:50 2018 +0100 >> >> sh: remove unused rtc_sh_get/set_time infrastructure >> >> All platforms are now converted to RTC drivers, so this has become >> obsolete. The board_time_init() callback still has one caller, but >> could otherwise also get killed. >> >> This removes one more usage of the deprecated timespec structure, >> which overflows in y2038. >> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > In principle this looks correct, but I don't think all the drivers > have been converted. I did a quick grep for where board_time_init is > set, and found mach-dreamcast and mach-sh03 boards have rtc drivers > that set rtc_sh_[gs]et_time pointers. These need to be converted to > proper rtc drivers, I think. Ah, of course. I fished the one patch out of my stash of unsubmitted patches, but missed the fact that I had three of them, the other ones converting sh03 and dreamcast, respectively. ;-) Let me send all three for proper review then. > The of-generic board also uses board_time_init, but only to call the > arch-independent timer_probe which uses devicetree to get > clocksource/clockevent devices. This probe should probably be moved to > the place where board_timer_init is called, right? Good idea. My patches don't do that, but that seems like a good follow-up. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/sh/include/asm/rtc.h b/arch/sh/include/asm/rtc.h index c63555ee1255..fe55fbb181aa 100644 --- a/arch/sh/include/asm/rtc.h +++ b/arch/sh/include/asm/rtc.h @@ -4,8 +4,6 @@ void time_init(void); extern void (*board_time_init)(void); -extern void (*rtc_sh_get_time)(struct timespec *); -extern int (*rtc_sh_set_time)(const time_t); #define RTC_CAP_4_DIGIT_YEAR (1 << 0) diff --git a/arch/sh/kernel/time.c b/arch/sh/kernel/time.c index fcd5e41977d1..eb0a91270499 100644 --- a/arch/sh/kernel/time.c +++ b/arch/sh/kernel/time.c @@ -22,75 +22,6 @@ #include <asm/clock.h> #include <asm/rtc.h> -/* Dummy RTC ops */ -static void null_rtc_get_time(struct timespec *tv) -{ - tv->tv_sec = mktime(2000, 1, 1, 0, 0, 0); - tv->tv_nsec = 0; -} - -static int null_rtc_set_time(const time_t secs) -{ - return 0; -} - -void (*rtc_sh_get_time)(struct timespec *) = null_rtc_get_time; -int (*rtc_sh_set_time)(const time_t) = null_rtc_set_time; - -void read_persistent_clock(struct timespec *ts) -{ - rtc_sh_get_time(ts); -} - -#ifdef CONFIG_GENERIC_CMOS_UPDATE -int update_persistent_clock(struct timespec now) -{ - return rtc_sh_set_time(now.tv_sec); -} -#endif - -static int rtc_generic_get_time(struct device *dev, struct rtc_time *tm) -{ - struct timespec tv; - - rtc_sh_get_time(&tv); - rtc_time_to_tm(tv.tv_sec, tm); - return 0; -} - - &rtc_generic_ops, - sizeof(rtc_generic_ops)); - - - return PTR_ERR_OR_ZERO(pdev); -} -device_initcall(rtc_generic_init); - void (*board_time_init)(void); static void __init sh_late_time_init(void) -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html