All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/4] Add rtc 64bit epoch offset for rtc hardware that only provides 32bit time
@ 2014-11-27 12:02 Xunlei Pang
  2014-11-27 12:02 ` [RFC PATCH 1/4] rtc/mxc: Convert get_alarm_or_time()/set_alarm_or_time() to use time64_t Xunlei Pang
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Xunlei Pang @ 2014-11-27 12:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: rtc-linux, Thomas Gleixner, Alessandro Zummo, Sven Schnelle,
	Xunlei Pang, John Stultz, Arnd Bergmann

On some systems the rtc hardware may only provide 32bits of time.
This is problematic, as the hardware won't be able to provide
post-y2038 times to initialize the system at boot.

Thus this patch series provides a 64bit epoch offset, which can be
configured to allow the 32bit rtc hardware to represent times past
2038, and also demonstrates how to use the newly-added interfaces
based on rtc epoch offset to eliminate y2038/y2106 issues existing
in many current rtc drivers despite the above-mentioned rtc hardware
limitations.

Patch 1:
mxc_rtc_set_mmss() is one y2038-unsafe user of rtc_class_ops.set_mmss(),
but it uses get_alarm_or_time()/set_alarm_or_time() internal interfaces
which are also y2038 unsafe. The handling of these two internal interfaces
has enough complexity to require a separate patch, so we process it here
before converting rtc_class_ops.set_mmss() which will be done in Patch 2.
Currently, "mxc" is the only driver with such issue.

Patch 2:
Converts rtc_class_ops.set_mmss() to use time64_t, so lays some groundwork
for making rtc drivers y2038/y2106 safe.

Patch 3:
Provides two new interfaces based on the rtc epoch offset to map between
32bit hardware time and 64bit time. We can use them to make 32bit hardware
rtc drivers y2038/y2106 safe.

Patch 4:
Now thanks to foregoing jobs, it's time to provide an example to demonstrate
how to address all 32bit time issues in a single rtc driver(imxdi) whose
rtc hardware only provides 32bit time.

If there're no objections with my approach, I'll go on with the work
to make all the rtc drivers under "drivers/rtc" y2038/y2106 safe.

NOTE: This patch series relies on the former patch series named:
"y2038 in-kernel interface changes for drivers/rtc".

Cc: John Stultz <john.stultz@linaro.org>
Cc: Arnd Bergmann <arnd.bergmann@linaro.org>

Xunlei Pang (4):
  rtc/mxc: Convert get_alarm_or_time()/set_alarm_or_time() to use
    time64_t
  rtc: Convert rtc_class_ops.set_mmss() to use time64_t
  rtc/lib: Provide interfaces to map between 32bit hardware and 64bit
    time
  rtc/imxdi: Update driver to address time issues

 arch/alpha/kernel/rtc.c     |    4 ++--
 drivers/rtc/class.c         |   25 +++++++++++++++++++++++++
 drivers/rtc/interface.c     |    8 ++++----
 drivers/rtc/rtc-ab3100.c    |    4 ++--
 drivers/rtc/rtc-coh901331.c |    8 +++++++-
 drivers/rtc/rtc-ds1672.c    |    8 +++++++-
 drivers/rtc/rtc-ds2404.c    |    9 ++++++++-
 drivers/rtc/rtc-ep93xx.c    |    8 +++++++-
 drivers/rtc/rtc-imxdi.c     |   37 ++++++++++++++++++++++---------------
 drivers/rtc/rtc-jz4740.c    |    8 +++++++-
 drivers/rtc/rtc-lib.c       |   28 ++++++++++++++++++++++++++++
 drivers/rtc/rtc-lpc32xx.c   |    9 ++++++++-
 drivers/rtc/rtc-mc13xxx.c   |    5 ++---
 drivers/rtc/rtc-mxc.c       |   37 ++++++++++++++++++-------------------
 drivers/rtc/rtc-pcap.c      |    6 ++----
 drivers/rtc/rtc-stmp3xxx.c  |    8 +++++++-
 drivers/rtc/rtc-test.c      |    4 ++--
 drivers/rtc/rtc-tx4939.c    |    8 +++++++-
 drivers/rtc/rtc-wm831x.c    |   16 +++++++++-------
 drivers/rtc/rtc-xgene.c     |    6 +++++-
 include/linux/rtc.h         |    5 ++++-
 21 files changed, 183 insertions(+), 68 deletions(-)

-- 
1.7.9.5


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [RFC PATCH 1/4] rtc/mxc: Convert get_alarm_or_time()/set_alarm_or_time() to use time64_t
  2014-11-27 12:02 [RFC PATCH 0/4] Add rtc 64bit epoch offset for rtc hardware that only provides 32bit time Xunlei Pang
@ 2014-11-27 12:02 ` Xunlei Pang
  2014-11-27 23:02   ` Thomas Gleixner
  2014-11-27 12:02 ` [RFC PATCH 2/4] rtc: Convert rtc_class_ops.set_mmss() " Xunlei Pang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Xunlei Pang @ 2014-11-27 12:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: rtc-linux, Thomas Gleixner, Alessandro Zummo, Sven Schnelle,
	Xunlei Pang, John Stultz, Arnd Bergmann

We want to convert y2038 unsafe rtc_class_ops.set_mmss() and all its
users to use time64_t. mxc_rtc_set_mmss() in "mxc" driver is one of
its users, but it uses get_alarm_or_time()/set_alarm_or_time() internal
interfaces which are also y2038 unsafe.

So here as a separate patch, it converts these two internal interfaces
of "mxc" to use safe time64_t first, so as to make some preparations
for the rtc_class_ops.set_mmss() conversion.

Currently, "mxc" is the only driver with such issue.

Cc: John Stultz <john.stultz@linaro.org>
Cc: Arnd Bergmann <arnd.bergmann@linaro.org>
Signed-off-by: Xunlei Pang <pang.xunlei@linaro.org>
---
 drivers/rtc/rtc-mxc.c |   31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/rtc/rtc-mxc.c b/drivers/rtc/rtc-mxc.c
index 419874f..07bff34 100644
--- a/drivers/rtc/rtc-mxc.c
+++ b/drivers/rtc/rtc-mxc.c
@@ -106,7 +106,7 @@ static inline int is_imx1_rtc(struct rtc_plat_data *data)
  * This function is used to obtain the RTC time or the alarm value in
  * second.
  */
-static u32 get_alarm_or_time(struct device *dev, int time_alarm)
+static time64_t get_alarm_or_time(struct device *dev, int time_alarm)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct rtc_plat_data *pdata = platform_get_drvdata(pdev);
@@ -129,29 +129,28 @@ static u32 get_alarm_or_time(struct device *dev, int time_alarm)
 	hr = hr_min >> 8;
 	min = hr_min & 0xff;
 
-	return (((day * 24 + hr) * 60) + min) * 60 + sec;
+	return ((((time64_t)day * 24 + hr) * 60) + min) * 60 + sec;
 }
 
 /*
  * This function sets the RTC alarm value or the time value.
  */
-static void set_alarm_or_time(struct device *dev, int time_alarm, u32 time)
+static void set_alarm_or_time(struct device *dev, int time_alarm, time64_t time)
 {
-	u32 day, hr, min, sec, temp;
+	u32 tod, day, hr, min, sec, temp;
 	struct platform_device *pdev = to_platform_device(dev);
 	struct rtc_plat_data *pdata = platform_get_drvdata(pdev);
 	void __iomem *ioaddr = pdata->ioaddr;
 
-	day = time / 86400;
-	time -= day * 86400;
+	day = div_s64_rem(time, 86400, &tod);
 
 	/* time is within a day now */
-	hr = time / 3600;
-	time -= hr * 3600;
+	hr = tod / 3600;
+	tod -= hr * 3600;
 
 	/* time is within an hour now */
-	min = time / 60;
-	sec = time - min * 60;
+	min = tod / 60;
+	sec = tod - min * 60;
 
 	temp = (hr << 8) + min;
 
@@ -176,20 +175,20 @@ static void set_alarm_or_time(struct device *dev, int time_alarm, u32 time)
 static int rtc_update_alarm(struct device *dev, struct rtc_time *alrm)
 {
 	struct rtc_time alarm_tm, now_tm;
-	unsigned long now, time;
+	time64_t now, time;
 	struct platform_device *pdev = to_platform_device(dev);
 	struct rtc_plat_data *pdata = platform_get_drvdata(pdev);
 	void __iomem *ioaddr = pdata->ioaddr;
 
 	now = get_alarm_or_time(dev, MXC_RTC_TIME);
-	rtc_time_to_tm(now, &now_tm);
+	rtc_time64_to_tm(now, &now_tm);
 	alarm_tm.tm_year = now_tm.tm_year;
 	alarm_tm.tm_mon = now_tm.tm_mon;
 	alarm_tm.tm_mday = now_tm.tm_mday;
 	alarm_tm.tm_hour = alrm->tm_hour;
 	alarm_tm.tm_min = alrm->tm_min;
 	alarm_tm.tm_sec = alrm->tm_sec;
-	rtc_tm_to_time(&alarm_tm, &time);
+	time = rtc_tm_to_time64(&alarm_tm);
 
 	/* clear all the interrupt status bits */
 	writew(readw(ioaddr + RTC_RTCISR), ioaddr + RTC_RTCISR);
@@ -283,14 +282,14 @@ static int mxc_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
  */
 static int mxc_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
-	u32 val;
+	time64_t val;
 
 	/* Avoid roll-over from reading the different registers */
 	do {
 		val = get_alarm_or_time(dev, MXC_RTC_TIME);
 	} while (val != get_alarm_or_time(dev, MXC_RTC_TIME));
 
-	rtc_time_to_tm(val, tm);
+	rtc_time64_to_tm(val, tm);
 
 	return 0;
 }
@@ -333,7 +332,7 @@ static int mxc_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	struct rtc_plat_data *pdata = platform_get_drvdata(pdev);
 	void __iomem *ioaddr = pdata->ioaddr;
 
-	rtc_time_to_tm(get_alarm_or_time(dev, MXC_RTC_ALARM), &alrm->time);
+	rtc_time64_to_tm(get_alarm_or_time(dev, MXC_RTC_ALARM), &alrm->time);
 	alrm->pending = ((readw(ioaddr + RTC_RTCISR) & RTC_ALM_BIT)) ? 1 : 0;
 
 	return 0;
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 2/4] rtc: Convert rtc_class_ops.set_mmss() to use time64_t
  2014-11-27 12:02 [RFC PATCH 0/4] Add rtc 64bit epoch offset for rtc hardware that only provides 32bit time Xunlei Pang
  2014-11-27 12:02 ` [RFC PATCH 1/4] rtc/mxc: Convert get_alarm_or_time()/set_alarm_or_time() to use time64_t Xunlei Pang
@ 2014-11-27 12:02 ` Xunlei Pang
  2014-11-27 23:05   ` Thomas Gleixner
  2014-11-28 16:49   ` pang.xunlei
  2014-11-27 12:02 ` [RFC PATCH 3/4] rtc/lib: Provide interfaces to map between 32bit hardware and 64bit time Xunlei Pang
  2014-11-27 12:02 ` [RFC PATCH 4/4] rtc/imxdi: Update driver to address time issues Xunlei Pang
  3 siblings, 2 replies; 18+ messages in thread
