[PATCH RESEND 1/3] drivers/rtc/pcf8563: Replace deprecated rtc_time_to_tm() and rtc_tm_to_time()

[PATCH RESEND 1/3] drivers/rtc/pcf8563: Replace deprecated rtc_time_to_tm() and rtc_tm_to_time()

[Xunlei Pang]

From: Xunlei Pang <pang.xunlei@linaro.org>

pcf8563_rtc_set_alarm() uses deprecated rtc_tm_to_time()
and rtc_time_to_tm(), which will overflow in year 2106
on 32-bit machines.

This patch solves this by:
 - Replacing rtc_time_to_tm() with rtc_time64_to_tm()
 - Replacing rtc_tm_to_time() with rtc_tm_to_time64()

Signed-off-by: Xunlei Pang <pang.xunlei@linaro.org>
---
 drivers/rtc/rtc-pcf8563.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
index 0ba7e59..5f87f84 100644
--- a/drivers/rtc/rtc-pcf8563.c
+++ b/drivers/rtc/rtc-pcf8563.c
@@ -363,13 +363,13 @@ static int pcf8563_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm)
 	struct i2c_client *client = to_i2c_client(dev);
 	unsigned char buf[4];
 	int err;
-	unsigned long alarm_time;
 
 	/* The alarm has no seconds, round up to nearest minute */
 	if (tm->time.tm_sec) {
-		rtc_tm_to_time(&tm->time, &alarm_time);
-		alarm_time += 60-tm->time.tm_sec;
-		rtc_time_to_tm(alarm_time, &tm->time);
+		time64_t alarm_time = rtc_tm_to_time64(&tm->time);
+
+		alarm_time += 60 - tm->time.tm_sec;
+		rtc_time64_to_tm(alarm_time, &tm->time);
 	}
 
 	dev_dbg(dev, "%s, min=%d hour=%d wday=%d mday=%d "
-- 
1.9.1

[PATCH RESEND 2/3] drivers/rtc/isl1208: Replace deprecated rtc_tm_to_time()

[Xunlei Pang]

From: Xunlei Pang <pang.xunlei@linaro.org>

isl1208_i2c_set_alarm() uses deprecated rtc_tm_to_time(),
which will overflow in year 2106 on 32-bit machines.

This patch solves this by:
 - Replacing rtc_tm_to_time() with rtc_tm_to_time64()

Cc: Herbert Valerio Riedel <hvr@gnu.org>
Signed-off-by: Xunlei Pang <pang.xunlei@linaro.org>
---
 drivers/rtc/rtc-isl1208.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
index c3c549d..06113e8 100644
--- a/drivers/rtc/rtc-isl1208.c
+++ b/drivers/rtc/rtc-isl1208.c
@@ -370,19 +370,16 @@ isl1208_i2c_set_alarm(struct i2c_client *client, struct rtc_wkalrm *alarm)
 	struct rtc_time *alarm_tm = &alarm->time;
 	u8 regs[ISL1208_ALARM_SECTION_LEN] = { 0, };
 	const int offs = ISL1208_REG_SCA;
-	unsigned long rtc_secs, alarm_secs;
+	time64_t rtc_secs, alarm_secs;
 	struct rtc_time rtc_tm;
 	int err, enable;
 
 	err = isl1208_i2c_read_time(client, &rtc_tm);
 	if (err)
 		return err;
-	err = rtc_tm_to_time(&rtc_tm, &rtc_secs);
-	if (err)
-		return err;
-	err = rtc_tm_to_time(alarm_tm, &alarm_secs);
-	if (err)
-		return err;
+
+	rtc_secs = rtc_tm_to_time64(&rtc_tm);
+	alarm_secs = rtc_tm_to_time64(alarm_tm);
 
 	/* If the alarm time is before the current time disable the alarm */
 	if (!alarm->enabled || alarm_secs <= rtc_secs)
-- 
1.9.1

[PATCH RESEND 3/3] drivers/rtc/sunxi: Replace deprecated rtc_tm_to_time()

[Xunlei Pang]

From: Xunlei Pang <pang.xunlei@linaro.org>

sunxi_rtc_setalarm() uses deprecated rtc_tm_to_time(),
which will overflow in year 2106 on 32-bit machines.

This patch solves this by:
- Replacing rtc_tm_to_time() with rtc_tm_to_time64()

Also remove the unnecessary initial zeroing of some
local variables in sunxi_rtc_setalarm().

Cc: Carlo Caione <carlo.caione@gmail.com>
Signed-off-by: Xunlei Pang <pang.xunlei@linaro.org>
---
 drivers/rtc/rtc-sunxi.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/rtc/rtc-sunxi.c b/drivers/rtc/rtc-sunxi.c
index 6e678fa..7f22753 100644
--- a/drivers/rtc/rtc-sunxi.c
+++ b/drivers/rtc/rtc-sunxi.c
@@ -269,14 +269,13 @@ static int sunxi_rtc_setalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
 	struct sunxi_rtc_dev *chip = dev_get_drvdata(dev);
 	struct rtc_time *alrm_tm = &wkalrm->time;
 	struct rtc_time tm_now;
-	u32 alrm = 0;
-	unsigned long time_now = 0;
-	unsigned long time_set = 0;
-	unsigned long time_gap = 0;
-	unsigned long time_gap_day = 0;
-	unsigned long time_gap_hour = 0;
-	unsigned long time_gap_min = 0;
-	int ret = 0;
+	u32 alrm;
+	time64_t time_set, time_now;
+	unsigned long time_gap;
+	unsigned long time_gap_day;
+	unsigned long time_gap_hour;
+	unsigned long time_gap_min;
+	int ret;
 
 	ret = sunxi_rtc_gettime(dev, &tm_now);
 	if (ret < 0) {
@@ -284,13 +283,18 @@ static int sunxi_rtc_setalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
 		return -EINVAL;
 	}
 
-	rtc_tm_to_time(alrm_tm, &time_set);
-	rtc_tm_to_time(&tm_now, &time_now);
+	time_set = rtc_tm_to_time64(alrm_tm);
+	time_now = rtc_tm_to_time64(&tm_now);
 	if (time_set <= time_now) {
 		dev_err(dev, "Date to set in the past\n");
 		return -EINVAL;
 	}
 
+	if (time_set > time_now + 255 * SEC_IN_DAY) {
+		dev_err(dev, "Day must be in the range 0 - 255\n");
+		return -EINVAL;
+	}
+
 	time_gap = time_set - time_now;
 	time_gap_day = time_gap / SEC_IN_DAY;
 	time_gap -= time_gap_day * SEC_IN_DAY;
@@ -299,11 +303,6 @@ static int sunxi_rtc_setalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
 	time_gap_min = time_gap / SEC_IN_MIN;
 	time_gap -= time_gap_min * SEC_IN_MIN;
 
-	if (time_gap_day > 255) {
-		dev_err(dev, "Day must be in the range 0 - 255\n");
-		return -EINVAL;
-	}
-
 	sunxi_rtc_setaie(0, chip);
 	writel(0, chip->base + SUNXI_ALRM_DHMS);
 	usleep_range(100, 300);
-- 
1.9.1

Re: [PATCH RESEND 1/3] drivers/rtc/pcf8563: Replace deprecated rtc_time_to_tm() and rtc_tm_to_time()

[Arnd Bergmann]

On Friday 29 May 2015 23:04:35 Xunlei Pang wrote:
> From: Xunlei Pang <pang.xunlei@linaro.org>
> 
> pcf8563_rtc_set_alarm() uses deprecated rtc_tm_to_time()
> and rtc_time_to_tm(), which will overflow in year 2106
> on 32-bit machines.
> 
> This patch solves this by:
>  - Replacing rtc_time_to_tm() with rtc_time64_to_tm()
>  - Replacing rtc_tm_to_time() with rtc_tm_to_time64()
> 
> Signed-off-by: Xunlei Pang <pang.xunlei@linaro.org>

(adding alexandre to Cc)

Looks good to me,

Acked-by: Arnd Bergmann <arnd@arndb.de>


>  drivers/rtc/rtc-pcf8563.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-pcf8563.c b/drivers/rtc/rtc-pcf8563.c
> index 0ba7e59..5f87f84 100644
> --- a/drivers/rtc/rtc-pcf8563.c
> +++ b/drivers/rtc/rtc-pcf8563.c
> @@ -363,13 +363,13 @@ static int pcf8563_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm)
>  	struct i2c_client *client = to_i2c_client(dev);
>  	unsigned char buf[4];
>  	int err;
> -	unsigned long alarm_time;
>  
>  	/* The alarm has no seconds, round up to nearest minute */
>  	if (tm->time.tm_sec) {
> -		rtc_tm_to_time(&tm->time, &alarm_time);
> -		alarm_time += 60-tm->time.tm_sec;
> -		rtc_time_to_tm(alarm_time, &tm->time);
> +		time64_t alarm_time = rtc_tm_to_time64(&tm->time);
> +
> +		alarm_time += 60 - tm->time.tm_sec;
> +		rtc_time64_to_tm(alarm_time, &tm->time);
>  	}
>  
>  	dev_dbg(dev, "%s, min=%d hour=%d wday=%d mday=%d "
> 

Re: [PATCH RESEND 2/3] drivers/rtc/isl1208: Replace deprecated rtc_tm_to_time()

[Arnd Bergmann]

On Friday 29 May 2015 23:04:36 Xunlei Pang wrote:
> From: Xunlei Pang <pang.xunlei@linaro.org>
> 
> isl1208_i2c_set_alarm() uses deprecated rtc_tm_to_time(),
> which will overflow in year 2106 on 32-bit machines.
> 
> This patch solves this by:
>  - Replacing rtc_tm_to_time() with rtc_tm_to_time64()
> 
> Cc: Herbert Valerio Riedel <hvr@gnu.org>
> Signed-off-by: Xunlei Pang <pang.xunlei@linaro.org>
> ---
>  drivers/rtc/rtc-isl1208.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)

