* Re: [PATCH] sh: time: Remove the read_persistent_clock()
2018-04-19 6:06 [PATCH] sh: time: Remove the read_persistent_clock() Baolin Wang
@ 2018-04-20 15:19 ` Arnd Bergmann
2018-04-20 15:32 ` Rich Felker
2018-04-20 15:43 ` Arnd Bergmann
2 siblings, 0 replies; 4+ messages in thread
From: Arnd Bergmann @ 2018-04-20 15:19 UTC (permalink / raw)
To: linux-sh
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>
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)
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] sh: time: Remove the read_persistent_clock()
2018-04-19 6:06 [PATCH] sh: time: Remove the read_persistent_clock() Baolin Wang
2018-04-20 15:19 ` Arnd Bergmann
@ 2018-04-20 15:32 ` Rich Felker
2018-04-20 15:43 ` Arnd Bergmann
2 siblings, 0 replies; 4+ messages in thread
From: Rich Felker @ 2018-04-20 15:32 UTC (permalink / raw)
To: linux-sh
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)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] sh: time: Remove the read_persistent_clock()
2018-04-19 6:06 [PATCH] sh: time: Remove the read_persistent_clock() Baolin Wang
2018-04-20 15:19 ` Arnd Bergmann
2018-04-20 15:32 ` Rich Felker
@ 2018-04-20 15:43 ` Arnd Bergmann
2 siblings, 0 replies; 4+ messages in thread
From: Arnd Bergmann @ 2018-04-20 15:43 UTC (permalink / raw)
To: linux-sh
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
^ permalink raw reply [flat|nested] 4+ messages in thread