2018-05-04 07:09:17

by Baolin Wang

[permalink] [raw]
Subject: [PATCH v2 1/2] MIPS: Convert read_persistent_clock() to read_persistent_clock64()

Since struct timespec is not y2038 safe on 32bit machines, this patch
converts read_persistent_clock() to read_persistent_clock64() using
struct timespec64, as well as converting mktime() to mktime64().

Signed-off-by: Baolin Wang <[email protected]>
---
Changes since v1:
- No updates.
---
arch/mips/dec/time.c | 4 ++--
arch/mips/include/asm/mc146818-time.h | 4 ++--
arch/mips/lasat/ds1603.c | 2 +-
arch/mips/loongson64/common/time.c | 2 +-
arch/mips/mti-malta/malta-time.c | 2 +-
arch/mips/sibyte/swarm/rtc_m41t81.c | 4 ++--
arch/mips/sibyte/swarm/rtc_xicor1241.c | 4 ++--
arch/mips/sibyte/swarm/setup.c | 10 +++++-----
8 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/arch/mips/dec/time.c b/arch/mips/dec/time.c
index a2a150e..9e992cf 100644
--- a/arch/mips/dec/time.c
+++ b/arch/mips/dec/time.c
@@ -19,7 +19,7 @@
#include <asm/dec/ioasic.h>
#include <asm/dec/machtype.h>

-void read_persistent_clock(struct timespec *ts)
+void read_persistent_clock64(struct timespec64 *ts)
{
unsigned int year, mon, day, hour, min, sec, real_year;
unsigned long flags;
@@ -54,7 +54,7 @@ void read_persistent_clock(struct timespec *ts)

year += real_year - 72 + 2000;

- ts->tv_sec = mktime(year, mon, day, hour, min, sec);
+ ts->tv_sec = mktime64(year, mon, day, hour, min, sec);
ts->tv_nsec = 0;
}

diff --git a/arch/mips/include/asm/mc146818-time.h b/arch/mips/include/asm/mc146818-time.h
index 9e1ad26..cbf5cec 100644
--- a/arch/mips/include/asm/mc146818-time.h
+++ b/arch/mips/include/asm/mc146818-time.h
@@ -86,7 +86,7 @@ static inline int mc146818_set_rtc_mmss(unsigned long nowtime)
return retval;
}

-static inline unsigned long mc146818_get_cmos_time(void)
+static inline time64_t mc146818_get_cmos_time(void)
{
unsigned int year, mon, day, hour, min, sec;
unsigned long flags;
@@ -113,7 +113,7 @@ static inline unsigned long mc146818_get_cmos_time(void)
spin_unlock_irqrestore(&rtc_lock, flags);
year = mc146818_decode_year(year);

- return mktime(year, mon, day, hour, min, sec);
+ return mktime64(year, mon, day, hour, min, sec);
}

#endif /* __ASM_MC146818_TIME_H */
diff --git a/arch/mips/lasat/ds1603.c b/arch/mips/lasat/ds1603.c
index 8bd5cf8..d75c887 100644
--- a/arch/mips/lasat/ds1603.c
+++ b/arch/mips/lasat/ds1603.c
@@ -136,7 +136,7 @@ static void rtc_end_op(void)
lasat_ndelay(1000);
}

-void read_persistent_clock(struct timespec *ts)
+void read_persistent_clock64(struct timespec64 *ts)
{
unsigned long word;
unsigned long flags;
diff --git a/arch/mips/loongson64/common/time.c b/arch/mips/loongson64/common/time.c
index e1a5382a..0ba53c5 100644
--- a/arch/mips/loongson64/common/time.c
+++ b/arch/mips/loongson64/common/time.c
@@ -29,7 +29,7 @@ void __init plat_time_init(void)
#endif
}