The patch looks correct, but I wonder if this is the best solution.
The only use of the two time values in the function is in the
'alarm_secs <= rtc_secs' comparison.

Maybe we should introduce an 'rtc_tm_before' helper function that
performs the same check? Do other drivers use the same piece
of code as this one?

	Arnd

> diff --git a/drivers/rtc/rtc-isl1208.c b/drivers/rtc/rtc-isl1208.c
> index c3c549d..06113e8 100644
> --- a/drivers/rtc/rtc-isl1208.c
> +++ b/drivers/rtc/rtc-isl1208.c
> @@ -370,19 +370,16 @@ isl1208_i2c_set_alarm(struct i2c_client *client, struct rtc_wkalrm *alarm)
>  	struct rtc_time *alarm_tm = &alarm->time;
>  	u8 regs[ISL1208_ALARM_SECTION_LEN] = { 0, };
>  	const int offs = ISL1208_REG_SCA;
> -	unsigned long rtc_secs, alarm_secs;
> +	time64_t rtc_secs, alarm_secs;
>  	struct rtc_time rtc_tm;
>  	int err, enable;
>  
>  	err = isl1208_i2c_read_time(client, &rtc_tm);
>  	if (err)
>  		return err;
> -	err = rtc_tm_to_time(&rtc_tm, &rtc_secs);
> -	if (err)
> -		return err;
> -	err = rtc_tm_to_time(alarm_tm, &alarm_secs);
> -	if (err)
> -		return err;
> +
> +	rtc_secs = rtc_tm_to_time64(&rtc_tm);
> +	alarm_secs = rtc_tm_to_time64(alarm_tm);
>  
>  	/* If the alarm time is before the current time disable the alarm */
>  	if (!alarm->enabled || alarm_secs <= rtc_secs)
> 

