diff mbox series

[35/51] rtc: pm8xxx: stop using rtc deprecated functions

Message ID 1497951359-13334-36-git-send-email-benjamin.gaignard@linaro.org
State New
Headers show
Series rtc: stop using rtc deprecated functions | expand

Commit Message

Benjamin Gaignard June 20, 2017, 9:35 a.m. UTC 
rtc_time_to_tm() and rtc_tm_to_time() are deprecated because they
rely on 32bits variables and that will make rtc break in y2038/2016.
Stop using those two functions to safer 64bits ones.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>

CC: Alessandro Zummo <a.zummo@towertech.it>
CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
CC: rtc-linux@googlegroups.com
CC: linux-kernel@vger.kernel.org
---
 drivers/rtc/rtc-pm8xxx.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

-- 
1.9.1

Comments

kernel test robot June 21, 2017, 4:54 p.m. UTC | #1 
Hi Benjamin,

[auto build test ERROR on abelloni/rtc-next]
[also build test ERROR on v4.12-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Benjamin-Gaignard/rtc-stop-using-rtc-deprecated-functions/20170621-044455
base:   https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/linux.git rtc-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

>> ERROR: "__udivdi3" [drivers/rtc/rtc-pcap.ko] undefined!

>> ERROR: "__umoddi3" [drivers/rtc/rtc-pcap.ko] undefined!

>> ERROR: "__divdi3" [drivers/rtc/rtc-pcap.ko] undefined!

>> ERROR: "__moddi3" [drivers/rtc/rtc-pcap.ko] undefined!

   ERROR: "__umoddi3" [drivers/rtc/rtc-cpcap.ko] undefined!
   ERROR: "__udivdi3" [drivers/rtc/rtc-cpcap.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Arnd Bergmann July 3, 2017, 10:12 a.m. UTC | #2 
On Tue, Jun 20, 2017 at 11:35 AM, Benjamin Gaignard
<benjamin.gaignard@linaro.org> wrote:
> rtc_time_to_tm() and rtc_tm_to_time() are deprecated because they

> rely on 32bits variables and that will make rtc break in y2038/2016.

> Stop using those two functions to safer 64bits ones.

>

> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>

> CC: Alessandro Zummo <a.zummo@towertech.it>

> CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>

> CC: rtc-linux@googlegroups.com

> CC: linux-kernel@vger.kernel.org

> ---

>  drivers/rtc/rtc-pm8xxx.c | 22 ++++++++++++----------

>  1 file changed, 12 insertions(+), 10 deletions(-)

>

> diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c

> index fac8355..e6b52bd 100644

> --- a/drivers/rtc/rtc-pm8xxx.c

> +++ b/drivers/rtc/rtc-pm8xxx.c

> @@ -81,7 +81,8 @@ struct pm8xxx_rtc {

>  static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)

>  {

>         int rc, i;

> -       unsigned long secs, irq_flags;

> +       unsigned long long secs;

> +       unsigned long irq_flags;


'secs' should be 'time64_t' here, which is signed.

>         u8 value[NUM_8_BIT_RTC_REGS], alarm_enabled = 0;

>         unsigned int ctrl_reg;

>         struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);

> @@ -90,14 +91,14 @@ static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)

>         if (!rtc_dd->allow_set_time)

>                 return -EACCES;

>

> -       rtc_tm_to_time(tm, &secs);

> +       secs = rtc_tm_to_time64(tm);

>

>         for (i = 0; i < NUM_8_BIT_RTC_REGS; i++) {

>                 value[i] = secs & 0xFF;

>                 secs >>= 8;

>         }

>

> -       dev_dbg(dev, "Seconds value to be written to RTC = %lu\n", secs);

> +       dev_dbg(dev, "Seconds value to be written to RTC = %llu\n", secs);

>


However, note that you only write 32 bits here, since NUM_8_BIT_RTC_REGS
is set to '4'. Is that a hardware constant? If yes, it would be best to return
-EINVAL or -EOVERFLOW for out of range times.

I think should really try to come up with a way to set the policy for
overflows in
RTC drivers globally in this case. There are many drivers that take a 32-bit
unsigned value (and many others that don't), but a nicer policy for the long
run might be to define some kind of windowing where values before e.g.
year 2000 or 2017 are wrapped around and used as future dates.

Unfortunately, the downside of this is that any RTC that defaults to '0'
and is now interpreted as year 1970 would then be interpreted as a future
data that can not be represented in today's 32-bit time_t.

       Arnd
diff mbox series

Patch

diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
index fac8355..e6b52bd 100644
--- a/drivers/rtc/rtc-pm8xxx.c
+++ b/drivers/rtc/rtc-pm8xxx.c
@@ -81,7 +81,8 @@  struct pm8xxx_rtc {
 static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
 	int rc, i;
-	unsigned long secs, irq_flags;
+	unsigned long long secs;
+	unsigned long irq_flags;
 	u8 value[NUM_8_BIT_RTC_REGS], alarm_enabled = 0;
 	unsigned int ctrl_reg;
 	struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
@@ -90,14 +91,14 @@  static int pm8xxx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	if (!rtc_dd->allow_set_time)
 		return -EACCES;
 
-	rtc_tm_to_time(tm, &secs);
+	secs = rtc_tm_to_time64(tm);
 
 	for (i = 0; i < NUM_8_BIT_RTC_REGS; i++) {
 		value[i] = secs & 0xFF;
 		secs >>= 8;
 	}
 
-	dev_dbg(dev, "Seconds value to be written to RTC = %lu\n", secs);
+	dev_dbg(dev, "Seconds value to be written to RTC = %llu\n", secs);
 
 	spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
 
@@ -156,7 +157,7 @@  static int pm8xxx_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
 	int rc;
 	u8 value[NUM_8_BIT_RTC_REGS];
-	unsigned long secs;
+	unsigned long long secs;
 	unsigned int reg;
 	struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
 	const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
@@ -188,7 +189,7 @@  static int pm8xxx_rtc_read_time(struct device *dev, struct rtc_time *tm)
 
 	secs = value[0] | (value[1] << 8) | (value[2] << 16) | (value[3] << 24);
 
-	rtc_time_to_tm(secs, tm);
+	rtc_time64_to_tm(secs, tm);
 
 	rc = rtc_valid_tm(tm);
 	if (rc < 0) {
@@ -196,7 +197,7 @@  static int pm8xxx_rtc_read_time(struct device *dev, struct rtc_time *tm)
 		return rc;
 	}
 
-	dev_dbg(dev, "secs = %lu, h:m:s == %d:%d:%d, d/m/y = %d/%d/%d\n",
+	dev_dbg(dev, "secs = %llu, h:m:s == %d:%d:%d, d/m/y = %d/%d/%d\n",
 		secs, tm->tm_hour, tm->tm_min, tm->tm_sec,
 		tm->tm_mday, tm->tm_mon, tm->tm_year);
 
@@ -208,11 +209,12 @@  static int pm8xxx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 	int rc, i;
 	u8 value[NUM_8_BIT_RTC_REGS];
 	unsigned int ctrl_reg;
-	unsigned long secs, irq_flags;
+	unsigned long long secs;
+	unsigned long irq_flags;
 	struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
 	const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
 
-	rtc_tm_to_time(&alarm->time, &secs);
+	secs = rtc_tm_to_time64(&alarm->time);
 
 	for (i = 0; i < NUM_8_BIT_RTC_REGS; i++) {
 		value[i] = secs & 0xFF;
@@ -256,7 +258,7 @@  static int pm8xxx_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 {
 	int rc;
 	u8 value[NUM_8_BIT_RTC_REGS];
-	unsigned long secs;
+	unsigned long long secs;
 	struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
 	const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
 
@@ -269,7 +271,7 @@  static int pm8xxx_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 
 	secs = value[0] | (value[1] << 8) | (value[2] << 16) | (value[3] << 24);
 
-	rtc_time_to_tm(secs, &alarm->time);
+	rtc_time64_to_tm(secs, &alarm->time);
 
 	rc = rtc_valid_tm(&alarm->time);
 	if (rc < 0) {