-void read_persistent_clock(struct timespec *ts)
+void read_persistent_clock64(struct timespec64 *ts)
{
ts->tv_sec = mc146818_get_cmos_time();
ts->tv_nsec = 0;
diff --git a/arch/mips/mti-malta/malta-time.c b/arch/mips/mti-malta/malta-time.c
index 66c8667..d22b7ed 100644
--- a/arch/mips/mti-malta/malta-time.c
+++ b/arch/mips/mti-malta/malta-time.c
@@ -134,7 +134,7 @@ static void __init estimate_frequencies(void)
}
}

-void read_persistent_clock(struct timespec *ts)
+void read_persistent_clock64(struct timespec64 *ts)
{
ts->tv_sec = mc146818_get_cmos_time();
ts->tv_nsec = 0;
diff --git a/arch/mips/sibyte/swarm/rtc_m41t81.c b/arch/mips/sibyte/swarm/rtc_m41t81.c
index e624664..aa27a22 100644
--- a/arch/mips/sibyte/swarm/rtc_m41t81.c
+++ b/arch/mips/sibyte/swarm/rtc_m41t81.c
@@ -188,7 +188,7 @@ int m41t81_set_time(unsigned long t)
return 0;
}

-unsigned long m41t81_get_time(void)
+time64_t m41t81_get_time(void)
{
unsigned int year, mon, day, hour, min, sec;
unsigned long flags;
@@ -218,7 +218,7 @@ unsigned long m41t81_get_time(void)

year += 2000;

- return mktime(year, mon, day, hour, min, sec);
+ return mktime64(year, mon, day, hour, min, sec);
}

int m41t81_probe(void)
diff --git a/arch/mips/sibyte/swarm/rtc_xicor1241.c b/arch/mips/sibyte/swarm/rtc_xicor1241.c
index 50a82c4..a2121c1 100644
--- a/arch/mips/sibyte/swarm/rtc_xicor1241.c
+++ b/arch/mips/sibyte/swarm/rtc_xicor1241.c
@@ -168,7 +168,7 @@ int xicor_set_time(unsigned long t)
return 0;
}

-unsigned long xicor_get_time(void)
+time64_t xicor_get_time(void)
{
unsigned int year, mon, day, hour, min, sec, y2k;
unsigned long flags;
@@ -201,7 +201,7 @@ unsigned long xicor_get_time(void)

year += (y2k * 100);

- return mktime(year, mon, day, hour, min, sec);
+ return mktime64(year, mon, day, hour, min, sec);
}

int xicor_probe(void)
diff --git a/arch/mips/sibyte/swarm/setup.c b/arch/mips/sibyte/swarm/setup.c
index 494fb0a..7073940 100644
--- a/arch/mips/sibyte/swarm/setup.c
+++ b/arch/mips/sibyte/swarm/setup.c
@@ -58,11 +58,11 @@

extern int xicor_probe(void);
extern int xicor_set_time(unsigned long);
-extern unsigned long xicor_get_time(void);
+extern time64_t xicor_get_time(void);

extern int m41t81_probe(void);
extern int m41t81_set_time(unsigned long);
-extern unsigned long m41t81_get_time(void);
+extern time64_t m41t81_get_time(void);