From: Xunlei Pang @ 2014-11-27 12:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: rtc-linux, Thomas Gleixner, Alessandro Zummo, Sven Schnelle,
	Xunlei Pang, John Stultz, Arnd Bergmann

Currently the rtc_class_op's set_mmss() function takes a 32bit second
value (on 32bit systems), which is problematic for dates past y2038.

This patch resolves it by changing the interface and all users to use
y2038 safe time64_t.

Cc: John Stultz <john.stultz@linaro.org>
Cc: Arnd Bergmann <arnd.bergmann@linaro.org>
Signed-off-by: Xunlei Pang <pang.xunlei@linaro.org>
---
This patch leaves some "y2106 issue" tags in the problematic drivers
whose rtc hardware only provides 32bit time. The handling of these
problematic drivers will be left to the succeeding patch series.
But, to better understand the purpose of this patch series, one of
those drivers is picked and resolved in the last patch as an example.

 arch/alpha/kernel/rtc.c     |    4 ++--
 drivers/rtc/interface.c     |    8 ++++----
 drivers/rtc/rtc-ab3100.c    |    4 ++--
 drivers/rtc/rtc-coh901331.c |    8 +++++++-
 drivers/rtc/rtc-ds1672.c    |    8 +++++++-
 drivers/rtc/rtc-ds2404.c    |    9 ++++++++-
 drivers/rtc/rtc-ep93xx.c    |    8 +++++++-
 drivers/rtc/rtc-imxdi.c     |    8 +++++++-
 drivers/rtc/rtc-jz4740.c    |    8 +++++++-
 drivers/rtc/rtc-lpc32xx.c   |    9 ++++++++-
 drivers/rtc/rtc-mc13xxx.c   |    5 ++---
 drivers/rtc/rtc-mxc.c       |    6 +++---
 drivers/rtc/rtc-pcap.c      |    6 ++----
 drivers/rtc/rtc-stmp3xxx.c  |    8 +++++++-
 drivers/rtc/rtc-test.c      |    4 ++--
 drivers/rtc/rtc-tx4939.c    |    8 +++++++-
 drivers/rtc/rtc-wm831x.c    |   16 +++++++++-------
 drivers/rtc/rtc-xgene.c     |    6 +++++-
 include/linux/rtc.h         |    2 +-
 19 files changed, 97 insertions(+), 38 deletions(-)

diff --git a/arch/alpha/kernel/rtc.c b/arch/alpha/kernel/rtc.c
index c8d284d..c398a23 100644
--- a/arch/alpha/kernel/rtc.c
+++ b/arch/alpha/kernel/rtc.c
@@ -116,7 +116,7 @@ alpha_rtc_set_time(struct device *dev, struct rtc_time *tm)
 }
 
 static int
-alpha_rtc_set_mmss(struct device *dev, unsigned long nowtime)
+alpha_rtc_set_mmss(struct device *dev, time64_t nowtime)
 {
 	int retval = 0;
 	int real_seconds, real_minutes, cmos_minutes;
@@ -276,7 +276,7 @@ do_remote_mmss(void *data)
 }
 
 static int
-remote_set_mmss(struct device *dev, unsigned long now)
+remote_set_mmss(struct device *dev, time64_t now)
 {
 	union remote_data x;
 	if (smp_processor_id() != boot_cpuid) {
diff --git a/drivers/rtc/interface.c b/drivers/rtc/interface.c
index 5b2717f..f409a36 100644
--- a/drivers/rtc/interface.c
+++ b/drivers/rtc/interface.c
@@ -65,10 +65,10 @@ int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm)
 	else if (rtc->ops->set_time)
 		err = rtc->ops->set_time(rtc->dev.parent, tm);
 	else if (rtc->ops->set_mmss) {
-		unsigned long secs;
-		err = rtc_tm_to_time(tm, &secs);
-		if (err == 0)
-			err = rtc->ops->set_mmss(rtc->dev.parent, secs);
+		time64_t secs;
+
+		secs = rtc_tm_to_time64(tm);
+		err = rtc->ops->set_mmss(rtc->dev.parent, secs);
 	} else
 		err = -EINVAL;
 
diff --git a/drivers/rtc/rtc-ab3100.c b/drivers/rtc/rtc-ab3100.c
index ff43534..1cf4cd4 100644
--- a/drivers/rtc/rtc-ab3100.c
+++ b/drivers/rtc/rtc-ab3100.c
@@ -43,12 +43,12 @@
 /*
  * RTC clock functions and device struct declaration
  */
-static int ab3100_rtc_set_mmss(struct device *dev, unsigned long secs)
+static int ab3100_rtc_set_mmss(struct device *dev, time64_t secs)
 {
 	u8 regs[] = {AB3100_TI0, AB3100_TI1, AB3100_TI2,
 		     AB3100_TI3, AB3100_TI4, AB3100_TI5};
 	unsigned char buf[6];
-	u64 fat_time = (u64) secs * AB3100_RTC_CLOCK_RATE * 2;
+	time64_t fat_time = secs * AB3100_RTC_CLOCK_RATE * 2;
 	int err = 0;
 	int i;
 
diff --git a/drivers/rtc/rtc-coh901331.c b/drivers/rtc/rtc-coh901331.c
index 869cae2..85fd650 100644
--- a/drivers/rtc/rtc-coh901331.c
+++ b/drivers/rtc/rtc-coh901331.c
@@ -88,11 +88,17 @@ static int coh901331_read_time(struct device *dev, struct rtc_time *tm)
 	return -EINVAL;
 }
 
-static int coh901331_set_mmss(struct device *dev, unsigned long secs)
+static int coh901331_set_mmss(struct device *dev, time64_t secs)
 {
 	struct coh901331_port *rtap = dev_get_drvdata(dev);
 
 	clk_enable(rtap->clk);
+	/*
+	 * y2106 issue:
+	 *   On 32bit systems the time64_t secs value gets cast to
+	 *   a 32bit long, and thus we can only write a maximum value
+	 *   of y2016
+	 */
 	writel(secs, rtap->virtbase + COH901331_SET_TIME);
 	clk_disable(rtap->clk);
 
diff --git a/drivers/rtc/rtc-ds1672.c b/drivers/rtc/rtc-ds1672.c
index a4888db..4da0f4a 100644
--- a/drivers/rtc/rtc-ds1672.c
+++ b/drivers/rtc/rtc-ds1672.c
@@ -98,8 +98,14 @@ static int ds1672_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	return ds1672_get_datetime(to_i2c_client(dev), tm);
 }
 
-static int ds1672_rtc_set_mmss(struct device *dev, unsigned long secs)
+static int ds1672_rtc_set_mmss(struct device *dev, time64_t secs)
 {
+	/*
+	 * y2106 issue:
+	 *   On 32bit systems the time64_t secs value gets cast to
+	 *   a 32bit long, and thus we can only write a maximum value
+	 *   of y2016
+	 */
 	return ds1672_set_mmss(to_i2c_client(dev), secs);
 }
 
diff --git a/drivers/rtc/rtc-ds2404.c b/drivers/rtc/rtc-ds2404.c
index fc209dc..ec50757 100644
--- a/drivers/rtc/rtc-ds2404.c
+++ b/drivers/rtc/rtc-ds2404.c
@@ -210,9 +210,16 @@ static int ds2404_read_time(struct device *dev, struct rtc_time *dt)
 	return rtc_valid_tm(dt);
 }
 
-static int ds2404_set_mmss(struct device *dev, unsigned long secs)
+static int ds2404_set_mmss(struct device *dev, time64_t secs)
 {
+	/*
+	 * y2106 issue:
+	 *   On 32bit systems the time64_t secs value gets cast to
+	 *   a 32bit long, and thus we can only write a maximum value
+	 *   of y2016
+	 */
 	u32 time = cpu_to_le32(secs);
+
 	ds2404_write_memory(dev, 0x203, 4, (u8 *)&time);
 	return 0;
 }
diff --git a/drivers/rtc/rtc-ep93xx.c b/drivers/rtc/rtc-ep93xx.c
index 5e4f5dc..6e50aaa8 100644
--- a/drivers/rtc/rtc-ep93xx.c
+++ b/drivers/rtc/rtc-ep93xx.c
@@ -69,10 +69,16 @@ static int ep93xx_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	return 0;
 }
 
-static int ep93xx_rtc_set_mmss(struct device *dev, unsigned long secs)
+static int ep93xx_rtc_set_mmss(struct device *dev, time64_t secs)
 {
 	struct ep93xx_rtc *ep93xx_rtc = dev_get_platdata(dev);
 
+	/*
+	 * y2106 issue:
+	 *   On 32bit systems the time64_t secs value gets cast to
+	 *   a 32bit long, and thus we can only write a maximum value
+	 *   of y2016
+	 */
 	__raw_writel(secs + 1, ep93xx_rtc->mmio_base + EP93XX_RTC_LOAD);
 	return 0;
 }
diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index cd741c7..34d76b2 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -209,7 +209,7 @@ static int dryice_rtc_read_time(struct device *dev, struct rtc_time *tm)
  * set the seconds portion of dryice time counter and clear the
  * fractional part.
  */
-static int dryice_rtc_set_mmss(struct device *dev, unsigned long secs)
+static int dryice_rtc_set_mmss(struct device *dev, time64_t secs)
 {
 	struct imxdi_dev *imxdi = dev_get_drvdata(dev);
 	int rc;
@@ -217,6 +217,12 @@ static int dryice_rtc_set_mmss(struct device *dev, unsigned long secs)
 	/* zero the fractional part first */
 	rc = di_write_wait(imxdi, 0, DTCLR);
 	if (rc == 0)
+		/*
+		 * y2106 issue:
+		 *   On 32bit systems the time64_t secs value gets cast to
+		 *   a 32bit long, and thus we can only write a maximum value
+		 *   of y2016
+		 */
 		rc = di_write_wait(imxdi, secs, DTCMR);
 
 	return rc;
diff --git a/drivers/rtc/rtc-jz4740.c b/drivers/rtc/rtc-jz4740.c
index 08f5160..06a8917 100644
--- a/drivers/rtc/rtc-jz4740.c
+++ b/drivers/rtc/rtc-jz4740.c
@@ -127,10 +127,16 @@ static int jz4740_rtc_read_time(struct device *dev, struct rtc_time *time)
 	return rtc_valid_tm(time);
 }
 
-static int jz4740_rtc_set_mmss(struct device *dev, unsigned long secs)
+static int jz4740_rtc_set_mmss(struct device *dev, time64_t secs)
 {
 	struct jz4740_rtc *rtc = dev_get_drvdata(dev);
 
+	/*
+	 * y2106 issue:
+	 *   On 32bit systems the time64_t secs value gets cast to
+	 *   a 32bit long, and thus we can only write a maximum value
+	 *   of y2016
+	 */
 	return jz4740_rtc_reg_write(rtc, JZ_REG_RTC_SEC, secs);
 }
 
diff --git a/drivers/rtc/rtc-lpc32xx.c b/drivers/rtc/rtc-lpc32xx.c
index f130c08..4b782a4 100644
--- a/drivers/rtc/rtc-lpc32xx.c
+++ b/drivers/rtc/rtc-lpc32xx.c
@@ -73,7 +73,7 @@ static int lpc32xx_rtc_read_time(struct device *dev, struct rtc_time *time)
 	return rtc_valid_tm(time);
 }
 
-static int lpc32xx_rtc_set_mmss(struct device *dev, unsigned long secs)
+static int lpc32xx_rtc_set_mmss(struct device *dev, time64_t secs)
 {
 	struct lpc32xx_rtc *rtc = dev_get_drvdata(dev);
 	u32 tmp;
@@ -83,6 +83,13 @@ static int lpc32xx_rtc_set_mmss(struct device *dev, unsigned long secs)
 	/* RTC must be disabled during count update */
 	tmp = rtc_readl(rtc, LPC32XX_RTC_CTRL);
 	rtc_writel(rtc, LPC32XX_RTC_CTRL, tmp | LPC32XX_RTC_CTRL_CNTR_DIS);
+
+	/*
+	 * y2106 issue:
+	 *   On 32bit systems the time64_t secs value gets cast to
+	 *   a 32bit long, and thus we can only write a maximum value
+	 *   of y2016
+	 */
 	rtc_writel(rtc, LPC32XX_RTC_UCOUNT, secs);
 	rtc_writel(rtc, LPC32XX_RTC_DCOUNT, 0xFFFFFFFF - secs);
 	rtc_writel(rtc, LPC32XX_RTC_CTRL, tmp &= ~LPC32XX_RTC_CTRL_CNTR_DIS);
diff --git a/drivers/rtc/rtc-mc13xxx.c b/drivers/rtc/rtc-mc13xxx.c
index 124996d..d87edf1 100644
--- a/drivers/rtc/rtc-mc13xxx.c
+++ b/drivers/rtc/rtc-mc13xxx.c
@@ -88,15 +88,14 @@ static int mc13xxx_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	return rtc_valid_tm(tm);
 }
 
-static int mc13xxx_rtc_set_mmss(struct device *dev, unsigned long secs)
+static int mc13xxx_rtc_set_mmss(struct device *dev, time64_t secs)
 {
 	struct mc13xxx_rtc *priv = dev_get_drvdata(dev);
 	unsigned int seconds, days;
 	unsigned int alarmseconds;
 	int ret;
 
-	seconds = secs % SEC_PER_DAY;
-	days = secs / SEC_PER_DAY;
+	days = div_s64_rem(secs, SEC_PER_DAY, &seconds);
 
 	mc13xxx_lock(priv->mc13xxx);
 
diff --git a/drivers/rtc/rtc-mxc.c b/drivers/rtc/rtc-mxc.c
index 07bff34..3339571 100644
--- a/drivers/rtc/rtc-mxc.c
+++ b/drivers/rtc/rtc-mxc.c
@@ -297,7 +297,7 @@ static int mxc_rtc_read_time(struct device *dev, struct rtc_time *tm)
 /*
  * This function sets the internal RTC time based on tm in Gregorian date.
  */
-static int mxc_rtc_set_mmss(struct device *dev, unsigned long time)
+static int mxc_rtc_set_mmss(struct device *dev, time64_t time)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct rtc_plat_data *pdata = platform_get_drvdata(pdev);
@@ -308,9 +308,9 @@ static int mxc_rtc_set_mmss(struct device *dev, unsigned long time)
 	if (is_imx1_rtc(pdata)) {
 		struct rtc_time tm;
 
-		rtc_time_to_tm(time, &tm);
+		rtc_time64_to_tm(time, &tm);
 		tm.tm_year = 70;
-		rtc_tm_to_time(&tm, &time);
+		time = rtc_tm_to_time64(&tm);
 	}
 
 	/* Avoid roll-over from reading the different registers */
diff --git a/drivers/rtc/rtc-pcap.c b/drivers/rtc/rtc-pcap.c
index 40b5c63..1fe5849 100644
--- a/drivers/rtc/rtc-pcap.c
+++ b/drivers/rtc/rtc-pcap.c
@@ -98,16 +98,14 @@ static int pcap_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	return rtc_valid_tm(tm);
 }
 
-static int pcap_rtc_set_mmss(struct device *dev, unsigned long secs)
+static int pcap_rtc_set_mmss(struct device *dev, time64_t secs)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct pcap_rtc *pcap_rtc = platform_get_drvdata(pdev);
 	u32 tod, days;
 
-	tod = secs % SEC_PER_DAY;
+	days = div_s64_rem(secs, SEC_PER_DAY, &tod);
 	ezx_pcap_write(pcap_rtc->pcap, PCAP_REG_RTC_TOD, tod);
-
-	days = secs / SEC_PER_DAY;
 	ezx_pcap_write(pcap_rtc->pcap, PCAP_REG_RTC_DAY, days);
 
 	return 0;
diff --git a/drivers/rtc/rtc-stmp3xxx.c b/drivers/rtc/rtc-stmp3xxx.c
index ea96492..fe5f8bb 100644
--- a/drivers/rtc/rtc-stmp3xxx.c
+++ b/drivers/rtc/rtc-stmp3xxx.c
@@ -157,10 +157,16 @@ static int stmp3xxx_rtc_gettime(struct device *dev, struct rtc_time *rtc_tm)
 	return 0;
 }
 
-static int stmp3xxx_rtc_set_mmss(struct device *dev, unsigned long t)
+static int stmp3xxx_rtc_set_mmss(struct device *dev, time64_t t)
 {
 	struct stmp3xxx_rtc_data *rtc_data = dev_get_drvdata(dev);
 
+	/*
+	 * y2106 issue:
+	 *   On 32bit systems the time64_t secs value gets cast to
+	 *   a 32bit long, and thus we can only write a maximum value
+	 *   of y2016
+	 */
 	writel(t, rtc_data->io + STMP3XXX_RTC_SECONDS);
 	return stmp3xxx_wait_time(rtc_data);
 }
diff --git a/drivers/rtc/rtc-test.c b/drivers/rtc/rtc-test.c
index 6599c20..3e831bc 100644
--- a/drivers/rtc/rtc-test.c
+++ b/drivers/rtc/rtc-test.c
@@ -34,9 +34,9 @@ static int test_rtc_read_time(struct device *dev,
 	return 0;
 }
 
-static int test_rtc_set_mmss(struct device *dev, unsigned long secs)
+static int test_rtc_set_mmss(struct device *dev, time64_t secs)
 {
-	dev_info(dev, "%s, secs = %lu\n", __func__, secs);
+	dev_info(dev, "%s, secs = %lld\n", __func__, (long long)secs);
 	return 0;
 }
 
diff --git a/drivers/rtc/rtc-tx4939.c b/drivers/rtc/rtc-tx4939.c
index 2e678c6..4165442 100644
--- a/drivers/rtc/rtc-tx4939.c
+++ b/drivers/rtc/rtc-tx4939.c
@@ -42,13 +42,19 @@ static int tx4939_rtc_cmd(struct tx4939_rtc_reg __iomem *rtcreg, int cmd)
 	return 0;
 }
 
