| Message ID | 1417089760-26848-5-git-send-email-pang.xunlei@linaro.org |
|---|---|
| State | Rejected |
| Headers | show |
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
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 >
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?
On 1 December 2014 at 08:13, Alessandro Zummo <a.zummo@towertech.it> wrote: > 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? Hi Alessandro, Sorry for the confusion. Here are some background of this patchset: The kernel uses 32-bit signed value(time_t) for seconds since 1970-01-01:00:00:00, so it will overflow at 2038-01-19 03:14:08 on 32-bit systems. This is widely known as the y2038 problem. Currently, we're tackling y2038 problems of rtc subsystem. For most rtc drivers, they have y2038 issues, and now we've already provided do_settimeofday64(), rtc_tm_to_time64(), rtc_time64_to_tm(), ... to use time64_t which is s64, so those y2038 issues of y2038-safe rtc hardware(for example, hardware using 64-bit counter, using years/days/seconds regs) has been eliminated. But for the rtc hardware which is 32bit counter, those y2038 issues has become y2106 issues. please refer to my 1st patchset for details: *https://lkml.org/lkml/2014/11/18/218 <https://lkml.org/lkml/2014/11/18/218>* Thus, this patchset is made to further solve those y2106 issues (previously y2038 issues) for the y2038-unsafe rtc hardware. Then, with these 2 patchset, I will hopefully be able to convert the whole rtc subsystem over to eliminate all time issues. Thanks, Xunlei > > -- > > Best regards, > > Alessandro Zummo, > Tower Technologies - Torino, Italy > > http://www.towertech.it >
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;
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(-)