diff mbox

[RFC,4/4] rtc/imxdi: Update driver to address time issues

Message ID 1417089760-26848-5-git-send-email-pang.xunlei@linaro.org
State New
Headers show

Commit Message

pang.xunlei Nov. 27, 2014, 12:02 p.m. UTC 
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(-)

Comments

pang.xunlei Nov. 28, 2014, 4:20 p.m. UTC | #1 
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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
diff mbox

Patch

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;