Re: [PATCH RESEND 3/3] drivers/rtc/sunxi: Replace deprecated rtc_tm_to_time()

[Arnd Bergmann]

On Friday 29 May 2015 23:04:37 Xunlei Pang wrote:
>         }
>  
> -       rtc_tm_to_time(alrm_tm, &time_set);
> -       rtc_tm_to_time(&tm_now, &time_now);
> +       time_set = rtc_tm_to_time64(alrm_tm);
> +       time_now = rtc_tm_to_time64(&tm_now);
>         if (time_set <= time_now) {
>                 dev_err(dev, "Date to set in the past\n");
>                 return -EINVAL;
>         }
>  
> +       if (time_set > time_now + 255 * SEC_IN_DAY) {
> +               dev_err(dev, "Day must be in the range 0 - 255\n");
> +               return -EINVAL;
> +       }
> +
> 

So this driver also uses the two values just to do a comparison
and to take the difference in seconds.

If we have a helper function that returns the difference between
two rtc_tm values as a time64_t, we can use that for both
this driver and isl1208.

	Arnd

Re: [PATCH RESEND 3/3] drivers/rtc/sunxi: Replace deprecated rtc_tm_to_time()

[pang.xunlei]

Hi Arnd,

Arnd Bergmann <arnd@arndb.de> wrote 2015-06-02 AM 03:53:03:
> Re: [PATCH RESEND 3/3] drivers/rtc/sunxi: Replace deprecated 
rtc_tm_to_time()
> 
> On Friday 29 May 2015 23:04:37 Xunlei Pang wrote:
> >         }
> > 
> > -       rtc_tm_to_time(alrm_tm, &time_set);
> > -       rtc_tm_to_time(&tm_now, &time_now);
> > +       time_set = rtc_tm_to_time64(alrm_tm);
> > +       time_now = rtc_tm_to_time64(&tm_now);
> >         if (time_set <= time_now) {
> >                 dev_err(dev, "Date to set in the past\n");
> >                 return -EINVAL;
> >         }
> > 
> > +       if (time_set > time_now + 255 * SEC_IN_DAY) {
> > +               dev_err(dev, "Day must be in the range 0 - 255\n");
> > +               return -EINVAL;
> > +       }
> > +
> > 
> 
> So this driver also uses the two values just to do a comparison
> and to take the difference in seconds.
> 
> If we have a helper function that returns the difference between
> two rtc_tm values as a time64_t, we can use that for both
> this driver and isl1208.

Indeed, will do.

Thanks,
-Xunlei
--------------------------------------------------------
ZTE Information Security Notice: The information contained in this mail (and any attachment transmitted herewith) is privileged and confidential and is intended for the exclusive use of the addressee(s).  If you are not an intended recipient, any disclosure, reproduction, distribution or other dissemination or use of the information contained is strictly prohibited.  If you have received this mail in error, please delete it and notify us immediately.