* [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-03 1:53 ` Julius Werner 0 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-03 1:53 UTC (permalink / raw) To: Andrew Morton Cc: Alessandro Zummo, Doug Anderson, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux, Julius Werner In Fuzhou, China, the month of November seems to be having 31 days. That's nice and all (I'm sure you can get a lot more done in a year that way), but back here in other parts of the world we are not so lucky. Therefore, if we read that date from the RTC we should correct it to December 1st. This is not a full workaround. Since the RTC actually ticks all the way through that imaginary day, there's no easy way to detect and correct this issue if the device was offline the whole time and allowed it to tick through to December 1st on the Rockchip calendar (which would be December 2nd on the Gregorian one). Those edge cases can only really be solved by regularly syncing to an external time source (e.g. NTP). Signed-off-by: Julius Werner <jwerner@chromium.org> Reviewed-by: Chris Zhong <zyw@rock-chips.com> --- drivers/rtc/rtc-rk808.c | 93 ++++++++++++++++++++++++++----------------------- 1 file changed, 50 insertions(+), 43 deletions(-) diff --git a/drivers/rtc/rtc-rk808.c b/drivers/rtc/rtc-rk808.c index 91ca0bc..b605b35 100644 --- a/drivers/rtc/rtc-rk808.c +++ b/drivers/rtc/rtc-rk808.c @@ -56,6 +56,50 @@ struct rk808_rtc { int irq; }; +/* Set current time and date in RTC */ +static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) +{ + struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); + struct rk808 *rk808 = rk808_rtc->rk808; + u8 rtc_data[NUM_TIME_REGS]; + int ret; + + rtc_data[0] = bin2bcd(tm->tm_sec); + rtc_data[1] = bin2bcd(tm->tm_min); + rtc_data[2] = bin2bcd(tm->tm_hour); + rtc_data[3] = bin2bcd(tm->tm_mday); + rtc_data[4] = bin2bcd(tm->tm_mon + 1); + rtc_data[5] = bin2bcd(tm->tm_year - 100); + rtc_data[6] = bin2bcd(tm->tm_wday); + dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", + 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, + tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); + + /* Stop RTC while updating the RTC registers */ + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, + BIT_RTC_CTRL_REG_STOP_RTC_M, + BIT_RTC_CTRL_REG_STOP_RTC_M); + if (ret) { + dev_err(dev, "Failed to update RTC control: %d\n", ret); + return ret; + } + + ret = regmap_bulk_write(rk808->regmap, RK808_SECONDS_REG, + rtc_data, NUM_TIME_REGS); + if (ret) { + dev_err(dev, "Failed to bull write rtc_data: %d\n", ret); + return ret; + } + /* Start RTC again */ + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, + BIT_RTC_CTRL_REG_STOP_RTC_M, 0); + if (ret) { + dev_err(dev, "Failed to update RTC control: %d\n", ret); + return ret; + } + return 0; +} + /* Read current time and date in RTC */ static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) { @@ -105,51 +149,14 @@ static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); - return ret; -} - -/* Set current time and date in RTC */ -static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) -{ - struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); - struct rk808 *rk808 = rk808_rtc->rk808; - u8 rtc_data[NUM_TIME_REGS]; - int ret; - - rtc_data[0] = bin2bcd(tm->tm_sec); - rtc_data[1] = bin2bcd(tm->tm_min); - rtc_data[2] = bin2bcd(tm->tm_hour); - rtc_data[3] = bin2bcd(tm->tm_mday); - rtc_data[4] = bin2bcd(tm->tm_mon + 1); - rtc_data[5] = bin2bcd(tm->tm_year - 100); - rtc_data[6] = bin2bcd(tm->tm_wday); - dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", - 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); - - /* Stop RTC while updating the RTC registers */ - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, - BIT_RTC_CTRL_REG_STOP_RTC_M, - BIT_RTC_CTRL_REG_STOP_RTC_M); - if (ret) { - dev_err(dev, "Failed to update RTC control: %d\n", ret); - return ret; + if (tm->tm_mon == 10 && tm->tm_mday == 31) { + dev_warn(dev, "correcting Nov 31st to Dec 1st (HW bug)\n"); + tm->tm_mon = 11; + tm->tm_mday = 1; + rk808_rtc_set_time(dev, tm); } - ret = regmap_bulk_write(rk808->regmap, RK808_SECONDS_REG, - rtc_data, NUM_TIME_REGS); - if (ret) { - dev_err(dev, "Failed to bull write rtc_data: %d\n", ret); - return ret; - } - /* Start RTC again */ - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, - BIT_RTC_CTRL_REG_STOP_RTC_M, 0); - if (ret) { - dev_err(dev, "Failed to update RTC control: %d\n", ret); - return ret; - } - return 0; + return ret; } /* Read alarm time and date in RTC */ -- 2.1.2 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* [rtc-linux] [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-03 1:53 ` Julius Werner 0 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-03 1:53 UTC (permalink / raw) To: Andrew Morton Cc: Alessandro Zummo, Doug Anderson, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux, Julius Werner In Fuzhou, China, the month of November seems to be having 31 days. That's nice and all (I'm sure you can get a lot more done in a year that way), but back here in other parts of the world we are not so lucky. Therefore, if we read that date from the RTC we should correct it to December 1st. This is not a full workaround. Since the RTC actually ticks all the way through that imaginary day, there's no easy way to detect and correct this issue if the device was offline the whole time and allowed it to tick through to December 1st on the Rockchip calendar (which would be December 2nd on the Gregorian one). Those edge cases can only really be solved by regularly syncing to an external time source (e.g. NTP). Signed-off-by: Julius Werner <jwerner@chromium.org> Reviewed-by: Chris Zhong <zyw@rock-chips.com> --- drivers/rtc/rtc-rk808.c | 93 ++++++++++++++++++++++++++----------------------- 1 file changed, 50 insertions(+), 43 deletions(-) diff --git a/drivers/rtc/rtc-rk808.c b/drivers/rtc/rtc-rk808.c index 91ca0bc..b605b35 100644 --- a/drivers/rtc/rtc-rk808.c +++ b/drivers/rtc/rtc-rk808.c @@ -56,6 +56,50 @@ struct rk808_rtc { int irq; }; +/* Set current time and date in RTC */ +static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) +{ + struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); + struct rk808 *rk808 = rk808_rtc->rk808; + u8 rtc_data[NUM_TIME_REGS]; + int ret; + + rtc_data[0] = bin2bcd(tm->tm_sec); + rtc_data[1] = bin2bcd(tm->tm_min); + rtc_data[2] = bin2bcd(tm->tm_hour); + rtc_data[3] = bin2bcd(tm->tm_mday); + rtc_data[4] = bin2bcd(tm->tm_mon + 1); + rtc_data[5] = bin2bcd(tm->tm_year - 100); + rtc_data[6] = bin2bcd(tm->tm_wday); + dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", + 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, + tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); + + /* Stop RTC while updating the RTC registers */ + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, + BIT_RTC_CTRL_REG_STOP_RTC_M, + BIT_RTC_CTRL_REG_STOP_RTC_M); + if (ret) { + dev_err(dev, "Failed to update RTC control: %d\n", ret); + return ret; + } + + ret = regmap_bulk_write(rk808->regmap, RK808_SECONDS_REG, + rtc_data, NUM_TIME_REGS); + if (ret) { + dev_err(dev, "Failed to bull write rtc_data: %d\n", ret); + return ret; + } + /* Start RTC again */ + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, + BIT_RTC_CTRL_REG_STOP_RTC_M, 0); + if (ret) { + dev_err(dev, "Failed to update RTC control: %d\n", ret); + return ret; + } + return 0; +} + /* Read current time and date in RTC */ static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) { @@ -105,51 +149,14 @@ static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); - return ret; -} - -/* Set current time and date in RTC */ -static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) -{ - struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); - struct rk808 *rk808 = rk808_rtc->rk808; - u8 rtc_data[NUM_TIME_REGS]; - int ret; - - rtc_data[0] = bin2bcd(tm->tm_sec); - rtc_data[1] = bin2bcd(tm->tm_min); - rtc_data[2] = bin2bcd(tm->tm_hour); - rtc_data[3] = bin2bcd(tm->tm_mday); - rtc_data[4] = bin2bcd(tm->tm_mon + 1); - rtc_data[5] = bin2bcd(tm->tm_year - 100); - rtc_data[6] = bin2bcd(tm->tm_wday); - dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", - 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); - - /* Stop RTC while updating the RTC registers */ - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, - BIT_RTC_CTRL_REG_STOP_RTC_M, - BIT_RTC_CTRL_REG_STOP_RTC_M); - if (ret) { - dev_err(dev, "Failed to update RTC control: %d\n", ret); - return ret; + if (tm->tm_mon == 10 && tm->tm_mday == 31) { + dev_warn(dev, "correcting Nov 31st to Dec 1st (HW bug)\n"); + tm->tm_mon = 11; + tm->tm_mday = 1; + rk808_rtc_set_time(dev, tm); } - ret = regmap_bulk_write(rk808->regmap, RK808_SECONDS_REG, - rtc_data, NUM_TIME_REGS); - if (ret) { - dev_err(dev, "Failed to bull write rtc_data: %d\n", ret); - return ret; - } - /* Start RTC again */ - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, - BIT_RTC_CTRL_REG_STOP_RTC_M, 0); - if (ret) { - dev_err(dev, "Failed to update RTC control: %d\n", ret); - return ret; - } - return 0; + return ret; } /* Read alarm time and date in RTC */ -- 2.1.2 -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [rtc-linux] [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-03 1:53 ` [rtc-linux] " Julius Werner @ 2015-12-03 14:42 ` Alexandre Belloni -1 siblings, 0 replies; 62+ messages in thread From: Alexandre Belloni @ 2015-12-03 14:42 UTC (permalink / raw) To: Julius Werner Cc: Andrew Morton, Alessandro Zummo, Doug Anderson, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Hi, I would have liked to be in copy of that mail. Maybe you used get_maintainers on an old tree? On 02/12/2015 at 17:53:04 -0800, Julius Werner wrote : > In Fuzhou, China, the month of November seems to be having 31 days. > That's nice and all (I'm sure you can get a lot more done in a year that > way), but back here in other parts of the world we are not so lucky. > Therefore, if we read that date from the RTC we should correct it to > December 1st. > Wow, nice one... I hope reading the time properly fails thanks to the rtc_valid_tm(tm) in __rtc_read_time(). > This is not a full workaround. Since the RTC actually ticks all the way > through that imaginary day, there's no easy way to detect and correct > this issue if the device was offline the whole time and allowed it to > tick through to December 1st on the Rockchip calendar (which would be > December 2nd on the Gregorian one). Those edge cases can only really be > solved by regularly syncing to an external time source (e.g. NTP). > It will also happen if nobody reads the RTC time for that day (highly improbable in a default configuration). Do you need that patch for 4.4 or can it wait 4.5? > Signed-off-by: Julius Werner <jwerner@chromium.org> > Reviewed-by: Chris Zhong <zyw@rock-chips.com> > --- > drivers/rtc/rtc-rk808.c | 93 ++++++++++++++++++++++++++----------------------- > 1 file changed, 50 insertions(+), 43 deletions(-) > -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [rtc-linux] [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-03 14:42 ` Alexandre Belloni 0 siblings, 0 replies; 62+ messages in thread From: Alexandre Belloni @ 2015-12-03 14:42 UTC (permalink / raw) To: Julius Werner Cc: Andrew Morton, Alessandro Zummo, Doug Anderson, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Hi, I would have liked to be in copy of that mail. Maybe you used get_maintainers on an old tree? On 02/12/2015 at 17:53:04 -0800, Julius Werner wrote : > In Fuzhou, China, the month of November seems to be having 31 days. > That's nice and all (I'm sure you can get a lot more done in a year that > way), but back here in other parts of the world we are not so lucky. > Therefore, if we read that date from the RTC we should correct it to > December 1st. > Wow, nice one... I hope reading the time properly fails thanks to the rtc_valid_tm(tm) in __rtc_read_time(). > This is not a full workaround. Since the RTC actually ticks all the way > through that imaginary day, there's no easy way to detect and correct > this issue if the device was offline the whole time and allowed it to > tick through to December 1st on the Rockchip calendar (which would be > December 2nd on the Gregorian one). Those edge cases can only really be > solved by regularly syncing to an external time source (e.g. NTP). > It will also happen if nobody reads the RTC time for that day (highly improbable in a default configuration). Do you need that patch for 4.4 or can it wait 4.5? > Signed-off-by: Julius Werner <jwerner@chromium.org> > Reviewed-by: Chris Zhong <zyw@rock-chips.com> > --- > drivers/rtc/rtc-rk808.c | 93 ++++++++++++++++++++++++++----------------------- > 1 file changed, 50 insertions(+), 43 deletions(-) > -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [rtc-linux] [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-03 14:42 ` Alexandre Belloni @ 2015-12-03 16:53 ` Julius Werner -1 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-03 16:53 UTC (permalink / raw) To: Alexandre Belloni Cc: Julius Werner, Andrew Morton, Alessandro Zummo, Doug Anderson, Sonny Rao, Chris Zhong, Heiko Stuebner, LKML, rtc-linux > I would have liked to be in copy of that mail. Maybe you used > get_maintainers on an old tree? Oops, yes, sorry, I ran it on a 3.14 backport and then just added people I found in older patches for that file. > I hope reading the time properly fails thanks to the rtc_valid_tm(tm) in > __rtc_read_time(). It should, yeah. I haven't tested it (beyond compiling) on master, and our version doesn't have that code yet, so I can't say for sure. > It will also happen if nobody reads the RTC time for that day (highly > improbable in a default configuration). Do you need that patch for 4.4 > or can it wait 4.5? True. I don't really need the patch to land in a particular release since we maintain our own tree with backports. It would probably still be nice if you can get it into the 4.4 LTS to reduce the chance this will hit anyone again next year. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [rtc-linux] [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-03 16:53 ` Julius Werner 0 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-03 16:53 UTC (permalink / raw) To: Alexandre Belloni Cc: Julius Werner, Andrew Morton, Alessandro Zummo, Doug Anderson, Sonny Rao, Chris Zhong, Heiko Stuebner, LKML, rtc-linux > I would have liked to be in copy of that mail. Maybe you used > get_maintainers on an old tree? Oops, yes, sorry, I ran it on a 3.14 backport and then just added people I found in older patches for that file. > I hope reading the time properly fails thanks to the rtc_valid_tm(tm) in > __rtc_read_time(). It should, yeah. I haven't tested it (beyond compiling) on master, and our version doesn't have that code yet, so I can't say for sure. > It will also happen if nobody reads the RTC time for that day (highly > improbable in a default configuration). Do you need that patch for 4.4 > or can it wait 4.5? True. I don't really need the patch to land in a particular release since we maintain our own tree with backports. It would probably still be nice if you can get it into the 4.4 LTS to reduce the chance this will hit anyone again next year. -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-03 1:53 ` [rtc-linux] " Julius Werner @ 2015-12-04 23:50 ` Doug Anderson -1 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-04 23:50 UTC (permalink / raw) To: Julius Werner Cc: Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Julius, On Wed, Dec 2, 2015 at 5:53 PM, Julius Werner <jwerner@chromium.org> wrote: > In Fuzhou, China, the month of November seems to be having 31 days. > That's nice and all (I'm sure you can get a lot more done in a year that > way), but back here in other parts of the world we are not so lucky. > Therefore, if we read that date from the RTC we should correct it to > December 1st. > > This is not a full workaround. Since the RTC actually ticks all the way > through that imaginary day, there's no easy way to detect and correct > this issue if the device was offline the whole time and allowed it to > tick through to December 1st on the Rockchip calendar (which would be > December 2nd on the Gregorian one). Those edge cases can only really be > solved by regularly syncing to an external time source (e.g. NTP). > > Signed-off-by: Julius Werner <jwerner@chromium.org> > Reviewed-by: Chris Zhong <zyw@rock-chips.com> > --- > drivers/rtc/rtc-rk808.c | 93 ++++++++++++++++++++++++++----------------------- > 1 file changed, 50 insertions(+), 43 deletions(-) > > diff --git a/drivers/rtc/rtc-rk808.c b/drivers/rtc/rtc-rk808.c > index 91ca0bc..b605b35 100644 > --- a/drivers/rtc/rtc-rk808.c > +++ b/drivers/rtc/rtc-rk808.c > @@ -56,6 +56,50 @@ struct rk808_rtc { > int irq; > }; > > +/* Set current time and date in RTC */ > +static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) > +{ > + struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); > + struct rk808 *rk808 = rk808_rtc->rk808; > + u8 rtc_data[NUM_TIME_REGS]; > + int ret; > + > + rtc_data[0] = bin2bcd(tm->tm_sec); > + rtc_data[1] = bin2bcd(tm->tm_min); > + rtc_data[2] = bin2bcd(tm->tm_hour); > + rtc_data[3] = bin2bcd(tm->tm_mday); > + rtc_data[4] = bin2bcd(tm->tm_mon + 1); > + rtc_data[5] = bin2bcd(tm->tm_year - 100); > + rtc_data[6] = bin2bcd(tm->tm_wday); > + dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > + 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > + tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > + > + /* Stop RTC while updating the RTC registers */ > + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > + BIT_RTC_CTRL_REG_STOP_RTC_M, > + BIT_RTC_CTRL_REG_STOP_RTC_M); > + if (ret) { > + dev_err(dev, "Failed to update RTC control: %d\n", ret); > + return ret; > + } > + > + ret = regmap_bulk_write(rk808->regmap, RK808_SECONDS_REG, > + rtc_data, NUM_TIME_REGS); > + if (ret) { > + dev_err(dev, "Failed to bull write rtc_data: %d\n", ret); > + return ret; > + } > + /* Start RTC again */ > + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > + BIT_RTC_CTRL_REG_STOP_RTC_M, 0); > + if (ret) { > + dev_err(dev, "Failed to update RTC control: %d\n", ret); > + return ret; > + } > + return 0; > +} > + > /* Read current time and date in RTC */ > static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) > { > @@ -105,51 +149,14 @@ static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) > 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > > - return ret; > -} > - > -/* Set current time and date in RTC */ > -static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) > -{ > - struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); > - struct rk808 *rk808 = rk808_rtc->rk808; > - u8 rtc_data[NUM_TIME_REGS]; > - int ret; > - > - rtc_data[0] = bin2bcd(tm->tm_sec); > - rtc_data[1] = bin2bcd(tm->tm_min); > - rtc_data[2] = bin2bcd(tm->tm_hour); > - rtc_data[3] = bin2bcd(tm->tm_mday); > - rtc_data[4] = bin2bcd(tm->tm_mon + 1); > - rtc_data[5] = bin2bcd(tm->tm_year - 100); > - rtc_data[6] = bin2bcd(tm->tm_wday); > - dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > - 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > - > - /* Stop RTC while updating the RTC registers */ > - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > - BIT_RTC_CTRL_REG_STOP_RTC_M, > - BIT_RTC_CTRL_REG_STOP_RTC_M); > - if (ret) { > - dev_err(dev, "Failed to update RTC control: %d\n", ret); > - return ret; > + if (tm->tm_mon == 10 && tm->tm_mday == 31) { > + dev_warn(dev, "correcting Nov 31st to Dec 1st (HW bug)\n"); > + tm->tm_mon = 11; > + tm->tm_mday = 1; > + rk808_rtc_set_time(dev, tm); If a device is in S3 for the whole day that the glitch occurs and then we wake up then we'll end up thinking it's Dec 1st instead of Dec 2nd, right? That case _could_ be handled by knowing that the last time we read the clock it was before 12/1 and that this time it is after 11/30. Then we add the extra day. In order to do this, we'd have to know that we're on hardware with the glitch, which I guess could either be done with a device tree property or by spending 1 second probing the device at bootup (that would be a bit of a pain...). Obviously the trick above wouldn't handle if the clock ticked when the device was in S5, but I'd imagine that most systems treat the RTC as slightly questionable on an initial bootup anyway (though I'd imagine that they rely on it working across S3). What do you think? Is that a worthwhile thing to handle too? It's pretty common for me to not turn all my devices on every day. -Doug ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-04 23:50 ` Doug Anderson 0 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-04 23:50 UTC (permalink / raw) To: Julius Werner Cc: Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Julius, On Wed, Dec 2, 2015 at 5:53 PM, Julius Werner <jwerner@chromium.org> wrote: > In Fuzhou, China, the month of November seems to be having 31 days. > That's nice and all (I'm sure you can get a lot more done in a year that > way), but back here in other parts of the world we are not so lucky. > Therefore, if we read that date from the RTC we should correct it to > December 1st. > > This is not a full workaround. Since the RTC actually ticks all the way > through that imaginary day, there's no easy way to detect and correct > this issue if the device was offline the whole time and allowed it to > tick through to December 1st on the Rockchip calendar (which would be > December 2nd on the Gregorian one). Those edge cases can only really be > solved by regularly syncing to an external time source (e.g. NTP). > > Signed-off-by: Julius Werner <jwerner@chromium.org> > Reviewed-by: Chris Zhong <zyw@rock-chips.com> > --- > drivers/rtc/rtc-rk808.c | 93 ++++++++++++++++++++++++++----------------------- > 1 file changed, 50 insertions(+), 43 deletions(-) > > diff --git a/drivers/rtc/rtc-rk808.c b/drivers/rtc/rtc-rk808.c > index 91ca0bc..b605b35 100644 > --- a/drivers/rtc/rtc-rk808.c > +++ b/drivers/rtc/rtc-rk808.c > @@ -56,6 +56,50 @@ struct rk808_rtc { > int irq; > }; > > +/* Set current time and date in RTC */ > +static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) > +{ > + struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); > + struct rk808 *rk808 = rk808_rtc->rk808; > + u8 rtc_data[NUM_TIME_REGS]; > + int ret; > + > + rtc_data[0] = bin2bcd(tm->tm_sec); > + rtc_data[1] = bin2bcd(tm->tm_min); > + rtc_data[2] = bin2bcd(tm->tm_hour); > + rtc_data[3] = bin2bcd(tm->tm_mday); > + rtc_data[4] = bin2bcd(tm->tm_mon + 1); > + rtc_data[5] = bin2bcd(tm->tm_year - 100); > + rtc_data[6] = bin2bcd(tm->tm_wday); > + dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > + 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > + tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > + > + /* Stop RTC while updating the RTC registers */ > + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > + BIT_RTC_CTRL_REG_STOP_RTC_M, > + BIT_RTC_CTRL_REG_STOP_RTC_M); > + if (ret) { > + dev_err(dev, "Failed to update RTC control: %d\n", ret); > + return ret; > + } > + > + ret = regmap_bulk_write(rk808->regmap, RK808_SECONDS_REG, > + rtc_data, NUM_TIME_REGS); > + if (ret) { > + dev_err(dev, "Failed to bull write rtc_data: %d\n", ret); > + return ret; > + } > + /* Start RTC again */ > + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > + BIT_RTC_CTRL_REG_STOP_RTC_M, 0); > + if (ret) { > + dev_err(dev, "Failed to update RTC control: %d\n", ret); > + return ret; > + } > + return 0; > +} > + > /* Read current time and date in RTC */ > static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) > { > @@ -105,51 +149,14 @@ static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) > 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > > - return ret; > -} > - > -/* Set current time and date in RTC */ > -static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) > -{ > - struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); > - struct rk808 *rk808 = rk808_rtc->rk808; > - u8 rtc_data[NUM_TIME_REGS]; > - int ret; > - > - rtc_data[0] = bin2bcd(tm->tm_sec); > - rtc_data[1] = bin2bcd(tm->tm_min); > - rtc_data[2] = bin2bcd(tm->tm_hour); > - rtc_data[3] = bin2bcd(tm->tm_mday); > - rtc_data[4] = bin2bcd(tm->tm_mon + 1); > - rtc_data[5] = bin2bcd(tm->tm_year - 100); > - rtc_data[6] = bin2bcd(tm->tm_wday); > - dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > - 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > - > - /* Stop RTC while updating the RTC registers */ > - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > - BIT_RTC_CTRL_REG_STOP_RTC_M, > - BIT_RTC_CTRL_REG_STOP_RTC_M); > - if (ret) { > - dev_err(dev, "Failed to update RTC control: %d\n", ret); > - return ret; > + if (tm->tm_mon == 10 && tm->tm_mday == 31) { > + dev_warn(dev, "correcting Nov 31st to Dec 1st (HW bug)\n"); > + tm->tm_mon = 11; > + tm->tm_mday = 1; > + rk808_rtc_set_time(dev, tm); If a device is in S3 for the whole day that the glitch occurs and then we wake up then we'll end up thinking it's Dec 1st instead of Dec 2nd, right? That case _could_ be handled by knowing that the last time we read the clock it was before 12/1 and that this time it is after 11/30. Then we add the extra day. In order to do this, we'd have to know that we're on hardware with the glitch, which I guess could either be done with a device tree property or by spending 1 second probing the device at bootup (that would be a bit of a pain...). Obviously the trick above wouldn't handle if the clock ticked when the device was in S5, but I'd imagine that most systems treat the RTC as slightly questionable on an initial bootup anyway (though I'd imagine that they rely on it working across S3). What do you think? Is that a worthwhile thing to handle too? It's pretty common for me to not turn all my devices on every day. -Doug -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-04 23:50 ` [rtc-linux] " Doug Anderson @ 2015-12-05 0:25 ` Julius Werner -1 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-05 0:25 UTC (permalink / raw) To: Doug Anderson Cc: Julius Werner, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux > If a device is in S3 for the whole day that the glitch occurs and then > we wake up then we'll end up thinking it's Dec 1st instead of Dec 2nd, > right? That case _could_ be handled by knowing that the last time we > read the clock it was before 12/1 and that this time it is after > 11/30. Then we add the extra day. In order to do this, we'd have to > know that we're on hardware with the glitch, which I guess could > either be done with a device tree property or by spending 1 second > probing the device at bootup (that would be a bit of a pain...). > > Obviously the trick above wouldn't handle if the clock ticked when the > device was in S5, but I'd imagine that most systems treat the RTC as > slightly questionable on an initial bootup anyway (though I'd imagine > that they rely on it working across S3). True, we could do that. I don't think it makes much sense to differentiate between S3 and S5 like that, though... the problem can happen just the same after both, and I don't think there's a practical difference in how systems treat that (if userspace has ways to double-check the system time, such as syncing to a network time source, it should really be doing that after both resume and reboot). Of course, building a work-around like that for S5 will become more complicated and requires persistent storage. For Chromium OS, we're already planning to improve tlsdated such that I don't think this will be an issue anymore (making it schedule a resync after resume, not just after reboot, which is a probably a good idea in general). For other systems that don't have any kind of network time sync, I think the best solution would be to handle this completely with a small userspace hook on boot and resume (because you probably need to access the file system to keep track of the last seen time anyway, you can do the device identification through /proc/device-tree just as well, and this avoids putting too much hacky workaround logic into the kernel). The other thing that would worry me about this approach is that it requires perfect identification of the problem, and Rockchip will hopefully eventually be able to fix this either in RK808 or a successor chip that might use the same RTC interface (and thus driver). Detecting it at boot is probably a bad idea because a crash/brownout at the wrong moment will permanently leave you with a bad time. I really think fixing this as best as we easily can and leaving the hard edge-cases to userspace is the best approach here. ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-05 0:25 ` Julius Werner 0 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-05 0:25 UTC (permalink / raw) To: Doug Anderson Cc: Julius Werner, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux > If a device is in S3 for the whole day that the glitch occurs and then > we wake up then we'll end up thinking it's Dec 1st instead of Dec 2nd, > right? That case _could_ be handled by knowing that the last time we > read the clock it was before 12/1 and that this time it is after > 11/30. Then we add the extra day. In order to do this, we'd have to > know that we're on hardware with the glitch, which I guess could > either be done with a device tree property or by spending 1 second > probing the device at bootup (that would be a bit of a pain...). > > Obviously the trick above wouldn't handle if the clock ticked when the > device was in S5, but I'd imagine that most systems treat the RTC as > slightly questionable on an initial bootup anyway (though I'd imagine > that they rely on it working across S3). True, we could do that. I don't think it makes much sense to differentiate between S3 and S5 like that, though... the problem can happen just the same after both, and I don't think there's a practical difference in how systems treat that (if userspace has ways to double-check the system time, such as syncing to a network time source, it should really be doing that after both resume and reboot). Of course, building a work-around like that for S5 will become more complicated and requires persistent storage. For Chromium OS, we're already planning to improve tlsdated such that I don't think this will be an issue anymore (making it schedule a resync after resume, not just after reboot, which is a probably a good idea in general). For other systems that don't have any kind of network time sync, I think the best solution would be to handle this completely with a small userspace hook on boot and resume (because you probably need to access the file system to keep track of the last seen time anyway, you can do the device identification through /proc/device-tree just as well, and this avoids putting too much hacky workaround logic into the kernel). The other thing that would worry me about this approach is that it requires perfect identification of the problem, and Rockchip will hopefully eventually be able to fix this either in RK808 or a successor chip that might use the same RTC interface (and thus driver). Detecting it at boot is probably a bad idea because a crash/brownout at the wrong moment will permanently leave you with a bad time. I really think fixing this as best as we easily can and leaving the hard edge-cases to userspace is the best approach here. -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-05 0:25 ` [rtc-linux] " Julius Werner @ 2015-12-05 0:58 ` Doug Anderson -1 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-05 0:58 UTC (permalink / raw) To: Julius Werner Cc: Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Julius, On Fri, Dec 4, 2015 at 4:25 PM, Julius Werner <jwerner@chromium.org> wrote: >> If a device is in S3 for the whole day that the glitch occurs and then >> we wake up then we'll end up thinking it's Dec 1st instead of Dec 2nd, >> right? That case _could_ be handled by knowing that the last time we >> read the clock it was before 12/1 and that this time it is after >> 11/30. Then we add the extra day. In order to do this, we'd have to >> know that we're on hardware with the glitch, which I guess could >> either be done with a device tree property or by spending 1 second >> probing the device at bootup (that would be a bit of a pain...). >> >> Obviously the trick above wouldn't handle if the clock ticked when the >> device was in S5, but I'd imagine that most systems treat the RTC as >> slightly questionable on an initial bootup anyway (though I'd imagine >> that they rely on it working across S3). > > True, we could do that. I don't think it makes much sense to > differentiate between S3 and S5 like that, though... the problem can > happen just the same after both, and I don't think there's a practical > difference in how systems treat that (if userspace has ways to > double-check the system time, such as syncing to a network time > source, it should really be doing that after both resume and reboot). > Of course, building a work-around like that for S5 will become more > complicated and requires persistent storage. Right, the need for persistent storage is what makes S5 hard... > For Chromium OS, we're already planning to improve tlsdated such that > I don't think this will be an issue anymore (making it schedule a > resync after resume, not just after reboot, which is a probably a good > idea in general). For other systems that don't have any kind of > network time sync, I think the best solution would be to handle this > completely with a small userspace hook on boot and resume (because you > probably need to access the file system to keep track of the last seen > time anyway, you can do the device identification through > /proc/device-tree just as well, and this avoids putting too much hacky > workaround logic into the kernel). How would such a hook work? If userspace sees the system suspend on Nov 30th and sees the system wake up on Dec 1st, how does it know whether it should adjust? If it's truly Dec 1st then the kernel will have adjusted the date from Nov 31st to Dec 1st. If it's truly Dec 2nd then the kernel will not have adjusted the date and the RTC will have ticked past Nov 31 and onto Dec 1st. Userspace can't tell. Userspace could try to parse "dmesg" and look to see if the kernel adjusted, but that's ugly. We could add a sysfs entry, but it seems pretty hard to imagine that all Linux distros using rk808 will add this type of hook... > The other thing that would worry me about this approach is that it > requires perfect identification of the problem, and Rockchip will > hopefully eventually be able to fix this either in RK808 or a > successor chip that might use the same RTC interface (and thus > driver). Detecting it at boot is probably a bad idea because a > crash/brownout at the wrong moment will permanently leave you with a > bad time. I really think fixing this as best as we easily can and > leaving the hard edge-cases to userspace is the best approach here. Yes, you're right. Detecting is a bit scary. Chris: any chance there's an RK808 revision hiding somewhere in the i2c register banks that we could rely on? Adding a device tree hook doesn't seem insane, but you're right that Rockchip could start producing new revisions of rk808 with this fixed and all of sudden we'd be adjusting the wrong way. ...so you're probably right that this is a bad idea... So I guess my #1 choice would be to find a revision somewhere in the rk808 i2c register space. If that's not there, then I guess you're patch is probably better than trying to adjust and maybe being wrong when newer rk808 revisions fix this... -Doug ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-05 0:58 ` Doug Anderson 0 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-05 0:58 UTC (permalink / raw) To: Julius Werner Cc: Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Julius, On Fri, Dec 4, 2015 at 4:25 PM, Julius Werner <jwerner@chromium.org> wrote: >> If a device is in S3 for the whole day that the glitch occurs and then >> we wake up then we'll end up thinking it's Dec 1st instead of Dec 2nd, >> right? That case _could_ be handled by knowing that the last time we >> read the clock it was before 12/1 and that this time it is after >> 11/30. Then we add the extra day. In order to do this, we'd have to >> know that we're on hardware with the glitch, which I guess could >> either be done with a device tree property or by spending 1 second >> probing the device at bootup (that would be a bit of a pain...). >> >> Obviously the trick above wouldn't handle if the clock ticked when the >> device was in S5, but I'd imagine that most systems treat the RTC as >> slightly questionable on an initial bootup anyway (though I'd imagine >> that they rely on it working across S3). > > True, we could do that. I don't think it makes much sense to > differentiate between S3 and S5 like that, though... the problem can > happen just the same after both, and I don't think there's a practical > difference in how systems treat that (if userspace has ways to > double-check the system time, such as syncing to a network time > source, it should really be doing that after both resume and reboot). > Of course, building a work-around like that for S5 will become more > complicated and requires persistent storage. Right, the need for persistent storage is what makes S5 hard... > For Chromium OS, we're already planning to improve tlsdated such that > I don't think this will be an issue anymore (making it schedule a > resync after resume, not just after reboot, which is a probably a good > idea in general). For other systems that don't have any kind of > network time sync, I think the best solution would be to handle this > completely with a small userspace hook on boot and resume (because you > probably need to access the file system to keep track of the last seen > time anyway, you can do the device identification through > /proc/device-tree just as well, and this avoids putting too much hacky > workaround logic into the kernel). How would such a hook work? If userspace sees the system suspend on Nov 30th and sees the system wake up on Dec 1st, how does it know whether it should adjust? If it's truly Dec 1st then the kernel will have adjusted the date from Nov 31st to Dec 1st. If it's truly Dec 2nd then the kernel will not have adjusted the date and the RTC will have ticked past Nov 31 and onto Dec 1st. Userspace can't tell. Userspace could try to parse "dmesg" and look to see if the kernel adjusted, but that's ugly. We could add a sysfs entry, but it seems pretty hard to imagine that all Linux distros using rk808 will add this type of hook... > The other thing that would worry me about this approach is that it > requires perfect identification of the problem, and Rockchip will > hopefully eventually be able to fix this either in RK808 or a > successor chip that might use the same RTC interface (and thus > driver). Detecting it at boot is probably a bad idea because a > crash/brownout at the wrong moment will permanently leave you with a > bad time. I really think fixing this as best as we easily can and > leaving the hard edge-cases to userspace is the best approach here. Yes, you're right. Detecting is a bit scary. Chris: any chance there's an RK808 revision hiding somewhere in the i2c register banks that we could rely on? Adding a device tree hook doesn't seem insane, but you're right that Rockchip could start producing new revisions of rk808 with this fixed and all of sudden we'd be adjusting the wrong way. ...so you're probably right that this is a bad idea... So I guess my #1 choice would be to find a revision somewhere in the rk808 i2c register space. If that's not there, then I guess you're patch is probably better than trying to adjust and maybe being wrong when newer rk808 revisions fix this... -Doug -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-05 0:58 ` [rtc-linux] " Doug Anderson @ 2015-12-05 1:54 ` Julius Werner -1 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-05 1:54 UTC (permalink / raw) To: Doug Anderson Cc: Julius Werner, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux > How would such a hook work? If userspace sees the system suspend on > Nov 30th and sees the system wake up on Dec 1st, how does it know > whether it should adjust? If it's truly Dec 1st then the kernel will > have adjusted the date from Nov 31st to Dec 1st. If it's truly Dec > 2nd then the kernel will not have adjusted the date and the RTC will > have ticked past Nov 31 and onto Dec 1st. Userspace can't tell. > Userspace could try to parse "dmesg" and look to see if the kernel > adjusted, but that's ugly. Good point, I didn't think that through far enough. I guess parsing dmesg would be an option, but a pretty ugly one and it wouldn't be guaranteed to work if you got an early boot kernel crash after the correction. So, really, it seems like there's no reliable way to fix this for S5 (unless we start doing crazy things like writing to disk from kernel code). ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-05 1:54 ` Julius Werner 0 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-05 1:54 UTC (permalink / raw) To: Doug Anderson Cc: Julius Werner, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux > How would such a hook work? If userspace sees the system suspend on > Nov 30th and sees the system wake up on Dec 1st, how does it know > whether it should adjust? If it's truly Dec 1st then the kernel will > have adjusted the date from Nov 31st to Dec 1st. If it's truly Dec > 2nd then the kernel will not have adjusted the date and the RTC will > have ticked past Nov 31 and onto Dec 1st. Userspace can't tell. > Userspace could try to parse "dmesg" and look to see if the kernel > adjusted, but that's ugly. Good point, I didn't think that through far enough. I guess parsing dmesg would be an option, but a pretty ugly one and it wouldn't be guaranteed to work if you got an early boot kernel crash after the correction. So, really, it seems like there's no reliable way to fix this for S5 (unless we start doing crazy things like writing to disk from kernel code). -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-05 1:54 ` [rtc-linux] " Julius Werner @ 2015-12-05 4:02 ` Doug Anderson -1 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-05 4:02 UTC (permalink / raw) To: Julius Werner Cc: Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Hi, On Fri, Dec 4, 2015 at 5:54 PM, Julius Werner <jwerner@chromium.org> wrote: >> How would such a hook work? If userspace sees the system suspend on >> Nov 30th and sees the system wake up on Dec 1st, how does it know >> whether it should adjust? If it's truly Dec 1st then the kernel will >> have adjusted the date from Nov 31st to Dec 1st. If it's truly Dec >> 2nd then the kernel will not have adjusted the date and the RTC will >> have ticked past Nov 31 and onto Dec 1st. Userspace can't tell. >> Userspace could try to parse "dmesg" and look to see if the kernel >> adjusted, but that's ugly. > > Good point, I didn't think that through far enough. I guess parsing > dmesg would be an option, but a pretty ugly one and it wouldn't be > guaranteed to work if you got an early boot kernel crash after the > correction. So, really, it seems like there's no reliable way to fix > this for S5 (unless we start doing crazy things like writing to disk > from kernel code). Hmmm, this made me think. We _do_ have some storage we could use, depending on how hacky^H^H^H^H^H^H clever we wanted to be. We've got the alarm registers in the RTC. If we set the alarm to something but then turn the alarm off then we can use that to store information that will persist in S5 (as long as the RTC is ticking). What do you think? I'd have to think of a scheme, but we could certainly use alarms that are several years in the future (or the past) as a sentinel, then use the day/month of the last time the kernel saw the time.... ...and speaking of the alarm, we also need to handle the RTC bug for setting the alarm. If you set an alarm for 10 seconds after Nov 30, you need to set the alarm for Nov 31st or it will actually fire 10 seconds + 1 day later. ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-05 4:02 ` Doug Anderson 0 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-05 4:02 UTC (permalink / raw) To: Julius Werner Cc: Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Hi, On Fri, Dec 4, 2015 at 5:54 PM, Julius Werner <jwerner@chromium.org> wrote: >> How would such a hook work? If userspace sees the system suspend on >> Nov 30th and sees the system wake up on Dec 1st, how does it know >> whether it should adjust? If it's truly Dec 1st then the kernel will >> have adjusted the date from Nov 31st to Dec 1st. If it's truly Dec >> 2nd then the kernel will not have adjusted the date and the RTC will >> have ticked past Nov 31 and onto Dec 1st. Userspace can't tell. >> Userspace could try to parse "dmesg" and look to see if the kernel >> adjusted, but that's ugly. > > Good point, I didn't think that through far enough. I guess parsing > dmesg would be an option, but a pretty ugly one and it wouldn't be > guaranteed to work if you got an early boot kernel crash after the > correction. So, really, it seems like there's no reliable way to fix > this for S5 (unless we start doing crazy things like writing to disk > from kernel code). Hmmm, this made me think. We _do_ have some storage we could use, depending on how hacky^H^H^H^H^H^H clever we wanted to be. We've got the alarm registers in the RTC. If we set the alarm to something but then turn the alarm off then we can use that to store information that will persist in S5 (as long as the RTC is ticking). What do you think? I'd have to think of a scheme, but we could certainly use alarms that are several years in the future (or the past) as a sentinel, then use the day/month of the last time the kernel saw the time.... ...and speaking of the alarm, we also need to handle the RTC bug for setting the alarm. If you set an alarm for 10 seconds after Nov 30, you need to set the alarm for Nov 31st or it will actually fire 10 seconds + 1 day later. -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-05 4:02 ` [rtc-linux] " Doug Anderson @ 2015-12-05 4:53 ` Doug Anderson -1 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-05 4:53 UTC (permalink / raw) To: Julius Werner Cc: Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Hi, On Fri, Dec 4, 2015 at 8:02 PM, Doug Anderson <dianders@chromium.org> wrote: > Hi, > > On Fri, Dec 4, 2015 at 5:54 PM, Julius Werner <jwerner@chromium.org> wrote: >>> How would such a hook work? If userspace sees the system suspend on >>> Nov 30th and sees the system wake up on Dec 1st, how does it know >>> whether it should adjust? If it's truly Dec 1st then the kernel will >>> have adjusted the date from Nov 31st to Dec 1st. If it's truly Dec >>> 2nd then the kernel will not have adjusted the date and the RTC will >>> have ticked past Nov 31 and onto Dec 1st. Userspace can't tell. >>> Userspace could try to parse "dmesg" and look to see if the kernel >>> adjusted, but that's ugly. >> >> Good point, I didn't think that through far enough. I guess parsing >> dmesg would be an option, but a pretty ugly one and it wouldn't be >> guaranteed to work if you got an early boot kernel crash after the >> correction. So, really, it seems like there's no reliable way to fix >> this for S5 (unless we start doing crazy things like writing to disk >> from kernel code). > > Hmmm, this made me think. We _do_ have some storage we could use, > depending on how hacky^H^H^H^H^H^H clever we wanted to be. We've got > the alarm registers in the RTC. If we set the alarm to something but > then turn the alarm off then we can use that to store information that > will persist in S5 (as long as the RTC is ticking). What do you > think? I'd have to think of a scheme, but we could certainly use > alarms that are several years in the future (or the past) as a > sentinel, then use the day/month of the last time the kernel saw the > time.... Actually, it wouldn't even be that terribly hacky Whenever the alarm isn't in use then set it to the next Nov 31st. At boot time, if the alarm is set to Nov 31st and the current date is >= the alarm time then you adjust. At shutdown time always disable the alarm (and set to Nov 31st). To handle resume time you might as well just keep the state somewhere in RAM (see below for why). Whenever you set the alarm for real then presumably you'll need to adjust for Nov 31st and presumably you'll wake up and deal with the alarm, then turn it off. So if you set the alarm for July 4th then (presumably) you'll wake up way before Nov 31st. If you set the alarm for Dec 25th and it's currently Oct 31st then you'll have to adjust in the alarm code and you'll really set it for Dec 24th. As per above, we're in S3 (presumably) or have some persistent kernel state so we know to adjust everything when we wake up (even if we wake up for a non-alarm reason). You'll still get a failure if you set the alarm and then forcibly go into S5 without software knowledge, then stay in S5 long enough to cross over Nov 31st without seeing it (but somehow keep the RTC state). ...but come on, that seems so incredibly rare! :-P Of course, all this hinges on being able to tell whether we've got the bug or not so we know whether to adjust. Assuming that there is no ID register, we could get someone from Rockchip to agree to change _something_ in a way that it's visible to us if they ever fix the bug... -Doug ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-05 4:53 ` Doug Anderson 0 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-05 4:53 UTC (permalink / raw) To: Julius Werner Cc: Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Hi, On Fri, Dec 4, 2015 at 8:02 PM, Doug Anderson <dianders@chromium.org> wrote: > Hi, > > On Fri, Dec 4, 2015 at 5:54 PM, Julius Werner <jwerner@chromium.org> wrote: >>> How would such a hook work? If userspace sees the system suspend on >>> Nov 30th and sees the system wake up on Dec 1st, how does it know >>> whether it should adjust? If it's truly Dec 1st then the kernel will >>> have adjusted the date from Nov 31st to Dec 1st. If it's truly Dec >>> 2nd then the kernel will not have adjusted the date and the RTC will >>> have ticked past Nov 31 and onto Dec 1st. Userspace can't tell. >>> Userspace could try to parse "dmesg" and look to see if the kernel >>> adjusted, but that's ugly. >> >> Good point, I didn't think that through far enough. I guess parsing >> dmesg would be an option, but a pretty ugly one and it wouldn't be >> guaranteed to work if you got an early boot kernel crash after the >> correction. So, really, it seems like there's no reliable way to fix >> this for S5 (unless we start doing crazy things like writing to disk >> from kernel code). > > Hmmm, this made me think. We _do_ have some storage we could use, > depending on how hacky^H^H^H^H^H^H clever we wanted to be. We've got > the alarm registers in the RTC. If we set the alarm to something but > then turn the alarm off then we can use that to store information that > will persist in S5 (as long as the RTC is ticking). What do you > think? I'd have to think of a scheme, but we could certainly use > alarms that are several years in the future (or the past) as a > sentinel, then use the day/month of the last time the kernel saw the > time.... Actually, it wouldn't even be that terribly hacky Whenever the alarm isn't in use then set it to the next Nov 31st. At boot time, if the alarm is set to Nov 31st and the current date is >= the alarm time then you adjust. At shutdown time always disable the alarm (and set to Nov 31st). To handle resume time you might as well just keep the state somewhere in RAM (see below for why). Whenever you set the alarm for real then presumably you'll need to adjust for Nov 31st and presumably you'll wake up and deal with the alarm, then turn it off. So if you set the alarm for July 4th then (presumably) you'll wake up way before Nov 31st. If you set the alarm for Dec 25th and it's currently Oct 31st then you'll have to adjust in the alarm code and you'll really set it for Dec 24th. As per above, we're in S3 (presumably) or have some persistent kernel state so we know to adjust everything when we wake up (even if we wake up for a non-alarm reason). You'll still get a failure if you set the alarm and then forcibly go into S5 without software knowledge, then stay in S5 long enough to cross over Nov 31st without seeing it (but somehow keep the RTC state). ...but come on, that seems so incredibly rare! :-P Of course, all this hinges on being able to tell whether we've got the bug or not so we know whether to adjust. Assuming that there is no ID register, we could get someone from Rockchip to agree to change _something_ in a way that it's visible to us if they ever fix the bug... -Doug -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-05 4:53 ` [rtc-linux] " Doug Anderson @ 2015-12-05 7:17 ` Julius Werner -1 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-05 7:17 UTC (permalink / raw) To: Doug Anderson Cc: Julius Werner, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux > If you set the alarm > for Dec 25th and it's currently Oct 31st then you'll have to adjust in > the alarm code and you'll really set it for Dec 24th. As per above, > we're in S3 (presumably) or have some persistent kernel state so we > know to adjust everything when we wake up (even if we wake up for a > non-alarm reason). How do you deal with the case where you set an alarm in late December, but you then power the system on earlier in December by other means? I think then you couldn't tell if the fix had already been applied? > You'll still get a failure if you set the alarm and then forcibly go > into S5 without software knowledge, then stay in S5 long enough to > cross over Nov 31st without seeing it (but somehow keep the RTC > state). ...but come on, that seems so incredibly rare! :-P I think you could just set it to "November 31st, disabled" at probe time (if it isn't already) and keep it like that indefinitely? I think as long as you don't need to actually use the alarm, that would work fine. Still, with the vast majority of practically existing devices with an RK808 almost constantly connected to some network, I'm not sure if a huge hack-around is really worth it here. (I guess we could still just do the S3 thing which is much less complicated, assuming we can guarantee correct identification.) ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-05 7:17 ` Julius Werner 0 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-05 7:17 UTC (permalink / raw) To: Doug Anderson Cc: Julius Werner, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux > If you set the alarm > for Dec 25th and it's currently Oct 31st then you'll have to adjust in > the alarm code and you'll really set it for Dec 24th. As per above, > we're in S3 (presumably) or have some persistent kernel state so we > know to adjust everything when we wake up (even if we wake up for a > non-alarm reason). How do you deal with the case where you set an alarm in late December, but you then power the system on earlier in December by other means? I think then you couldn't tell if the fix had already been applied? > You'll still get a failure if you set the alarm and then forcibly go > into S5 without software knowledge, then stay in S5 long enough to > cross over Nov 31st without seeing it (but somehow keep the RTC > state). ...but come on, that seems so incredibly rare! :-P I think you could just set it to "November 31st, disabled" at probe time (if it isn't already) and keep it like that indefinitely? I think as long as you don't need to actually use the alarm, that would work fine. Still, with the vast majority of practically existing devices with an RK808 almost constantly connected to some network, I'm not sure if a huge hack-around is really worth it here. (I guess we could still just do the S3 thing which is much less complicated, assuming we can guarantee correct identification.) -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-05 7:17 ` [rtc-linux] " Julius Werner @ 2015-12-06 0:36 ` Doug Anderson -1 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-06 0:36 UTC (permalink / raw) To: Julius Werner Cc: Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Julius, On Fri, Dec 4, 2015 at 11:17 PM, Julius Werner <jwerner@chromium.org> wrote: >> If you set the alarm >> for Dec 25th and it's currently Oct 31st then you'll have to adjust in >> the alarm code and you'll really set it for Dec 24th. As per above, >> we're in S3 (presumably) or have some persistent kernel state so we >> know to adjust everything when we wake up (even if we wake up for a >> non-alarm reason). > > How do you deal with the case where you set an alarm in late December, > but you then power the system on earlier in December by other means? I > think then you couldn't tell if the fix had already been applied? I was presuming that alarms were never set at power off time unless power off happened due to an exceptional case. AKA: normal Linux shutdown disables all alarms. If you happened to have an exceptional shutdown (out of battery?) while an alarm is set then, yes, my solution fails. Presumably if the RTC keeps state but the system ran out of battery, you've got two batteries in your system (something none of the rk808-based Chromebooks have--we back rk808 state up with the main battery, not a coin cell). In Chromebooks you could possibly get a shutdown that Linux didn't know anything about if you got a forcible reboot (watchdog, crash, or Refresh-Power) and then you managed to convince the BIOS to shut you down (you have the dev screen and press power off there). Again, this seems pretty darn rare. >> You'll still get a failure if you set the alarm and then forcibly go >> into S5 without software knowledge, then stay in S5 long enough to >> cross over Nov 31st without seeing it (but somehow keep the RTC >> state). ...but come on, that seems so incredibly rare! :-P > > I think you could just set it to "November 31st, disabled" at probe > time (if it isn't already) and keep it like that indefinitely? I think > as long as you don't need to actually use the alarm, that would work > fine. Yup. In ChromeOS we only use the alarm in suspend stress testing, but I'd believe that anyone using Android on one of these systems would use the RTC much more. We might (?) use the RTC alarm in ChromeOS if we ever support dark resume etc on rk3288-based devices, right? > Still, with the vast majority of practically existing devices with an > RK808 almost constantly connected to some network, I'm not sure if a > huge hack-around is really worth it here. (I guess we could still just > do the S3 thing which is much less complicated, assuming we can > guarantee correct identification.) I'm pretty certain that we need to handle correcting the alarm. Setting an alarm for 10 seconds from now and getting the alarm firing 1 day and 10 seconds from now seems like a pretty huge problem, at least for any system that relies on the RTC alarm a lot. That pretty much means that we need some way to identify problematic devices. ...so I think if we have no revision register then we should either assume all rk808 devices have this problem (and hope and pray that Rockchip gives us a way to ID things if they ever add a fix) or come up with some other type of solution (probe one time when the clock is presumably wrong and store something somewhere in rk808 to indicate that we've already probed) Once we have to fix the alarm, handling S3 seems like it will be not much more work. I'm OK with not handling S5, but I think with my proposed scheme we could also handle it if we wanted. All hacks should be contained in the rk808 driver, which should make them much less objectionable in general. -Doug ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-06 0:36 ` Doug Anderson 0 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-06 0:36 UTC (permalink / raw) To: Julius Werner Cc: Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Julius, On Fri, Dec 4, 2015 at 11:17 PM, Julius Werner <jwerner@chromium.org> wrote: >> If you set the alarm >> for Dec 25th and it's currently Oct 31st then you'll have to adjust in >> the alarm code and you'll really set it for Dec 24th. As per above, >> we're in S3 (presumably) or have some persistent kernel state so we >> know to adjust everything when we wake up (even if we wake up for a >> non-alarm reason). > > How do you deal with the case where you set an alarm in late December, > but you then power the system on earlier in December by other means? I > think then you couldn't tell if the fix had already been applied? I was presuming that alarms were never set at power off time unless power off happened due to an exceptional case. AKA: normal Linux shutdown disables all alarms. If you happened to have an exceptional shutdown (out of battery?) while an alarm is set then, yes, my solution fails. Presumably if the RTC keeps state but the system ran out of battery, you've got two batteries in your system (something none of the rk808-based Chromebooks have--we back rk808 state up with the main battery, not a coin cell). In Chromebooks you could possibly get a shutdown that Linux didn't know anything about if you got a forcible reboot (watchdog, crash, or Refresh-Power) and then you managed to convince the BIOS to shut you down (you have the dev screen and press power off there). Again, this seems pretty darn rare. >> You'll still get a failure if you set the alarm and then forcibly go >> into S5 without software knowledge, then stay in S5 long enough to >> cross over Nov 31st without seeing it (but somehow keep the RTC >> state). ...but come on, that seems so incredibly rare! :-P > > I think you could just set it to "November 31st, disabled" at probe > time (if it isn't already) and keep it like that indefinitely? I think > as long as you don't need to actually use the alarm, that would work > fine. Yup. In ChromeOS we only use the alarm in suspend stress testing, but I'd believe that anyone using Android on one of these systems would use the RTC much more. We might (?) use the RTC alarm in ChromeOS if we ever support dark resume etc on rk3288-based devices, right? > Still, with the vast majority of practically existing devices with an > RK808 almost constantly connected to some network, I'm not sure if a > huge hack-around is really worth it here. (I guess we could still just > do the S3 thing which is much less complicated, assuming we can > guarantee correct identification.) I'm pretty certain that we need to handle correcting the alarm. Setting an alarm for 10 seconds from now and getting the alarm firing 1 day and 10 seconds from now seems like a pretty huge problem, at least for any system that relies on the RTC alarm a lot. That pretty much means that we need some way to identify problematic devices. ...so I think if we have no revision register then we should either assume all rk808 devices have this problem (and hope and pray that Rockchip gives us a way to ID things if they ever add a fix) or come up with some other type of solution (probe one time when the clock is presumably wrong and store something somewhere in rk808 to indicate that we've already probed) Once we have to fix the alarm, handling S3 seems like it will be not much more work. I'm OK with not handling S5, but I think with my proposed scheme we could also handle it if we wanted. All hacks should be contained in the rk808 driver, which should make them much less objectionable in general. -Doug -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-06 0:36 ` [rtc-linux] " Doug Anderson @ 2015-12-07 1:33 ` Chris Zhong -1 siblings, 0 replies; 62+ messages in thread From: Chris Zhong @ 2015-12-07 1:33 UTC (permalink / raw) To: Doug Anderson, Julius Werner Cc: Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux Hi Doug RK808 has a shadowed register for saving a "frozen" RTC time. When we setting "GET_TIME" to 1, the time will save in this shadowed register. So if we do not set the "GET_TIME", we always get the last time. read the old time before "get_time", and then read the time again for new time. If the old time earlier than 12.1 && new time later than 12.1, we should +1 day for the correct rtc time. On the other hand, we should set the "GET_TIME" after rk808_rtc_set_time, for restore the time before suspend/shut_down. regmap_bulk_read(rk808->regmap, RK808_SECONDS_REG, rtc_data, NUM_TIME_REGS); <....> /* Force an update of the shadowed registers right now */ ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, BIT_RTC_CTRL_REG_RTC_GET_TIME, BIT_RTC_CTRL_REG_RTC_GET_TIME); regmap_bulk_read(rk808->regmap, RK808_SECONDS_REG, rtc_data, NUM_TIME_REGS); On 12/06/2015 08:36 AM, Doug Anderson wrote: > Julius, > > On Fri, Dec 4, 2015 at 11:17 PM, Julius Werner <jwerner@chromium.org> wrote: >>> If you set the alarm >>> for Dec 25th and it's currently Oct 31st then you'll have to adjust in >>> the alarm code and you'll really set it for Dec 24th. As per above, >>> we're in S3 (presumably) or have some persistent kernel state so we >>> know to adjust everything when we wake up (even if we wake up for a >>> non-alarm reason). >> How do you deal with the case where you set an alarm in late December, >> but you then power the system on earlier in December by other means? I >> think then you couldn't tell if the fix had already been applied? > I was presuming that alarms were never set at power off time unless > power off happened due to an exceptional case. AKA: normal Linux > shutdown disables all alarms. If you happened to have an exceptional > shutdown (out of battery?) while an alarm is set then, yes, my > solution fails. Presumably if the RTC keeps state but the system ran > out of battery, you've got two batteries in your system (something > none of the rk808-based Chromebooks have--we back rk808 state up with > the main battery, not a coin cell). > > In Chromebooks you could possibly get a shutdown that Linux didn't > know anything about if you got a forcible reboot (watchdog, crash, or > Refresh-Power) and then you managed to convince the BIOS to shut you > down (you have the dev screen and press power off there). Again, this > seems pretty darn rare. > > >>> You'll still get a failure if you set the alarm and then forcibly go >>> into S5 without software knowledge, then stay in S5 long enough to >>> cross over Nov 31st without seeing it (but somehow keep the RTC >>> state). ...but come on, that seems so incredibly rare! :-P >> I think you could just set it to "November 31st, disabled" at probe >> time (if it isn't already) and keep it like that indefinitely? I think >> as long as you don't need to actually use the alarm, that would work >> fine. > Yup. In ChromeOS we only use the alarm in suspend stress testing, but > I'd believe that anyone using Android on one of these systems would > use the RTC much more. > > We might (?) use the RTC alarm in ChromeOS if we ever support dark > resume etc on rk3288-based devices, right? > > >> Still, with the vast majority of practically existing devices with an >> RK808 almost constantly connected to some network, I'm not sure if a >> huge hack-around is really worth it here. (I guess we could still just >> do the S3 thing which is much less complicated, assuming we can >> guarantee correct identification.) > I'm pretty certain that we need to handle correcting the alarm. > Setting an alarm for 10 seconds from now and getting the alarm firing > 1 day and 10 seconds from now seems like a pretty huge problem, at > least for any system that relies on the RTC alarm a lot. That pretty > much means that we need some way to identify problematic devices. > ...so I think if we have no revision register then we should either > assume all rk808 devices have this problem (and hope and pray that > Rockchip gives us a way to ID things if they ever add a fix) or come > up with some other type of solution (probe one time when the clock is > presumably wrong and store something somewhere in rk808 to indicate > that we've already probed) > > Once we have to fix the alarm, handling S3 seems like it will be not > much more work. > > I'm OK with not handling S5, but I think with my proposed scheme we > could also handle it if we wanted. > > > All hacks should be contained in the rk808 driver, which should make > them much less objectionable in general. > > -Doug > > > ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-07 1:33 ` Chris Zhong 0 siblings, 0 replies; 62+ messages in thread From: Chris Zhong @ 2015-12-07 1:33 UTC (permalink / raw) To: Doug Anderson, Julius Werner Cc: Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux Hi Doug RK808 has a shadowed register for saving a "frozen" RTC time. When we setting "GET_TIME" to 1, the time will save in this shadowed register. So if we do not set the "GET_TIME", we always get the last time. read the old time before "get_time", and then read the time again for new time. If the old time earlier than 12.1 && new time later than 12.1, we should +1 day for the correct rtc time. On the other hand, we should set the "GET_TIME" after rk808_rtc_set_time, for restore the time before suspend/shut_down. regmap_bulk_read(rk808->regmap, RK808_SECONDS_REG, rtc_data, NUM_TIME_REGS); <....> /* Force an update of the shadowed registers right now */ ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, BIT_RTC_CTRL_REG_RTC_GET_TIME, BIT_RTC_CTRL_REG_RTC_GET_TIME); regmap_bulk_read(rk808->regmap, RK808_SECONDS_REG, rtc_data, NUM_TIME_REGS); On 12/06/2015 08:36 AM, Doug Anderson wrote: > Julius, > > On Fri, Dec 4, 2015 at 11:17 PM, Julius Werner <jwerner@chromium.org> wrote: >>> If you set the alarm >>> for Dec 25th and it's currently Oct 31st then you'll have to adjust in >>> the alarm code and you'll really set it for Dec 24th. As per above, >>> we're in S3 (presumably) or have some persistent kernel state so we >>> know to adjust everything when we wake up (even if we wake up for a >>> non-alarm reason). >> How do you deal with the case where you set an alarm in late December, >> but you then power the system on earlier in December by other means? I >> think then you couldn't tell if the fix had already been applied? > I was presuming that alarms were never set at power off time unless > power off happened due to an exceptional case. AKA: normal Linux > shutdown disables all alarms. If you happened to have an exceptional > shutdown (out of battery?) while an alarm is set then, yes, my > solution fails. Presumably if the RTC keeps state but the system ran > out of battery, you've got two batteries in your system (something > none of the rk808-based Chromebooks have--we back rk808 state up with > the main battery, not a coin cell). > > In Chromebooks you could possibly get a shutdown that Linux didn't > know anything about if you got a forcible reboot (watchdog, crash, or > Refresh-Power) and then you managed to convince the BIOS to shut you > down (you have the dev screen and press power off there). Again, this > seems pretty darn rare. > > >>> You'll still get a failure if you set the alarm and then forcibly go >>> into S5 without software knowledge, then stay in S5 long enough to >>> cross over Nov 31st without seeing it (but somehow keep the RTC >>> state). ...but come on, that seems so incredibly rare! :-P >> I think you could just set it to "November 31st, disabled" at probe >> time (if it isn't already) and keep it like that indefinitely? I think >> as long as you don't need to actually use the alarm, that would work >> fine. > Yup. In ChromeOS we only use the alarm in suspend stress testing, but > I'd believe that anyone using Android on one of these systems would > use the RTC much more. > > We might (?) use the RTC alarm in ChromeOS if we ever support dark > resume etc on rk3288-based devices, right? > > >> Still, with the vast majority of practically existing devices with an >> RK808 almost constantly connected to some network, I'm not sure if a >> huge hack-around is really worth it here. (I guess we could still just >> do the S3 thing which is much less complicated, assuming we can >> guarantee correct identification.) > I'm pretty certain that we need to handle correcting the alarm. > Setting an alarm for 10 seconds from now and getting the alarm firing > 1 day and 10 seconds from now seems like a pretty huge problem, at > least for any system that relies on the RTC alarm a lot. That pretty > much means that we need some way to identify problematic devices. > ...so I think if we have no revision register then we should either > assume all rk808 devices have this problem (and hope and pray that > Rockchip gives us a way to ID things if they ever add a fix) or come > up with some other type of solution (probe one time when the clock is > presumably wrong and store something somewhere in rk808 to indicate > that we've already probed) > > Once we have to fix the alarm, handling S3 seems like it will be not > much more work. > > I'm OK with not handling S5, but I think with my proposed scheme we > could also handle it if we wanted. > > > All hacks should be contained in the rk808 driver, which should make > them much less objectionable in general. > > -Doug > > > -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-07 1:33 ` [rtc-linux] " Chris Zhong @ 2015-12-07 2:50 ` Doug Anderson -1 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-07 2:50 UTC (permalink / raw) To: Chris Zhong Cc: Julius Werner, Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux Chris, On Sun, Dec 6, 2015 at 5:33 PM, Chris Zhong <zyw@rock-chips.com> wrote: > Hi Doug > > RK808 has a shadowed register for saving a "frozen" RTC time. > When we setting "GET_TIME" to 1, the time will save in this shadowed > register. So if we do not set the "GET_TIME", we always get the last time. > > read the old time before "get_time", and then read the time again for new > time. If the old time earlier than 12.1 && new time later than 12.1, we > should > +1 day for the correct rtc time. > > On the other hand, we should set the "GET_TIME" after rk808_rtc_set_time, > for restore the time before suspend/shut_down. Ah, good idea using the shadow registers. The whole point of the shadow registers is to enable atomic read of time, right? So if the clock ticks as you are reading 23:59:59 you don't end up reading 23:59:00 or 24:59:59 (you'll get either 23:59:59 or 24:00:00). So right, time read will now be: 1. Read GET_TIME. Confirm it's 1. 2. Read the time. 3. Set GET_TIME to 0. 4. Set GET_TIME to 1. 5. Read the time. If time from #2 < 11/31 and time from #5 >= 11/31 then we do the adjust. If GET_TIME wasn't 1 in step #1 then we won't do any adjusting unless the time is actually 11/31. Between steps #4 and #5 we'll need to add a small delay since old code used to use the setting to 0 as a delay (as commented). We should presumably always leave GET_TIME as 1 unless we're actively reading the time for the most reliability. Also, if we've already read the time this bootup, we can certainly optimize the above by skipping #1 and #2. -Doug ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-07 2:50 ` Doug Anderson 0 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-07 2:50 UTC (permalink / raw) To: Chris Zhong Cc: Julius Werner, Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux Chris, On Sun, Dec 6, 2015 at 5:33 PM, Chris Zhong <zyw@rock-chips.com> wrote: > Hi Doug > > RK808 has a shadowed register for saving a "frozen" RTC time. > When we setting "GET_TIME" to 1, the time will save in this shadowed > register. So if we do not set the "GET_TIME", we always get the last time. > > read the old time before "get_time", and then read the time again for new > time. If the old time earlier than 12.1 && new time later than 12.1, we > should > +1 day for the correct rtc time. > > On the other hand, we should set the "GET_TIME" after rk808_rtc_set_time, > for restore the time before suspend/shut_down. Ah, good idea using the shadow registers. The whole point of the shadow registers is to enable atomic read of time, right? So if the clock ticks as you are reading 23:59:59 you don't end up reading 23:59:00 or 24:59:59 (you'll get either 23:59:59 or 24:00:00). So right, time read will now be: 1. Read GET_TIME. Confirm it's 1. 2. Read the time. 3. Set GET_TIME to 0. 4. Set GET_TIME to 1. 5. Read the time. If time from #2 < 11/31 and time from #5 >= 11/31 then we do the adjust. If GET_TIME wasn't 1 in step #1 then we won't do any adjusting unless the time is actually 11/31. Between steps #4 and #5 we'll need to add a small delay since old code used to use the setting to 0 as a delay (as commented). We should presumably always leave GET_TIME as 1 unless we're actively reading the time for the most reliability. Also, if we've already read the time this bootup, we can certainly optimize the above by skipping #1 and #2. -Doug -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-07 2:50 ` [rtc-linux] " Doug Anderson @ 2015-12-07 2:52 ` Doug Anderson -1 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-07 2:52 UTC (permalink / raw) To: Chris Zhong Cc: Julius Werner, Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux Hi, On Sun, Dec 6, 2015 at 6:50 PM, Doug Anderson <dianders@chromium.org> wrote: > Chris, > > On Sun, Dec 6, 2015 at 5:33 PM, Chris Zhong <zyw@rock-chips.com> wrote: >> Hi Doug >> >> RK808 has a shadowed register for saving a "frozen" RTC time. >> When we setting "GET_TIME" to 1, the time will save in this shadowed >> register. So if we do not set the "GET_TIME", we always get the last time. >> >> read the old time before "get_time", and then read the time again for new >> time. If the old time earlier than 12.1 && new time later than 12.1, we >> should >> +1 day for the correct rtc time. >> >> On the other hand, we should set the "GET_TIME" after rk808_rtc_set_time, >> for restore the time before suspend/shut_down. > > Ah, good idea using the shadow registers. The whole point of the > shadow registers is to enable atomic read of time, right? So if the > clock ticks as you are reading 23:59:59 you don't end up reading > 23:59:00 or 24:59:59 (you'll get either 23:59:59 or 24:00:00). So > right, time read will now be: > > 1. Read GET_TIME. Confirm it's 1. > 2. Read the time. > 3. Set GET_TIME to 0. > 4. Set GET_TIME to 1. > 5. Read the time. > > If time from #2 < 11/31 and time from #5 >= 11/31 then we do the > adjust. If GET_TIME wasn't 1 in step #1 then we won't do any > adjusting unless the time is actually 11/31. > > Between steps #4 and #5 we'll need to add a small delay since old code > used to use the setting to 0 as a delay (as commented). > > We should presumably always leave GET_TIME as 1 unless we're actively > reading the time for the most reliability. Also, if we've already > read the time this bootup, we can certainly optimize the above by > skipping #1 and #2. Oh, but also we still need to know whether to adjust the alarm. I think you said that all existing rk808 chips have this bug and that you'll set a bit (to be determined later) if/when this bug is fixed. So we still need to assume that all rk808 chips have this bug... -Doug ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-07 2:52 ` Doug Anderson 0 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-07 2:52 UTC (permalink / raw) To: Chris Zhong Cc: Julius Werner, Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux Hi, On Sun, Dec 6, 2015 at 6:50 PM, Doug Anderson <dianders@chromium.org> wrote: > Chris, > > On Sun, Dec 6, 2015 at 5:33 PM, Chris Zhong <zyw@rock-chips.com> wrote: >> Hi Doug >> >> RK808 has a shadowed register for saving a "frozen" RTC time. >> When we setting "GET_TIME" to 1, the time will save in this shadowed >> register. So if we do not set the "GET_TIME", we always get the last time. >> >> read the old time before "get_time", and then read the time again for new >> time. If the old time earlier than 12.1 && new time later than 12.1, we >> should >> +1 day for the correct rtc time. >> >> On the other hand, we should set the "GET_TIME" after rk808_rtc_set_time, >> for restore the time before suspend/shut_down. > > Ah, good idea using the shadow registers. The whole point of the > shadow registers is to enable atomic read of time, right? So if the > clock ticks as you are reading 23:59:59 you don't end up reading > 23:59:00 or 24:59:59 (you'll get either 23:59:59 or 24:00:00). So > right, time read will now be: > > 1. Read GET_TIME. Confirm it's 1. > 2. Read the time. > 3. Set GET_TIME to 0. > 4. Set GET_TIME to 1. > 5. Read the time. > > If time from #2 < 11/31 and time from #5 >= 11/31 then we do the > adjust. If GET_TIME wasn't 1 in step #1 then we won't do any > adjusting unless the time is actually 11/31. > > Between steps #4 and #5 we'll need to add a small delay since old code > used to use the setting to 0 as a delay (as commented). > > We should presumably always leave GET_TIME as 1 unless we're actively > reading the time for the most reliability. Also, if we've already > read the time this bootup, we can certainly optimize the above by > skipping #1 and #2. Oh, but also we still need to know whether to adjust the alarm. I think you said that all existing rk808 chips have this bug and that you'll set a bit (to be determined later) if/when this bug is fixed. So we still need to assume that all rk808 chips have this bug... -Doug -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-07 2:52 ` [rtc-linux] " Doug Anderson @ 2015-12-07 3:08 ` Chris Zhong -1 siblings, 0 replies; 62+ messages in thread From: Chris Zhong @ 2015-12-07 3:08 UTC (permalink / raw) To: Doug Anderson Cc: Julius Werner, Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux On 12/07/2015 10:52 AM, Doug Anderson wrote: > Hi, > > On Sun, Dec 6, 2015 at 6:50 PM, Doug Anderson <dianders@chromium.org> wrote: >> Chris, >> >> On Sun, Dec 6, 2015 at 5:33 PM, Chris Zhong <zyw@rock-chips.com> wrote: >>> Hi Doug >>> >>> RK808 has a shadowed register for saving a "frozen" RTC time. >>> When we setting "GET_TIME" to 1, the time will save in this shadowed >>> register. So if we do not set the "GET_TIME", we always get the last time. >>> >>> read the old time before "get_time", and then read the time again for new >>> time. If the old time earlier than 12.1 && new time later than 12.1, we >>> should >>> +1 day for the correct rtc time. >>> >>> On the other hand, we should set the "GET_TIME" after rk808_rtc_set_time, >>> for restore the time before suspend/shut_down. >> Ah, good idea using the shadow registers. The whole point of the >> shadow registers is to enable atomic read of time, right? So if the >> clock ticks as you are reading 23:59:59 you don't end up reading >> 23:59:00 or 24:59:59 (you'll get either 23:59:59 or 24:00:00). So >> right, time read will now be: >> >> 1. Read GET_TIME. Confirm it's 1. >> 2. Read the time. >> 3. Set GET_TIME to 0. >> 4. Set GET_TIME to 1. >> 5. Read the time. >> >> If time from #2 < 11/31 and time from #5 >= 11/31 then we do the >> adjust. If GET_TIME wasn't 1 in step #1 then we won't do any >> adjusting unless the time is actually 11/31. >> >> Between steps #4 and #5 we'll need to add a small delay since old code >> used to use the setting to 0 as a delay (as commented). >> >> We should presumably always leave GET_TIME as 1 unless we're actively >> reading the time for the most reliability. Also, if we've already >> read the time this bootup, we can certainly optimize the above by >> skipping #1 and #2. GET_TIME: Rising transition of this register transfers dynamic registers into static shadowed registers. So only the rising of GET_TIME would update the "static shadowed registers". We only need ensure that the rising occurs on condition that we want to the really time. > Oh, but also we still need to know whether to adjust the alarm. I > think you said that all existing rk808 chips have this bug and that > you'll set a bit (to be determined later) if/when this bug is fixed. > So we still need to assume that all rk808 chips have this bug... I think so, all rk808 chips have this bug. And we can read the version register to differentiate the PMICs, once this bug is fixed. > > -Doug > > > ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-07 3:08 ` Chris Zhong 0 siblings, 0 replies; 62+ messages in thread From: Chris Zhong @ 2015-12-07 3:08 UTC (permalink / raw) To: Doug Anderson Cc: Julius Werner, Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux On 12/07/2015 10:52 AM, Doug Anderson wrote: > Hi, > > On Sun, Dec 6, 2015 at 6:50 PM, Doug Anderson <dianders@chromium.org> wrote: >> Chris, >> >> On Sun, Dec 6, 2015 at 5:33 PM, Chris Zhong <zyw@rock-chips.com> wrote: >>> Hi Doug >>> >>> RK808 has a shadowed register for saving a "frozen" RTC time. >>> When we setting "GET_TIME" to 1, the time will save in this shadowed >>> register. So if we do not set the "GET_TIME", we always get the last time. >>> >>> read the old time before "get_time", and then read the time again for new >>> time. If the old time earlier than 12.1 && new time later than 12.1, we >>> should >>> +1 day for the correct rtc time. >>> >>> On the other hand, we should set the "GET_TIME" after rk808_rtc_set_time, >>> for restore the time before suspend/shut_down. >> Ah, good idea using the shadow registers. The whole point of the >> shadow registers is to enable atomic read of time, right? So if the >> clock ticks as you are reading 23:59:59 you don't end up reading >> 23:59:00 or 24:59:59 (you'll get either 23:59:59 or 24:00:00). So >> right, time read will now be: >> >> 1. Read GET_TIME. Confirm it's 1. >> 2. Read the time. >> 3. Set GET_TIME to 0. >> 4. Set GET_TIME to 1. >> 5. Read the time. >> >> If time from #2 < 11/31 and time from #5 >= 11/31 then we do the >> adjust. If GET_TIME wasn't 1 in step #1 then we won't do any >> adjusting unless the time is actually 11/31. >> >> Between steps #4 and #5 we'll need to add a small delay since old code >> used to use the setting to 0 as a delay (as commented). >> >> We should presumably always leave GET_TIME as 1 unless we're actively >> reading the time for the most reliability. Also, if we've already >> read the time this bootup, we can certainly optimize the above by >> skipping #1 and #2. GET_TIME: Rising transition of this register transfers dynamic registers into static shadowed registers. So only the rising of GET_TIME would update the "static shadowed registers". We only need ensure that the rising occurs on condition that we want to the really time. > Oh, but also we still need to know whether to adjust the alarm. I > think you said that all existing rk808 chips have this bug and that > you'll set a bit (to be determined later) if/when this bug is fixed. > So we still need to assume that all rk808 chips have this bug... I think so, all rk808 chips have this bug. And we can read the version register to differentiate the PMICs, once this bug is fixed. > > -Doug > > > -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-07 3:08 ` [rtc-linux] " Chris Zhong @ 2015-12-07 20:28 ` Julius Werner -1 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-07 20:28 UTC (permalink / raw) To: Chris Zhong Cc: Doug Anderson, Julius Werner, Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux > I was presuming that alarms were never set at power off time unless > power off happened due to an exceptional case. AKA: normal Linux > shutdown disables all alarms. Hmm... maybe I misunderstand how this works. Are alarms never used to wake from S5? (It doesn't seem to work on my HiSense Chromebook right now, but maybe that's just an artifact of our board design? I'm pretty sure I've seen S5-wakes work on other platforms... I doubt that RK808 is intrinsically incapable of that, it just depends on how you hook it up.) > RK808 has a shadowed register for saving a "frozen" RTC time. > When we setting "GET_TIME" to 1, the time will save in this shadowed > register. So if we do not set the "GET_TIME", we always get the last > time. > > read the old time before "get_time", and then read the time again for new > time. If the old time earlier than 12.1 && new time later than 12.1, we should > +1 day for the correct rtc time. Unfortunately, this doesn't work through S5 if system firmware also accesses the RTC and transitions GET_TIME on boot (which it does on Chromebooks). We could fix this for coreboot, but it's probably still a bad idea to rely on it since Linux should be as firmware-agnostic as possible. > I'm pretty certain that we need to handle correcting the alarm. > Setting an alarm for 10 seconds from now and getting the alarm firing > 1 day and 10 seconds from now seems like a pretty huge problem, at > least for any system that relies on the RTC alarm a lot. That pretty > much means that we need some way to identify problematic devices. > ...so I think if we have no revision register then we should either > assume all rk808 devices have this problem (and hope and pray that > Rockchip gives us a way to ID things if they ever add a fix) or come > up with some other type of solution (probe one time when the clock is > presumably wrong and store something somewhere in rk808 to indicate > that we've already probed) > Once we have to fix the alarm, handling S3 seems like it will be not > much more work. Agreed, scheduling alarms in the future is also an important point. So I guess I'll update the patch to fix alarm scheduling and S3 as well, and we'll just ignore S5 as infeasible? ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-07 20:28 ` Julius Werner 0 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-07 20:28 UTC (permalink / raw) To: Chris Zhong Cc: Doug Anderson, Julius Werner, Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux > I was presuming that alarms were never set at power off time unless > power off happened due to an exceptional case. AKA: normal Linux > shutdown disables all alarms. Hmm... maybe I misunderstand how this works. Are alarms never used to wake from S5? (It doesn't seem to work on my HiSense Chromebook right now, but maybe that's just an artifact of our board design? I'm pretty sure I've seen S5-wakes work on other platforms... I doubt that RK808 is intrinsically incapable of that, it just depends on how you hook it up.) > RK808 has a shadowed register for saving a "frozen" RTC time. > When we setting "GET_TIME" to 1, the time will save in this shadowed > register. So if we do not set the "GET_TIME", we always get the last > time. > > read the old time before "get_time", and then read the time again for new > time. If the old time earlier than 12.1 && new time later than 12.1, we should > +1 day for the correct rtc time. Unfortunately, this doesn't work through S5 if system firmware also accesses the RTC and transitions GET_TIME on boot (which it does on Chromebooks). We could fix this for coreboot, but it's probably still a bad idea to rely on it since Linux should be as firmware-agnostic as possible. > I'm pretty certain that we need to handle correcting the alarm. > Setting an alarm for 10 seconds from now and getting the alarm firing > 1 day and 10 seconds from now seems like a pretty huge problem, at > least for any system that relies on the RTC alarm a lot. That pretty > much means that we need some way to identify problematic devices. > ...so I think if we have no revision register then we should either > assume all rk808 devices have this problem (and hope and pray that > Rockchip gives us a way to ID things if they ever add a fix) or come > up with some other type of solution (probe one time when the clock is > presumably wrong and store something somewhere in rk808 to indicate > that we've already probed) > Once we have to fix the alarm, handling S3 seems like it will be not > much more work. Agreed, scheduling alarms in the future is also an important point. So I guess I'll update the patch to fix alarm scheduling and S3 as well, and we'll just ignore S5 as infeasible? -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-07 20:28 ` [rtc-linux] " Julius Werner @ 2015-12-07 22:40 ` Julius Werner -1 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-07 22:40 UTC (permalink / raw) To: Julius Werner Cc: Chris Zhong, Doug Anderson, Alexandre Belloni, Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux [+Alexandre... sorry, didn't notice that you somehow fell off the thread again, but I presume you get rtc-linux anyway] > Agreed, scheduling alarms in the future is also an important point. So > I guess I'll update the patch to fix alarm scheduling and S3 as well, > and we'll just ignore S5 as infeasible? Found another edge case trying to implement this: if you set an alarm on Nov 25th for Dec 5th, the code will have to adjust it to Dec 4th to account for the extra day. But if you then power off (or brownout) to S5 and don't power it on again until December, you have no way of knowing whether the already set alarm timestamp is on the Rockchip or the Gregorian calendar. You may or may not sync your current time back through an external source, but you have no way of knowing what you should do with the alarm. Other than that my current idea is roughly: - if the current time gets written, always assume the new date is correct - store a "last known timestamp" in memory on boot and on every write - if the current time gets read, adjust it forward by the amount of Nov 31sts since the stored timestamp (this adjustment itself constitutes a write) - if an alarm is written, adjust the timestamp written to hardware backward by the amount of Nov 31sts between the stored timestamp and the alarm time - if an alarm is read, adjust the returned result forward by the amount of Nov 31sts between the stored timestamp and the alarm time - if the current time gets written, read the alarm time before the write and write it back afterwards (the last known timestamp will change as a result of writing the current time, meaning the newly written hardware alarm time may be different from the old one) ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-07 22:40 ` Julius Werner 0 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-07 22:40 UTC (permalink / raw) To: Julius Werner Cc: Chris Zhong, Doug Anderson, Alexandre Belloni, Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux [+Alexandre... sorry, didn't notice that you somehow fell off the thread again, but I presume you get rtc-linux anyway] > Agreed, scheduling alarms in the future is also an important point. So > I guess I'll update the patch to fix alarm scheduling and S3 as well, > and we'll just ignore S5 as infeasible? Found another edge case trying to implement this: if you set an alarm on Nov 25th for Dec 5th, the code will have to adjust it to Dec 4th to account for the extra day. But if you then power off (or brownout) to S5 and don't power it on again until December, you have no way of knowing whether the already set alarm timestamp is on the Rockchip or the Gregorian calendar. You may or may not sync your current time back through an external source, but you have no way of knowing what you should do with the alarm. Other than that my current idea is roughly: - if the current time gets written, always assume the new date is correct - store a "last known timestamp" in memory on boot and on every write - if the current time gets read, adjust it forward by the amount of Nov 31sts since the stored timestamp (this adjustment itself constitutes a write) - if an alarm is written, adjust the timestamp written to hardware backward by the amount of Nov 31sts between the stored timestamp and the alarm time - if an alarm is read, adjust the returned result forward by the amount of Nov 31sts between the stored timestamp and the alarm time - if the current time gets written, read the alarm time before the write and write it back afterwards (the last known timestamp will change as a result of writing the current time, meaning the newly written hardware alarm time may be different from the old one) -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-07 20:28 ` [rtc-linux] " Julius Werner @ 2015-12-08 1:17 ` Doug Anderson -1 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-08 1:17 UTC (permalink / raw) To: Julius Werner Cc: Chris Zhong, Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux, Alexandre Belloni Julius, On Mon, Dec 7, 2015 at 12:28 PM, Julius Werner <jwerner@chromium.org> wrote: >> I was presuming that alarms were never set at power off time unless >> power off happened due to an exceptional case. AKA: normal Linux >> shutdown disables all alarms. > > Hmm... maybe I misunderstand how this works. Are alarms never used to > wake from S5? (It doesn't seem to work on my HiSense Chromebook right > now, but maybe that's just an artifact of our board design? I'm pretty > sure I've seen S5-wakes work on other platforms... I doubt that RK808 > is intrinsically incapable of that, it just depends on how you hook it > up.) Good point. The hardware out to be capable of it. >> RK808 has a shadowed register for saving a "frozen" RTC time. >> When we setting "GET_TIME" to 1, the time will save in this shadowed >> register. So if we do not set the "GET_TIME", we always get the last >> time. >> >> read the old time before "get_time", and then read the time again for new >> time. If the old time earlier than 12.1 && new time later than 12.1, we should >> +1 day for the correct rtc time. > > Unfortunately, this doesn't work through S5 if system firmware also > accesses the RTC and transitions GET_TIME on boot (which it does on > Chromebooks). We could fix this for coreboot, but it's probably still > a bad idea to rely on it since Linux should be as firmware-agnostic as > possible. Dang. Who wrote that firmware, anyway? :-P Technically you could still implement this and if firmware happens to read the RTC (and doesn't correct things) then we won't be able to correct things. ...but certainly if you read the old time and then the new time and the old time was < 11/31 and the new time was >= 11/31 you know you need to correct. I'd say that there's a good chance that other firmware (UBoot) doesn't actually read the RTC anyway. Why would it? We only do it for even log, right? >> I'm pretty certain that we need to handle correcting the alarm. >> Setting an alarm for 10 seconds from now and getting the alarm firing >> 1 day and 10 seconds from now seems like a pretty huge problem, at >> least for any system that relies on the RTC alarm a lot. That pretty >> much means that we need some way to identify problematic devices. >> ...so I think if we have no revision register then we should either >> assume all rk808 devices have this problem (and hope and pray that >> Rockchip gives us a way to ID things if they ever add a fix) or come >> up with some other type of solution (probe one time when the clock is >> presumably wrong and store something somewhere in rk808 to indicate >> that we've already probed) > >> Once we have to fix the alarm, handling S3 seems like it will be not >> much more work. > > Agreed, scheduling alarms in the future is also an important point. So > I guess I'll update the patch to fix alarm scheduling and S3 as well, > and we'll just ignore S5 as infeasible? ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-08 1:17 ` Doug Anderson 0 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-08 1:17 UTC (permalink / raw) To: Julius Werner Cc: Chris Zhong, Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux, Alexandre Belloni Julius, On Mon, Dec 7, 2015 at 12:28 PM, Julius Werner <jwerner@chromium.org> wrote: >> I was presuming that alarms were never set at power off time unless >> power off happened due to an exceptional case. AKA: normal Linux >> shutdown disables all alarms. > > Hmm... maybe I misunderstand how this works. Are alarms never used to > wake from S5? (It doesn't seem to work on my HiSense Chromebook right > now, but maybe that's just an artifact of our board design? I'm pretty > sure I've seen S5-wakes work on other platforms... I doubt that RK808 > is intrinsically incapable of that, it just depends on how you hook it > up.) Good point. The hardware out to be capable of it. >> RK808 has a shadowed register for saving a "frozen" RTC time. >> When we setting "GET_TIME" to 1, the time will save in this shadowed >> register. So if we do not set the "GET_TIME", we always get the last >> time. >> >> read the old time before "get_time", and then read the time again for new >> time. If the old time earlier than 12.1 && new time later than 12.1, we should >> +1 day for the correct rtc time. > > Unfortunately, this doesn't work through S5 if system firmware also > accesses the RTC and transitions GET_TIME on boot (which it does on > Chromebooks). We could fix this for coreboot, but it's probably still > a bad idea to rely on it since Linux should be as firmware-agnostic as > possible. Dang. Who wrote that firmware, anyway? :-P Technically you could still implement this and if firmware happens to read the RTC (and doesn't correct things) then we won't be able to correct things. ...but certainly if you read the old time and then the new time and the old time was < 11/31 and the new time was >= 11/31 you know you need to correct. I'd say that there's a good chance that other firmware (UBoot) doesn't actually read the RTC anyway. Why would it? We only do it for even log, right? >> I'm pretty certain that we need to handle correcting the alarm. >> Setting an alarm for 10 seconds from now and getting the alarm firing >> 1 day and 10 seconds from now seems like a pretty huge problem, at >> least for any system that relies on the RTC alarm a lot. That pretty >> much means that we need some way to identify problematic devices. >> ...so I think if we have no revision register then we should either >> assume all rk808 devices have this problem (and hope and pray that >> Rockchip gives us a way to ID things if they ever add a fix) or come >> up with some other type of solution (probe one time when the clock is >> presumably wrong and store something somewhere in rk808 to indicate >> that we've already probed) > >> Once we have to fix the alarm, handling S3 seems like it will be not >> much more work. > > Agreed, scheduling alarms in the future is also an important point. So > I guess I'll update the patch to fix alarm scheduling and S3 as well, > and we'll just ignore S5 as infeasible? -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-08 1:17 ` [rtc-linux] " Doug Anderson @ 2015-12-08 1:41 ` Julius Werner -1 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-08 1:41 UTC (permalink / raw) To: Doug Anderson Cc: Julius Werner, Chris Zhong, Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux, Alexandre Belloni > Technically you could still implement this and if firmware happens to > read the RTC (and doesn't correct things) then we won't be able to > correct things. ...but certainly if you read the old time and then > the new time and the old time was < 11/31 and the new time was >= > 11/31 you know you need to correct. > > I'd say that there's a good chance that other firmware (UBoot) doesn't > actually read the RTC anyway. Why would it? We only do it for even > log, right? Hah! Sounds like you assume U-Boot was doing the things it does with comprehensible reasoning. From my experience, that's a dangerous wager (not that I'm biased or anything... ;) ). FWIW, they seem to be having a huge (175kloc, of only the finest copied kernel code from 10+ years ago, I presume) repository of RTC drivers, a separate command line command to read/write to it, and an SNTP client. Still, you're right that adding the GET_TIME check wouldn't hurt... at worst it just does nothing. I'll try it out. ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-08 1:41 ` Julius Werner 0 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-08 1:41 UTC (permalink / raw) To: Doug Anderson Cc: Julius Werner, Chris Zhong, Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux, Alexandre Belloni > Technically you could still implement this and if firmware happens to > read the RTC (and doesn't correct things) then we won't be able to > correct things. ...but certainly if you read the old time and then > the new time and the old time was < 11/31 and the new time was >= > 11/31 you know you need to correct. > > I'd say that there's a good chance that other firmware (UBoot) doesn't > actually read the RTC anyway. Why would it? We only do it for even > log, right? Hah! Sounds like you assume U-Boot was doing the things it does with comprehensible reasoning. From my experience, that's a dangerous wager (not that I'm biased or anything... ;) ). FWIW, they seem to be having a huge (175kloc, of only the finest copied kernel code from 10+ years ago, I presume) repository of RTC drivers, a separate command line command to read/write to it, and an SNTP client. Still, you're right that adding the GET_TIME check wouldn't hurt... at worst it just does nothing. I'll try it out. -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st 2015-12-08 1:41 ` [rtc-linux] " Julius Werner @ 2015-12-08 5:19 ` Julius Werner -1 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-08 5:19 UTC (permalink / raw) To: Julius Werner Cc: Doug Anderson, Chris Zhong, Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux, Alexandre Belloni > Still, you're right that adding the GET_TIME check wouldn't hurt... at > worst it just does nothing. I'll try it out. Hmm... so it basically works (when you hack the RTC read out of the firmware), but only on reboot for some reason (which makes it way less useful). After a full power off I'm reading only zeroes out until I transition GET_TIME again. My guess would be that these shadow registers are not in the RTC power well... I've talked to Alex and he said that we disable the RK808's main power source in S5, but that devices without an EC (e.g. phones) would likely leave it on. So I guess it's still a good extra workaround to keep in the code, but it will never help on Chromebooks (and there's no point in hacking up our firmware for it). ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-08 5:19 ` Julius Werner 0 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-08 5:19 UTC (permalink / raw) To: Julius Werner Cc: Doug Anderson, Chris Zhong, Andrew Morton, Alessandro Zummo, Sonny Rao, Heiko Stuebner, linux-kernel, rtc-linux, Alexandre Belloni > Still, you're right that adding the GET_TIME check wouldn't hurt... at > worst it just does nothing. I'll try it out. Hmm... so it basically works (when you hack the RTC read out of the firmware), but only on reboot for some reason (which makes it way less useful). After a full power off I'm reading only zeroes out until I transition GET_TIME again. My guess would be that these shadow registers are not in the RTC power well... I've talked to Alex and he said that we disable the RK808's main power source in S5, but that devices without an EC (e.g. phones) would likely leave it on. So I guess it's still a good extra workaround to keep in the code, but it will never help on Chromebooks (and there's no point in hacking up our firmware for it). -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCH v2] RTC: RK808: Work around hardware bug on November 31st 2015-12-08 5:19 ` [rtc-linux] " Julius Werner @ 2015-12-08 5:21 ` Julius Werner -1 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-08 5:21 UTC (permalink / raw) To: Alexandre Belloni Cc: Andrew Morton, Alessandro Zummo, Doug Anderson, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux, Julius Werner In Fuzhou, China, the month of November seems to be having 31 days. That's nice and all (I'm sure you can get a lot more done in a year that way), but back here in other parts of the world we are not so lucky. Therefore, we need to compensate for these extra days existing only in the RTC's imagination when reading the time and dealing with alarms. This patch is not a perfect workaround -- it only keeps the time stable as long as the system is running or suspended. If the system is fully shut down in November and only booted back up in December, the system time may be incorrect and alarms that had been set before the shutdown may fire on the wrong day. We're trying to catch and recover from this by reading the RTC's last "shadow timestamp" (which only gets resynced when transitioning the GET_TIME control bit) to figure out when the system was shut down, but this is only reliable if no other code (e.g. firmware) has read the RTC in-between. Basic idea for the workaround: - Whenever we set the time, we assume that timestamp to be correct (synced to the real world). We store a copy of it in memory as an anchor point (where we know our calendar matched the real world). - Whenever we read the time, we can tell how many days of desync we have by counting November/December transitions between the anchor timestamp and the time read from the hardware. We adjust the hardware clock accordingly to get back in sync (which also resets the anchor time). - Whenever we set an alarm, we adjust the alarm time backwards by the amount of days that we know we will lag behind at that point (by counting the November/December transitions between our anchor point and the alarm). This way, we will wake up on the right real world date even though we cannot make adjustments while suspended. - Whenever we read an alarm, we do the opposite (forward) adjustment for the returned result to keep our outside interface free from this madness (callers expect to be able to read back the alarm they wrote). - Whenever we set the system time (which adjusts the anchor point), we read out the (adjusted) alarm time beforehand and write it (newly adjusted) back afterwards. This way, system time and alarm time will always stay on the same calendar (as long as we're able to keep track of our anchor point, at least). Signed-off-by: Julius Werner <jwerner@chromium.org> --- drivers/rtc/rtc-rk808.c | 282 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 190 insertions(+), 92 deletions(-) diff --git a/drivers/rtc/rtc-rk808.c b/drivers/rtc/rtc-rk808.c index 91ca0bc..2a6cd6f 100644 --- a/drivers/rtc/rtc-rk808.c +++ b/drivers/rtc/rtc-rk808.c @@ -54,103 +54,30 @@ struct rk808_rtc { struct rk808 *rk808; struct rtc_device *rtc; int irq; + struct rtc_time anchor_time; /* Last sync point with real world */ }; -/* Read current time and date in RTC */ -static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) +/* + * RK808 has a hardware bug causing it to count 31 days in November. This + * function can calculate the amount of days that code needs to adjust for + * between two timestamps to compensate for this. + */ +static int nov31st_transitions(struct rtc_time *from, struct rtc_time *to) { - struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); - struct rk808 *rk808 = rk808_rtc->rk808; - u8 rtc_data[NUM_TIME_REGS]; - int ret; - - /* Force an update of the shadowed registers right now */ - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, - BIT_RTC_CTRL_REG_RTC_GET_TIME, - BIT_RTC_CTRL_REG_RTC_GET_TIME); - if (ret) { - dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret); - return ret; - } - - /* - * After we set the GET_TIME bit, the rtc time can't be read - * immediately. So we should wait up to 31.25 us, about one cycle of - * 32khz. If we clear the GET_TIME bit here, the time of i2c transfer - * certainly more than 31.25us: 16 * 2.5us at 400kHz bus frequency. - */ - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, - BIT_RTC_CTRL_REG_RTC_GET_TIME, - 0); - if (ret) { - dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret); - return ret; - } + int extra_days = to->tm_year - from->tm_year; - ret = regmap_bulk_read(rk808->regmap, RK808_SECONDS_REG, - rtc_data, NUM_TIME_REGS); - if (ret) { - dev_err(dev, "Failed to bulk read rtc_data: %d\n", ret); - return ret; - } + /* Avoid adjusting anything for uninitialized timestamps */ + if (from->tm_mday == 0 || to->tm_mday == 0) + return 0; - tm->tm_sec = bcd2bin(rtc_data[0] & SECONDS_REG_MSK); - tm->tm_min = bcd2bin(rtc_data[1] & MINUTES_REG_MAK); - tm->tm_hour = bcd2bin(rtc_data[2] & HOURS_REG_MSK); - tm->tm_mday = bcd2bin(rtc_data[3] & DAYS_REG_MSK); - tm->tm_mon = (bcd2bin(rtc_data[4] & MONTHS_REG_MSK)) - 1; - tm->tm_year = (bcd2bin(rtc_data[5] & YEARS_REG_MSK)) + 100; - tm->tm_wday = bcd2bin(rtc_data[6] & WEEKS_REG_MSK); - dev_dbg(dev, "RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", - 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); + if (from->tm_mon > 10) + extra_days--; - return ret; -} + if (to->tm_mon > 10) + extra_days++; -/* Set current time and date in RTC */ -static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) -{ - struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); - struct rk808 *rk808 = rk808_rtc->rk808; - u8 rtc_data[NUM_TIME_REGS]; - int ret; - - rtc_data[0] = bin2bcd(tm->tm_sec); - rtc_data[1] = bin2bcd(tm->tm_min); - rtc_data[2] = bin2bcd(tm->tm_hour); - rtc_data[3] = bin2bcd(tm->tm_mday); - rtc_data[4] = bin2bcd(tm->tm_mon + 1); - rtc_data[5] = bin2bcd(tm->tm_year - 100); - rtc_data[6] = bin2bcd(tm->tm_wday); - dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", - 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); - - /* Stop RTC while updating the RTC registers */ - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, - BIT_RTC_CTRL_REG_STOP_RTC_M, - BIT_RTC_CTRL_REG_STOP_RTC_M); - if (ret) { - dev_err(dev, "Failed to update RTC control: %d\n", ret); - return ret; - } - - ret = regmap_bulk_write(rk808->regmap, RK808_SECONDS_REG, - rtc_data, NUM_TIME_REGS); - if (ret) { - dev_err(dev, "Failed to bull write rtc_data: %d\n", ret); - return ret; - } - /* Start RTC again */ - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, - BIT_RTC_CTRL_REG_STOP_RTC_M, 0); - if (ret) { - dev_err(dev, "Failed to update RTC control: %d\n", ret); - return ret; - } - return 0; -} + return extra_days; +}; /* Read alarm time and date in RTC */ static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm) @@ -159,7 +86,7 @@ static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm) struct rk808 *rk808 = rk808_rtc->rk808; u8 alrm_data[NUM_ALARM_REGS]; uint32_t int_reg; - int ret; + int ret, extra_days; ret = regmap_bulk_read(rk808->regmap, RK808_ALARM_SECONDS_REG, alrm_data, NUM_ALARM_REGS); @@ -171,6 +98,19 @@ static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm) alrm->time.tm_mon = (bcd2bin(alrm_data[4] & MONTHS_REG_MSK)) - 1; alrm->time.tm_year = (bcd2bin(alrm_data[5] & YEARS_REG_MSK)) + 100; + extra_days = nov31st_transitions(&rk808_rtc->anchor_time, &alrm->time); + if (alrm->time.tm_mon == 10 && alrm->time.tm_mday == 31) { + dev_warn(dev, "read HW alarm date as Nov 31, compensating\n"); + alrm->time.tm_mon = 11; + alrm->time.tm_mday = 1 + extra_days; + } else if (extra_days) { + unsigned long time; + dev_warn(dev, "compensating for %d Nov31 until HW alarm date\n", + extra_days); + rtc_tm_to_time(&alrm->time, &time); + rtc_time_to_tm(time + extra_days * 86400, &alrm->time); + } + ret = regmap_read(rk808->regmap, RK808_RTC_INT_REG, &int_reg); if (ret) { dev_err(dev, "Failed to read RTC INT REG: %d\n", ret); @@ -215,7 +155,7 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm) struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); struct rk808 *rk808 = rk808_rtc->rk808; u8 alrm_data[NUM_ALARM_REGS]; - int ret; + int ret, extra_days; ret = rk808_rtc_stop_alarm(rk808_rtc); if (ret) { @@ -227,6 +167,19 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm) alrm->time.tm_mday, alrm->time.tm_wday, alrm->time.tm_hour, alrm->time.tm_min, alrm->time.tm_sec); + extra_days = nov31st_transitions(&rk808_rtc->anchor_time, &alrm->time); + if (extra_days) { + unsigned long time; + dev_warn(dev, "writing HW alarm date adjusted for %d Nov31\n", + extra_days); + rtc_tm_to_time(&alrm->time, &time); + rtc_time_to_tm(time - extra_days * 86400, &alrm->time); + /* Compensate in case the subtraction went back over Nov 31st */ + if (alrm->time.tm_mon == 10 && + alrm->time.tm_mday == 31 - extra_days) + alrm->time.tm_mday++; /* This can result in 31! */ + } + alrm_data[0] = bin2bcd(alrm->time.tm_sec); alrm_data[1] = bin2bcd(alrm->time.tm_min); alrm_data[2] = bin2bcd(alrm->time.tm_hour); @@ -250,6 +203,141 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm) return 0; } +/* Set current time and date in RTC */ +static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) +{ + struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); + struct rk808 *rk808 = rk808_rtc->rk808; + struct rtc_wkalrm alrm; + u8 rtc_data[NUM_TIME_REGS]; + int ret; + + /* Read out wake alarm with old Nov 31st adjustment */ + rk808_rtc_readalarm(dev, &alrm); + + rtc_data[0] = bin2bcd(tm->tm_sec); + rtc_data[1] = bin2bcd(tm->tm_min); + rtc_data[2] = bin2bcd(tm->tm_hour); + rtc_data[3] = bin2bcd(tm->tm_mday); + rtc_data[4] = bin2bcd(tm->tm_mon + 1); + rtc_data[5] = bin2bcd(tm->tm_year - 100); + rtc_data[6] = bin2bcd(tm->tm_wday); + dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", + 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, + tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); + + /* Stop RTC while updating the RTC registers */ + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, + BIT_RTC_CTRL_REG_STOP_RTC_M, + BIT_RTC_CTRL_REG_STOP_RTC_M); + if (ret) { + dev_err(dev, "Failed to update RTC control: %d\n", ret); + return ret; + } + + ret = regmap_bulk_write(rk808->regmap, RK808_SECONDS_REG, + rtc_data, NUM_TIME_REGS); + if (ret) { + dev_err(dev, "Failed to bull write rtc_data: %d\n", ret); + return ret; + } + /* Start RTC again */ + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, + BIT_RTC_CTRL_REG_STOP_RTC_M, 0); + if (ret) { + dev_err(dev, "Failed to update RTC control: %d\n", ret); + return ret; + } + + /* Assume a newly set time is always correct (regardless of source) */ + rk808_rtc->anchor_time = *tm; + + /* Write back wake alarm with new Nov 31st adjustment */ + rk808_rtc_setalarm(dev, &alrm); + + return 0; +} + +/* Read time from static shadow registers (without updating) */ +static int rk808_rtc_raw_read(struct device *dev, struct rtc_time *tm) +{ + struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); + struct rk808 *rk808 = rk808_rtc->rk808; + u8 rtc_data[NUM_TIME_REGS]; + int ret; + + ret = regmap_bulk_read(rk808->regmap, RK808_SECONDS_REG, + rtc_data, NUM_TIME_REGS); + if (ret) { + dev_err(dev, "Failed to bulk read rtc_data: %d\n", ret); + return ret; + } + + tm->tm_sec = bcd2bin(rtc_data[0] & SECONDS_REG_MSK); + tm->tm_min = bcd2bin(rtc_data[1] & MINUTES_REG_MAK); + tm->tm_hour = bcd2bin(rtc_data[2] & HOURS_REG_MSK); + tm->tm_mday = bcd2bin(rtc_data[3] & DAYS_REG_MSK); + tm->tm_mon = (bcd2bin(rtc_data[4] & MONTHS_REG_MSK)) - 1; + tm->tm_year = (bcd2bin(rtc_data[5] & YEARS_REG_MSK)) + 100; + tm->tm_wday = bcd2bin(rtc_data[6] & WEEKS_REG_MSK); + dev_dbg(dev, "RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", + 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, + tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); + + return ret; +} + +/* Read current time and date in RTC */ +static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) +{ + struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); + struct rk808 *rk808 = rk808_rtc->rk808; + int ret, extra_days; + + /* Force an update of the shadowed registers right now */ + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, + BIT_RTC_CTRL_REG_RTC_GET_TIME, + BIT_RTC_CTRL_REG_RTC_GET_TIME); + if (ret) { + dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret); + return ret; + } + + /* + * After we set the GET_TIME bit, the rtc time can't be read + * immediately. So we should wait up to 31.25 us, about one cycle of + * 32khz. If we clear the GET_TIME bit here, the time of i2c transfer + * certainly more than 31.25us: 16 * 2.5us at 400kHz bus frequency. + */ + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, + BIT_RTC_CTRL_REG_RTC_GET_TIME, + 0); + if (ret) { + dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret); + return ret; + } + + ret = rk808_rtc_raw_read(dev, tm); + if (ret) + return ret; + + extra_days = nov31st_transitions(&rk808_rtc->anchor_time, tm); + if (tm->tm_mon == 10 && tm->tm_mday == 31) { + dev_warn(dev, "read Nov 31, correcting to Dec 1 (HW bug)\n"); + tm->tm_mon = 11; + tm->tm_mday = 1 + extra_days; /* don't S2R for over 30 years! */ + rk808_rtc_set_time(dev, tm); + } else if (extra_days) { + unsigned long time; + dev_warn(dev, "compensating for %d skips over Nov 31\n", + extra_days); + rtc_tm_to_time(tm, &time); + rtc_time_to_tm(time + extra_days * 86400, tm); + rk808_rtc_set_time(dev, tm); + } + return ret; +} + static int rk808_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled) { @@ -364,6 +452,16 @@ static int rk808_rtc_probe(struct platform_device *pdev) return ret; } + /* + * Try to initialize anchor point by reading "last read" shadow + * timestamp, to catch Nov 31st transitions that happened while shut + * down. This only works if no other code (e.g. firmware) has + * transitioned GET_TIME before this point. + */ + ret = rk808_rtc_raw_read(&pdev->dev, &rk808_rtc->anchor_time); + if (ret || rtc_valid_tm(&rk808_rtc->anchor_time)) + rk808_rtc->anchor_time.tm_mday = 0; /* invalidate */ + /* set init time */ ret = rk808_rtc_readtime(&pdev->dev, &tm); if (ret) { -- 2.1.2 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* [rtc-linux] [PATCH v2] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-08 5:21 ` Julius Werner 0 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-08 5:21 UTC (permalink / raw) To: Alexandre Belloni Cc: Andrew Morton, Alessandro Zummo, Doug Anderson, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux, Julius Werner In Fuzhou, China, the month of November seems to be having 31 days. That's nice and all (I'm sure you can get a lot more done in a year that way), but back here in other parts of the world we are not so lucky. Therefore, we need to compensate for these extra days existing only in the RTC's imagination when reading the time and dealing with alarms. This patch is not a perfect workaround -- it only keeps the time stable as long as the system is running or suspended. If the system is fully shut down in November and only booted back up in December, the system time may be incorrect and alarms that had been set before the shutdown may fire on the wrong day. We're trying to catch and recover from this by reading the RTC's last "shadow timestamp" (which only gets resynced when transitioning the GET_TIME control bit) to figure out when the system was shut down, but this is only reliable if no other code (e.g. firmware) has read the RTC in-between. Basic idea for the workaround: - Whenever we set the time, we assume that timestamp to be correct (synced to the real world). We store a copy of it in memory as an anchor point (where we know our calendar matched the real world). - Whenever we read the time, we can tell how many days of desync we have by counting November/December transitions between the anchor timestamp and the time read from the hardware. We adjust the hardware clock accordingly to get back in sync (which also resets the anchor time). - Whenever we set an alarm, we adjust the alarm time backwards by the amount of days that we know we will lag behind at that point (by counting the November/December transitions between our anchor point and the alarm). This way, we will wake up on the right real world date even though we cannot make adjustments while suspended. - Whenever we read an alarm, we do the opposite (forward) adjustment for the returned result to keep our outside interface free from this madness (callers expect to be able to read back the alarm they wrote). - Whenever we set the system time (which adjusts the anchor point), we read out the (adjusted) alarm time beforehand and write it (newly adjusted) back afterwards. This way, system time and alarm time will always stay on the same calendar (as long as we're able to keep track of our anchor point, at least). Signed-off-by: Julius Werner <jwerner@chromium.org> --- drivers/rtc/rtc-rk808.c | 282 ++++++++++++++++++++++++++++++++---------------- 1 file changed, 190 insertions(+), 92 deletions(-) diff --git a/drivers/rtc/rtc-rk808.c b/drivers/rtc/rtc-rk808.c index 91ca0bc..2a6cd6f 100644 --- a/drivers/rtc/rtc-rk808.c +++ b/drivers/rtc/rtc-rk808.c @@ -54,103 +54,30 @@ struct rk808_rtc { struct rk808 *rk808; struct rtc_device *rtc; int irq; + struct rtc_time anchor_time; /* Last sync point with real world */ }; -/* Read current time and date in RTC */ -static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) +/* + * RK808 has a hardware bug causing it to count 31 days in November. This + * function can calculate the amount of days that code needs to adjust for + * between two timestamps to compensate for this. + */ +static int nov31st_transitions(struct rtc_time *from, struct rtc_time *to) { - struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); - struct rk808 *rk808 = rk808_rtc->rk808; - u8 rtc_data[NUM_TIME_REGS]; - int ret; - - /* Force an update of the shadowed registers right now */ - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, - BIT_RTC_CTRL_REG_RTC_GET_TIME, - BIT_RTC_CTRL_REG_RTC_GET_TIME); - if (ret) { - dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret); - return ret; - } - - /* - * After we set the GET_TIME bit, the rtc time can't be read - * immediately. So we should wait up to 31.25 us, about one cycle of - * 32khz. If we clear the GET_TIME bit here, the time of i2c transfer - * certainly more than 31.25us: 16 * 2.5us at 400kHz bus frequency. - */ - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, - BIT_RTC_CTRL_REG_RTC_GET_TIME, - 0); - if (ret) { - dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret); - return ret; - } + int extra_days = to->tm_year - from->tm_year; - ret = regmap_bulk_read(rk808->regmap, RK808_SECONDS_REG, - rtc_data, NUM_TIME_REGS); - if (ret) { - dev_err(dev, "Failed to bulk read rtc_data: %d\n", ret); - return ret; - } + /* Avoid adjusting anything for uninitialized timestamps */ + if (from->tm_mday == 0 || to->tm_mday == 0) + return 0; - tm->tm_sec = bcd2bin(rtc_data[0] & SECONDS_REG_MSK); - tm->tm_min = bcd2bin(rtc_data[1] & MINUTES_REG_MAK); - tm->tm_hour = bcd2bin(rtc_data[2] & HOURS_REG_MSK); - tm->tm_mday = bcd2bin(rtc_data[3] & DAYS_REG_MSK); - tm->tm_mon = (bcd2bin(rtc_data[4] & MONTHS_REG_MSK)) - 1; - tm->tm_year = (bcd2bin(rtc_data[5] & YEARS_REG_MSK)) + 100; - tm->tm_wday = bcd2bin(rtc_data[6] & WEEKS_REG_MSK); - dev_dbg(dev, "RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", - 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); + if (from->tm_mon > 10) + extra_days--; - return ret; -} + if (to->tm_mon > 10) + extra_days++; -/* Set current time and date in RTC */ -static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) -{ - struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); - struct rk808 *rk808 = rk808_rtc->rk808; - u8 rtc_data[NUM_TIME_REGS]; - int ret; - - rtc_data[0] = bin2bcd(tm->tm_sec); - rtc_data[1] = bin2bcd(tm->tm_min); - rtc_data[2] = bin2bcd(tm->tm_hour); - rtc_data[3] = bin2bcd(tm->tm_mday); - rtc_data[4] = bin2bcd(tm->tm_mon + 1); - rtc_data[5] = bin2bcd(tm->tm_year - 100); - rtc_data[6] = bin2bcd(tm->tm_wday); - dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", - 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); - - /* Stop RTC while updating the RTC registers */ - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, - BIT_RTC_CTRL_REG_STOP_RTC_M, - BIT_RTC_CTRL_REG_STOP_RTC_M); - if (ret) { - dev_err(dev, "Failed to update RTC control: %d\n", ret); - return ret; - } - - ret = regmap_bulk_write(rk808->regmap, RK808_SECONDS_REG, - rtc_data, NUM_TIME_REGS); - if (ret) { - dev_err(dev, "Failed to bull write rtc_data: %d\n", ret); - return ret; - } - /* Start RTC again */ - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, - BIT_RTC_CTRL_REG_STOP_RTC_M, 0); - if (ret) { - dev_err(dev, "Failed to update RTC control: %d\n", ret); - return ret; - } - return 0; -} + return extra_days; +}; /* Read alarm time and date in RTC */ static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm) @@ -159,7 +86,7 @@ static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm) struct rk808 *rk808 = rk808_rtc->rk808; u8 alrm_data[NUM_ALARM_REGS]; uint32_t int_reg; - int ret; + int ret, extra_days; ret = regmap_bulk_read(rk808->regmap, RK808_ALARM_SECONDS_REG, alrm_data, NUM_ALARM_REGS); @@ -171,6 +98,19 @@ static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm) alrm->time.tm_mon = (bcd2bin(alrm_data[4] & MONTHS_REG_MSK)) - 1; alrm->time.tm_year = (bcd2bin(alrm_data[5] & YEARS_REG_MSK)) + 100; + extra_days = nov31st_transitions(&rk808_rtc->anchor_time, &alrm->time); + if (alrm->time.tm_mon == 10 && alrm->time.tm_mday == 31) { + dev_warn(dev, "read HW alarm date as Nov 31, compensating\n"); + alrm->time.tm_mon = 11; + alrm->time.tm_mday = 1 + extra_days; + } else if (extra_days) { + unsigned long time; + dev_warn(dev, "compensating for %d Nov31 until HW alarm date\n", + extra_days); + rtc_tm_to_time(&alrm->time, &time); + rtc_time_to_tm(time + extra_days * 86400, &alrm->time); + } + ret = regmap_read(rk808->regmap, RK808_RTC_INT_REG, &int_reg); if (ret) { dev_err(dev, "Failed to read RTC INT REG: %d\n", ret); @@ -215,7 +155,7 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm) struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); struct rk808 *rk808 = rk808_rtc->rk808; u8 alrm_data[NUM_ALARM_REGS]; - int ret; + int ret, extra_days; ret = rk808_rtc_stop_alarm(rk808_rtc); if (ret) { @@ -227,6 +167,19 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm) alrm->time.tm_mday, alrm->time.tm_wday, alrm->time.tm_hour, alrm->time.tm_min, alrm->time.tm_sec); + extra_days = nov31st_transitions(&rk808_rtc->anchor_time, &alrm->time); + if (extra_days) { + unsigned long time; + dev_warn(dev, "writing HW alarm date adjusted for %d Nov31\n", + extra_days); + rtc_tm_to_time(&alrm->time, &time); + rtc_time_to_tm(time - extra_days * 86400, &alrm->time); + /* Compensate in case the subtraction went back over Nov 31st */ + if (alrm->time.tm_mon == 10 && + alrm->time.tm_mday == 31 - extra_days) + alrm->time.tm_mday++; /* This can result in 31! */ + } + alrm_data[0] = bin2bcd(alrm->time.tm_sec); alrm_data[1] = bin2bcd(alrm->time.tm_min); alrm_data[2] = bin2bcd(alrm->time.tm_hour); @@ -250,6 +203,141 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm) return 0; } +/* Set current time and date in RTC */ +static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) +{ + struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); + struct rk808 *rk808 = rk808_rtc->rk808; + struct rtc_wkalrm alrm; + u8 rtc_data[NUM_TIME_REGS]; + int ret; + + /* Read out wake alarm with old Nov 31st adjustment */ + rk808_rtc_readalarm(dev, &alrm); + + rtc_data[0] = bin2bcd(tm->tm_sec); + rtc_data[1] = bin2bcd(tm->tm_min); + rtc_data[2] = bin2bcd(tm->tm_hour); + rtc_data[3] = bin2bcd(tm->tm_mday); + rtc_data[4] = bin2bcd(tm->tm_mon + 1); + rtc_data[5] = bin2bcd(tm->tm_year - 100); + rtc_data[6] = bin2bcd(tm->tm_wday); + dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", + 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, + tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); + + /* Stop RTC while updating the RTC registers */ + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, + BIT_RTC_CTRL_REG_STOP_RTC_M, + BIT_RTC_CTRL_REG_STOP_RTC_M); + if (ret) { + dev_err(dev, "Failed to update RTC control: %d\n", ret); + return ret; + } + + ret = regmap_bulk_write(rk808->regmap, RK808_SECONDS_REG, + rtc_data, NUM_TIME_REGS); + if (ret) { + dev_err(dev, "Failed to bull write rtc_data: %d\n", ret); + return ret; + } + /* Start RTC again */ + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, + BIT_RTC_CTRL_REG_STOP_RTC_M, 0); + if (ret) { + dev_err(dev, "Failed to update RTC control: %d\n", ret); + return ret; + } + + /* Assume a newly set time is always correct (regardless of source) */ + rk808_rtc->anchor_time = *tm; + + /* Write back wake alarm with new Nov 31st adjustment */ + rk808_rtc_setalarm(dev, &alrm); + + return 0; +} + +/* Read time from static shadow registers (without updating) */ +static int rk808_rtc_raw_read(struct device *dev, struct rtc_time *tm) +{ + struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); + struct rk808 *rk808 = rk808_rtc->rk808; + u8 rtc_data[NUM_TIME_REGS]; + int ret; + + ret = regmap_bulk_read(rk808->regmap, RK808_SECONDS_REG, + rtc_data, NUM_TIME_REGS); + if (ret) { + dev_err(dev, "Failed to bulk read rtc_data: %d\n", ret); + return ret; + } + + tm->tm_sec = bcd2bin(rtc_data[0] & SECONDS_REG_MSK); + tm->tm_min = bcd2bin(rtc_data[1] & MINUTES_REG_MAK); + tm->tm_hour = bcd2bin(rtc_data[2] & HOURS_REG_MSK); + tm->tm_mday = bcd2bin(rtc_data[3] & DAYS_REG_MSK); + tm->tm_mon = (bcd2bin(rtc_data[4] & MONTHS_REG_MSK)) - 1; + tm->tm_year = (bcd2bin(rtc_data[5] & YEARS_REG_MSK)) + 100; + tm->tm_wday = bcd2bin(rtc_data[6] & WEEKS_REG_MSK); + dev_dbg(dev, "RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", + 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, + tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); + + return ret; +} + +/* Read current time and date in RTC */ +static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) +{ + struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); + struct rk808 *rk808 = rk808_rtc->rk808; + int ret, extra_days; + + /* Force an update of the shadowed registers right now */ + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, + BIT_RTC_CTRL_REG_RTC_GET_TIME, + BIT_RTC_CTRL_REG_RTC_GET_TIME); + if (ret) { + dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret); + return ret; + } + + /* + * After we set the GET_TIME bit, the rtc time can't be read + * immediately. So we should wait up to 31.25 us, about one cycle of + * 32khz. If we clear the GET_TIME bit here, the time of i2c transfer + * certainly more than 31.25us: 16 * 2.5us at 400kHz bus frequency. + */ + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, + BIT_RTC_CTRL_REG_RTC_GET_TIME, + 0); + if (ret) { + dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret); + return ret; + } + + ret = rk808_rtc_raw_read(dev, tm); + if (ret) + return ret; + + extra_days = nov31st_transitions(&rk808_rtc->anchor_time, tm); + if (tm->tm_mon == 10 && tm->tm_mday == 31) { + dev_warn(dev, "read Nov 31, correcting to Dec 1 (HW bug)\n"); + tm->tm_mon = 11; + tm->tm_mday = 1 + extra_days; /* don't S2R for over 30 years! */ + rk808_rtc_set_time(dev, tm); + } else if (extra_days) { + unsigned long time; + dev_warn(dev, "compensating for %d skips over Nov 31\n", + extra_days); + rtc_tm_to_time(tm, &time); + rtc_time_to_tm(time + extra_days * 86400, tm); + rk808_rtc_set_time(dev, tm); + } + return ret; +} + static int rk808_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled) { @@ -364,6 +452,16 @@ static int rk808_rtc_probe(struct platform_device *pdev) return ret; } + /* + * Try to initialize anchor point by reading "last read" shadow + * timestamp, to catch Nov 31st transitions that happened while shut + * down. This only works if no other code (e.g. firmware) has + * transitioned GET_TIME before this point. + */ + ret = rk808_rtc_raw_read(&pdev->dev, &rk808_rtc->anchor_time); + if (ret || rtc_valid_tm(&rk808_rtc->anchor_time)) + rk808_rtc->anchor_time.tm_mday = 0; /* invalidate */ + /* set init time */ ret = rk808_rtc_readtime(&pdev->dev, &tm); if (ret) { -- 2.1.2 -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCH v2] RTC: RK808: Work around hardware bug on November 31st 2015-12-08 5:21 ` [rtc-linux] " Julius Werner @ 2015-12-09 5:44 ` Doug Anderson -1 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-09 5:44 UTC (permalink / raw) To: Julius Werner Cc: Alexandre Belloni, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Julius, On Mon, Dec 7, 2015 at 9:21 PM, Julius Werner <jwerner@chromium.org> wrote: > In Fuzhou, China, the month of November seems to be having 31 days. > That's nice and all (I'm sure you can get a lot more done in a year that > way), but back here in other parts of the world we are not so lucky. > Therefore, we need to compensate for these extra days existing only in > the RTC's imagination when reading the time and dealing with alarms. > > This patch is not a perfect workaround -- it only keeps the time stable > as long as the system is running or suspended. If the system is fully > shut down in November and only booted back up in December, the system > time may be incorrect and alarms that had been set before the shutdown > may fire on the wrong day. We're trying to catch and recover from this > by reading the RTC's last "shadow timestamp" (which only gets resynced > when transitioning the GET_TIME control bit) to figure out when the > system was shut down, but this is only reliable if no other code (e.g. > firmware) has read the RTC in-between. > > Basic idea for the workaround: > > - Whenever we set the time, we assume that timestamp to be correct > (synced to the real world). We store a copy of it in memory as an > anchor point (where we know our calendar matched the real world). > - Whenever we read the time, we can tell how many days of desync we have > by counting November/December transitions between the anchor timestamp > and the time read from the hardware. We adjust the hardware clock > accordingly to get back in sync (which also resets the anchor time). > - Whenever we set an alarm, we adjust the alarm time backwards by the > amount of days that we know we will lag behind at that point (by > counting the November/December transitions between our anchor point > and the alarm). This way, we will wake up on the right real world date > even though we cannot make adjustments while suspended. > - Whenever we read an alarm, we do the opposite (forward) adjustment for > the returned result to keep our outside interface free from this > madness (callers expect to be able to read back the alarm they wrote). > - Whenever we set the system time (which adjusts the anchor point), we > read out the (adjusted) alarm time beforehand and write it (newly > adjusted) back afterwards. This way, system time and alarm time will > always stay on the same calendar (as long as we're able to keep track > of our anchor point, at least). Thinking about all this: these's actually a totally different alternative approach we could take if you wanted. It would fix S5 and avoid all the anchor stuff, unless I'm crazy. Basically totally give up on the RTC time reflecting reality. Add a "real time to rk808" and "rk808 time to real time" function. Always use it when reading from rk808 and always use it when writing to rk808. Choose 2015 as the "truth" year if you want (or pick another year). In early 2016 the rk808 always contains 1 day back. In 2017 the rk808 always contains 2 days back. Etc, etc, etc. The firmware would get confused, but ... > Signed-off-by: Julius Werner <jwerner@chromium.org> > --- > drivers/rtc/rtc-rk808.c | 282 ++++++++++++++++++++++++++++++++---------------- > 1 file changed, 190 insertions(+), 92 deletions(-) > > diff --git a/drivers/rtc/rtc-rk808.c b/drivers/rtc/rtc-rk808.c > index 91ca0bc..2a6cd6f 100644 > --- a/drivers/rtc/rtc-rk808.c > +++ b/drivers/rtc/rtc-rk808.c > @@ -54,103 +54,30 @@ struct rk808_rtc { > struct rk808 *rk808; > struct rtc_device *rtc; > int irq; > + struct rtc_time anchor_time; /* Last sync point with real world */ Is this ever Nov 31st? Looks like it never is... > }; > > -/* Read current time and date in RTC */ > -static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) > +/* > + * RK808 has a hardware bug causing it to count 31 days in November. This > + * function can calculate the amount of days that code needs to adjust for > + * between two timestamps to compensate for this. Document what this does if "from" is Nov 31st or if "to" is Nov 31st. > + */ > +static int nov31st_transitions(struct rtc_time *from, struct rtc_time *to) Review will be significantly easier if we break this up into two patches: 1. Brain-dead code movement. Move rk808_rtc_readtime() and rk808_rtc_set_time() with no functional changes. 2. This actual change. That changed the diffstat of the important patch from: 1 file changed, 190 insertions(+), 92 deletions(-) ...to... 1 file changed, 118 insertions(+), 20 deletions(-) ...possibly you could even add a 3rd patch splitting out raw_read. Not sure... > { > - struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); > - struct rk808 *rk808 = rk808_rtc->rk808; > - u8 rtc_data[NUM_TIME_REGS]; > - int ret; > - > - /* Force an update of the shadowed registers right now */ > - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > - BIT_RTC_CTRL_REG_RTC_GET_TIME, > - BIT_RTC_CTRL_REG_RTC_GET_TIME); > - if (ret) { > - dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret); > - return ret; > - } > - > - /* > - * After we set the GET_TIME bit, the rtc time can't be read > - * immediately. So we should wait up to 31.25 us, about one cycle of > - * 32khz. If we clear the GET_TIME bit here, the time of i2c transfer > - * certainly more than 31.25us: 16 * 2.5us at 400kHz bus frequency. > - */ > - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > - BIT_RTC_CTRL_REG_RTC_GET_TIME, > - 0); > - if (ret) { > - dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret); > - return ret; > - } > + int extra_days = to->tm_year - from->tm_year; > > - ret = regmap_bulk_read(rk808->regmap, RK808_SECONDS_REG, > - rtc_data, NUM_TIME_REGS); > - if (ret) { > - dev_err(dev, "Failed to bulk read rtc_data: %d\n", ret); > - return ret; > - } > + /* Avoid adjusting anything for uninitialized timestamps */ > + if (from->tm_mday == 0 || to->tm_mday == 0) > + return 0; > > - tm->tm_sec = bcd2bin(rtc_data[0] & SECONDS_REG_MSK); > - tm->tm_min = bcd2bin(rtc_data[1] & MINUTES_REG_MAK); > - tm->tm_hour = bcd2bin(rtc_data[2] & HOURS_REG_MSK); > - tm->tm_mday = bcd2bin(rtc_data[3] & DAYS_REG_MSK); > - tm->tm_mon = (bcd2bin(rtc_data[4] & MONTHS_REG_MSK)) - 1; > - tm->tm_year = (bcd2bin(rtc_data[5] & YEARS_REG_MSK)) + 100; > - tm->tm_wday = bcd2bin(rtc_data[6] & WEEKS_REG_MSK); > - dev_dbg(dev, "RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > - 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > + if (from->tm_mon > 10) > + extra_days--; Hmm, need to think about this a bit. I'm bad at math like this, but let me try an example from = Nov (AKA 10) 30, 2015 to = Dec (AKA 11) 1, 2020 extra days = 6: Nov 31, 2015 Nov 31, 2016 Nov 31, 2017 Nov 31, 2018 Nov 31, 2019 Nov 31, 2020 I think you'll do 20 - 15 + 1 = 6. OK, good. from = Nov (AKA 10) 30, 2015 to = Nov (AKA 10) 30, 2020 extra days = 5: Nov 31, 2015 Nov 31, 2016 Nov 31, 2017 Nov 31, 2018 Nov 31, 2019 You'll do 20 - 15 = 5. OK Good. from = Nov (AKA 10) 30, 2015 to = Nov (AKA 10) 31, 2015 You'll do 15 - 15 = 0. Document. I think this is OK. from = Nov (AKA 10) 31, 2015 to = Dec (AKA 11) 1, 2015 You'll do 15 - 15 + 1= 1. Document, though I think code never actually does this because anchor is never Nov 31st. from = Dec (AKA 11) 1, 2015 to = Nov (AKA 10) 31, 2016 You'll do 16 - 15 - 1 = 0. Matches above. > > - return ret; > -} > + if (to->tm_mon > 10) > + extra_days++; > > -/* Set current time and date in RTC */ > -static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) > -{ > - struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); > - struct rk808 *rk808 = rk808_rtc->rk808; > - u8 rtc_data[NUM_TIME_REGS]; > - int ret; > - > - rtc_data[0] = bin2bcd(tm->tm_sec); > - rtc_data[1] = bin2bcd(tm->tm_min); > - rtc_data[2] = bin2bcd(tm->tm_hour); > - rtc_data[3] = bin2bcd(tm->tm_mday); > - rtc_data[4] = bin2bcd(tm->tm_mon + 1); > - rtc_data[5] = bin2bcd(tm->tm_year - 100); > - rtc_data[6] = bin2bcd(tm->tm_wday); > - dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > - 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > - > - /* Stop RTC while updating the RTC registers */ > - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > - BIT_RTC_CTRL_REG_STOP_RTC_M, > - BIT_RTC_CTRL_REG_STOP_RTC_M); > - if (ret) { > - dev_err(dev, "Failed to update RTC control: %d\n", ret); > - return ret; > - } > - > - ret = regmap_bulk_write(rk808->regmap, RK808_SECONDS_REG, > - rtc_data, NUM_TIME_REGS); > - if (ret) { > - dev_err(dev, "Failed to bull write rtc_data: %d\n", ret); > - return ret; > - } > - /* Start RTC again */ > - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > - BIT_RTC_CTRL_REG_STOP_RTC_M, 0); > - if (ret) { > - dev_err(dev, "Failed to update RTC control: %d\n", ret); > - return ret; > - } > - return 0; > -} > + return extra_days; > +}; > > /* Read alarm time and date in RTC */ > static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm) > @@ -159,7 +86,7 @@ static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm) > struct rk808 *rk808 = rk808_rtc->rk808; > u8 alrm_data[NUM_ALARM_REGS]; > uint32_t int_reg; > - int ret; > + int ret, extra_days; > > ret = regmap_bulk_read(rk808->regmap, RK808_ALARM_SECONDS_REG, > alrm_data, NUM_ALARM_REGS); > @@ -171,6 +98,19 @@ static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm) > alrm->time.tm_mon = (bcd2bin(alrm_data[4] & MONTHS_REG_MSK)) - 1; > alrm->time.tm_year = (bcd2bin(alrm_data[5] & YEARS_REG_MSK)) + 100; > > + extra_days = nov31st_transitions(&rk808_rtc->anchor_time, &alrm->time); > + if (alrm->time.tm_mon == 10 && alrm->time.tm_mday == 31) { > + dev_warn(dev, "read HW alarm date as Nov 31, compensating\n"); > + alrm->time.tm_mon = 11; > + alrm->time.tm_mday = 1 + extra_days; But what is it's been 30+ years?!?! :-P Not that this is seriously a problem, but you already handle this in the "else" case. Why not just set to 11/1 and get rid of the "else" below, so: ... alrm->time.tm_mon = 11; alrm->time.tm_mday = 1; } if (extra_days) { ... > + } else if (extra_days) { > + unsigned long time; > + dev_warn(dev, "compensating for %d Nov31 until HW alarm date\n", > + extra_days); > + rtc_tm_to_time(&alrm->time, &time); > + rtc_time_to_tm(time + extra_days * 86400, &alrm->time); > + } > + > ret = regmap_read(rk808->regmap, RK808_RTC_INT_REG, &int_reg); > if (ret) { > dev_err(dev, "Failed to read RTC INT REG: %d\n", ret); > @@ -215,7 +155,7 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm) > struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); > struct rk808 *rk808 = rk808_rtc->rk808; > u8 alrm_data[NUM_ALARM_REGS]; > - int ret; > + int ret, extra_days; > > ret = rk808_rtc_stop_alarm(rk808_rtc); > if (ret) { > @@ -227,6 +167,19 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm) > alrm->time.tm_mday, alrm->time.tm_wday, alrm->time.tm_hour, > alrm->time.tm_min, alrm->time.tm_sec); > > + extra_days = nov31st_transitions(&rk808_rtc->anchor_time, &alrm->time); > + if (extra_days) { > + unsigned long time; > + dev_warn(dev, "writing HW alarm date adjusted for %d Nov31\n", > + extra_days); > + rtc_tm_to_time(&alrm->time, &time); > + rtc_time_to_tm(time - extra_days * 86400, &alrm->time); > + /* Compensate in case the subtraction went back over Nov 31st */ > + if (alrm->time.tm_mon == 10 && > + alrm->time.tm_mday == 31 - extra_days) > + alrm->time.tm_mday++; /* This can result in 31! */ Seems fishy somehow. You should be able to come up with scenarios where you need an alarm on 11/31 on each future year. Here I think you can only possibly get 11/31 if extra days == 1, right? So if it is Nov 30, 2015 and we want alarm on Dec 6, 2020 extra days = 6: Nov 31, 2015 Nov 31, 2016 Nov 31, 2017 Nov 31, 2018 Nov 31, 2019 Nov 31, 2020 I think we need to set the alarm for Nov 31st 2020, right? Your subtraction should get you to Nov 30th and you need to add an extra day for that, but I don't _think_ your if test will. Did I mess up? As I said, I'm bad at this... > + } > + > alrm_data[0] = bin2bcd(alrm->time.tm_sec); > alrm_data[1] = bin2bcd(alrm->time.tm_min); > alrm_data[2] = bin2bcd(alrm->time.tm_hour); > @@ -250,6 +203,141 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm) > return 0; > } > > +/* Set current time and date in RTC */ > +static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) > +{ > + struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); > + struct rk808 *rk808 = rk808_rtc->rk808; > + struct rtc_wkalrm alrm; > + u8 rtc_data[NUM_TIME_REGS]; > + int ret; > + > + /* Read out wake alarm with old Nov 31st adjustment */ > + rk808_rtc_readalarm(dev, &alrm); > + > + rtc_data[0] = bin2bcd(tm->tm_sec); > + rtc_data[1] = bin2bcd(tm->tm_min); > + rtc_data[2] = bin2bcd(tm->tm_hour); > + rtc_data[3] = bin2bcd(tm->tm_mday); > + rtc_data[4] = bin2bcd(tm->tm_mon + 1); > + rtc_data[5] = bin2bcd(tm->tm_year - 100); > + rtc_data[6] = bin2bcd(tm->tm_wday); > + dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > + 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > + tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > + > + /* Stop RTC while updating the RTC registers */ > + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > + BIT_RTC_CTRL_REG_STOP_RTC_M, > + BIT_RTC_CTRL_REG_STOP_RTC_M); > + if (ret) { > + dev_err(dev, "Failed to update RTC control: %d\n", ret); > + return ret; > + } > + > + ret = regmap_bulk_write(rk808->regmap, RK808_SECONDS_REG, > + rtc_data, NUM_TIME_REGS); > + if (ret) { > + dev_err(dev, "Failed to bull write rtc_data: %d\n", ret); > + return ret; > + } > + /* Start RTC again */ > + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > + BIT_RTC_CTRL_REG_STOP_RTC_M, 0); > + if (ret) { > + dev_err(dev, "Failed to update RTC control: %d\n", ret); > + return ret; > + } > + > + /* Assume a newly set time is always correct (regardless of source) */ > + rk808_rtc->anchor_time = *tm; > + > + /* Write back wake alarm with new Nov 31st adjustment */ > + rk808_rtc_setalarm(dev, &alrm); > + > + return 0; > +} > + > +/* Read time from static shadow registers (without updating) */ > +static int rk808_rtc_raw_read(struct device *dev, struct rtc_time *tm) > +{ > + struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); > + struct rk808 *rk808 = rk808_rtc->rk808; > + u8 rtc_data[NUM_TIME_REGS]; > + int ret; > + > + ret = regmap_bulk_read(rk808->regmap, RK808_SECONDS_REG, > + rtc_data, NUM_TIME_REGS); > + if (ret) { > + dev_err(dev, "Failed to bulk read rtc_data: %d\n", ret); > + return ret; > + } > + > + tm->tm_sec = bcd2bin(rtc_data[0] & SECONDS_REG_MSK); > + tm->tm_min = bcd2bin(rtc_data[1] & MINUTES_REG_MAK); > + tm->tm_hour = bcd2bin(rtc_data[2] & HOURS_REG_MSK); > + tm->tm_mday = bcd2bin(rtc_data[3] & DAYS_REG_MSK); > + tm->tm_mon = (bcd2bin(rtc_data[4] & MONTHS_REG_MSK)) - 1; > + tm->tm_year = (bcd2bin(rtc_data[5] & YEARS_REG_MSK)) + 100; > + tm->tm_wday = bcd2bin(rtc_data[6] & WEEKS_REG_MSK); > + dev_dbg(dev, "RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > + 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > + tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > + > + return ret; > +} > + > +/* Read current time and date in RTC */ > +static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) > +{ > + struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); > + struct rk808 *rk808 = rk808_rtc->rk808; > + int ret, extra_days; > + > + /* Force an update of the shadowed registers right now */ > + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > + BIT_RTC_CTRL_REG_RTC_GET_TIME, > + BIT_RTC_CTRL_REG_RTC_GET_TIME); > + if (ret) { > + dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret); > + return ret; > + } > + > + /* > + * After we set the GET_TIME bit, the rtc time can't be read > + * immediately. So we should wait up to 31.25 us, about one cycle of > + * 32khz. If we clear the GET_TIME bit here, the time of i2c transfer > + * certainly more than 31.25us: 16 * 2.5us at 400kHz bus frequency. > + */ > + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > + BIT_RTC_CTRL_REG_RTC_GET_TIME, > + 0); > + if (ret) { > + dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret); > + return ret; > + } > + > + ret = rk808_rtc_raw_read(dev, tm); > + if (ret) > + return ret; > + > + extra_days = nov31st_transitions(&rk808_rtc->anchor_time, tm); > + if (tm->tm_mon == 10 && tm->tm_mday == 31) { > + dev_warn(dev, "read Nov 31, correcting to Dec 1 (HW bug)\n"); > + tm->tm_mon = 11; > + tm->tm_mday = 1 + extra_days; /* don't S2R for over 30 years! */ > + rk808_rtc_set_time(dev, tm); > + } else if (extra_days) { > + unsigned long time; > + dev_warn(dev, "compensating for %d skips over Nov 31\n", > + extra_days); > + rtc_tm_to_time(tm, &time); > + rtc_time_to_tm(time + extra_days * 86400, tm); > + rk808_rtc_set_time(dev, tm); > + } > + return ret; > +} > + > static int rk808_rtc_alarm_irq_enable(struct device *dev, > unsigned int enabled) > { > @@ -364,6 +452,16 @@ static int rk808_rtc_probe(struct platform_device *pdev) > return ret; > } > > + /* > + * Try to initialize anchor point by reading "last read" shadow > + * timestamp, to catch Nov 31st transitions that happened while shut > + * down. This only works if no other code (e.g. firmware) has > + * transitioned GET_TIME before this point. > + */ > + ret = rk808_rtc_raw_read(&pdev->dev, &rk808_rtc->anchor_time); > + if (ret || rtc_valid_tm(&rk808_rtc->anchor_time)) > + rk808_rtc->anchor_time.tm_mday = 0; /* invalidate */ Ah, the rtc_valid_tm() keeps the anchor from every being Nov 31st. Got it. > + > /* set init time */ > ret = rk808_rtc_readtime(&pdev->dev, &tm); > if (ret) { OK, that's a first pass anyway... Thanks for taking this on! :) -Doug ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH v2] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-09 5:44 ` Doug Anderson 0 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-09 5:44 UTC (permalink / raw) To: Julius Werner Cc: Alexandre Belloni, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Julius, On Mon, Dec 7, 2015 at 9:21 PM, Julius Werner <jwerner@chromium.org> wrote: > In Fuzhou, China, the month of November seems to be having 31 days. > That's nice and all (I'm sure you can get a lot more done in a year that > way), but back here in other parts of the world we are not so lucky. > Therefore, we need to compensate for these extra days existing only in > the RTC's imagination when reading the time and dealing with alarms. > > This patch is not a perfect workaround -- it only keeps the time stable > as long as the system is running or suspended. If the system is fully > shut down in November and only booted back up in December, the system > time may be incorrect and alarms that had been set before the shutdown > may fire on the wrong day. We're trying to catch and recover from this > by reading the RTC's last "shadow timestamp" (which only gets resynced > when transitioning the GET_TIME control bit) to figure out when the > system was shut down, but this is only reliable if no other code (e.g. > firmware) has read the RTC in-between. > > Basic idea for the workaround: > > - Whenever we set the time, we assume that timestamp to be correct > (synced to the real world). We store a copy of it in memory as an > anchor point (where we know our calendar matched the real world). > - Whenever we read the time, we can tell how many days of desync we have > by counting November/December transitions between the anchor timestamp > and the time read from the hardware. We adjust the hardware clock > accordingly to get back in sync (which also resets the anchor time). > - Whenever we set an alarm, we adjust the alarm time backwards by the > amount of days that we know we will lag behind at that point (by > counting the November/December transitions between our anchor point > and the alarm). This way, we will wake up on the right real world date > even though we cannot make adjustments while suspended. > - Whenever we read an alarm, we do the opposite (forward) adjustment for > the returned result to keep our outside interface free from this > madness (callers expect to be able to read back the alarm they wrote). > - Whenever we set the system time (which adjusts the anchor point), we > read out the (adjusted) alarm time beforehand and write it (newly > adjusted) back afterwards. This way, system time and alarm time will > always stay on the same calendar (as long as we're able to keep track > of our anchor point, at least). Thinking about all this: these's actually a totally different alternative approach we could take if you wanted. It would fix S5 and avoid all the anchor stuff, unless I'm crazy. Basically totally give up on the RTC time reflecting reality. Add a "real time to rk808" and "rk808 time to real time" function. Always use it when reading from rk808 and always use it when writing to rk808. Choose 2015 as the "truth" year if you want (or pick another year). In early 2016 the rk808 always contains 1 day back. In 2017 the rk808 always contains 2 days back. Etc, etc, etc. The firmware would get confused, but ... > Signed-off-by: Julius Werner <jwerner@chromium.org> > --- > drivers/rtc/rtc-rk808.c | 282 ++++++++++++++++++++++++++++++++---------------- > 1 file changed, 190 insertions(+), 92 deletions(-) > > diff --git a/drivers/rtc/rtc-rk808.c b/drivers/rtc/rtc-rk808.c > index 91ca0bc..2a6cd6f 100644 > --- a/drivers/rtc/rtc-rk808.c > +++ b/drivers/rtc/rtc-rk808.c > @@ -54,103 +54,30 @@ struct rk808_rtc { > struct rk808 *rk808; > struct rtc_device *rtc; > int irq; > + struct rtc_time anchor_time; /* Last sync point with real world */ Is this ever Nov 31st? Looks like it never is... > }; > > -/* Read current time and date in RTC */ > -static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) > +/* > + * RK808 has a hardware bug causing it to count 31 days in November. This > + * function can calculate the amount of days that code needs to adjust for > + * between two timestamps to compensate for this. Document what this does if "from" is Nov 31st or if "to" is Nov 31st. > + */ > +static int nov31st_transitions(struct rtc_time *from, struct rtc_time *to) Review will be significantly easier if we break this up into two patches: 1. Brain-dead code movement. Move rk808_rtc_readtime() and rk808_rtc_set_time() with no functional changes. 2. This actual change. That changed the diffstat of the important patch from: 1 file changed, 190 insertions(+), 92 deletions(-) ...to... 1 file changed, 118 insertions(+), 20 deletions(-) ...possibly you could even add a 3rd patch splitting out raw_read. Not sure... > { > - struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); > - struct rk808 *rk808 = rk808_rtc->rk808; > - u8 rtc_data[NUM_TIME_REGS]; > - int ret; > - > - /* Force an update of the shadowed registers right now */ > - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > - BIT_RTC_CTRL_REG_RTC_GET_TIME, > - BIT_RTC_CTRL_REG_RTC_GET_TIME); > - if (ret) { > - dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret); > - return ret; > - } > - > - /* > - * After we set the GET_TIME bit, the rtc time can't be read > - * immediately. So we should wait up to 31.25 us, about one cycle of > - * 32khz. If we clear the GET_TIME bit here, the time of i2c transfer > - * certainly more than 31.25us: 16 * 2.5us at 400kHz bus frequency. > - */ > - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > - BIT_RTC_CTRL_REG_RTC_GET_TIME, > - 0); > - if (ret) { > - dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret); > - return ret; > - } > + int extra_days = to->tm_year - from->tm_year; > > - ret = regmap_bulk_read(rk808->regmap, RK808_SECONDS_REG, > - rtc_data, NUM_TIME_REGS); > - if (ret) { > - dev_err(dev, "Failed to bulk read rtc_data: %d\n", ret); > - return ret; > - } > + /* Avoid adjusting anything for uninitialized timestamps */ > + if (from->tm_mday == 0 || to->tm_mday == 0) > + return 0; > > - tm->tm_sec = bcd2bin(rtc_data[0] & SECONDS_REG_MSK); > - tm->tm_min = bcd2bin(rtc_data[1] & MINUTES_REG_MAK); > - tm->tm_hour = bcd2bin(rtc_data[2] & HOURS_REG_MSK); > - tm->tm_mday = bcd2bin(rtc_data[3] & DAYS_REG_MSK); > - tm->tm_mon = (bcd2bin(rtc_data[4] & MONTHS_REG_MSK)) - 1; > - tm->tm_year = (bcd2bin(rtc_data[5] & YEARS_REG_MSK)) + 100; > - tm->tm_wday = bcd2bin(rtc_data[6] & WEEKS_REG_MSK); > - dev_dbg(dev, "RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > - 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > + if (from->tm_mon > 10) > + extra_days--; Hmm, need to think about this a bit. I'm bad at math like this, but let me try an example from = Nov (AKA 10) 30, 2015 to = Dec (AKA 11) 1, 2020 extra days = 6: Nov 31, 2015 Nov 31, 2016 Nov 31, 2017 Nov 31, 2018 Nov 31, 2019 Nov 31, 2020 I think you'll do 20 - 15 + 1 = 6. OK, good. from = Nov (AKA 10) 30, 2015 to = Nov (AKA 10) 30, 2020 extra days = 5: Nov 31, 2015 Nov 31, 2016 Nov 31, 2017 Nov 31, 2018 Nov 31, 2019 You'll do 20 - 15 = 5. OK Good. from = Nov (AKA 10) 30, 2015 to = Nov (AKA 10) 31, 2015 You'll do 15 - 15 = 0. Document. I think this is OK. from = Nov (AKA 10) 31, 2015 to = Dec (AKA 11) 1, 2015 You'll do 15 - 15 + 1= 1. Document, though I think code never actually does this because anchor is never Nov 31st. from = Dec (AKA 11) 1, 2015 to = Nov (AKA 10) 31, 2016 You'll do 16 - 15 - 1 = 0. Matches above. > > - return ret; > -} > + if (to->tm_mon > 10) > + extra_days++; > > -/* Set current time and date in RTC */ > -static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) > -{ > - struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); > - struct rk808 *rk808 = rk808_rtc->rk808; > - u8 rtc_data[NUM_TIME_REGS]; > - int ret; > - > - rtc_data[0] = bin2bcd(tm->tm_sec); > - rtc_data[1] = bin2bcd(tm->tm_min); > - rtc_data[2] = bin2bcd(tm->tm_hour); > - rtc_data[3] = bin2bcd(tm->tm_mday); > - rtc_data[4] = bin2bcd(tm->tm_mon + 1); > - rtc_data[5] = bin2bcd(tm->tm_year - 100); > - rtc_data[6] = bin2bcd(tm->tm_wday); > - dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > - 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > - > - /* Stop RTC while updating the RTC registers */ > - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > - BIT_RTC_CTRL_REG_STOP_RTC_M, > - BIT_RTC_CTRL_REG_STOP_RTC_M); > - if (ret) { > - dev_err(dev, "Failed to update RTC control: %d\n", ret); > - return ret; > - } > - > - ret = regmap_bulk_write(rk808->regmap, RK808_SECONDS_REG, > - rtc_data, NUM_TIME_REGS); > - if (ret) { > - dev_err(dev, "Failed to bull write rtc_data: %d\n", ret); > - return ret; > - } > - /* Start RTC again */ > - ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > - BIT_RTC_CTRL_REG_STOP_RTC_M, 0); > - if (ret) { > - dev_err(dev, "Failed to update RTC control: %d\n", ret); > - return ret; > - } > - return 0; > -} > + return extra_days; > +}; > > /* Read alarm time and date in RTC */ > static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm) > @@ -159,7 +86,7 @@ static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm) > struct rk808 *rk808 = rk808_rtc->rk808; > u8 alrm_data[NUM_ALARM_REGS]; > uint32_t int_reg; > - int ret; > + int ret, extra_days; > > ret = regmap_bulk_read(rk808->regmap, RK808_ALARM_SECONDS_REG, > alrm_data, NUM_ALARM_REGS); > @@ -171,6 +98,19 @@ static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm) > alrm->time.tm_mon = (bcd2bin(alrm_data[4] & MONTHS_REG_MSK)) - 1; > alrm->time.tm_year = (bcd2bin(alrm_data[5] & YEARS_REG_MSK)) + 100; > > + extra_days = nov31st_transitions(&rk808_rtc->anchor_time, &alrm->time); > + if (alrm->time.tm_mon == 10 && alrm->time.tm_mday == 31) { > + dev_warn(dev, "read HW alarm date as Nov 31, compensating\n"); > + alrm->time.tm_mon = 11; > + alrm->time.tm_mday = 1 + extra_days; But what is it's been 30+ years?!?! :-P Not that this is seriously a problem, but you already handle this in the "else" case. Why not just set to 11/1 and get rid of the "else" below, so: ... alrm->time.tm_mon = 11; alrm->time.tm_mday = 1; } if (extra_days) { ... > + } else if (extra_days) { > + unsigned long time; > + dev_warn(dev, "compensating for %d Nov31 until HW alarm date\n", > + extra_days); > + rtc_tm_to_time(&alrm->time, &time); > + rtc_time_to_tm(time + extra_days * 86400, &alrm->time); > + } > + > ret = regmap_read(rk808->regmap, RK808_RTC_INT_REG, &int_reg); > if (ret) { > dev_err(dev, "Failed to read RTC INT REG: %d\n", ret); > @@ -215,7 +155,7 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm) > struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); > struct rk808 *rk808 = rk808_rtc->rk808; > u8 alrm_data[NUM_ALARM_REGS]; > - int ret; > + int ret, extra_days; > > ret = rk808_rtc_stop_alarm(rk808_rtc); > if (ret) { > @@ -227,6 +167,19 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm) > alrm->time.tm_mday, alrm->time.tm_wday, alrm->time.tm_hour, > alrm->time.tm_min, alrm->time.tm_sec); > > + extra_days = nov31st_transitions(&rk808_rtc->anchor_time, &alrm->time); > + if (extra_days) { > + unsigned long time; > + dev_warn(dev, "writing HW alarm date adjusted for %d Nov31\n", > + extra_days); > + rtc_tm_to_time(&alrm->time, &time); > + rtc_time_to_tm(time - extra_days * 86400, &alrm->time); > + /* Compensate in case the subtraction went back over Nov 31st */ > + if (alrm->time.tm_mon == 10 && > + alrm->time.tm_mday == 31 - extra_days) > + alrm->time.tm_mday++; /* This can result in 31! */ Seems fishy somehow. You should be able to come up with scenarios where you need an alarm on 11/31 on each future year. Here I think you can only possibly get 11/31 if extra days == 1, right? So if it is Nov 30, 2015 and we want alarm on Dec 6, 2020 extra days = 6: Nov 31, 2015 Nov 31, 2016 Nov 31, 2017 Nov 31, 2018 Nov 31, 2019 Nov 31, 2020 I think we need to set the alarm for Nov 31st 2020, right? Your subtraction should get you to Nov 30th and you need to add an extra day for that, but I don't _think_ your if test will. Did I mess up? As I said, I'm bad at this... > + } > + > alrm_data[0] = bin2bcd(alrm->time.tm_sec); > alrm_data[1] = bin2bcd(alrm->time.tm_min); > alrm_data[2] = bin2bcd(alrm->time.tm_hour); > @@ -250,6 +203,141 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm) > return 0; > } > > +/* Set current time and date in RTC */ > +static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) > +{ > + struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); > + struct rk808 *rk808 = rk808_rtc->rk808; > + struct rtc_wkalrm alrm; > + u8 rtc_data[NUM_TIME_REGS]; > + int ret; > + > + /* Read out wake alarm with old Nov 31st adjustment */ > + rk808_rtc_readalarm(dev, &alrm); > + > + rtc_data[0] = bin2bcd(tm->tm_sec); > + rtc_data[1] = bin2bcd(tm->tm_min); > + rtc_data[2] = bin2bcd(tm->tm_hour); > + rtc_data[3] = bin2bcd(tm->tm_mday); > + rtc_data[4] = bin2bcd(tm->tm_mon + 1); > + rtc_data[5] = bin2bcd(tm->tm_year - 100); > + rtc_data[6] = bin2bcd(tm->tm_wday); > + dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > + 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > + tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > + > + /* Stop RTC while updating the RTC registers */ > + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > + BIT_RTC_CTRL_REG_STOP_RTC_M, > + BIT_RTC_CTRL_REG_STOP_RTC_M); > + if (ret) { > + dev_err(dev, "Failed to update RTC control: %d\n", ret); > + return ret; > + } > + > + ret = regmap_bulk_write(rk808->regmap, RK808_SECONDS_REG, > + rtc_data, NUM_TIME_REGS); > + if (ret) { > + dev_err(dev, "Failed to bull write rtc_data: %d\n", ret); > + return ret; > + } > + /* Start RTC again */ > + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > + BIT_RTC_CTRL_REG_STOP_RTC_M, 0); > + if (ret) { > + dev_err(dev, "Failed to update RTC control: %d\n", ret); > + return ret; > + } > + > + /* Assume a newly set time is always correct (regardless of source) */ > + rk808_rtc->anchor_time = *tm; > + > + /* Write back wake alarm with new Nov 31st adjustment */ > + rk808_rtc_setalarm(dev, &alrm); > + > + return 0; > +} > + > +/* Read time from static shadow registers (without updating) */ > +static int rk808_rtc_raw_read(struct device *dev, struct rtc_time *tm) > +{ > + struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); > + struct rk808 *rk808 = rk808_rtc->rk808; > + u8 rtc_data[NUM_TIME_REGS]; > + int ret; > + > + ret = regmap_bulk_read(rk808->regmap, RK808_SECONDS_REG, > + rtc_data, NUM_TIME_REGS); > + if (ret) { > + dev_err(dev, "Failed to bulk read rtc_data: %d\n", ret); > + return ret; > + } > + > + tm->tm_sec = bcd2bin(rtc_data[0] & SECONDS_REG_MSK); > + tm->tm_min = bcd2bin(rtc_data[1] & MINUTES_REG_MAK); > + tm->tm_hour = bcd2bin(rtc_data[2] & HOURS_REG_MSK); > + tm->tm_mday = bcd2bin(rtc_data[3] & DAYS_REG_MSK); > + tm->tm_mon = (bcd2bin(rtc_data[4] & MONTHS_REG_MSK)) - 1; > + tm->tm_year = (bcd2bin(rtc_data[5] & YEARS_REG_MSK)) + 100; > + tm->tm_wday = bcd2bin(rtc_data[6] & WEEKS_REG_MSK); > + dev_dbg(dev, "RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > + 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > + tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > + > + return ret; > +} > + > +/* Read current time and date in RTC */ > +static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) > +{ > + struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev); > + struct rk808 *rk808 = rk808_rtc->rk808; > + int ret, extra_days; > + > + /* Force an update of the shadowed registers right now */ > + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > + BIT_RTC_CTRL_REG_RTC_GET_TIME, > + BIT_RTC_CTRL_REG_RTC_GET_TIME); > + if (ret) { > + dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret); > + return ret; > + } > + > + /* > + * After we set the GET_TIME bit, the rtc time can't be read > + * immediately. So we should wait up to 31.25 us, about one cycle of > + * 32khz. If we clear the GET_TIME bit here, the time of i2c transfer > + * certainly more than 31.25us: 16 * 2.5us at 400kHz bus frequency. > + */ > + ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > + BIT_RTC_CTRL_REG_RTC_GET_TIME, > + 0); > + if (ret) { > + dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret); > + return ret; > + } > + > + ret = rk808_rtc_raw_read(dev, tm); > + if (ret) > + return ret; > + > + extra_days = nov31st_transitions(&rk808_rtc->anchor_time, tm); > + if (tm->tm_mon == 10 && tm->tm_mday == 31) { > + dev_warn(dev, "read Nov 31, correcting to Dec 1 (HW bug)\n"); > + tm->tm_mon = 11; > + tm->tm_mday = 1 + extra_days; /* don't S2R for over 30 years! */ > + rk808_rtc_set_time(dev, tm); > + } else if (extra_days) { > + unsigned long time; > + dev_warn(dev, "compensating for %d skips over Nov 31\n", > + extra_days); > + rtc_tm_to_time(tm, &time); > + rtc_time_to_tm(time + extra_days * 86400, tm); > + rk808_rtc_set_time(dev, tm); > + } > + return ret; > +} > + > static int rk808_rtc_alarm_irq_enable(struct device *dev, > unsigned int enabled) > { > @@ -364,6 +452,16 @@ static int rk808_rtc_probe(struct platform_device *pdev) > return ret; > } > > + /* > + * Try to initialize anchor point by reading "last read" shadow > + * timestamp, to catch Nov 31st transitions that happened while shut > + * down. This only works if no other code (e.g. firmware) has > + * transitioned GET_TIME before this point. > + */ > + ret = rk808_rtc_raw_read(&pdev->dev, &rk808_rtc->anchor_time); > + if (ret || rtc_valid_tm(&rk808_rtc->anchor_time)) > + rk808_rtc->anchor_time.tm_mday = 0; /* invalidate */ Ah, the rtc_valid_tm() keeps the anchor from every being Nov 31st. Got it. > + > /* set init time */ > ret = rk808_rtc_readtime(&pdev->dev, &tm); > if (ret) { OK, that's a first pass anyway... Thanks for taking this on! :) -Doug -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2] RTC: RK808: Work around hardware bug on November 31st 2015-12-09 5:44 ` [rtc-linux] " Doug Anderson @ 2015-12-09 21:32 ` Julius Werner -1 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-09 21:32 UTC (permalink / raw) To: Doug Anderson Cc: Julius Werner, Alexandre Belloni, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux > Thinking about all this: these's actually a totally different > alternative approach we could take if you wanted. It would fix S5 and > avoid all the anchor stuff, unless I'm crazy. > > Basically totally give up on the RTC time reflecting reality. Add a > "real time to rk808" and "rk808 time to real time" function. Always > use it when reading from rk808 and always use it when writing to > rk808. Choose 2015 as the "truth" year if you want (or pick another > year). In early 2016 the rk808 always contains 1 day back. In 2017 > the rk808 always contains 2 days back. Etc, etc, etc. > > The firmware would get confused, but ... Well... other than that it's crazy and that I'd have to rewrite the whole patch again, I can't come up with a good argument against this. In Chromebook firmware the time is only needed for a debug log, so we'd probably just be willing to accept it being wrong. If U-Boot ever gets RK808 support, they'll probably just copy the Linux driver wholesale anyway so they'll conform to the same system. So if nobody else raises fundamental objections to this approach, I guess I'll get started on another patch version. (Further replies below for reference, but most of that stuff would then become moot.) >> + struct rtc_time anchor_time; /* Last sync point with real world */ > > Is this ever Nov 31st? Looks like it never is... No, it's always an already corrected date. > Review will be significantly easier if we break this up into two patches: > > 1. Brain-dead code movement. Move rk808_rtc_readtime() and > rk808_rtc_set_time() with no functional changes. > > 2. This actual change. Okay, I'll do that for the next rewrite. >> + extra_days = nov31st_transitions(&rk808_rtc->anchor_time, &alrm->time); >> + if (alrm->time.tm_mon == 10 && alrm->time.tm_mday == 31) { >> + dev_warn(dev, "read HW alarm date as Nov 31, compensating\n"); >> + alrm->time.tm_mon = 11; >> + alrm->time.tm_mday = 1 + extra_days; > > But what is it's been 30+ years?!?! :-P Not that this is seriously a > problem, but you already handle this in the "else" case. Why not just > set to 11/1 and get rid of the "else" below, so: > > ... > alrm->time.tm_mon = 11; > alrm->time.tm_mday = 1; > } > if (extra_days) { > ... Good point, that makes more sense. >> + extra_days = nov31st_transitions(&rk808_rtc->anchor_time, &alrm->time); >> + if (extra_days) { >> + unsigned long time; >> + dev_warn(dev, "writing HW alarm date adjusted for %d Nov31\n", >> + extra_days); >> + rtc_tm_to_time(&alrm->time, &time); >> + rtc_time_to_tm(time - extra_days * 86400, &alrm->time); >> + /* Compensate in case the subtraction went back over Nov 31st */ >> + if (alrm->time.tm_mon == 10 && >> + alrm->time.tm_mday == 31 - extra_days) >> + alrm->time.tm_mday++; /* This can result in 31! */ > > Seems fishy somehow. You should be able to come up with scenarios > where you need an alarm on 11/31 on each future year. Here I think > you can only possibly get 11/31 if extra days == 1, right? > > So if it is Nov 30, 2015 and we want alarm on Dec 6, 2020 > extra days = 6: > Nov 31, 2015 > Nov 31, 2016 > Nov 31, 2017 > Nov 31, 2018 > Nov 31, 2019 > Nov 31, 2020 > > I think we need to set the alarm for Nov 31st 2020, right? Your > subtraction should get you to Nov 30th and you need to add an extra > day for that, but I don't _think_ your if test will. Did I mess up? > As I said, I'm bad at this... Hmm... right, I didn't think far enough here. I think I only considered "what if it's Dec 1st in year X", but of course Dec 2nd and so on may have the same issues. I think I really just need to change that condition to (alrm->time.tm_mday >= 31 - extra_days) to get what we need (at least up to 30 years in the future, after that it reaches into October). >> + /* >> + * Try to initialize anchor point by reading "last read" shadow >> + * timestamp, to catch Nov 31st transitions that happened while shut >> + * down. This only works if no other code (e.g. firmware) has >> + * transitioned GET_TIME before this point. >> + */ >> + ret = rk808_rtc_raw_read(&pdev->dev, &rk808_rtc->anchor_time); >> + if (ret || rtc_valid_tm(&rk808_rtc->anchor_time)) >> + rk808_rtc->anchor_time.tm_mday = 0; /* invalidate */ > > Ah, the rtc_valid_tm() keeps the anchor from every being Nov 31st. Got it. At this point we cannot really get Nov 31st (in a way we'd want), because we want the last time that was read by this driver before shutdown. If that time was Nov 31st, it would have been immediately corrected by the driver code (although, now that I think of it, there is no further read() after that so the set() might not actually update the shadow registers). So if this is Nov 31st it means that the RTC was last read by some non-aware software and therefore there's no use trying to recover the anchor date from it. ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH v2] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-09 21:32 ` Julius Werner 0 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-09 21:32 UTC (permalink / raw) To: Doug Anderson Cc: Julius Werner, Alexandre Belloni, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux > Thinking about all this: these's actually a totally different > alternative approach we could take if you wanted. It would fix S5 and > avoid all the anchor stuff, unless I'm crazy. > > Basically totally give up on the RTC time reflecting reality. Add a > "real time to rk808" and "rk808 time to real time" function. Always > use it when reading from rk808 and always use it when writing to > rk808. Choose 2015 as the "truth" year if you want (or pick another > year). In early 2016 the rk808 always contains 1 day back. In 2017 > the rk808 always contains 2 days back. Etc, etc, etc. > > The firmware would get confused, but ... Well... other than that it's crazy and that I'd have to rewrite the whole patch again, I can't come up with a good argument against this. In Chromebook firmware the time is only needed for a debug log, so we'd probably just be willing to accept it being wrong. If U-Boot ever gets RK808 support, they'll probably just copy the Linux driver wholesale anyway so they'll conform to the same system. So if nobody else raises fundamental objections to this approach, I guess I'll get started on another patch version. (Further replies below for reference, but most of that stuff would then become moot.) >> + struct rtc_time anchor_time; /* Last sync point with real world */ > > Is this ever Nov 31st? Looks like it never is... No, it's always an already corrected date. > Review will be significantly easier if we break this up into two patches: > > 1. Brain-dead code movement. Move rk808_rtc_readtime() and > rk808_rtc_set_time() with no functional changes. > > 2. This actual change. Okay, I'll do that for the next rewrite. >> + extra_days = nov31st_transitions(&rk808_rtc->anchor_time, &alrm->time); >> + if (alrm->time.tm_mon == 10 && alrm->time.tm_mday == 31) { >> + dev_warn(dev, "read HW alarm date as Nov 31, compensating\n"); >> + alrm->time.tm_mon = 11; >> + alrm->time.tm_mday = 1 + extra_days; > > But what is it's been 30+ years?!?! :-P Not that this is seriously a > problem, but you already handle this in the "else" case. Why not just > set to 11/1 and get rid of the "else" below, so: > > ... > alrm->time.tm_mon = 11; > alrm->time.tm_mday = 1; > } > if (extra_days) { > ... Good point, that makes more sense. >> + extra_days = nov31st_transitions(&rk808_rtc->anchor_time, &alrm->time); >> + if (extra_days) { >> + unsigned long time; >> + dev_warn(dev, "writing HW alarm date adjusted for %d Nov31\n", >> + extra_days); >> + rtc_tm_to_time(&alrm->time, &time); >> + rtc_time_to_tm(time - extra_days * 86400, &alrm->time); >> + /* Compensate in case the subtraction went back over Nov 31st */ >> + if (alrm->time.tm_mon == 10 && >> + alrm->time.tm_mday == 31 - extra_days) >> + alrm->time.tm_mday++; /* This can result in 31! */ > > Seems fishy somehow. You should be able to come up with scenarios > where you need an alarm on 11/31 on each future year. Here I think > you can only possibly get 11/31 if extra days == 1, right? > > So if it is Nov 30, 2015 and we want alarm on Dec 6, 2020 > extra days = 6: > Nov 31, 2015 > Nov 31, 2016 > Nov 31, 2017 > Nov 31, 2018 > Nov 31, 2019 > Nov 31, 2020 > > I think we need to set the alarm for Nov 31st 2020, right? Your > subtraction should get you to Nov 30th and you need to add an extra > day for that, but I don't _think_ your if test will. Did I mess up? > As I said, I'm bad at this... Hmm... right, I didn't think far enough here. I think I only considered "what if it's Dec 1st in year X", but of course Dec 2nd and so on may have the same issues. I think I really just need to change that condition to (alrm->time.tm_mday >= 31 - extra_days) to get what we need (at least up to 30 years in the future, after that it reaches into October). >> + /* >> + * Try to initialize anchor point by reading "last read" shadow >> + * timestamp, to catch Nov 31st transitions that happened while shut >> + * down. This only works if no other code (e.g. firmware) has >> + * transitioned GET_TIME before this point. >> + */ >> + ret = rk808_rtc_raw_read(&pdev->dev, &rk808_rtc->anchor_time); >> + if (ret || rtc_valid_tm(&rk808_rtc->anchor_time)) >> + rk808_rtc->anchor_time.tm_mday = 0; /* invalidate */ > > Ah, the rtc_valid_tm() keeps the anchor from every being Nov 31st. Got it. At this point we cannot really get Nov 31st (in a way we'd want), because we want the last time that was read by this driver before shutdown. If that time was Nov 31st, it would have been immediately corrected by the driver code (although, now that I think of it, there is no further read() after that so the set() might not actually update the shadow registers). So if this is Nov 31st it means that the RTC was last read by some non-aware software and therefore there's no use trying to recover the anchor date from it. -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2] RTC: RK808: Work around hardware bug on November 31st 2015-12-09 21:32 ` [rtc-linux] " Julius Werner @ 2015-12-10 18:41 ` Alexandre Belloni -1 siblings, 0 replies; 62+ messages in thread From: Alexandre Belloni @ 2015-12-10 18:41 UTC (permalink / raw) To: Julius Werner Cc: Doug Anderson, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Hi Julius, Doug, On 09/12/2015 at 13:32:52 -0800, Julius Werner wrote : > > Thinking about all this: these's actually a totally different > > alternative approach we could take if you wanted. It would fix S5 and > > avoid all the anchor stuff, unless I'm crazy. > > > > Basically totally give up on the RTC time reflecting reality. Add a > > "real time to rk808" and "rk808 time to real time" function. Always > > use it when reading from rk808 and always use it when writing to > > rk808. Choose 2015 as the "truth" year if you want (or pick another > > year). In early 2016 the rk808 always contains 1 day back. In 2017 > > the rk808 always contains 2 days back. Etc, etc, etc. > > > > The firmware would get confused, but ... > > Well... other than that it's crazy and that I'd have to rewrite the > whole patch again, I can't come up with a good argument against this. > In Chromebook firmware the time is only needed for a debug log, so > we'd probably just be willing to accept it being wrong. If U-Boot ever > gets RK808 support, they'll probably just copy the Linux driver > wholesale anyway so they'll conform to the same system. > > So if nobody else raises fundamental objections to this approach, I > guess I'll get started on another patch version. (Further replies > below for reference, but most of that stuff would then become moot.) > I'll try to review and evaluate both solution by the end of the week (no guarantee though). -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH v2] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-10 18:41 ` Alexandre Belloni 0 siblings, 0 replies; 62+ messages in thread From: Alexandre Belloni @ 2015-12-10 18:41 UTC (permalink / raw) To: Julius Werner Cc: Doug Anderson, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Hi Julius, Doug, On 09/12/2015 at 13:32:52 -0800, Julius Werner wrote : > > Thinking about all this: these's actually a totally different > > alternative approach we could take if you wanted. It would fix S5 and > > avoid all the anchor stuff, unless I'm crazy. > > > > Basically totally give up on the RTC time reflecting reality. Add a > > "real time to rk808" and "rk808 time to real time" function. Always > > use it when reading from rk808 and always use it when writing to > > rk808. Choose 2015 as the "truth" year if you want (or pick another > > year). In early 2016 the rk808 always contains 1 day back. In 2017 > > the rk808 always contains 2 days back. Etc, etc, etc. > > > > The firmware would get confused, but ... > > Well... other than that it's crazy and that I'd have to rewrite the > whole patch again, I can't come up with a good argument against this. > In Chromebook firmware the time is only needed for a debug log, so > we'd probably just be willing to accept it being wrong. If U-Boot ever > gets RK808 support, they'll probably just copy the Linux driver > wholesale anyway so they'll conform to the same system. > > So if nobody else raises fundamental objections to this approach, I > guess I'll get started on another patch version. (Further replies > below for reference, but most of that stuff would then become moot.) > I'll try to review and evaluate both solution by the end of the week (no guarantee though). -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCH v2] RTC: RK808: Work around hardware bug on November 31st 2015-12-10 18:41 ` [rtc-linux] " Alexandre Belloni @ 2015-12-10 18:57 ` Julius Werner -1 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-10 18:57 UTC (permalink / raw) To: Alexandre Belloni Cc: Julius Werner, Doug Anderson, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux > I'll try to review and evaluate both solution by the end of the week (no > guarantee though). To summarize, it's a pretty simple trade-off. Do you: a) try to detect every time the RTC deviated from the real-world time and correct it instantly? This can be done most of the time but there are edge cases (when the system is completely powered off while the RTC ticks over Nov 31st) where you cannot detect it. Or, b) just completely let the RTC tick in its own world where every year has an extra day and convert that "calendar" to/from the real-world calendar on every access. This essentially relegates the RTC to a "dumb second counter" and the timestamp in its registers doesn't have any direct connection to the real world time anymore (unless you know the conversion algorithm). This has the nice advantage that it perfectly solves all use cases (time-keeping, accurate alarms, even when shut down or browning out at inopportune times)... the only real disadvantage is that other software (e.g. U-Boot) reading the same RTC must know about this and use the same conversion code to get the correct real-world time from it. ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCH v2] RTC: RK808: Work around hardware bug on November 31st @ 2015-12-10 18:57 ` Julius Werner 0 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-10 18:57 UTC (permalink / raw) To: Alexandre Belloni Cc: Julius Werner, Doug Anderson, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux > I'll try to review and evaluate both solution by the end of the week (no > guarantee though). To summarize, it's a pretty simple trade-off. Do you: a) try to detect every time the RTC deviated from the real-world time and correct it instantly? This can be done most of the time but there are edge cases (when the system is completely powered off while the RTC ticks over Nov 31st) where you cannot detect it. Or, b) just completely let the RTC tick in its own world where every year has an extra day and convert that "calendar" to/from the real-world calendar on every access. This essentially relegates the RTC to a "dumb second counter" and the timestamp in its registers doesn't have any direct connection to the real world time anymore (unless you know the conversion algorithm). This has the nice advantage that it perfectly solves all use cases (time-keeping, accurate alarms, even when shut down or browning out at inopportune times)... the only real disadvantage is that other software (e.g. U-Boot) reading the same RTC must know about this and use the same conversion code to get the correct real-world time from it. -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st 2015-12-10 18:57 ` [rtc-linux] " Julius Werner @ 2015-12-15 23:02 ` Julius Werner -1 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-15 23:02 UTC (permalink / raw) To: Alexandre Belloni Cc: Andrew Morton, Alessandro Zummo, Doug Anderson, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux, Julius Werner In A.D. 1582 Pope Gregory XIII found that the existing Julian calendar insufficiently represented reality, and changed the rules about calculating leap years to account for this. Similarly, in A.D. 2013 Rockchip hardware engineers found that the new Gregorian calendar still contained flaws, and that the month of November should be counted up to 31 days instead. Unfortunately it takes a long time for calendar changes to gain widespread adoption, and just like more than 300 years went by before the last Protestant nation implemented Greg's proposal, we will have to wait a while until all religions and operating system kernels acknowledge the inherent advantages of the Rockchip system. Until then we need to translate dates read from (and written to) Rockchip hardware back to the Gregorian format. This patch works by defining Jan 1st, 2016 as the arbitrary anchor date on which Rockchip and Gregorian calendars are in sync. From that we can translate arbitrary later dates back and forth by counting the number of November/December transitons since the anchor date to determine the offset between the calendars. We choose this method (rather than trying to regularly "correct" the date stored in hardware) since it's the only way to ensure perfect time-keeping even if the system may be shut down for an unknown number of years. The drawback is that other software reading the same hardware (e.g. mainboard firmware) must use the same translation convention (including the same anchor date) to be able to read and write correct timestamps from/to the RTC. Signed-off-by: Julius Werner <jwerner@chromium.org> --- drivers/rtc/rtc-rk808.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/drivers/rtc/rtc-rk808.c b/drivers/rtc/rtc-rk808.c index 91ca0bc..35c9aad 100644 --- a/drivers/rtc/rtc-rk808.c +++ b/drivers/rtc/rtc-rk808.c @@ -56,6 +56,42 @@ struct rk808_rtc { int irq; }; +/* + * The Rockchip calendar used by the RK808 counts November with 31 days. We use + * these translation functions to convert its dates to/from the Gregorian + * calendar used by the rest of the world. We arbitrarily define Jan 1st, 2016 + * as the day when both calendars were in sync, and treat all other dates + * relative to that. + * NOTE: Other system software (e.g. firmware) that reads the same hardware must + * implement this exact same conversion algorithm, with the same anchor date. + */ +static time64_t nov2dec_transitions(struct rtc_time *tm) +{ + return (tm->tm_year + 1900) - 2016 + (tm->tm_mon + 1 > 11 ? 1 : 0); +} + +static void rockchip_to_gregorian(struct rtc_time *tm) +{ + /* If it's Nov 31st, rtc_tm_to_time64() will count that like Dec 1st */ + time64_t time = rtc_tm_to_time64(tm); + rtc_time64_to_tm(time + nov2dec_transitions(tm) * 86400, tm); +} + +static void gregorian_to_rockchip(struct rtc_time *tm) +{ + time64_t extra_days = nov2dec_transitions(tm); + time64_t time = rtc_tm_to_time64(tm); + rtc_time64_to_tm(time - extra_days * 86400, tm); + + /* Compensate if we went back over Nov 31st (will work up to 2381) */ + if (nov2dec_transitions(tm) < extra_days) { + if (tm->tm_mon + 1 == 11) + tm->tm_mday++; /* This may result in 31! */ + else + rtc_time64_to_tm(time - (extra_days - 1) * 86400, tm); + } +} + /* Read current time and date in RTC */ static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) { @@ -101,9 +137,10 @@ static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) tm->tm_mon = (bcd2bin(rtc_data[4] & MONTHS_REG_MSK)) - 1; tm->tm_year = (bcd2bin(rtc_data[5] & YEARS_REG_MSK)) + 100; tm->tm_wday = bcd2bin(rtc_data[6] & WEEKS_REG_MSK); + rockchip_to_gregorian(tm); dev_dbg(dev, "RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); + tm->tm_wday, tm->tm_hour, tm->tm_min, tm->tm_sec); return ret; } @@ -116,6 +153,10 @@ static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) u8 rtc_data[NUM_TIME_REGS]; int ret; + dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", + 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, + tm->tm_wday, tm->tm_hour, tm->tm_min, tm->tm_sec); + gregorian_to_rockchip(tm); rtc_data[0] = bin2bcd(tm->tm_sec); rtc_data[1] = bin2bcd(tm->tm_min); rtc_data[2] = bin2bcd(tm->tm_hour); @@ -123,9 +164,6 @@ static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) rtc_data[4] = bin2bcd(tm->tm_mon + 1); rtc_data[5] = bin2bcd(tm->tm_year - 100); rtc_data[6] = bin2bcd(tm->tm_wday); - dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", - 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); /* Stop RTC while updating the RTC registers */ ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, @@ -170,6 +208,7 @@ static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm) alrm->time.tm_mday = bcd2bin(alrm_data[3] & DAYS_REG_MSK); alrm->time.tm_mon = (bcd2bin(alrm_data[4] & MONTHS_REG_MSK)) - 1; alrm->time.tm_year = (bcd2bin(alrm_data[5] & YEARS_REG_MSK)) + 100; + rockchip_to_gregorian(&alrm->time); ret = regmap_read(rk808->regmap, RK808_RTC_INT_REG, &int_reg); if (ret) { @@ -227,6 +266,7 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm) alrm->time.tm_mday, alrm->time.tm_wday, alrm->time.tm_hour, alrm->time.tm_min, alrm->time.tm_sec); + gregorian_to_rockchip(&alrm->time); alrm_data[0] = bin2bcd(alrm->time.tm_sec); alrm_data[1] = bin2bcd(alrm->time.tm_min); alrm_data[2] = bin2bcd(alrm->time.tm_hour); -- 2.1.2 ^ permalink raw reply related [flat|nested] 62+ messages in thread
* [rtc-linux] [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st @ 2015-12-15 23:02 ` Julius Werner 0 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-15 23:02 UTC (permalink / raw) To: Alexandre Belloni Cc: Andrew Morton, Alessandro Zummo, Doug Anderson, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux, Julius Werner In A.D. 1582 Pope Gregory XIII found that the existing Julian calendar insufficiently represented reality, and changed the rules about calculating leap years to account for this. Similarly, in A.D. 2013 Rockchip hardware engineers found that the new Gregorian calendar still contained flaws, and that the month of November should be counted up to 31 days instead. Unfortunately it takes a long time for calendar changes to gain widespread adoption, and just like more than 300 years went by before the last Protestant nation implemented Greg's proposal, we will have to wait a while until all religions and operating system kernels acknowledge the inherent advantages of the Rockchip system. Until then we need to translate dates read from (and written to) Rockchip hardware back to the Gregorian format. This patch works by defining Jan 1st, 2016 as the arbitrary anchor date on which Rockchip and Gregorian calendars are in sync. From that we can translate arbitrary later dates back and forth by counting the number of November/December transitons since the anchor date to determine the offset between the calendars. We choose this method (rather than trying to regularly "correct" the date stored in hardware) since it's the only way to ensure perfect time-keeping even if the system may be shut down for an unknown number of years. The drawback is that other software reading the same hardware (e.g. mainboard firmware) must use the same translation convention (including the same anchor date) to be able to read and write correct timestamps from/to the RTC. Signed-off-by: Julius Werner <jwerner@chromium.org> --- drivers/rtc/rtc-rk808.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/drivers/rtc/rtc-rk808.c b/drivers/rtc/rtc-rk808.c index 91ca0bc..35c9aad 100644 --- a/drivers/rtc/rtc-rk808.c +++ b/drivers/rtc/rtc-rk808.c @@ -56,6 +56,42 @@ struct rk808_rtc { int irq; }; +/* + * The Rockchip calendar used by the RK808 counts November with 31 days. We use + * these translation functions to convert its dates to/from the Gregorian + * calendar used by the rest of the world. We arbitrarily define Jan 1st, 2016 + * as the day when both calendars were in sync, and treat all other dates + * relative to that. + * NOTE: Other system software (e.g. firmware) that reads the same hardware must + * implement this exact same conversion algorithm, with the same anchor date. + */ +static time64_t nov2dec_transitions(struct rtc_time *tm) +{ + return (tm->tm_year + 1900) - 2016 + (tm->tm_mon + 1 > 11 ? 1 : 0); +} + +static void rockchip_to_gregorian(struct rtc_time *tm) +{ + /* If it's Nov 31st, rtc_tm_to_time64() will count that like Dec 1st */ + time64_t time = rtc_tm_to_time64(tm); + rtc_time64_to_tm(time + nov2dec_transitions(tm) * 86400, tm); +} + +static void gregorian_to_rockchip(struct rtc_time *tm) +{ + time64_t extra_days = nov2dec_transitions(tm); + time64_t time = rtc_tm_to_time64(tm); + rtc_time64_to_tm(time - extra_days * 86400, tm); + + /* Compensate if we went back over Nov 31st (will work up to 2381) */ + if (nov2dec_transitions(tm) < extra_days) { + if (tm->tm_mon + 1 == 11) + tm->tm_mday++; /* This may result in 31! */ + else + rtc_time64_to_tm(time - (extra_days - 1) * 86400, tm); + } +} + /* Read current time and date in RTC */ static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) { @@ -101,9 +137,10 @@ static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) tm->tm_mon = (bcd2bin(rtc_data[4] & MONTHS_REG_MSK)) - 1; tm->tm_year = (bcd2bin(rtc_data[5] & YEARS_REG_MSK)) + 100; tm->tm_wday = bcd2bin(rtc_data[6] & WEEKS_REG_MSK); + rockchip_to_gregorian(tm); dev_dbg(dev, "RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); + tm->tm_wday, tm->tm_hour, tm->tm_min, tm->tm_sec); return ret; } @@ -116,6 +153,10 @@ static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) u8 rtc_data[NUM_TIME_REGS]; int ret; + dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", + 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, + tm->tm_wday, tm->tm_hour, tm->tm_min, tm->tm_sec); + gregorian_to_rockchip(tm); rtc_data[0] = bin2bcd(tm->tm_sec); rtc_data[1] = bin2bcd(tm->tm_min); rtc_data[2] = bin2bcd(tm->tm_hour); @@ -123,9 +164,6 @@ static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) rtc_data[4] = bin2bcd(tm->tm_mon + 1); rtc_data[5] = bin2bcd(tm->tm_year - 100); rtc_data[6] = bin2bcd(tm->tm_wday); - dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", - 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); /* Stop RTC while updating the RTC registers */ ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, @@ -170,6 +208,7 @@ static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm) alrm->time.tm_mday = bcd2bin(alrm_data[3] & DAYS_REG_MSK); alrm->time.tm_mon = (bcd2bin(alrm_data[4] & MONTHS_REG_MSK)) - 1; alrm->time.tm_year = (bcd2bin(alrm_data[5] & YEARS_REG_MSK)) + 100; + rockchip_to_gregorian(&alrm->time); ret = regmap_read(rk808->regmap, RK808_RTC_INT_REG, &int_reg); if (ret) { @@ -227,6 +266,7 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm) alrm->time.tm_mday, alrm->time.tm_wday, alrm->time.tm_hour, alrm->time.tm_min, alrm->time.tm_sec); + gregorian_to_rockchip(&alrm->time); alrm_data[0] = bin2bcd(alrm->time.tm_sec); alrm_data[1] = bin2bcd(alrm->time.tm_min); alrm_data[2] = bin2bcd(alrm->time.tm_hour); -- 2.1.2 -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply related [flat|nested] 62+ messages in thread
* Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st 2015-12-15 23:02 ` [rtc-linux] " Julius Werner @ 2015-12-15 23:14 ` Julius Werner -1 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-15 23:14 UTC (permalink / raw) To: Julius Werner Cc: Alexandre Belloni, Andrew Morton, Alessandro Zummo, Doug Anderson, Sonny Rao, Chris Zhong, Heiko Stuebner, LKML, rtc-linux Okay, wrote up and tested the anchor date version. I think once you get over the initial weirdness of the approach this one is really much cleaner and safer. I tested this with the older rtc_tm_to_time() API and only ported it over to rtc_tm_to_time64() for submission, since my 3.14 kernel didn't have that yet... but it still compiles fine and the change was very trivial so I'm confident that it should work. I also did a big manual test for my conversion functions where I just threw a whole bunch of dates at them, results below for reference: [ 1.431216] jwerner: Testing translation functions: [ 1.431221] 2015-01-01 to_rockchip: 2015-01-02 to_gregorian: 2014-12-31 [ 1.431224] 2015-10-30 to_rockchip: 2015-10-31 to_gregorian: 2015-10-29 [ 1.431228] 2015-10-31 to_rockchip: 2015-11-01 to_gregorian: 2015-10-30 [ 1.431231] 2015-11-01 to_rockchip: 2015-11-02 to_gregorian: 2015-10-31 [ 1.431235] 2015-11-27 to_rockchip: 2015-11-28 to_gregorian: 2015-11-26 [ 1.431238] 2015-11-28 to_rockchip: 2015-11-29 to_gregorian: 2015-11-27 [ 1.431242] 2015-11-29 to_rockchip: 2015-11-30 to_gregorian: 2015-11-28 [ 1.431245] 2015-11-30 to_rockchip: 2015-12-01 to_gregorian: 2015-11-29 This one is actually a bug... to_rockchip should be 2015-11-31 here. It happens because the "compensate if we went back over" part of gregorian_to_rockchip() only checks whether we went over *backwards*, which happens if the date is after the anchor date. If it was before we can go back over forwards and I didn't bother to handle that case. I think this is fine since all affected dates lie in the past and there's no real-world use case where you'd ever need them to work again. [ 1.431249] 2015-11-31 to_rockchip: 2015-12-02 to_gregorian: 2015-11-30 [ 1.431252] 2015-12-01 to_rockchip: 2015-12-01 to_gregorian: 2015-12-01 [ 1.431256] 2015-12-02 to_rockchip: 2015-12-02 to_gregorian: 2015-12-02 [ 1.431259] 2015-12-03 to_rockchip: 2015-12-03 to_gregorian: 2015-12-03 [ 1.431262] 2015-12-04 to_rockchip: 2015-12-04 to_gregorian: 2015-12-04 [ 1.431266] 2015-12-05 to_rockchip: 2015-12-05 to_gregorian: 2015-12-05 [ 1.431269] 2015-12-30 to_rockchip: 2015-12-30 to_gregorian: 2015-12-30 [ 1.431273] 2015-12-31 to_rockchip: 2015-12-31 to_gregorian: 2015-12-31 [ 1.431276] 2016-01-01 to_rockchip: 2016-01-01 to_gregorian: 2016-01-01 [ 1.431279] 2016-10-30 to_rockchip: 2016-10-30 to_gregorian: 2016-10-30 [ 1.431283] 2016-10-31 to_rockchip: 2016-10-31 to_gregorian: 2016-10-31 [ 1.431287] 2016-11-01 to_rockchip: 2016-11-01 to_gregorian: 2016-11-01 [ 1.431291] 2016-11-27 to_rockchip: 2016-11-27 to_gregorian: 2016-11-27 [ 1.431295] 2016-11-28 to_rockchip: 2016-11-28 to_gregorian: 2016-11-28 [ 1.431299] 2016-11-29 to_rockchip: 2016-11-29 to_gregorian: 2016-11-29 [ 1.431302] 2016-11-30 to_rockchip: 2016-11-30 to_gregorian: 2016-11-30 [ 1.431306] 2016-11-31 to_rockchip: 2016-12-01 to_gregorian: 2016-12-01 [ 1.431310] 2016-12-01 to_rockchip: 2016-11-31 to_gregorian: 2016-12-02 [ 1.431313] 2016-12-02 to_rockchip: 2016-12-01 to_gregorian: 2016-12-03 [ 1.431317] 2016-12-03 to_rockchip: 2016-12-02 to_gregorian: 2016-12-04 [ 1.431321] 2016-12-04 to_rockchip: 2016-12-03 to_gregorian: 2016-12-05 [ 1.431324] 2016-12-05 to_rockchip: 2016-12-04 to_gregorian: 2016-12-06 [ 1.431328] 2016-12-30 to_rockchip: 2016-12-29 to_gregorian: 2016-12-31 [ 1.431332] 2016-12-31 to_rockchip: 2016-12-30 to_gregorian: 2017-01-01 [ 1.431335] 2017-01-01 to_rockchip: 2016-12-31 to_gregorian: 2017-01-02 [ 1.431338] 2017-10-30 to_rockchip: 2017-10-29 to_gregorian: 2017-10-31 [ 1.431342] 2017-10-31 to_rockchip: 2017-10-30 to_gregorian: 2017-11-01 [ 1.431345] 2017-11-01 to_rockchip: 2017-10-31 to_gregorian: 2017-11-02 [ 1.431349] 2017-11-27 to_rockchip: 2017-11-26 to_gregorian: 2017-11-28 [ 1.431352] 2017-11-28 to_rockchip: 2017-11-27 to_gregorian: 2017-11-29 [ 1.431356] 2017-11-29 to_rockchip: 2017-11-28 to_gregorian: 2017-11-30 [ 1.431359] 2017-11-30 to_rockchip: 2017-11-29 to_gregorian: 2017-12-01 [ 1.431363] 2017-11-31 to_rockchip: 2017-11-30 to_gregorian: 2017-12-02 [ 1.431366] 2017-12-01 to_rockchip: 2017-11-30 to_gregorian: 2017-12-03 [ 1.431369] 2017-12-02 to_rockchip: 2017-11-31 to_gregorian: 2017-12-04 [ 1.431373] 2017-12-03 to_rockchip: 2017-12-01 to_gregorian: 2017-12-05 [ 1.431376] 2017-12-04 to_rockchip: 2017-12-02 to_gregorian: 2017-12-06 [ 1.431380] 2017-12-05 to_rockchip: 2017-12-03 to_gregorian: 2017-12-07 [ 1.431383] 2017-12-30 to_rockchip: 2017-12-28 to_gregorian: 2018-01-01 [ 1.431386] 2017-12-31 to_rockchip: 2017-12-29 to_gregorian: 2018-01-02 [ 1.431389] 2020-01-01 to_rockchip: 2019-12-28 to_gregorian: 2020-01-05 [ 1.431393] 2020-10-30 to_rockchip: 2020-10-26 to_gregorian: 2020-11-03 [ 1.431397] 2020-10-31 to_rockchip: 2020-10-27 to_gregorian: 2020-11-04 [ 1.431400] 2020-11-01 to_rockchip: 2020-10-28 to_gregorian: 2020-11-05 [ 1.431404] 2020-11-27 to_rockchip: 2020-11-23 to_gregorian: 2020-12-01 [ 1.431408] 2020-11-28 to_rockchip: 2020-11-24 to_gregorian: 2020-12-02 [ 1.431411] 2020-11-29 to_rockchip: 2020-11-25 to_gregorian: 2020-12-03 [ 1.431415] 2020-11-30 to_rockchip: 2020-11-26 to_gregorian: 2020-12-04 [ 1.431419] 2020-11-31 to_rockchip: 2020-11-27 to_gregorian: 2020-12-05 [ 1.431422] 2020-12-01 to_rockchip: 2020-11-27 to_gregorian: 2020-12-06 [ 1.431426] 2020-12-02 to_rockchip: 2020-11-28 to_gregorian: 2020-12-07 [ 1.431430] 2020-12-03 to_rockchip: 2020-11-29 to_gregorian: 2020-12-08 [ 1.431434] 2020-12-04 to_rockchip: 2020-11-30 to_gregorian: 2020-12-09 [ 1.431437] 2020-12-05 to_rockchip: 2020-11-31 to_gregorian: 2020-12-10 [ 1.431441] 2020-12-30 to_rockchip: 2020-12-25 to_gregorian: 2021-01-04 [ 1.431444] 2020-12-31 to_rockchip: 2020-12-26 to_gregorian: 2021-01-05 [ 1.431447] 2030-01-01 to_rockchip: 2029-12-18 to_gregorian: 2030-01-15 [ 1.431450] 2030-10-30 to_rockchip: 2030-10-16 to_gregorian: 2030-11-13 [ 1.431454] 2030-10-31 to_rockchip: 2030-10-17 to_gregorian: 2030-11-14 [ 1.431457] 2030-11-01 to_rockchip: 2030-10-18 to_gregorian: 2030-11-15 [ 1.431461] 2030-11-27 to_rockchip: 2030-11-13 to_gregorian: 2030-12-11 [ 1.431464] 2030-11-28 to_rockchip: 2030-11-14 to_gregorian: 2030-12-12 [ 1.431468] 2030-11-29 to_rockchip: 2030-11-15 to_gregorian: 2030-12-13 [ 1.431471] 2030-11-30 to_rockchip: 2030-11-16 to_gregorian: 2030-12-14 [ 1.431474] 2030-11-31 to_rockchip: 2030-11-17 to_gregorian: 2030-12-15 [ 1.431478] 2030-12-01 to_rockchip: 2030-11-17 to_gregorian: 2030-12-16 [ 1.431481] 2030-12-02 to_rockchip: 2030-11-18 to_gregorian: 2030-12-17 [ 1.431485] 2030-12-03 to_rockchip: 2030-11-19 to_gregorian: 2030-12-18 [ 1.431488] 2030-12-04 to_rockchip: 2030-11-20 to_gregorian: 2030-12-19 [ 1.431492] 2030-12-05 to_rockchip: 2030-11-21 to_gregorian: 2030-12-20 [ 1.431495] 2030-12-30 to_rockchip: 2030-12-15 to_gregorian: 2031-01-14 [ 1.431498] 2030-12-31 to_rockchip: 2030-12-16 to_gregorian: 2031-01-15 [ 1.431501] 2060-01-01 to_rockchip: 2059-11-19 to_gregorian: 2060-02-14 [ 1.431505] 2060-10-30 to_rockchip: 2060-09-16 to_gregorian: 2060-12-13 [ 1.431509] 2060-10-31 to_rockchip: 2060-09-17 to_gregorian: 2060-12-14 [ 1.431512] 2060-11-01 to_rockchip: 2060-09-18 to_gregorian: 2060-12-15 [ 1.431516] 2060-11-27 to_rockchip: 2060-10-14 to_gregorian: 2061-01-10 [ 1.431519] 2060-11-28 to_rockchip: 2060-10-15 to_gregorian: 2061-01-11 [ 1.431522] 2060-11-29 to_rockchip: 2060-10-16 to_gregorian: 2061-01-12 [ 1.431525] 2060-11-30 to_rockchip: 2060-10-17 to_gregorian: 2061-01-13 [ 1.431528] 2060-11-31 to_rockchip: 2060-10-18 to_gregorian: 2061-01-14 [ 1.431532] 2060-12-01 to_rockchip: 2060-10-18 to_gregorian: 2061-01-15 [ 1.431536] 2060-12-02 to_rockchip: 2060-10-19 to_gregorian: 2061-01-16 [ 1.431540] 2060-12-03 to_rockchip: 2060-10-20 to_gregorian: 2061-01-17 [ 1.431544] 2060-12-04 to_rockchip: 2060-10-21 to_gregorian: 2061-01-18 [ 1.431548] 2060-12-05 to_rockchip: 2060-10-22 to_gregorian: 2061-01-19 [ 1.431551] 2060-12-30 to_rockchip: 2060-11-16 to_gregorian: 2061-02-13 [ 1.431554] 2060-12-31 to_rockchip: 2060-11-17 to_gregorian: 2061-02-14 ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st @ 2015-12-15 23:14 ` Julius Werner 0 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-15 23:14 UTC (permalink / raw) To: Julius Werner Cc: Alexandre Belloni, Andrew Morton, Alessandro Zummo, Doug Anderson, Sonny Rao, Chris Zhong, Heiko Stuebner, LKML, rtc-linux Okay, wrote up and tested the anchor date version. I think once you get over the initial weirdness of the approach this one is really much cleaner and safer. I tested this with the older rtc_tm_to_time() API and only ported it over to rtc_tm_to_time64() for submission, since my 3.14 kernel didn't have that yet... but it still compiles fine and the change was very trivial so I'm confident that it should work. I also did a big manual test for my conversion functions where I just threw a whole bunch of dates at them, results below for reference: [ 1.431216] jwerner: Testing translation functions: [ 1.431221] 2015-01-01 to_rockchip: 2015-01-02 to_gregorian: 2014-12-31 [ 1.431224] 2015-10-30 to_rockchip: 2015-10-31 to_gregorian: 2015-10-29 [ 1.431228] 2015-10-31 to_rockchip: 2015-11-01 to_gregorian: 2015-10-30 [ 1.431231] 2015-11-01 to_rockchip: 2015-11-02 to_gregorian: 2015-10-31 [ 1.431235] 2015-11-27 to_rockchip: 2015-11-28 to_gregorian: 2015-11-26 [ 1.431238] 2015-11-28 to_rockchip: 2015-11-29 to_gregorian: 2015-11-27 [ 1.431242] 2015-11-29 to_rockchip: 2015-11-30 to_gregorian: 2015-11-28 [ 1.431245] 2015-11-30 to_rockchip: 2015-12-01 to_gregorian: 2015-11-29 This one is actually a bug... to_rockchip should be 2015-11-31 here. It happens because the "compensate if we went back over" part of gregorian_to_rockchip() only checks whether we went over *backwards*, which happens if the date is after the anchor date. If it was before we can go back over forwards and I didn't bother to handle that case. I think this is fine since all affected dates lie in the past and there's no real-world use case where you'd ever need them to work again. [ 1.431249] 2015-11-31 to_rockchip: 2015-12-02 to_gregorian: 2015-11-30 [ 1.431252] 2015-12-01 to_rockchip: 2015-12-01 to_gregorian: 2015-12-01 [ 1.431256] 2015-12-02 to_rockchip: 2015-12-02 to_gregorian: 2015-12-02 [ 1.431259] 2015-12-03 to_rockchip: 2015-12-03 to_gregorian: 2015-12-03 [ 1.431262] 2015-12-04 to_rockchip: 2015-12-04 to_gregorian: 2015-12-04 [ 1.431266] 2015-12-05 to_rockchip: 2015-12-05 to_gregorian: 2015-12-05 [ 1.431269] 2015-12-30 to_rockchip: 2015-12-30 to_gregorian: 2015-12-30 [ 1.431273] 2015-12-31 to_rockchip: 2015-12-31 to_gregorian: 2015-12-31 [ 1.431276] 2016-01-01 to_rockchip: 2016-01-01 to_gregorian: 2016-01-01 [ 1.431279] 2016-10-30 to_rockchip: 2016-10-30 to_gregorian: 2016-10-30 [ 1.431283] 2016-10-31 to_rockchip: 2016-10-31 to_gregorian: 2016-10-31 [ 1.431287] 2016-11-01 to_rockchip: 2016-11-01 to_gregorian: 2016-11-01 [ 1.431291] 2016-11-27 to_rockchip: 2016-11-27 to_gregorian: 2016-11-27 [ 1.431295] 2016-11-28 to_rockchip: 2016-11-28 to_gregorian: 2016-11-28 [ 1.431299] 2016-11-29 to_rockchip: 2016-11-29 to_gregorian: 2016-11-29 [ 1.431302] 2016-11-30 to_rockchip: 2016-11-30 to_gregorian: 2016-11-30 [ 1.431306] 2016-11-31 to_rockchip: 2016-12-01 to_gregorian: 2016-12-01 [ 1.431310] 2016-12-01 to_rockchip: 2016-11-31 to_gregorian: 2016-12-02 [ 1.431313] 2016-12-02 to_rockchip: 2016-12-01 to_gregorian: 2016-12-03 [ 1.431317] 2016-12-03 to_rockchip: 2016-12-02 to_gregorian: 2016-12-04 [ 1.431321] 2016-12-04 to_rockchip: 2016-12-03 to_gregorian: 2016-12-05 [ 1.431324] 2016-12-05 to_rockchip: 2016-12-04 to_gregorian: 2016-12-06 [ 1.431328] 2016-12-30 to_rockchip: 2016-12-29 to_gregorian: 2016-12-31 [ 1.431332] 2016-12-31 to_rockchip: 2016-12-30 to_gregorian: 2017-01-01 [ 1.431335] 2017-01-01 to_rockchip: 2016-12-31 to_gregorian: 2017-01-02 [ 1.431338] 2017-10-30 to_rockchip: 2017-10-29 to_gregorian: 2017-10-31 [ 1.431342] 2017-10-31 to_rockchip: 2017-10-30 to_gregorian: 2017-11-01 [ 1.431345] 2017-11-01 to_rockchip: 2017-10-31 to_gregorian: 2017-11-02 [ 1.431349] 2017-11-27 to_rockchip: 2017-11-26 to_gregorian: 2017-11-28 [ 1.431352] 2017-11-28 to_rockchip: 2017-11-27 to_gregorian: 2017-11-29 [ 1.431356] 2017-11-29 to_rockchip: 2017-11-28 to_gregorian: 2017-11-30 [ 1.431359] 2017-11-30 to_rockchip: 2017-11-29 to_gregorian: 2017-12-01 [ 1.431363] 2017-11-31 to_rockchip: 2017-11-30 to_gregorian: 2017-12-02 [ 1.431366] 2017-12-01 to_rockchip: 2017-11-30 to_gregorian: 2017-12-03 [ 1.431369] 2017-12-02 to_rockchip: 2017-11-31 to_gregorian: 2017-12-04 [ 1.431373] 2017-12-03 to_rockchip: 2017-12-01 to_gregorian: 2017-12-05 [ 1.431376] 2017-12-04 to_rockchip: 2017-12-02 to_gregorian: 2017-12-06 [ 1.431380] 2017-12-05 to_rockchip: 2017-12-03 to_gregorian: 2017-12-07 [ 1.431383] 2017-12-30 to_rockchip: 2017-12-28 to_gregorian: 2018-01-01 [ 1.431386] 2017-12-31 to_rockchip: 2017-12-29 to_gregorian: 2018-01-02 [ 1.431389] 2020-01-01 to_rockchip: 2019-12-28 to_gregorian: 2020-01-05 [ 1.431393] 2020-10-30 to_rockchip: 2020-10-26 to_gregorian: 2020-11-03 [ 1.431397] 2020-10-31 to_rockchip: 2020-10-27 to_gregorian: 2020-11-04 [ 1.431400] 2020-11-01 to_rockchip: 2020-10-28 to_gregorian: 2020-11-05 [ 1.431404] 2020-11-27 to_rockchip: 2020-11-23 to_gregorian: 2020-12-01 [ 1.431408] 2020-11-28 to_rockchip: 2020-11-24 to_gregorian: 2020-12-02 [ 1.431411] 2020-11-29 to_rockchip: 2020-11-25 to_gregorian: 2020-12-03 [ 1.431415] 2020-11-30 to_rockchip: 2020-11-26 to_gregorian: 2020-12-04 [ 1.431419] 2020-11-31 to_rockchip: 2020-11-27 to_gregorian: 2020-12-05 [ 1.431422] 2020-12-01 to_rockchip: 2020-11-27 to_gregorian: 2020-12-06 [ 1.431426] 2020-12-02 to_rockchip: 2020-11-28 to_gregorian: 2020-12-07 [ 1.431430] 2020-12-03 to_rockchip: 2020-11-29 to_gregorian: 2020-12-08 [ 1.431434] 2020-12-04 to_rockchip: 2020-11-30 to_gregorian: 2020-12-09 [ 1.431437] 2020-12-05 to_rockchip: 2020-11-31 to_gregorian: 2020-12-10 [ 1.431441] 2020-12-30 to_rockchip: 2020-12-25 to_gregorian: 2021-01-04 [ 1.431444] 2020-12-31 to_rockchip: 2020-12-26 to_gregorian: 2021-01-05 [ 1.431447] 2030-01-01 to_rockchip: 2029-12-18 to_gregorian: 2030-01-15 [ 1.431450] 2030-10-30 to_rockchip: 2030-10-16 to_gregorian: 2030-11-13 [ 1.431454] 2030-10-31 to_rockchip: 2030-10-17 to_gregorian: 2030-11-14 [ 1.431457] 2030-11-01 to_rockchip: 2030-10-18 to_gregorian: 2030-11-15 [ 1.431461] 2030-11-27 to_rockchip: 2030-11-13 to_gregorian: 2030-12-11 [ 1.431464] 2030-11-28 to_rockchip: 2030-11-14 to_gregorian: 2030-12-12 [ 1.431468] 2030-11-29 to_rockchip: 2030-11-15 to_gregorian: 2030-12-13 [ 1.431471] 2030-11-30 to_rockchip: 2030-11-16 to_gregorian: 2030-12-14 [ 1.431474] 2030-11-31 to_rockchip: 2030-11-17 to_gregorian: 2030-12-15 [ 1.431478] 2030-12-01 to_rockchip: 2030-11-17 to_gregorian: 2030-12-16 [ 1.431481] 2030-12-02 to_rockchip: 2030-11-18 to_gregorian: 2030-12-17 [ 1.431485] 2030-12-03 to_rockchip: 2030-11-19 to_gregorian: 2030-12-18 [ 1.431488] 2030-12-04 to_rockchip: 2030-11-20 to_gregorian: 2030-12-19 [ 1.431492] 2030-12-05 to_rockchip: 2030-11-21 to_gregorian: 2030-12-20 [ 1.431495] 2030-12-30 to_rockchip: 2030-12-15 to_gregorian: 2031-01-14 [ 1.431498] 2030-12-31 to_rockchip: 2030-12-16 to_gregorian: 2031-01-15 [ 1.431501] 2060-01-01 to_rockchip: 2059-11-19 to_gregorian: 2060-02-14 [ 1.431505] 2060-10-30 to_rockchip: 2060-09-16 to_gregorian: 2060-12-13 [ 1.431509] 2060-10-31 to_rockchip: 2060-09-17 to_gregorian: 2060-12-14 [ 1.431512] 2060-11-01 to_rockchip: 2060-09-18 to_gregorian: 2060-12-15 [ 1.431516] 2060-11-27 to_rockchip: 2060-10-14 to_gregorian: 2061-01-10 [ 1.431519] 2060-11-28 to_rockchip: 2060-10-15 to_gregorian: 2061-01-11 [ 1.431522] 2060-11-29 to_rockchip: 2060-10-16 to_gregorian: 2061-01-12 [ 1.431525] 2060-11-30 to_rockchip: 2060-10-17 to_gregorian: 2061-01-13 [ 1.431528] 2060-11-31 to_rockchip: 2060-10-18 to_gregorian: 2061-01-14 [ 1.431532] 2060-12-01 to_rockchip: 2060-10-18 to_gregorian: 2061-01-15 [ 1.431536] 2060-12-02 to_rockchip: 2060-10-19 to_gregorian: 2061-01-16 [ 1.431540] 2060-12-03 to_rockchip: 2060-10-20 to_gregorian: 2061-01-17 [ 1.431544] 2060-12-04 to_rockchip: 2060-10-21 to_gregorian: 2061-01-18 [ 1.431548] 2060-12-05 to_rockchip: 2060-10-22 to_gregorian: 2061-01-19 [ 1.431551] 2060-12-30 to_rockchip: 2060-11-16 to_gregorian: 2061-02-13 [ 1.431554] 2060-12-31 to_rockchip: 2060-11-17 to_gregorian: 2061-02-14 -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st 2015-12-15 23:14 ` [rtc-linux] " Julius Werner @ 2015-12-19 0:25 ` Doug Anderson -1 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-19 0:25 UTC (permalink / raw) To: Julius Werner Cc: Alexandre Belloni, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, LKML, rtc-linux Julius, On Tue, Dec 15, 2015 at 3:14 PM, Julius Werner <jwerner@chromium.org> wrote: > Okay, wrote up and tested the anchor date version. I think once you > get over the initial weirdness of the approach this one is really much > cleaner and safer. > > I tested this with the older rtc_tm_to_time() API and only ported it > over to rtc_tm_to_time64() for submission, since my 3.14 kernel didn't > have that yet... but it still compiles fine and the change was very > trivial so I'm confident that it should work. > > I also did a big manual test for my conversion functions where I just > threw a whole bunch of dates at them, results below for reference: > > [ 1.431216] jwerner: Testing translation functions: > [ 1.431221] 2015-01-01 to_rockchip: 2015-01-02 to_gregorian: 2014-12-31 > [ 1.431224] 2015-10-30 to_rockchip: 2015-10-31 to_gregorian: 2015-10-29 > [ 1.431228] 2015-10-31 to_rockchip: 2015-11-01 to_gregorian: 2015-10-30 > [ 1.431231] 2015-11-01 to_rockchip: 2015-11-02 to_gregorian: 2015-10-31 > [ 1.431235] 2015-11-27 to_rockchip: 2015-11-28 to_gregorian: 2015-11-26 > [ 1.431238] 2015-11-28 to_rockchip: 2015-11-29 to_gregorian: 2015-11-27 > [ 1.431242] 2015-11-29 to_rockchip: 2015-11-30 to_gregorian: 2015-11-28 > [ 1.431245] 2015-11-30 to_rockchip: 2015-12-01 to_gregorian: 2015-11-29 > > This one is actually a bug... to_rockchip should be 2015-11-31 here. > It happens because the "compensate if we went back over" part of > gregorian_to_rockchip() only checks whether we went over *backwards*, > which happens if the date is after the anchor date. If it was before > we can go back over forwards and I didn't bother to handle that case. > I think this is fine since all affected dates lie in the past and > there's no real-world use case where you'd ever need them to work > again. Thanks for the testing. Ah, I see, so the problem with your patch is only right around 11/31 in years past. That seems OK to me. There's actually a real world case that's pretty common where we want to work with dates before 2016. When I power cycle my device and it totally loses battery, I notice that the firmware seems to start as: 2013-01-21 00:50:02 It's possible we could need to run for a while in this state and we possibly could even need alarms to fire. ...but that's nowhere near the problematic dates and presumably someone wouldn't have a system in the "clock set totally wrong" state for a really long time. -Doug ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st @ 2015-12-19 0:25 ` Doug Anderson 0 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-19 0:25 UTC (permalink / raw) To: Julius Werner Cc: Alexandre Belloni, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, LKML, rtc-linux Julius, On Tue, Dec 15, 2015 at 3:14 PM, Julius Werner <jwerner@chromium.org> wrote: > Okay, wrote up and tested the anchor date version. I think once you > get over the initial weirdness of the approach this one is really much > cleaner and safer. > > I tested this with the older rtc_tm_to_time() API and only ported it > over to rtc_tm_to_time64() for submission, since my 3.14 kernel didn't > have that yet... but it still compiles fine and the change was very > trivial so I'm confident that it should work. > > I also did a big manual test for my conversion functions where I just > threw a whole bunch of dates at them, results below for reference: > > [ 1.431216] jwerner: Testing translation functions: > [ 1.431221] 2015-01-01 to_rockchip: 2015-01-02 to_gregorian: 2014-12-31 > [ 1.431224] 2015-10-30 to_rockchip: 2015-10-31 to_gregorian: 2015-10-29 > [ 1.431228] 2015-10-31 to_rockchip: 2015-11-01 to_gregorian: 2015-10-30 > [ 1.431231] 2015-11-01 to_rockchip: 2015-11-02 to_gregorian: 2015-10-31 > [ 1.431235] 2015-11-27 to_rockchip: 2015-11-28 to_gregorian: 2015-11-26 > [ 1.431238] 2015-11-28 to_rockchip: 2015-11-29 to_gregorian: 2015-11-27 > [ 1.431242] 2015-11-29 to_rockchip: 2015-11-30 to_gregorian: 2015-11-28 > [ 1.431245] 2015-11-30 to_rockchip: 2015-12-01 to_gregorian: 2015-11-29 > > This one is actually a bug... to_rockchip should be 2015-11-31 here. > It happens because the "compensate if we went back over" part of > gregorian_to_rockchip() only checks whether we went over *backwards*, > which happens if the date is after the anchor date. If it was before > we can go back over forwards and I didn't bother to handle that case. > I think this is fine since all affected dates lie in the past and > there's no real-world use case where you'd ever need them to work > again. Thanks for the testing. Ah, I see, so the problem with your patch is only right around 11/31 in years past. That seems OK to me. There's actually a real world case that's pretty common where we want to work with dates before 2016. When I power cycle my device and it totally loses battery, I notice that the firmware seems to start as: 2013-01-21 00:50:02 It's possible we could need to run for a while in this state and we possibly could even need alarms to fire. ...but that's nowhere near the problematic dates and presumably someone wouldn't have a system in the "clock set totally wrong" state for a really long time. -Doug -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st 2015-12-19 0:25 ` [rtc-linux] " Doug Anderson @ 2015-12-19 0:31 ` Julius Werner -1 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-19 0:31 UTC (permalink / raw) To: Doug Anderson Cc: Julius Werner, Alexandre Belloni, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, LKML, rtc-linux > There's actually a real world case that's pretty common where we want > to work with dates before 2016. When I power cycle my device and it > totally loses battery, I notice that the firmware seems to start as: > > 2013-01-21 00:50:02 > > It's possible we could need to run for a while in this state and we > possibly could even need alarms to fire. ...but that's nowhere near > the problematic dates and presumably someone wouldn't have a system in > the "clock set totally wrong" state for a really long time. Yeah... I don't think it really makes much sense to worry about that. At that point it's much more likely that you will loose an alarm because the user finally fixes the clock at some point (either manually or by connecting to a network and having some automated sync service jump in), and we never worry about something like that either. I mean, fixing it wouldn't be a big deal (another 5 lines or so maybe), but I just don't think it's worth adding any complexity. ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st @ 2015-12-19 0:31 ` Julius Werner 0 siblings, 0 replies; 62+ messages in thread From: Julius Werner @ 2015-12-19 0:31 UTC (permalink / raw) To: Doug Anderson Cc: Julius Werner, Alexandre Belloni, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, LKML, rtc-linux > There's actually a real world case that's pretty common where we want > to work with dates before 2016. When I power cycle my device and it > totally loses battery, I notice that the firmware seems to start as: > > 2013-01-21 00:50:02 > > It's possible we could need to run for a while in this state and we > possibly could even need alarms to fire. ...but that's nowhere near > the problematic dates and presumably someone wouldn't have a system in > the "clock set totally wrong" state for a really long time. Yeah... I don't think it really makes much sense to worry about that. At that point it's much more likely that you will loose an alarm because the user finally fixes the clock at some point (either manually or by connecting to a network and having some automated sync service jump in), and we never worry about something like that either. I mean, fixing it wouldn't be a big deal (another 5 lines or so maybe), but I just don't think it's worth adding any complexity. -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st 2015-12-15 23:02 ` [rtc-linux] " Julius Werner @ 2015-12-19 0:26 ` Doug Anderson -1 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-19 0:26 UTC (permalink / raw) To: Julius Werner Cc: Alexandre Belloni, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Julius, On Tue, Dec 15, 2015 at 3:02 PM, Julius Werner <jwerner@chromium.org> wrote: > In A.D. 1582 Pope Gregory XIII found that the existing Julian calendar > insufficiently represented reality, and changed the rules about > calculating leap years to account for this. Similarly, in A.D. 2013 > Rockchip hardware engineers found that the new Gregorian calendar still > contained flaws, and that the month of November should be counted up to > 31 days instead. Unfortunately it takes a long time for calendar changes > to gain widespread adoption, and just like more than 300 years went by > before the last Protestant nation implemented Greg's proposal, we will > have to wait a while until all religions and operating system kernels > acknowledge the inherent advantages of the Rockchip system. Until then > we need to translate dates read from (and written to) Rockchip hardware > back to the Gregorian format. > > This patch works by defining Jan 1st, 2016 as the arbitrary anchor date > on which Rockchip and Gregorian calendars are in sync. From that we can > translate arbitrary later dates back and forth by counting the number > of November/December transitons since the anchor date to determine the > offset between the calendars. We choose this method (rather than trying > to regularly "correct" the date stored in hardware) since it's the only > way to ensure perfect time-keeping even if the system may be shut down > for an unknown number of years. The drawback is that other software > reading the same hardware (e.g. mainboard firmware) must use the same > translation convention (including the same anchor date) to be able to > read and write correct timestamps from/to the RTC. > > Signed-off-by: Julius Werner <jwerner@chromium.org> > --- > drivers/rtc/rtc-rk808.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 44 insertions(+), 4 deletions(-) I'm not terribly worried about the date in the past problem that you brought up in your own response. So: Reviewed-by: Douglas Anderson <dianders@chromium.org> ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st @ 2015-12-19 0:26 ` Doug Anderson 0 siblings, 0 replies; 62+ messages in thread From: Doug Anderson @ 2015-12-19 0:26 UTC (permalink / raw) To: Julius Werner Cc: Alexandre Belloni, Andrew Morton, Alessandro Zummo, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux Julius, On Tue, Dec 15, 2015 at 3:02 PM, Julius Werner <jwerner@chromium.org> wrote: > In A.D. 1582 Pope Gregory XIII found that the existing Julian calendar > insufficiently represented reality, and changed the rules about > calculating leap years to account for this. Similarly, in A.D. 2013 > Rockchip hardware engineers found that the new Gregorian calendar still > contained flaws, and that the month of November should be counted up to > 31 days instead. Unfortunately it takes a long time for calendar changes > to gain widespread adoption, and just like more than 300 years went by > before the last Protestant nation implemented Greg's proposal, we will > have to wait a while until all religions and operating system kernels > acknowledge the inherent advantages of the Rockchip system. Until then > we need to translate dates read from (and written to) Rockchip hardware > back to the Gregorian format. > > This patch works by defining Jan 1st, 2016 as the arbitrary anchor date > on which Rockchip and Gregorian calendars are in sync. From that we can > translate arbitrary later dates back and forth by counting the number > of November/December transitons since the anchor date to determine the > offset between the calendars. We choose this method (rather than trying > to regularly "correct" the date stored in hardware) since it's the only > way to ensure perfect time-keeping even if the system may be shut down > for an unknown number of years. The drawback is that other software > reading the same hardware (e.g. mainboard firmware) must use the same > translation convention (including the same anchor date) to be able to > read and write correct timestamps from/to the RTC. > > Signed-off-by: Julius Werner <jwerner@chromium.org> > --- > drivers/rtc/rtc-rk808.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 44 insertions(+), 4 deletions(-) I'm not terribly worried about the date in the past problem that you brought up in your own response. So: Reviewed-by: Douglas Anderson <dianders@chromium.org> -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
* Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st 2015-12-15 23:02 ` [rtc-linux] " Julius Werner @ 2015-12-21 8:16 ` Alexandre Belloni -1 siblings, 0 replies; 62+ messages in thread From: Alexandre Belloni @ 2015-12-21 8:16 UTC (permalink / raw) To: Julius Werner Cc: Andrew Morton, Alessandro Zummo, Doug Anderson, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux On 15/12/2015 at 15:02:49 -0800, Julius Werner wrote : > In A.D. 1582 Pope Gregory XIII found that the existing Julian calendar > insufficiently represented reality, and changed the rules about > calculating leap years to account for this. Similarly, in A.D. 2013 > Rockchip hardware engineers found that the new Gregorian calendar still > contained flaws, and that the month of November should be counted up to > 31 days instead. Unfortunately it takes a long time for calendar changes > to gain widespread adoption, and just like more than 300 years went by > before the last Protestant nation implemented Greg's proposal, we will > have to wait a while until all religions and operating system kernels > acknowledge the inherent advantages of the Rockchip system. Until then > we need to translate dates read from (and written to) Rockchip hardware > back to the Gregorian format. > > This patch works by defining Jan 1st, 2016 as the arbitrary anchor date > on which Rockchip and Gregorian calendars are in sync. From that we can > translate arbitrary later dates back and forth by counting the number > of November/December transitons since the anchor date to determine the > offset between the calendars. We choose this method (rather than trying > to regularly "correct" the date stored in hardware) since it's the only > way to ensure perfect time-keeping even if the system may be shut down > for an unknown number of years. The drawback is that other software > reading the same hardware (e.g. mainboard firmware) must use the same > translation convention (including the same anchor date) to be able to > read and write correct timestamps from/to the RTC. > > Signed-off-by: Julius Werner <jwerner@chromium.org> I forgot to inform you but I've applied it and it landed in 4.4-rc6. > --- > drivers/rtc/rtc-rk808.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 44 insertions(+), 4 deletions(-) > > diff --git a/drivers/rtc/rtc-rk808.c b/drivers/rtc/rtc-rk808.c > index 91ca0bc..35c9aad 100644 > --- a/drivers/rtc/rtc-rk808.c > +++ b/drivers/rtc/rtc-rk808.c > @@ -56,6 +56,42 @@ struct rk808_rtc { > int irq; > }; > > +/* > + * The Rockchip calendar used by the RK808 counts November with 31 days. We use > + * these translation functions to convert its dates to/from the Gregorian > + * calendar used by the rest of the world. We arbitrarily define Jan 1st, 2016 > + * as the day when both calendars were in sync, and treat all other dates > + * relative to that. > + * NOTE: Other system software (e.g. firmware) that reads the same hardware must > + * implement this exact same conversion algorithm, with the same anchor date. > + */ > +static time64_t nov2dec_transitions(struct rtc_time *tm) > +{ > + return (tm->tm_year + 1900) - 2016 + (tm->tm_mon + 1 > 11 ? 1 : 0); > +} > + > +static void rockchip_to_gregorian(struct rtc_time *tm) > +{ > + /* If it's Nov 31st, rtc_tm_to_time64() will count that like Dec 1st */ > + time64_t time = rtc_tm_to_time64(tm); > + rtc_time64_to_tm(time + nov2dec_transitions(tm) * 86400, tm); > +} > + > +static void gregorian_to_rockchip(struct rtc_time *tm) > +{ > + time64_t extra_days = nov2dec_transitions(tm); > + time64_t time = rtc_tm_to_time64(tm); > + rtc_time64_to_tm(time - extra_days * 86400, tm); > + > + /* Compensate if we went back over Nov 31st (will work up to 2381) */ > + if (nov2dec_transitions(tm) < extra_days) { > + if (tm->tm_mon + 1 == 11) > + tm->tm_mday++; /* This may result in 31! */ > + else > + rtc_time64_to_tm(time - (extra_days - 1) * 86400, tm); > + } > +} > + > /* Read current time and date in RTC */ > static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) > { > @@ -101,9 +137,10 @@ static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) > tm->tm_mon = (bcd2bin(rtc_data[4] & MONTHS_REG_MSK)) - 1; > tm->tm_year = (bcd2bin(rtc_data[5] & YEARS_REG_MSK)) + 100; > tm->tm_wday = bcd2bin(rtc_data[6] & WEEKS_REG_MSK); > + rockchip_to_gregorian(tm); > dev_dbg(dev, "RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > + tm->tm_wday, tm->tm_hour, tm->tm_min, tm->tm_sec); > > return ret; > } > @@ -116,6 +153,10 @@ static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) > u8 rtc_data[NUM_TIME_REGS]; > int ret; > > + dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > + 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > + tm->tm_wday, tm->tm_hour, tm->tm_min, tm->tm_sec); > + gregorian_to_rockchip(tm); > rtc_data[0] = bin2bcd(tm->tm_sec); > rtc_data[1] = bin2bcd(tm->tm_min); > rtc_data[2] = bin2bcd(tm->tm_hour); > @@ -123,9 +164,6 @@ static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) > rtc_data[4] = bin2bcd(tm->tm_mon + 1); > rtc_data[5] = bin2bcd(tm->tm_year - 100); > rtc_data[6] = bin2bcd(tm->tm_wday); > - dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > - 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > > /* Stop RTC while updating the RTC registers */ > ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > @@ -170,6 +208,7 @@ static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm) > alrm->time.tm_mday = bcd2bin(alrm_data[3] & DAYS_REG_MSK); > alrm->time.tm_mon = (bcd2bin(alrm_data[4] & MONTHS_REG_MSK)) - 1; > alrm->time.tm_year = (bcd2bin(alrm_data[5] & YEARS_REG_MSK)) + 100; > + rockchip_to_gregorian(&alrm->time); > > ret = regmap_read(rk808->regmap, RK808_RTC_INT_REG, &int_reg); > if (ret) { > @@ -227,6 +266,7 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm) > alrm->time.tm_mday, alrm->time.tm_wday, alrm->time.tm_hour, > alrm->time.tm_min, alrm->time.tm_sec); > > + gregorian_to_rockchip(&alrm->time); > alrm_data[0] = bin2bcd(alrm->time.tm_sec); > alrm_data[1] = bin2bcd(alrm->time.tm_min); > alrm_data[2] = bin2bcd(alrm->time.tm_hour); > -- > 2.1.2 > -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 62+ messages in thread
* [rtc-linux] Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st @ 2015-12-21 8:16 ` Alexandre Belloni 0 siblings, 0 replies; 62+ messages in thread From: Alexandre Belloni @ 2015-12-21 8:16 UTC (permalink / raw) To: Julius Werner Cc: Andrew Morton, Alessandro Zummo, Doug Anderson, Sonny Rao, Chris Zhong, Heiko Stuebner, linux-kernel, rtc-linux On 15/12/2015 at 15:02:49 -0800, Julius Werner wrote : > In A.D. 1582 Pope Gregory XIII found that the existing Julian calendar > insufficiently represented reality, and changed the rules about > calculating leap years to account for this. Similarly, in A.D. 2013 > Rockchip hardware engineers found that the new Gregorian calendar still > contained flaws, and that the month of November should be counted up to > 31 days instead. Unfortunately it takes a long time for calendar changes > to gain widespread adoption, and just like more than 300 years went by > before the last Protestant nation implemented Greg's proposal, we will > have to wait a while until all religions and operating system kernels > acknowledge the inherent advantages of the Rockchip system. Until then > we need to translate dates read from (and written to) Rockchip hardware > back to the Gregorian format. > > This patch works by defining Jan 1st, 2016 as the arbitrary anchor date > on which Rockchip and Gregorian calendars are in sync. From that we can > translate arbitrary later dates back and forth by counting the number > of November/December transitons since the anchor date to determine the > offset between the calendars. We choose this method (rather than trying > to regularly "correct" the date stored in hardware) since it's the only > way to ensure perfect time-keeping even if the system may be shut down > for an unknown number of years. The drawback is that other software > reading the same hardware (e.g. mainboard firmware) must use the same > translation convention (including the same anchor date) to be able to > read and write correct timestamps from/to the RTC. > > Signed-off-by: Julius Werner <jwerner@chromium.org> I forgot to inform you but I've applied it and it landed in 4.4-rc6. > --- > drivers/rtc/rtc-rk808.c | 48 ++++++++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 44 insertions(+), 4 deletions(-) > > diff --git a/drivers/rtc/rtc-rk808.c b/drivers/rtc/rtc-rk808.c > index 91ca0bc..35c9aad 100644 > --- a/drivers/rtc/rtc-rk808.c > +++ b/drivers/rtc/rtc-rk808.c > @@ -56,6 +56,42 @@ struct rk808_rtc { > int irq; > }; > > +/* > + * The Rockchip calendar used by the RK808 counts November with 31 days. We use > + * these translation functions to convert its dates to/from the Gregorian > + * calendar used by the rest of the world. We arbitrarily define Jan 1st, 2016 > + * as the day when both calendars were in sync, and treat all other dates > + * relative to that. > + * NOTE: Other system software (e.g. firmware) that reads the same hardware must > + * implement this exact same conversion algorithm, with the same anchor date. > + */ > +static time64_t nov2dec_transitions(struct rtc_time *tm) > +{ > + return (tm->tm_year + 1900) - 2016 + (tm->tm_mon + 1 > 11 ? 1 : 0); > +} > + > +static void rockchip_to_gregorian(struct rtc_time *tm) > +{ > + /* If it's Nov 31st, rtc_tm_to_time64() will count that like Dec 1st */ > + time64_t time = rtc_tm_to_time64(tm); > + rtc_time64_to_tm(time + nov2dec_transitions(tm) * 86400, tm); > +} > + > +static void gregorian_to_rockchip(struct rtc_time *tm) > +{ > + time64_t extra_days = nov2dec_transitions(tm); > + time64_t time = rtc_tm_to_time64(tm); > + rtc_time64_to_tm(time - extra_days * 86400, tm); > + > + /* Compensate if we went back over Nov 31st (will work up to 2381) */ > + if (nov2dec_transitions(tm) < extra_days) { > + if (tm->tm_mon + 1 == 11) > + tm->tm_mday++; /* This may result in 31! */ > + else > + rtc_time64_to_tm(time - (extra_days - 1) * 86400, tm); > + } > +} > + > /* Read current time and date in RTC */ > static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) > { > @@ -101,9 +137,10 @@ static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm) > tm->tm_mon = (bcd2bin(rtc_data[4] & MONTHS_REG_MSK)) - 1; > tm->tm_year = (bcd2bin(rtc_data[5] & YEARS_REG_MSK)) + 100; > tm->tm_wday = bcd2bin(rtc_data[6] & WEEKS_REG_MSK); > + rockchip_to_gregorian(tm); > dev_dbg(dev, "RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > + tm->tm_wday, tm->tm_hour, tm->tm_min, tm->tm_sec); > > return ret; > } > @@ -116,6 +153,10 @@ static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) > u8 rtc_data[NUM_TIME_REGS]; > int ret; > > + dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > + 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > + tm->tm_wday, tm->tm_hour, tm->tm_min, tm->tm_sec); > + gregorian_to_rockchip(tm); > rtc_data[0] = bin2bcd(tm->tm_sec); > rtc_data[1] = bin2bcd(tm->tm_min); > rtc_data[2] = bin2bcd(tm->tm_hour); > @@ -123,9 +164,6 @@ static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm) > rtc_data[4] = bin2bcd(tm->tm_mon + 1); > rtc_data[5] = bin2bcd(tm->tm_year - 100); > rtc_data[6] = bin2bcd(tm->tm_wday); > - dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n", > - 1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday, > - tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec); > > /* Stop RTC while updating the RTC registers */ > ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG, > @@ -170,6 +208,7 @@ static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm) > alrm->time.tm_mday = bcd2bin(alrm_data[3] & DAYS_REG_MSK); > alrm->time.tm_mon = (bcd2bin(alrm_data[4] & MONTHS_REG_MSK)) - 1; > alrm->time.tm_year = (bcd2bin(alrm_data[5] & YEARS_REG_MSK)) + 100; > + rockchip_to_gregorian(&alrm->time); > > ret = regmap_read(rk808->regmap, RK808_RTC_INT_REG, &int_reg); > if (ret) { > @@ -227,6 +266,7 @@ static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm) > alrm->time.tm_mday, alrm->time.tm_wday, alrm->time.tm_hour, > alrm->time.tm_min, alrm->time.tm_sec); > > + gregorian_to_rockchip(&alrm->time); > alrm_data[0] = bin2bcd(alrm->time.tm_sec); > alrm_data[1] = bin2bcd(alrm->time.tm_min); > alrm_data[2] = bin2bcd(alrm->time.tm_hour); > -- > 2.1.2 > -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. ^ permalink raw reply [flat|nested] 62+ messages in thread
end of thread, other threads:[~2015-12-21 8:16 UTC | newest] Thread overview: 62+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-03 1:53 [PATCH] RTC: RK808: Work around hardware bug on November 31st Julius Werner 2015-12-03 1:53 ` [rtc-linux] " Julius Werner 2015-12-03 14:42 ` Alexandre Belloni 2015-12-03 14:42 ` Alexandre Belloni 2015-12-03 16:53 ` Julius Werner 2015-12-03 16:53 ` Julius Werner 2015-12-04 23:50 ` Doug Anderson 2015-12-04 23:50 ` [rtc-linux] " Doug Anderson 2015-12-05 0:25 ` Julius Werner 2015-12-05 0:25 ` [rtc-linux] " Julius Werner 2015-12-05 0:58 ` Doug Anderson 2015-12-05 0:58 ` [rtc-linux] " Doug Anderson 2015-12-05 1:54 ` Julius Werner 2015-12-05 1:54 ` [rtc-linux] " Julius Werner 2015-12-05 4:02 ` Doug Anderson 2015-12-05 4:02 ` [rtc-linux] " Doug Anderson 2015-12-05 4:53 ` Doug Anderson 2015-12-05 4:53 ` [rtc-linux] " Doug Anderson 2015-12-05 7:17 ` Julius Werner 2015-12-05 7:17 ` [rtc-linux] " Julius Werner 2015-12-06 0:36 ` Doug Anderson 2015-12-06 0:36 ` [rtc-linux] " Doug Anderson 2015-12-07 1:33 ` Chris Zhong 2015-12-07 1:33 ` [rtc-linux] " Chris Zhong 2015-12-07 2:50 ` Doug Anderson 2015-12-07 2:50 ` [rtc-linux] " Doug Anderson 2015-12-07 2:52 ` Doug Anderson 2015-12-07 2:52 ` [rtc-linux] " Doug Anderson 2015-12-07 3:08 ` Chris Zhong 2015-12-07 3:08 ` [rtc-linux] " Chris Zhong 2015-12-07 20:28 ` Julius Werner 2015-12-07 20:28 ` [rtc-linux] " Julius Werner 2015-12-07 22:40 ` Julius Werner 2015-12-07 22:40 ` [rtc-linux] " Julius Werner 2015-12-08 1:17 ` Doug Anderson 2015-12-08 1:17 ` [rtc-linux] " Doug Anderson 2015-12-08 1:41 ` Julius Werner 2015-12-08 1:41 ` [rtc-linux] " Julius Werner 2015-12-08 5:19 ` Julius Werner 2015-12-08 5:19 ` [rtc-linux] " Julius Werner 2015-12-08 5:21 ` [PATCH v2] " Julius Werner 2015-12-08 5:21 ` [rtc-linux] " Julius Werner 2015-12-09 5:44 ` Doug Anderson 2015-12-09 5:44 ` [rtc-linux] " Doug Anderson 2015-12-09 21:32 ` Julius Werner 2015-12-09 21:32 ` [rtc-linux] " Julius Werner 2015-12-10 18:41 ` Alexandre Belloni 2015-12-10 18:41 ` [rtc-linux] " Alexandre Belloni 2015-12-10 18:57 ` Julius Werner 2015-12-10 18:57 ` [rtc-linux] " Julius Werner 2015-12-15 23:02 ` [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation " Julius Werner 2015-12-15 23:02 ` [rtc-linux] " Julius Werner 2015-12-15 23:14 ` Julius Werner 2015-12-15 23:14 ` [rtc-linux] " Julius Werner 2015-12-19 0:25 ` Doug Anderson 2015-12-19 0:25 ` [rtc-linux] " Doug Anderson 2015-12-19 0:31 ` Julius Werner 2015-12-19 0:31 ` [rtc-linux] " Julius Werner 2015-12-19 0:26 ` Doug Anderson 2015-12-19 0:26 ` [rtc-linux] " Doug Anderson 2015-12-21 8:16 ` Alexandre Belloni 2015-12-21 8:16 ` [rtc-linux] " Alexandre Belloni
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.