-static int tx4939_rtc_set_mmss(struct device *dev, unsigned long secs)
+static int tx4939_rtc_set_mmss(struct device *dev, time64_t secs)
 {
 	struct tx4939rtc_plat_data *pdata = get_tx4939rtc_plat_data(dev);
 	struct tx4939_rtc_reg __iomem *rtcreg = pdata->rtcreg;
 	int i, ret;
 	unsigned char buf[6];
 
+	/*
+	 * y2106 issue:
+	 *   On 32bit systems the time64_t secs value gets cast to
+	 *   a 32bit long, and thus we can only write a maximum value
+	 *   of y2016
+	 */
 	buf[0] = 0;
 	buf[1] = 0;
 	buf[2] = secs;
diff --git a/drivers/rtc/rtc-wm831x.c b/drivers/rtc/rtc-wm831x.c
index 75aea4c..f9c03f4 100644
--- a/drivers/rtc/rtc-wm831x.c
+++ b/drivers/rtc/rtc-wm831x.c
@@ -169,15 +169,21 @@ static int wm831x_rtc_readtime(struct device *dev, struct rtc_time *tm)
 /*
  * Set current time and date in RTC
  */
-static int wm831x_rtc_set_mmss(struct device *dev, unsigned long time)
+static int wm831x_rtc_set_mmss(struct device *dev, time64_t time)
 {
 	struct wm831x_rtc *wm831x_rtc = dev_get_drvdata(dev);
 	struct wm831x *wm831x = wm831x_rtc->wm831x;
 	struct rtc_time new_tm;
-	unsigned long new_time;
+	time64_t new_time;
 	int ret;
 	int count = 0;
 
+	/*
+	 * y2106 issue:
+	 *   On 32bit systems the time64_t secs value gets cast to
+	 *   a 32bit long, and thus we can only write a maximum value
+	 *   of y2016
+	 */
 	ret = wm831x_reg_write(wm831x, WM831X_RTC_TIME_1,
 			       (time >> 16) & 0xffff);
 	if (ret < 0) {
@@ -215,11 +221,7 @@ static int wm831x_rtc_set_mmss(struct device *dev, unsigned long time)
 	if (ret < 0)
 		return ret;
 
-	ret = rtc_tm_to_time(&new_tm, &new_time);
-	if (ret < 0) {
-		dev_err(dev, "Failed to convert time: %d\n", ret);
-		return ret;
-	}
+	new_time = rtc_tm_to_time64(&new_tm);
 
 	/* Allow a second of change in case of tick */
 	if (new_time - time > 1) {
diff --git a/drivers/rtc/rtc-xgene.c b/drivers/rtc/rtc-xgene.c
index 14129cc..5dd329d 100644
--- a/drivers/rtc/rtc-xgene.c
+++ b/drivers/rtc/rtc-xgene.c
@@ -62,13 +62,17 @@ static int xgene_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	return rtc_valid_tm(tm);
 }
 
-static int xgene_rtc_set_mmss(struct device *dev, unsigned long secs)
+static int xgene_rtc_set_mmss(struct device *dev, time64_t secs)
 {
 	struct xgene_rtc_dev *pdata = dev_get_drvdata(dev);
 
 	/*
 	 * NOTE: After the following write, the RTC_CCVR is only reflected
 	 *       after the update cycle of 1 seconds.
+	 * y2106 issue:
+	 *   On 32bit systems the time64_t secs value gets cast to
+	 *   a 32bit long, and thus we can only write a maximum value
+	 *   of y2016
 	 */
 	writel((u32) secs, pdata->csr_base + RTC_CLR);
 	readl(pdata->csr_base + RTC_CLR); /* Force a barrier */
diff --git a/include/linux/rtc.h b/include/linux/rtc.h
index 6d6be09..d080606 100644
--- a/include/linux/rtc.h
+++ b/include/linux/rtc.h
@@ -77,7 +77,7 @@ struct rtc_class_ops {
 	int (*read_alarm)(struct device *, struct rtc_wkalrm *);
 	int (*set_alarm)(struct device *, struct rtc_wkalrm *);
 	int (*proc)(struct device *, struct seq_file *);
-	int (*set_mmss)(struct device *, unsigned long secs);
+	int (*set_mmss)(struct device *, time64_t secs);
 	int (*read_callback)(struct device *, int data);
 	int (*alarm_irq_enable)(struct device *, unsigned int enabled);
 };
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 3/4] rtc/lib: Provide interfaces to map between 32bit hardware and 64bit time
  2014-11-27 12:02 [RFC PATCH 0/4] Add rtc 64bit epoch offset for rtc hardware that only provides 32bit time Xunlei Pang
  2014-11-27 12:02 ` [RFC PATCH 1/4] rtc/mxc: Convert get_alarm_or_time()/set_alarm_or_time() to use time64_t Xunlei Pang
  2014-11-27 12:02 ` [RFC PATCH 2/4] rtc: Convert rtc_class_ops.set_mmss() " Xunlei Pang
@ 2014-11-27 12:02 ` Xunlei Pang
  2014-11-27 23:16   ` Thomas Gleixner
  2014-11-27 12:02 ` [RFC PATCH 4/4] rtc/imxdi: Update driver to address time issues Xunlei Pang
  3 siblings, 1 reply; 18+ messages in thread
From: Xunlei Pang @ 2014-11-27 12:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: rtc-linux, Thomas Gleixner, Alessandro Zummo, Sven Schnelle,
	Xunlei Pang, John Stultz, Arnd Bergmann

To attack the 32bit rtc hardware limitations which will bring about
y2106 issues to their rtc drivers as those marked "y2106 issue" tags
in the former patch, this patch adds a common rtc epoch offset, and
provides two interfaces to map between 32bit rtc hardware and 64bit
time using this rtc epoch offset. These two interfaces can be used
to write rtc drivers for 32-bit rtc hardware.

- Add "rtc_epoch" boot parameter, default offset value is hardcoded
  to 1970 for consistency with Unix epoch. This default value can be
  overrided through boot command if you have problematic rtc hardware.
  Once you are doing this, should be aware of following side effects:
    a) Can't set time before your rtc epoch.
    b) Can't boot into different OSes relying on the same rtc hardware.
  "rtc epoch" must stay invariable once finalized on one machine.
- Add rtc_time64_to_hw32(): This interface converts 64-bit seconds
  since unix 1970 epoch to 32bit seconds since rtc epoch which can
  be maintained by 32-bit rtc hardware.
- Add rtc_hw32_to_time64(): This interface converts 32bit seconds
  since rtc epoch which can be used by 32-bit rtc hardware to 64bit
  seconds since unix 1970 epoch.

There're also other ways to implement this function: For example,
some driver(vr41xx) implements the same logic using RTC_EPOCH_SET
ioctl and an rtc epoch offset variable. But the approach in this
patch is more flexible and common than that.

Cc: John Stultz <john.stultz@linaro.org>
Cc: Arnd Bergmann <arnd.bergmann@linaro.org>
Signed-off-by: Xunlei Pang <pang.xunlei@linaro.org>
---
 drivers/rtc/class.c   |   25 +++++++++++++++++++++++++
 drivers/rtc/rtc-lib.c |   28 ++++++++++++++++++++++++++++
 include/linux/rtc.h   |    3 +++
 3 files changed, 56 insertions(+)

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index 38e26be..7f1950f8 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -358,6 +358,29 @@ void devm_rtc_device_unregister(struct device *dev, struct rtc_device *rtc)
 }
 EXPORT_SYMBOL_GPL(devm_rtc_device_unregister);
 
+/* Rtc epoch year, can be overrided by command line */
+static unsigned int rtc_epoch = 1970;
+time64_t rtc_epoch_secs_since_1970;
+
+static int __init
+set_rtc_epoch(char *str)
+{
+	unsigned int epoch;
+
+	if (kstrtou32(str, 0, &epoch) != 0) {
+		printk(KERN_WARNING "Invalid setup epoch %u!\n", epoch);
+		return 1;
+	}
+
+	if (rtc_epoch < 1970)
+		printk(KERN_WARNING "Epoch %u is smaller than 1970!\n", epoch);
+	else
+		rtc_epoch = epoch;
+
+	return 1;
+}
+__setup("rtc_epoch=", set_rtc_epoch);
+
 static int __init rtc_init(void)
 {
 	rtc_class = class_create(THIS_MODULE, "rtc");
@@ -366,6 +389,8 @@ static int __init rtc_init(void)
 		return PTR_ERR(rtc_class);
 	}
 	rtc_class->pm = RTC_CLASS_DEV_PM_OPS;
+
+	rtc_epoch_secs_since_1970 = mktime64(rtc_epoch, 1, 1, 0, 0, 0);
 	rtc_dev_init();
 	rtc_sysfs_init(rtc_class);
 	return 0;
diff --git a/drivers/rtc/rtc-lib.c b/drivers/rtc/rtc-lib.c
index e6bfb9c..d26814c 100644
--- a/drivers/rtc/rtc-lib.c
+++ b/drivers/rtc/rtc-lib.c
@@ -124,6 +124,34 @@ time64_t rtc_tm_to_time64(struct rtc_time *tm)
 EXPORT_SYMBOL(rtc_tm_to_time64);
 
 /*
+ * Convert seconds since rtc epoch to seconds since Unix epoch.
+ */
+time64_t rtc_hw32_to_time64(u32 hwtime)
+{
+	return (rtc_epoch_secs_since_1970 + hwtime);
+}
+EXPORT_SYMBOL_GPL(rtc_hw32_to_time64);
+
+/*
+ * Convert seconds since Unix epoch to seconds since rtc epoch.
+ */
+int rtc_time64_to_hw32(time64_t time, u32 *hwtime)
+{
+	/* time must be after rtc epoch */
+	if (time < rtc_epoch_secs_since_1970)
+		return -EINVAL;
+
+	/* rtc epoch may be too small? */
+	if (time - rtc_epoch_secs_since_1970 > UINT_MAX)
+		return -EOVERFLOW;
+
+	*hwtime = time - rtc_epoch_secs_since_1970;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(rtc_time64_to_hw32);
+
+/*
  * Convert rtc_time to ktime
  */
 ktime_t rtc_tm_to_ktime(struct rtc_time tm)
diff --git a/include/linux/rtc.h b/include/linux/rtc.h
index d080606..32f7c22 100644
--- a/include/linux/rtc.h
+++ b/include/linux/rtc.h
@@ -16,11 +16,14 @@
 #include <linux/interrupt.h>
 #include <uapi/linux/rtc.h>
 
+extern time64_t rtc_epoch_secs_since_1970;
 extern int rtc_month_days(unsigned int month, unsigned int year);
 extern int rtc_year_days(unsigned int day, unsigned int month, unsigned int year);
 extern int rtc_valid_tm(struct rtc_time *tm);
 extern time64_t rtc_tm_to_time64(struct rtc_time *tm);
 extern void rtc_time64_to_tm(time64_t time, struct rtc_time *tm);
+extern int rtc_time64_to_hw32(time64_t time, u32 *hwtime);
+extern time64_t rtc_hw32_to_time64(u32 hwtime);
 ktime_t rtc_tm_to_ktime(struct rtc_time tm);
 struct rtc_time rtc_ktime_to_tm(ktime_t kt);
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 4/4] rtc/imxdi: Update driver to address time issues
  2014-11-27 12:02 [RFC PATCH 0/4] Add rtc 64bit epoch offset for rtc hardware that only provides 32bit time Xunlei Pang
                   ` (2 preceding siblings ...)
  2014-11-27 12:02 ` [RFC PATCH 3/4] rtc/lib: Provide interfaces to map between 32bit hardware and 64bit time Xunlei Pang
@ 2014-11-27 12:02 ` Xunlei Pang
  2014-11-27 23:24   ` Thomas Gleixner
  3 siblings, 1 reply; 18+ messages in thread
From: Xunlei Pang @ 2014-11-27 12:02 UTC (permalink / raw)
  To: linux-kernel
  Cc: rtc-linux, Thomas Gleixner, Alessandro Zummo, Sven Schnelle,
	Xunlei Pang, John Stultz, Arnd Bergmann

The rtc/imxdi rtc driver has a number of y2038/y2106 issues, the worst
one is its hardware only supports 32-bit time.

This patch resolves them based on our foregoing efforts:
- Replace rtc_time_to_tm() with rtc_hw32_to_time64()/rtc_time64_to_tm()
- Replace rtc_tm_to_time() with rtc_tm_to_time64()/rtc_time64_to_hw32()
- Use rtc_time64_to_hw32() to handle dryice_rtc_set_mmss()

After this patch, the drivers should not have any remaining y2038/y2106
issues.

Cc: John Stultz <john.stultz@linaro.org>
Cc: Arnd Bergmann <arnd.bergmann@linaro.org>
Signed-off-by: Xunlei Pang <pang.xunlei@linaro.org>
---
 drivers/rtc/rtc-imxdi.c |   41 +++++++++++++++++++++--------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/drivers/rtc/rtc-imxdi.c b/drivers/rtc/rtc-imxdi.c
index 34d76b2..015290f 100644
--- a/drivers/rtc/rtc-imxdi.c
+++ b/drivers/rtc/rtc-imxdi.c
@@ -197,10 +197,10 @@ out:
 static int dryice_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
 	struct imxdi_dev *imxdi = dev_get_drvdata(dev);
-	unsigned long now;
+	time64_t now;
 
-	now = __raw_readl(imxdi->ioaddr + DTCMR);
-	rtc_time_to_tm(now, tm);
+	now = rtc_hw32_to_time64(__raw_readl(imxdi->ioaddr + DTCMR));
+	rtc_time64_to_tm(now, tm);
 
 	return 0;
 }
@@ -213,17 +213,16 @@ static int dryice_rtc_set_mmss(struct device *dev, time64_t secs)
 {
 	struct imxdi_dev *imxdi = dev_get_drvdata(dev);
 	int rc;
+	u32 hwtime;
+
+	rc = rtc_time64_to_hw32(secs, &hwtime);
+	if (rc < 0)
+		return rc;
 
 	/* zero the fractional part first */
 	rc = di_write_wait(imxdi, 0, DTCLR);
 	if (rc == 0)
-		/*
-		 * y2106 issue:
-		 *   On 32bit systems the time64_t secs value gets cast to
-		 *   a 32bit long, and thus we can only write a maximum value
-		 *   of y2016
-		 */
-		rc = di_write_wait(imxdi, secs, DTCMR);
+		rc = di_write_wait(imxdi, hwtime, DTCMR);
 
 	return rc;
 }
@@ -248,10 +247,10 @@ static int dryice_rtc_alarm_irq_enable(struct device *dev,
 static int dryice_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 {
 	struct imxdi_dev *imxdi = dev_get_drvdata(dev);
-	u32 dcamr;
+	time64_t alarm_time;
 
-	dcamr = __raw_readl(imxdi->ioaddr + DCAMR);
-	rtc_time_to_tm(dcamr, &alarm->time);
+	alarm_time = rtc_hw32_to_time64(__raw_readl(imxdi->ioaddr + DCAMR));
+	rtc_time64_to_tm(alarm_time, &alarm->time);
 
 	/* alarm is enabled if the interrupt is enabled */
 	alarm->enabled = (__raw_readl(imxdi->ioaddr + DIER) & DIER_CAIE) != 0;
@@ -273,21 +272,23 @@ static int dryice_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 static int dryice_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm)
 {
 	struct imxdi_dev *imxdi = dev_get_drvdata(dev);
-	unsigned long now;
-	unsigned long alarm_time;
+	time64_t alarm_time, now;
+	u32 alarm_hwtime;
 	int rc;
 
-	rc = rtc_tm_to_time(&alarm->time, &alarm_time);
-	if (rc)
-		return rc;
+	alarm_time = rtc_tm_to_time64(&alarm->time);
+	now = rtc_hw32_to_time64(__raw_readl(imxdi->ioaddr + DTCMR));
 
 	/* don't allow setting alarm in the past */
-	now = __raw_readl(imxdi->ioaddr + DTCMR);
 	if (alarm_time < now)
 		return -EINVAL;
 
+	rc = rtc_time64_to_hw32(alarm_time, &alarm_hwtime);
+	if (rc < 0)
+		return rc;
+
 	/* write the new alarm time */
-	rc = di_write_wait(imxdi, (u32)alarm_time, DCAMR);
+	rc = di_write_wait(imxdi, alarm_hwtime, DCAMR);
 	if (rc)
 		return rc;
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 1/4] rtc/mxc: Convert get_alarm_or_time()/set_alarm_or_time() to use time64_t
  2014-11-27 12:02 ` [RFC PATCH 1/4] rtc/mxc: Convert get_alarm_or_time()/set_alarm_or_time() to use time64_t Xunlei Pang
@ 2014-11-27 23:02   ` Thomas Gleixner
  2014-11-27 23:47     ` Arnd Bergmann
  0 siblings, 1 reply; 18+ messages in thread
From: Thomas Gleixner @ 2014-11-27 23:02 UTC (permalink / raw)
  To: Xunlei Pang
  Cc: linux-kernel, rtc-linux, Alessandro Zummo, Sven Schnelle,
	John Stultz, Arnd Bergmann

On Thu, 27 Nov 2014, Xunlei Pang wrote:
> We want to convert y2038 unsafe rtc_class_ops.set_mmss() and all its
> users to use time64_t. mxc_rtc_set_mmss() in "mxc" driver is one of
> its users, but it uses get_alarm_or_time()/set_alarm_or_time() internal
> interfaces which are also y2038 unsafe.
> 
> So here as a separate patch, it converts these two internal interfaces
> of "mxc" to use safe time64_t first, so as to make some preparations
> for the rtc_class_ops.set_mmss() conversion.
> 
> Currently, "mxc" is the only driver with such issue.

Well. The driver has some more issues and again you are just blindly
following your y2038 agenda instead of looking what this stuff is
actually doing.
 
> -static u32 get_alarm_or_time(struct device *dev, int time_alarm)
> +static time64_t get_alarm_or_time(struct device *dev, int time_alarm)
>  {
>  	struct platform_device *pdev = to_platform_device(dev);
>  	struct rtc_plat_data *pdata = platform_get_drvdata(pdev);
> @@ -129,29 +129,28 @@ static u32 get_alarm_or_time(struct device *dev, int time_alarm)
>  	hr = hr_min >> 8;
>  	min = hr_min & 0xff;
>  
> -	return (((day * 24 + hr) * 60) + min) * 60 + sec;
> +	return ((((time64_t)day * 24 + hr) * 60) + min) * 60 + sec;

So why does this convert a split representation, which could be easily
represented in a struct rtc_time to time64t?

Now looking at the usage sites:

>  static int rtc_update_alarm(struct device *dev, struct rtc_time *alrm)
>  {
>  	struct rtc_time alarm_tm, now_tm;
> -	unsigned long now, time;
> +	time64_t now, time;
>  	struct platform_device *pdev = to_platform_device(dev);
>  	struct rtc_plat_data *pdata = platform_get_drvdata(pdev);
>  	void __iomem *ioaddr = pdata->ioaddr;
>  
>  	now = get_alarm_or_time(dev, MXC_RTC_TIME);
> -	rtc_time_to_tm(now, &now_tm);
> +	rtc_time64_to_tm(now, &now_tm);

So here you convert that to struct rtc_time.

>  	alarm_tm.tm_year = now_tm.tm_year;
>  	alarm_tm.tm_mon = now_tm.tm_mon;
>  	alarm_tm.tm_mday = now_tm.tm_mday;
>  	alarm_tm.tm_hour = alrm->tm_hour;
>  	alarm_tm.tm_min = alrm->tm_min;
>  	alarm_tm.tm_sec = alrm->tm_sec;
> -	rtc_tm_to_time(&alarm_tm, &time);
> +	time = rtc_tm_to_time64(&alarm_tm);

Just to convert it back and then do the reverse operation in
set_alarm_or_time()
  
>  static int mxc_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  {
> -	u32 val;
> +	time64_t val;
>  
>  	/* Avoid roll-over from reading the different registers */
>  	do {
>  		val = get_alarm_or_time(dev, MXC_RTC_TIME);
>  	} while (val != get_alarm_or_time(dev, MXC_RTC_TIME));
>  
> -	rtc_time_to_tm(val, tm);
> +	rtc_time64_to_tm(val, tm);

Ditto
 
>  	return 0;
>  }
> @@ -333,7 +332,7 @@ static int mxc_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>  	struct rtc_plat_data *pdata = platform_get_drvdata(pdev);
>  	void __iomem *ioaddr = pdata->ioaddr;
>  
> -	rtc_time_to_tm(get_alarm_or_time(dev, MXC_RTC_ALARM), &alrm->time);
> +	rtc_time64_to_tm(get_alarm_or_time(dev, MXC_RTC_ALARM), &alrm->time);

Ditto.

Makes a lot of sense? NOT

I did not have to look at the actual code to notice that
nonsense. Looking at the patch was enough.

Can you folks please stop to run shell scripts without looking at the
actual code?

This mechanical attempt to fix the 2038 issue w/o switching on
braincells is just annoying as hell. Dammit, if you touch stuff then
you better look at it and fix it proper instead of copying the same
crap over and over.

Thanks,

	tglx


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 2/4] rtc: Convert rtc_class_ops.set_mmss() to use time64_t
  2014-11-27 12:02 ` [RFC PATCH 2/4] rtc: Convert rtc_class_ops.set_mmss() " Xunlei Pang
@ 2014-11-27 23:05   ` Thomas Gleixner
  2014-11-27 23:23     ` Arnd Bergmann
  2014-11-28 16:49   ` pang.xunlei
  1 sibling, 1 reply; 18+ messages in thread
From: Thomas Gleixner @ 2014-11-27 23:05 UTC (permalink / raw)
  To: Xunlei Pang
  Cc: linux-kernel, rtc-linux, Alessandro Zummo, Sven Schnelle,
	John Stultz, Arnd Bergmann

On Thu, 27 Nov 2014, Xunlei Pang wrote:
> -static int coh901331_set_mmss(struct device *dev, unsigned long secs)
> +static int coh901331_set_mmss(struct device *dev, time64_t secs)
>  {
>  	struct coh901331_port *rtap = dev_get_drvdata(dev);
>  
>  	clk_enable(rtap->clk);
> +	/*
> +	 * y2106 issue:
> +	 *   On 32bit systems the time64_t secs value gets cast to
> +	 *   a 32bit long, and thus we can only write a maximum value
> +	 *   of y2016

That really makes a lot of sense. Before that patch the driver was
safe up to 2038. Now it is facing the y2016 problem.

> +	/*
> +	 * y2106 issue:
> +	 *   On 32bit systems the time64_t secs value gets cast to
> +	 *   a 32bit long, and thus we can only write a maximum value
> +	 *   of y2016

Copy and paste is wonderful, right?

Sigh,

	tglx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 3/4] rtc/lib: Provide interfaces to map between 32bit hardware and 64bit time
  2014-11-27 12:02 ` [RFC PATCH 3/4] rtc/lib: Provide interfaces to map between 32bit hardware and 64bit time Xunlei Pang
@ 2014-11-27 23:16   ` Thomas Gleixner
  2014-11-28 16:10     ` pang.xunlei
  0 siblings, 1 reply; 18+ messages in thread
From: Thomas Gleixner @ 2014-11-27 23:16 UTC (permalink / raw)
  To: Xunlei Pang
  Cc: linux-kernel, rtc-linux, Alessandro Zummo, Sven Schnelle,
	John Stultz, Arnd Bergmann

On Thu, 27 Nov 2014, Xunlei Pang wrote:
> +/* Rtc epoch year, can be overrided by command line */
> +static unsigned int rtc_epoch = 1970;
> +time64_t rtc_epoch_secs_since_1970;
> +
> +static int __init
> +set_rtc_epoch(char *str)

One line please

> +{
> +	unsigned int epoch;
> +
> +	if (kstrtou32(str, 0, &epoch) != 0) {
> +		printk(KERN_WARNING "Invalid setup epoch %u!\n", epoch);

Sure that makes a lot of sense to print epoch if the string conversion
failed. epoch is going to be either uninitialized stack value or 0
depending on the kstrtou32 implementation.

> +	if (rtc_epoch < 1970)
> +		printk(KERN_WARNING "Epoch %u is smaller than 1970!\n", epoch);

  pr_warn() ....

+__setup("rtc_epoch=", set_rtc_epoch);

Not documented in Documentation/kernel-parameters.txt ...

>  /*
> + * Convert seconds since rtc epoch to seconds since Unix epoch.
> + */
> +time64_t rtc_hw32_to_time64(u32 hwtime)
> +{
> +	return (rtc_epoch_secs_since_1970 + hwtime);
> +}
> +EXPORT_SYMBOL_GPL(rtc_hw32_to_time64);

So we need a exported function to add a global variable and a function
argument? inline ?

> +/*
> + * Convert seconds since Unix epoch to seconds since rtc epoch.
> + */
> +int rtc_time64_to_hw32(time64_t time, u32 *hwtime)
> +{
> +	/* time must be after rtc epoch */
> +	if (time < rtc_epoch_secs_since_1970)
> +		return -EINVAL;
> +
> +	/* rtc epoch may be too small? */
> +	if (time - rtc_epoch_secs_since_1970 > UINT_MAX)
> +		return -EOVERFLOW;

And what's the caller supposed to do with that information?

> +	*hwtime = time - rtc_epoch_secs_since_1970;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(rtc_time64_to_hw32);

How is that going to be useful w/o a mechanism to adjust the epoch
offset at runtime by any means?

Thanks,

	tglx



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 2/4] rtc: Convert rtc_class_ops.set_mmss() to use time64_t
  2014-11-27 23:05   ` Thomas Gleixner
@ 2014-11-27 23:23     ` Arnd Bergmann
  2014-11-27 23:28       ` Thomas Gleixner
  0 siblings, 1 reply; 18+ messages in thread
From: Arnd Bergmann @ 2014-11-27 23:23 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Xunlei Pang, linux-kernel, rtc-linux, Alessandro Zummo,
	Sven Schnelle, John Stultz, Arnd Bergmann

On Friday 28 November 2014 00:05:34 Thomas Gleixner wrote:
> On Thu, 27 Nov 2014, Xunlei Pang wrote:
> > -static int coh901331_set_mmss(struct device *dev, unsigned long secs)
> > +static int coh901331_set_mmss(struct device *dev, time64_t secs)
> >  {
> >       struct coh901331_port *rtap = dev_get_drvdata(dev);
> >  
> >       clk_enable(rtap->clk);
> > +     /*
> > +      * y2106 issue:
> > +      *   On 32bit systems the time64_t secs value gets cast to
> > +      *   a 32bit long, and thus we can only write a maximum value
> > +      *   of y2016
> 
> That really makes a lot of sense. Before that patch the driver was
> safe up to 2038. Now it is facing the y2016 problem.

Actually the comment is still wrong with the number fixed, I hadn't
noticed when I looked at the patch earlier:

The cast happens on both 32-bit and 64-bit, as we cast into a u32
value through the writel(). The behavior of this driver doesn't
even change with this patch, it was good until y2106 and stays
that way because 'unsigned long', 'time64_t' and 'u32' can all represent
at least times between 1970 and 2106, the change is just to document
the time at which it will break, while changing the API.

Some other drivers in this patch actually get changed to work beyond
2106, to the full time span that their hardware register layout allows.

	Arnd

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 4/4] rtc/imxdi: Update driver to address time issues
  2014-11-27 12:02 ` [RFC PATCH 4/4] rtc/imxdi: Update driver to address time issues Xunlei Pang
@ 2014-11-27 23:24   ` Thomas Gleixner
  2014-11-28 16:20     ` pang.xunlei
  0 siblings, 1 reply; 18+ messages in thread
From: Thomas Gleixner @ 2014-11-27 23:24 UTC (permalink / raw)
  To: Xunlei Pang
  Cc: linux-kernel, rtc-linux, Alessandro Zummo, Sven Schnelle,
	John Stultz, Arnd Bergmann

On Thu, 27 Nov 2014, Xunlei Pang wrote:
> @@ -213,17 +213,16 @@ static int dryice_rtc_set_mmss(struct device *dev, time64_t secs)
>  {
>  	struct imxdi_dev *imxdi = dev_get_drvdata(dev);
>  	int rc;
> +	u32 hwtime;
> +
> +	rc = rtc_time64_to_hw32(secs, &hwtime);
> +	if (rc < 0)
> +		return rc;
>  
>  	/* zero the fractional part first */
>  	rc = di_write_wait(imxdi, 0, DTCLR);
>  	if (rc == 0)
> -		/*
> -		 * y2106 issue:
> -		 *   On 32bit systems the time64_t secs value gets cast to
> -		 *   a 32bit long, and thus we can only write a maximum value
> -		 *   of y2016
> -		 */
> -		rc = di_write_wait(imxdi, secs, DTCMR);
> +		rc = di_write_wait(imxdi, hwtime, DTCMR);

So you repeat the same thing what I complained about last time. First
you add crap to a driver then you remove it again instead of analyzing
the necessary conversions in the first place, provide the
infrastructure and then do a per driver conversion. It's not that
hard, really and I'm getting tired of your approach.

Step 1:	     Provide rtc.set_mmss64

Step 2:	     Implement the epoch helper functions for 32bit hardware

Step 3:	     Convert drivers

Step 4:	     Remove obsolete interfaces

I wont explain that once more, really.

Thanks,

	tglx


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 2/4] rtc: Convert rtc_class_ops.set_mmss() to use time64_t
  2014-11-27 23:23     ` Arnd Bergmann
@ 2014-11-27 23:28       ` Thomas Gleixner
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Gleixner @ 2014-11-27 23:28 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Xunlei Pang, linux-kernel, rtc-linux, Alessandro Zummo,
	Sven Schnelle, John Stultz, Arnd Bergmann

On Fri, 28 Nov 2014, Arnd Bergmann wrote:
> On Friday 28 November 2014 00:05:34 Thomas Gleixner wrote:
> > On Thu, 27 Nov 2014, Xunlei Pang wrote:
> > > -static int coh901331_set_mmss(struct device *dev, unsigned long secs)
> > > +static int coh901331_set_mmss(struct device *dev, time64_t secs)
> > >  {
> > >       struct coh901331_port *rtap = dev_get_drvdata(dev);
> > >  
> > >       clk_enable(rtap->clk);
> > > +     /*
> > > +      * y2106 issue:
> > > +      *   On 32bit systems the time64_t secs value gets cast to
> > > +      *   a 32bit long, and thus we can only write a maximum value
> > > +      *   of y2016
> > 
> > That really makes a lot of sense. Before that patch the driver was
> > safe up to 2038. Now it is facing the y2016 problem.
> 
> Actually the comment is still wrong with the number fixed, I hadn't
> noticed when I looked at the patch earlier:
> 
> The cast happens on both 32-bit and 64-bit, as we cast into a u32
> value through the writel(). The behavior of this driver doesn't
> even change with this patch, it was good until y2106 and stays
> that way because 'unsigned long', 'time64_t' and 'u32' can all represent
> at least times between 1970 and 2106, the change is just to document
> the time at which it will break, while changing the API.

Indeed. I did not bother to think about that as I was already in
grumpy mode due to the general aproach of creating the maximal churn
for no value.
 
Thanks,

	tglx

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 1/4] rtc/mxc: Convert get_alarm_or_time()/set_alarm_or_time() to use time64_t
  2014-11-27 23:02   ` Thomas Gleixner
@ 2014-11-27 23:47     ` Arnd Bergmann
  2014-11-28 15:58       ` pang.xunlei
  0 siblings, 1 reply; 18+ messages in thread
From: Arnd Bergmann @ 2014-11-27 23:47 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Xunlei Pang, linux-kernel, rtc-linux, Alessandro Zummo,
	Sven Schnelle, John Stultz, Arnd Bergmann

On Friday 28 November 2014 00:02:47 Thomas Gleixner wrote:
> 
> >  static int rtc_update_alarm(struct device *dev, struct rtc_time *alrm)
> >  {
> >       struct rtc_time alarm_tm, now_tm;
> > -     unsigned long now, time;
> > +     time64_t now, time;
> >       struct platform_device *pdev = to_platform_device(dev);
> >       struct rtc_plat_data *pdata = platform_get_drvdata(pdev);
> >       void __iomem *ioaddr = pdata->ioaddr;
> >  
> >       now = get_alarm_or_time(dev, MXC_RTC_TIME);
> > -     rtc_time_to_tm(now, &now_tm);
> > +     rtc_time64_to_tm(now, &now_tm);
> 
> So here you convert that to struct rtc_time.
> 
> >       alarm_tm.tm_year = now_tm.tm_year;
> >       alarm_tm.tm_mon = now_tm.tm_mon;
> >       alarm_tm.tm_mday = now_tm.tm_mday;
> >       alarm_tm.tm_hour = alrm->tm_hour;
> >       alarm_tm.tm_min = alrm->tm_min;
> >       alarm_tm.tm_sec = alrm->tm_sec;
> > -     rtc_tm_to_time(&alarm_tm, &time);
> > +     time = rtc_tm_to_time64(&alarm_tm);
> 
> Just to convert it back and then do the reverse operation in
> set_alarm_or_time()

Apparently the code originally tried to make the alarm fire within 
the next 24 hours. What you and the author of c92182ee0b5a3
("drivers/rtc/rtc-mxc.c: make alarm work") apparently missed is that
it is taking the year/mon/mday value of today and the hour/min/sec
value from the function argument.

I think the idea was to make it work with user space that sets only
the last three fields (which would work on the typical x86 rtc),
but after that other patch, the function no longer makes any sense,
since it will set the alarm in the past half of the time.

	Arnd

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 1/4] rtc/mxc: Convert get_alarm_or_time()/set_alarm_or_time() to use time64_t
  2014-11-27 23:47     ` Arnd Bergmann
@ 2014-11-28 15:58       ` pang.xunlei
  0 siblings, 0 replies; 18+ messages in thread
From: pang.xunlei @ 2014-11-28 15:58 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Thomas Gleixner, lkml, rtc-linux, Alessandro Zummo,
	Sven Schnelle, John Stultz, Arnd Bergmann

On 28 November 2014 at 07:47, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 28 November 2014 00:02:47 Thomas Gleixner wrote:
>>
>> >  static int rtc_update_alarm(struct device *dev, struct rtc_time *alrm)
>> >  {
>> >       struct rtc_time alarm_tm, now_tm;
>> > -     unsigned long now, time;
>> > +     time64_t now, time;
>> >       struct platform_device *pdev = to_platform_device(dev);
>> >       struct rtc_plat_data *pdata = platform_get_drvdata(pdev);
>> >       void __iomem *ioaddr = pdata->ioaddr;
>> >
>> >       now = get_alarm_or_time(dev, MXC_RTC_TIME);
>> > -     rtc_time_to_tm(now, &now_tm);
>> > +     rtc_time64_to_tm(now, &now_tm);
>>
>> So here you convert that to struct rtc_time.
>>
>> >       alarm_tm.tm_year = now_tm.tm_year;
>> >       alarm_tm.tm_mon = now_tm.tm_mon;
>> >       alarm_tm.tm_mday = now_tm.tm_mday;
>> >       alarm_tm.tm_hour = alrm->tm_hour;
>> >       alarm_tm.tm_min = alrm->tm_min;
>> >       alarm_tm.tm_sec = alrm->tm_sec;
>> > -     rtc_tm_to_time(&alarm_tm, &time);
>> > +     time = rtc_tm_to_time64(&alarm_tm);
>>
>> Just to convert it back and then do the reverse operation in
>> set_alarm_or_time()
>
> Apparently the code originally tried to make the alarm fire within
> the next 24 hours. What you and the author of c92182ee0b5a3
> ("drivers/rtc/rtc-mxc.c: make alarm work") apparently missed is that
> it is taking the year/mon/mday value of today and the hour/min/sec
> value from the function argument.
>
> I think the idea was to make it work with user space that sets only
> the last three fields (which would work on the typical x86 rtc),
> but after that other patch, the function no longer makes any sense,
> since it will set the alarm in the past half of the time.

In rtc_dev_ioctl() RTC_ALM_SET, there is some common code handling such issue:
/* alarm may need to wrap into tomorrow */
if (then < now) {
rtc_time_to_tm(now + 24 * 60 * 60, &tm);
alarm.time.tm_mday = tm.tm_mday;
alarm.time.tm_mon = tm.tm_mon;
alarm.time.tm_year = tm.tm_year;
}
}

return rtc_set_alarm(rtc, &alarm);

But it still has a small window may set the past alarm time, even in
rtc drivers' set_alarm().
I think the driver should behave as follows to completely avoid this issue:

disable local irq

make sure the alarm time is later than now by delta, this delta should
be long enough to finish regs operations.

set alarm regs

enable local irq

Sadly, lots of rtc drivers have problems.

Regards,
-Xunlei
>
>         Arnd

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 3/4] rtc/lib: Provide interfaces to map between 32bit hardware and 64bit time
  2014-11-27 23:16   ` Thomas Gleixner
@ 2014-11-28 16:10     ` pang.xunlei
  2014-12-01 21:12       ` Thomas Gleixner
  0 siblings, 1 reply; 18+ messages in thread
From: pang.xunlei @ 2014-11-28 16:10 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: lkml, rtc-linux, Alessandro Zummo, Sven Schnelle, John Stultz,
	Arnd Bergmann

On 28 November 2014 at 07:16, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, 27 Nov 2014, Xunlei Pang wrote:
>> +/* Rtc epoch year, can be overrided by command line */
>> +static unsigned int rtc_epoch = 1970;
>> +time64_t rtc_epoch_secs_since_1970;
>> +
>> +static int __init
>> +set_rtc_epoch(char *str)
>
> One line please
>
>> +{
>> +     unsigned int epoch;
>> +
>> +     if (kstrtou32(str, 0, &epoch) != 0) {
>> +             printk(KERN_WARNING "Invalid setup epoch %u!\n", epoch);
>
> Sure that makes a lot of sense to print epoch if the string conversion
> failed. epoch is going to be either uninitialized stack value or 0
> depending on the kstrtou32 implementation.
>
>> +     if (rtc_epoch < 1970)
>> +             printk(KERN_WARNING "Epoch %u is smaller than 1970!\n", epoch);
>
>   pr_warn() ....
>
> +__setup("rtc_epoch=", set_rtc_epoch);
>
> Not documented in Documentation/kernel-parameters.txt ...
>
>>  /*
>> + * Convert seconds since rtc epoch to seconds since Unix epoch.
>> + */
>> +time64_t rtc_hw32_to_time64(u32 hwtime)
>> +{
>> +     return (rtc_epoch_secs_since_1970 + hwtime);
>> +}
>> +EXPORT_SYMBOL_GPL(rtc_hw32_to_time64);
>
> So we need a exported function to add a global variable and a function
> argument? inline ?
>
>> +/*
>> + * Convert seconds since Unix epoch to seconds since rtc epoch.
>> + */
>> +int rtc_time64_to_hw32(time64_t time, u32 *hwtime)
>> +{
>> +     /* time must be after rtc epoch */
>> +     if (time < rtc_epoch_secs_since_1970)
>> +             return -EINVAL;
>> +
>> +     /* rtc epoch may be too small? */
>> +     if (time - rtc_epoch_secs_since_1970 > UINT_MAX)
>> +             return -EOVERFLOW;
>
> And what's the caller supposed to do with that information?

This probably means the current rtc hardware has pushed its envelope now,
the users should rely on the rtc_epoch command line to continue use it.

>
>> +     *hwtime = time - rtc_epoch_secs_since_1970;
>> +
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(rtc_time64_to_hw32);
>
> How is that going to be useful w/o a mechanism to adjust the epoch
> offset at runtime by any means?

If the rtc epoch changes at runtime, we'll not get the right time once
the system is rebooting.
So, it can only be altered throught command line, and must stay
invariable ever since unless
it uses NTP or something.

Thanks,
Xunlei

>
> Thanks,
>
>         tglx
>
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 4/4] rtc/imxdi: Update driver to address time issues
  2014-11-27 23:24   ` Thomas Gleixner
@ 2014-11-28 16:20     ` pang.xunlei
  2014-12-01  0:13       ` Alessandro Zummo
  0 siblings, 1 reply; 18+ messages in thread
From: pang.xunlei @ 2014-11-28 16:20 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: lkml, rtc-linux, Alessandro Zummo, Sven Schnelle, John Stultz,
	Arnd Bergmann

On 28 November 2014 at 07:24, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, 27 Nov 2014, Xunlei Pang wrote:
>> @@ -213,17 +213,16 @@ static int dryice_rtc_set_mmss(struct device *dev, time64_t secs)
>>  {
>>       struct imxdi_dev *imxdi = dev_get_drvdata(dev);
>>       int rc;
>> +     u32 hwtime;
>> +
>> +     rc = rtc_time64_to_hw32(secs, &hwtime);
>> +     if (rc < 0)
>> +             return rc;
>>
>>       /* zero the fractional part first */
>>       rc = di_write_wait(imxdi, 0, DTCLR);
>>       if (rc == 0)
>> -             /*
>> -              * y2106 issue:
>> -              *   On 32bit systems the time64_t secs value gets cast to
>> -              *   a 32bit long, and thus we can only write a maximum value
>> -              *   of y2016
>> -              */
>> -             rc = di_write_wait(imxdi, secs, DTCMR);
>> +             rc = di_write_wait(imxdi, hwtime, DTCMR);
>
> So you repeat the same thing what I complained about last time. First
> you add crap to a driver then you remove it again instead of analyzing
> the necessary conversions in the first place, provide the
> infrastructure and then do a per driver conversion. It's not that
> hard, really and I'm getting tired of your approach.
>
> Step 1:      Provide rtc.set_mmss64
>
> Step 2:      Implement the epoch helper functions for 32bit hardware
>
> Step 3:      Convert drivers
>
> Step 4:      Remove obsolete interfaces
>
> I wont explain that once more, really.

Actually, I want to do this eariler, just thought that the "y2106 issue"
tags aren't something my patches brought about, they are more like
reminders help me with future patches.

Anyway, they are ungraceful being there. I'll refine this patchset carefully
according to your valuable suggestions, and will send out another version.

Thanks for your review!

Regards,
Xunlei

>
> Thanks,
>
>         tglx
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 2/4] rtc: Convert rtc_class_ops.set_mmss() to use time64_t
  2014-11-27 12:02 ` [RFC PATCH 2/4] rtc: Convert rtc_class_ops.set_mmss() " Xunlei Pang
  2014-11-27 23:05   ` Thomas Gleixner
@ 2014-11-28 16:49   ` pang.xunlei
  1 sibling, 0 replies; 18+ messages in thread
From: pang.xunlei @ 2014-11-28 16:49 UTC (permalink / raw)
  To: lkml
  Cc: rtc-linux, Thomas Gleixner, Alessandro Zummo, Sven Schnelle,
	Xunlei Pang, John Stultz, Arnd Bergmann

On 27 November 2014 at 20:02, Xunlei Pang <pang.xunlei@linaro.org> wrote:
> Currently the rtc_class_op's set_mmss() function takes a 32bit second
> value (on 32bit systems), which is problematic for dates past y2038.
>
> This patch resolves it by changing the interface and all users to use
> y2038 safe time64_t.
>
> Cc: John Stultz <john.stultz@linaro.org>
> Cc: Arnd Bergmann <arnd.bergmann@linaro.org>
> Signed-off-by: Xunlei Pang <pang.xunlei@linaro.org>
>
> diff --git a/drivers/rtc/rtc-ds2404.c b/drivers/rtc/rtc-ds2404.c
> index fc209dc..ec50757 100644
> --- a/drivers/rtc/rtc-ds2404.c
> +++ b/drivers/rtc/rtc-ds2404.c
> @@ -210,9 +210,16 @@ static int ds2404_read_time(struct device *dev, struct rtc_time *dt)
>         return rtc_valid_tm(dt);
>  }
>
> -static int ds2404_set_mmss(struct device *dev, unsigned long secs)
> +static int ds2404_set_mmss(struct device *dev, time64_t secs)
>  {
> +       /*
> +        * y2106 issue:
> +        *   On 32bit systems the time64_t secs value gets cast to
> +        *   a 32bit long, and thus we can only write a maximum value
> +        *   of y2016
> +        */
>         u32 time = cpu_to_le32(secs);
> +
>         ds2404_write_memory(dev, 0x203, 4, (u8 *)&time);

Hi Sven,

Does this rtc hardware support ds2404_write_memory() with length > 4,
i.e. y2106 safe?

Thanks,
Xunlei
>         return 0;
>  }

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 4/4] rtc/imxdi: Update driver to address time issues
  2014-11-28 16:20     ` pang.xunlei
@ 2014-12-01  0:13       ` Alessandro Zummo
  0 siblings, 0 replies; 18+ messages in thread
From: Alessandro Zummo @ 2014-12-01  0:13 UTC (permalink / raw)
  To: pang.xunlei
  Cc: Thomas Gleixner, lkml, rtc-linux, Sven Schnelle, John Stultz,
	Arnd Bergmann

On Sat, 29 Nov 2014 00:20:00 +0800
"pang.xunlei" <pang.xunlei@linaro.org> wrote:

> Anyway, they are ungraceful being there. I'll refine this patchset carefully
> according to your valuable suggestions, and will send out another version.
> 
> Thanks for your review!

 I have quite the feeling those patches will cause headaches
 to a lot of people.
 
 Too many of this stuff seems a copy & paste work, without actually
 looking/thinking to the side effects.

 Is there any particular reason your are pursuing this?

-- 

 Best regards,

 Alessandro Zummo,
  Tower Technologies - Torino, Italy

  http://www.towertech.it


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 3/4] rtc/lib: Provide interfaces to map between 32bit hardware and 64bit time
  2014-11-28 16:10     ` pang.xunlei
@ 2014-12-01 21:12       ` Thomas Gleixner
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Gleixner @ 2014-12-01 21:12 UTC (permalink / raw)
  To: pang.xunlei
  Cc: lkml, rtc-linux, Alessandro Zummo, Sven Schnelle, John Stultz,
	Arnd Bergmann

On Sat, 29 Nov 2014, pang.xunlei wrote:
> On 28 November 2014 at 07:16, Thomas Gleixner <tglx@linutronix.de> wrote:
> > How is that going to be useful w/o a mechanism to adjust the epoch
> > offset at runtime by any means?
> 
> If the rtc epoch changes at runtime, we'll not get the right time once
> the system is rebooting.
> So, it can only be altered throught command line, and must stay
> invariable ever since unless it uses NTP or something.

You are imposing that any machine which needs to handle this must be
rebooted in order to get support for the HW wrap around of the 32bit
seconds counter register.

That's wrong to begin with, simply because you would have to update
the command line and reboot all those machines a few milliseconds
before the wrap around occurs. Not very practicable, right?

Your argument that we wont get the proper time once the
system is rebooting is moot, as in any case there needs to be some
persistant storage which must be updated either to hold the offset
itself or the kernel command line.

So we want a mechanism to apply that change at runtime by whatever
mechanism. And that can be an ad hoc change where we update the offset
and the counter value in one go or a simple store operation which gets
effective when the counter actually wraps around.

That needs a lot more thought so I think we should drop that whole
offset magic for now and concentrate on the far more pressing problem
of time_t which affects all machines in about 24 years.

Once we have done that we can revisit the 2106 problem and design a
well thought out mechanism to handle it and not some cobbled together
ad hoc solution.

That said, I'm not too happy about your comments copied all over the
place. We rather have something which can be easily grepped for, does
not lose context and can be in the best case runtime evaluated.

The simplest solution for this is to do the following:

struct rtc_class_ops {
       ...
       int (*set_mmss)(struct device *, unsigned long secs);
+      int (*set_mmss64)(struct device *, time64_t secs);

So any RTC which is capable of handling time past 2106 will be
converted to use the new interface and leave the set_mmss callback
NULL. The device drivers which cannot be converted to handle anything
past 2106 due to hardware limitations do not need to be touched at
all.

Though we might to consider changing set_mmss() to 
-      int (*set_mmss)(struct device *, unsigned long secs);
+      int (*set_mmss32(struct device *, u32 secs);

but that can be done when the bulk has been converted to
set_mmss64. That's a single coccinelle patch w/o adding randomly
typoed comments to the wrong places.

You can find that nicely with grep, you can detect it compile time by
removing the set_mmss callback and you can evaluate it at runtime.

Once we have a proper solution for the 2106 issue we have a clear
indicator which of the drivers needs that treatment and which ones do
not. We just can deduce it from the set_mmss callback assignment.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2014-12-01 21:12 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-27 12:02 [RFC PATCH 0/4] Add rtc 64bit epoch offset for rtc hardware that only provides 32bit time Xunlei Pang
2014-11-27 12:02 ` [RFC PATCH 1/4] rtc/mxc: Convert get_alarm_or_time()/set_alarm_or_time() to use time64_t Xunlei Pang
2014-11-27 23:02   ` Thomas Gleixner
2014-11-27 23:47     ` Arnd Bergmann
2014-11-28 15:58       ` pang.xunlei
2014-11-27 12:02 ` [RFC PATCH 2/4] rtc: Convert rtc_class_ops.set_mmss() " Xunlei Pang
2014-11-27 23:05   ` Thomas Gleixner
2014-11-27 23:23     ` Arnd Bergmann
2014-11-27 23:28       ` Thomas Gleixner
2014-11-28 16:49   ` pang.xunlei
2014-11-27 12:02 ` [RFC PATCH 3/4] rtc/lib: Provide interfaces to map between 32bit hardware and 64bit time Xunlei Pang
2014-11-27 23:16   ` Thomas Gleixner
2014-11-28 16:10     ` pang.xunlei
2014-12-01 21:12       ` Thomas Gleixner
2014-11-27 12:02 ` [RFC PATCH 4/4] rtc/imxdi: Update driver to address time issues Xunlei Pang
2014-11-27 23:24   ` Thomas Gleixner
2014-11-28 16:20     ` pang.xunlei
2014-12-01  0:13       ` Alessandro Zummo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.