[PATCH] RTC: RK808: Work around hardware bug on November 31st

[PATCH] RTC: RK808: Work around hardware bug on November 31st

[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

[rtc-linux] [PATCH] RTC: RK808: Work around hardware bug on November 31st

[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.

Re: [rtc-linux] [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Alexandre Belloni]

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

Re: [rtc-linux] [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Alexandre Belloni]

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.

Re: [rtc-linux] [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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.

Re: [rtc-linux] [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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.

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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).

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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.

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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.)

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Chris Zhong]

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
>
>
>

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Chris Zhong]

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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Chris Zhong]

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
>
>
>

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Chris Zhong]

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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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?

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

[+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)

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

[+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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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?

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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.

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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.

Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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).

[rtc-linux] Re: [PATCH] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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.

[PATCH v2] RTC: RK808: Work around hardware bug on November 31st

[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

[rtc-linux] [PATCH v2] RTC: RK808: Work around hardware bug on November 31st

[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.

Re: [PATCH v2] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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

[rtc-linux] Re: [PATCH v2] RTC: RK808: Work around hardware bug on November 31st

[Doug Anderson]

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.

Re: [PATCH v2] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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.

[rtc-linux] Re: [PATCH v2] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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.

Re: [PATCH v2] RTC: RK808: Work around hardware bug on November 31st

[Alexandre Belloni]

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

[rtc-linux] Re: [PATCH v2] RTC: RK808: Work around hardware bug on November 31st

[Alexandre Belloni]

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.

Re: [PATCH v2] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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.

[rtc-linux] Re: [PATCH v2] RTC: RK808: Work around hardware bug on November 31st

[Julius Werner]

> 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.

[PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st

[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

[rtc-linux] [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st

[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.

Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st

[Julius Werner]

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

[rtc-linux] Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st

[Julius Werner]

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.

Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st

[Doug Anderson]

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

[rtc-linux] Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st

[Doug Anderson]

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.

Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st

[Julius Werner]

> 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.

[rtc-linux] Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st

[Julius Werner]

> 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.

Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st

[Doug Anderson]

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>

[rtc-linux] Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st

[Doug Anderson]

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.

Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st

[Alexandre Belloni]

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

[rtc-linux] Re: [PATCHv3] RTC: RK808: Compensate for Rockchip calendar deviation on November 31st

[Alexandre Belloni]

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.