const char *get_system_type(void)
{
@@ -87,9 +87,9 @@ enum swarm_rtc_type {

enum swarm_rtc_type swarm_rtc_type;

-void read_persistent_clock(struct timespec *ts)
+void read_persistent_clock64(struct timespec64 *ts)
{
- unsigned long sec;
+ time64_t sec;

switch (swarm_rtc_type) {
case RTC_XICOR:
@@ -102,7 +102,7 @@ void read_persistent_clock(struct timespec *ts)

case RTC_NONE:
default:
- sec = mktime(2000, 1, 1, 0, 0, 0);
+ sec = mktime64(2000, 1, 1, 0, 0, 0);
break;
}
ts->tv_sec = sec;
--
1.7.9.5



2018-05-04 07:09:30

by Baolin Wang

[permalink] [raw]
Subject: [PATCH v2 2/2] MIPS: Convert update_persistent_clock() to update_persistent_clock64()

Since struct timespec is not y2038 safe on 32bit machines, this patch
converts update_persistent_clock() to update_persistent_clock64() using
struct timespec64.

The rtc_mips_set_time() and rtc_mips_set_mmss() interfaces were using
'unsigned long' type that is not y2038 safe on 32bit machines, moreover
there is only one platform implementing rtc_mips_set_time() and two
platforms implementing rtc_mips_set_mmss(), so we can just make them each
implement update_persistent_clock64() directly, to get that helper out
of the common mips code by removing rtc_mips_set_time() and
rtc_mips_set_mmss() interfaces.

Signed-off-by: Baolin Wang <[email protected]>
---
Changes since v1:
- Remove rtc_mips_set_time() and rtc_mips_set_mmss() interfaces.
---
arch/mips/dec/time.c | 5 +++--
arch/mips/include/asm/time.h | 9 ---------
arch/mips/kernel/time.c | 15 ---------------
arch/mips/lasat/ds1603.c | 5 +++--
arch/mips/lasat/sysctl.c | 8 ++++++--
arch/mips/sibyte/swarm/rtc_m41t81.c | 4 ++--
arch/mips/sibyte/swarm/rtc_xicor1241.c | 4 ++--
arch/mips/sibyte/swarm/setup.c | 8 +++++---
8 files changed, 21 insertions(+), 37 deletions(-)

diff --git a/arch/mips/dec/time.c b/arch/mips/dec/time.c
index 9e992cf..934db6f 100644
--- a/arch/mips/dec/time.c
+++ b/arch/mips/dec/time.c
@@ -59,14 +59,15 @@ void read_persistent_clock64(struct timespec64 *ts)
}

/*
- * In order to set the CMOS clock precisely, rtc_mips_set_mmss has to
+ * In order to set the CMOS clock precisely, update_persistent_clock64 has to
* be called 500 ms after the second nowtime has started, because when
* nowtime is written into the registers of the CMOS clock, it will
* jump to the next second precisely 500 ms later. Check the Dallas
* DS1287 data sheet for details.
*/
-int rtc_mips_set_mmss(unsigned long nowtime)
+int update_persistent_clock64(struct timespec64 now)
{
+ time64_t nowtime = now.tv_sec;
int retval = 0;
int real_seconds, real_minutes, cmos_minutes;
unsigned char save_control, save_freq_select;
diff --git a/arch/mips/include/asm/time.h b/arch/mips/include/asm/time.h
index 17d4cd2..b85ec64 100644
--- a/arch/mips/include/asm/time.h
+++ b/arch/mips/include/asm/time.h
@@ -22,15 +22,6 @@
extern spinlock_t rtc_lock;

/*
- * RTC ops. By default, they point to weak no-op RTC functions.
- * rtc_mips_set_time - reverse the above translation and set time to RTC.
- * 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);
-
-/*
* board specific routines required by time_init().
*/
extern void plat_time_init(void);
diff --git a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
index a6ebc81..bfe02de 100644
--- a/arch/mips/kernel/time.c
+++ b/arch/mips/kernel/time.c
@@ -34,21 +34,6 @@
DEFINE_SPINLOCK(rtc_lock);
EXPORT_SYMBOL(rtc_lock);

-int __weak rtc_mips_set_time(unsigned long sec)
-{
- return -ENODEV;
-}
-
-int __weak rtc_mips_set_mmss(unsigned long nowtime)
-{
- return rtc_mips_set_time(nowtime);
-}
-
-int update_persistent_clock(struct timespec now)
-{
- return rtc_mips_set_mmss(now.tv_sec);
-}
-
static int null_perf_irq(void)
{
return 0;
diff --git a/arch/mips/lasat/ds1603.c b/arch/mips/lasat/ds1603.c
index d75c887..061815e 100644
--- a/arch/mips/lasat/ds1603.c
+++ b/arch/mips/lasat/ds1603.c
@@ -98,7 +98,7 @@ static void rtc_write_byte(unsigned int byte)
}
}

-static void rtc_write_word(unsigned long word)
+static void rtc_write_word(time64_t word)
{
int i;

@@ -152,8 +152,9 @@ void read_persistent_clock64(struct timespec64 *ts)
ts->tv_nsec = 0;
}

-int rtc_mips_set_mmss(unsigned long time)
+int update_persistent_clock64(struct timespec64 now)
{
+ time64_t time = now.tv_sec;
unsigned long flags;

spin_lock_irqsave(&rtc_lock, flags);
diff --git a/arch/mips/lasat/sysctl.c b/arch/mips/lasat/sysctl.c
index 6f74224..76f7b62 100644
--- a/arch/mips/lasat/sysctl.c
+++ b/arch/mips/lasat/sysctl.c
@@ -73,8 +73,12 @@ int proc_dolasatrtc(struct ctl_table *table, int write,
if (r)
return r;

- if (write)
- rtc_mips_set_mmss(rtctmp);
+ if (write) {
+ ts.tv_sec = rtctmp;
+ ts.tv_nsec = 0;
+
+ update_persistent_clock64(ts);
+ }

return 0;
}
diff --git a/arch/mips/sibyte/swarm/rtc_m41t81.c b/arch/mips/sibyte/swarm/rtc_m41t81.c
index aa27a22..4ac8ccd 100644
--- a/arch/mips/sibyte/swarm/rtc_m41t81.c
+++ b/arch/mips/sibyte/swarm/rtc_m41t81.c
@@ -141,13 +141,13 @@ static int m41t81_write(uint8_t addr, int b)
return 0;
}

-int m41t81_set_time(unsigned long t)
+int m41t81_set_time(time64_t t)
{
struct rtc_time tm;
unsigned long flags;

/* Note we don't care about the century */
- rtc_time_to_tm(t, &tm);
+ rtc_time64_to_tm(t, &tm);

/*
* Note the write order matters as it ensures the correctness.
diff --git a/arch/mips/sibyte/swarm/rtc_xicor1241.c b/arch/mips/sibyte/swarm/rtc_xicor1241.c
index a2121c1..2dcaaa7 100644
--- a/arch/mips/sibyte/swarm/rtc_xicor1241.c
+++ b/arch/mips/sibyte/swarm/rtc_xicor1241.c
@@ -109,13 +109,13 @@ static int xicor_write(uint8_t addr, int b)
}
}

-int xicor_set_time(unsigned long t)
+int xicor_set_time(time64_t t)
{
struct rtc_time tm;
int tmp;
unsigned long flags;

- rtc_time_to_tm(t, &tm);
+ rtc_time64_to_tm(t, &tm);
tm.tm_year += 1900;

spin_lock_irqsave(&rtc_lock, flags);
diff --git a/arch/mips/sibyte/swarm/setup.c b/arch/mips/sibyte/swarm/setup.c
index 7073940..152ca71 100644
--- a/arch/mips/sibyte/swarm/setup.c
+++ b/arch/mips/sibyte/swarm/setup.c
@@ -57,11 +57,11 @@
#endif

extern int xicor_probe(void);
-extern int xicor_set_time(unsigned long);
+extern int xicor_set_time(time64_t);
extern time64_t xicor_get_time(void);

extern int m41t81_probe(void);
-extern int m41t81_set_time(unsigned long);
+extern int m41t81_set_time(time64_t);
extern time64_t m41t81_get_time(void);

const char *get_system_type(void)
@@ -109,8 +109,10 @@ void read_persistent_clock64(struct timespec64 *ts)
ts->tv_nsec = 0;
}

-int rtc_mips_set_time(unsigned long sec)
+int update_persistent_clock64(struct timespec64 now)
{
+ time64_t sec = now.tv_sec;
+
switch (swarm_rtc_type) {
case RTC_XICOR:
return xicor_set_time(sec);
--
1.7.9.5


2018-05-06 03:07:04

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] MIPS: Convert update_persistent_clock() to update_persistent_clock64()

On Fri, May 4, 2018 at 3:07 AM, Baolin Wang <[email protected]> wrote:
> Since struct timespec is not y2038 safe on 32bit machines, this patch
> converts update_persistent_clock() to update_persistent_clock64() using
> struct timespec64.
>
> The rtc_mips_set_time() and rtc_mips_set_mmss() interfaces were using
> 'unsigned long' type that is not y2038 safe on 32bit machines, moreover
> there is only one platform implementing rtc_mips_set_time() and two
> platforms implementing rtc_mips_set_mmss(), so we can just make them each
> implement update_persistent_clock64() directly, to get that helper out
> of the common mips code by removing rtc_mips_set_time() and
> rtc_mips_set_mmss() interfaces.
>
> Signed-off-by: Baolin Wang <[email protected]>

Looks good overall, but I still found a bug and one minor issue. With
those fixed,

Acked-by: Arnd Bergmann <[email protected]>

> diff --git a/arch/mips/dec/time.c b/arch/mips/dec/time.c
> index 9e992cf..934db6f 100644
> --- a/arch/mips/dec/time.c
> +++ b/arch/mips/dec/time.c
> @@ -59,14 +59,15 @@ void read_persistent_clock64(struct timespec64 *ts)
> }
>
> /*
> - * In order to set the CMOS clock precisely, rtc_mips_set_mmss has to
> + * In order to set the CMOS clock precisely, update_persistent_clock64 has to
> * be called 500 ms after the second nowtime has started, because when
> * nowtime is written into the registers of the CMOS clock, it will
> * jump to the next second precisely 500 ms later. Check the Dallas
> * DS1287 data sheet for details.
> */
> -int rtc_mips_set_mmss(unsigned long nowtime)
> +int update_persistent_clock64(struct timespec64 now)
> {
> + time64_t nowtime = now.tv_sec;
> int retval = 0;
> int real_seconds, real_minutes, cmos_minutes;
> unsigned char save_control, save_freq_select;


It looks like you now get an invalid 64-bit division in here,
you have to change it to either use div_u64_rem() or possibly
time64_to_tm() or rtc_time64_to_tm() (the latter requires
CONFIG_RTC_LIB).

> diff --git a/arch/mips/lasat/ds1603.c b/arch/mips/lasat/ds1603.c
> index d75c887..061815e 100644
> --- a/arch/mips/lasat/ds1603.c
> +++ b/arch/mips/lasat/ds1603.c
> @@ -98,7 +98,7 @@ static void rtc_write_byte(unsigned int byte)
> }
> }
>
> -static void rtc_write_word(unsigned long word)
> +static void rtc_write_word(time64_t word)
> {
> int i;
>

I would say this function should take a 'u32' argument (or keep the
unsigned long) to match the name and implementation, but then have a
type cast in the caller with a comment about the loss of range and overflow
in y2106.

> diff --git a/arch/mips/lasat/sysctl.c b/arch/mips/lasat/sysctl.c
> index 6f74224..76f7b62 100644
> --- a/arch/mips/lasat/sysctl.c
> +++ b/arch/mips/lasat/sysctl.c
> @@ -73,8 +73,12 @@ int proc_dolasatrtc(struct ctl_table *table, int write,
> if (r)
> return r;
>
> - if (write)
> - rtc_mips_set_mmss(rtctmp);
> + if (write) {
> + ts.tv_sec = rtctmp;
> + ts.tv_nsec = 0;
> +
> + update_persistent_clock64(ts);
> + }
>
... and probably also a comment here to explain that we can't actually use
the full 64-bit range because of HW limits.

Arnd

2018-05-07 07:59:24

by Baolin Wang

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] MIPS: Convert update_persistent_clock() to update_persistent_clock64()

On 6 May 2018 at 11:04, Arnd Bergmann <[email protected]> wrote:
> On Fri, May 4, 2018 at 3:07 AM, Baolin Wang <[email protected]> wrote:
>> Since struct timespec is not y2038 safe on 32bit machines, this patch
>> converts update_persistent_clock() to update_persistent_clock64() using
>> struct timespec64.
>>
>> The rtc_mips_set_time() and rtc_mips_set_mmss() interfaces were using
>> 'unsigned long' type that is not y2038 safe on 32bit machines, moreover
>> there is only one platform implementing rtc_mips_set_time() and two
>> platforms implementing rtc_mips_set_mmss(), so we can just make them each
>> implement update_persistent_clock64() directly, to get that helper out
>> of the common mips code by removing rtc_mips_set_time() and
>> rtc_mips_set_mmss() interfaces.
>>
>> Signed-off-by: Baolin Wang <[email protected]>
>
> Looks good overall, but I still found a bug and one minor issue. With
> those fixed,
>
> Acked-by: Arnd Bergmann <[email protected]>
>
>> diff --git a/arch/mips/dec/time.c b/arch/mips/dec/time.c
>> index 9e992cf..934db6f 100644
>> --- a/arch/mips/dec/time.c
>> +++ b/arch/mips/dec/time.c
>> @@ -59,14 +59,15 @@ void read_persistent_clock64(struct timespec64 *ts)
>> }
>>
>> /*
>> - * In order to set the CMOS clock precisely, rtc_mips_set_mmss has to
>> + * In order to set the CMOS clock precisely, update_persistent_clock64 has to
>> * be called 500 ms after the second nowtime has started, because when
>> * nowtime is written into the registers of the CMOS clock, it will
>> * jump to the next second precisely 500 ms later. Check the Dallas
>> * DS1287 data sheet for details.
>> */
>> -int rtc_mips_set_mmss(unsigned long nowtime)
>> +int update_persistent_clock64(struct timespec64 now)
>> {
>> + time64_t nowtime = now.tv_sec;
>> int retval = 0;
>> int real_seconds, real_minutes, cmos_minutes;
>> unsigned char save_control, save_freq_select;
>
>
> It looks like you now get an invalid 64-bit division in here,
> you have to change it to either use div_u64_rem() or possibly
> time64_to_tm() or rtc_time64_to_tm() (the latter requires
> CONFIG_RTC_LIB).

I will use div_u64_rem() to calculate real_seconds and real_minutes.

>
>> diff --git a/arch/mips/lasat/ds1603.c b/arch/mips/lasat/ds1603.c
>> index d75c887..061815e 100644
>> --- a/arch/mips/lasat/ds1603.c
>> +++ b/arch/mips/lasat/ds1603.c
>> @@ -98,7 +98,7 @@ static void rtc_write_byte(unsigned int byte)
>> }
>> }
>>
>> -static void rtc_write_word(unsigned long word)
>> +static void rtc_write_word(time64_t word)
>> {
>> int i;
>>
>
> I would say this function should take a 'u32' argument (or keep the
> unsigned long) to match the name and implementation, but then have a
> type cast in the caller with a comment about the loss of range and overflow
> in y2106.

OK.

>
>> diff --git a/arch/mips/lasat/sysctl.c b/arch/mips/lasat/sysctl.c
>> index 6f74224..76f7b62 100644
>> --- a/arch/mips/lasat/sysctl.c
>> +++ b/arch/mips/lasat/sysctl.c
>> @@ -73,8 +73,12 @@ int proc_dolasatrtc(struct ctl_table *table, int write,
>> if (r)
>> return r;
>>
>> - if (write)
>> - rtc_mips_set_mmss(rtctmp);
>> + if (write) {
>> + ts.tv_sec = rtctmp;
>> + ts.tv_nsec = 0;
>> +
>> + update_persistent_clock64(ts);
>> + }
>>
> ... and probably also a comment here to explain that we can't actually use
> the full 64-bit range because of HW limits.
>

OK. Thanks for your comments.

--
Baolin.wang
Best Regards