2022-02-09 06:24:49

by Hanna Hawa

Subject: [RFC PATCH 1/1] rtc: bq32k: update years calculation according century bit

tm_year filed hold the number of years since 1900, in case the century
was changed the driver will return invalid year, as it will not
increment the years field by 200.

This change update the years calculation in bq32k_rtc_read_time() and
bq32k_rtc_write_time(). By increasing the years by 100 always, and only
if the century enable bit (BQ32K_CENT_EN) is set and century bit
(BQ32K_CENT) is cleared will increase again by 100 to represent the next
century.

Signed-off-by: Hanna Hawa <[email protected]>
---
drivers/rtc/rtc-bq32k.c | 29 +++++++++++++++++++++++------
1 file changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c
index 2235c968842d..09795dd2728b 100644
--- a/drivers/rtc/rtc-bq32k.c
+++ b/drivers/rtc/rtc-bq32k.c
@@ -108,8 +108,20 @@ static int bq32k_rtc_read_time(struct device *dev, struct rtc_time *tm)
tm->tm_mday = bcd2bin(regs.date);
tm->tm_wday = bcd2bin(regs.day) - 1;
tm->tm_mon = bcd2bin(regs.month) - 1;
- tm->tm_year = bcd2bin(regs.years) +
- ((regs.cent_hours & BQ32K_CENT) ? 100 : 0);
+ /*
+ * tm_year is number of years since 1900. Need to increase the years by
+ * 100 always assuming we are on 20YY and not 19YY.
+ */
+ tm->tm_year = bcd2bin(regs.years) + 100;
+
+ /*
+ * If the century enable bit (BQ32K_CENT_EN) is set, and century bit
+ * (BQ32K_CENT) is cleared, that means we are on the next century, which
+ * required to increase by 100.
+ */
+ if ((regs.cent_hours & BQ32K_CENT_EN) &&
+ !(regs.cent_hours & BQ32K_CENT))
+ tm->tm_year += 100;

return 0;
}
@@ -117,6 +129,7 @@ static int bq32k_rtc_read_time(struct device *dev, struct rtc_time *tm)
static int bq32k_rtc_set_time(struct device *dev, struct rtc_time *tm)
{
struct bq32k_regs regs;
+ int year;

regs.seconds = bin2bcd(tm->tm_sec);
regs.minutes = bin2bcd(tm->tm_min);
@@ -125,11 +138,15 @@ static int bq32k_rtc_set_time(struct device *dev, struct rtc_time *tm)
regs.date = bin2bcd(tm->tm_mday);
regs.month = bin2bcd(tm->tm_mon + 1);

- if (tm->tm_year >= 100) {
+ /* Assume we are on 20YY and not 19YY */
+ year = tm->tm_year - 100;
+
+ if (year < 100) {
regs.cent_hours |= BQ32K_CENT;
- regs.years = bin2bcd(tm->tm_year - 100);
- } else
- regs.years = bin2bcd(tm->tm_year);
+ regs.years = bin2bcd(year);
+ } else {
+ regs.years = bin2bcd(year - 100);
+ }

return bq32k_write(dev, &regs, 0, sizeof(regs));
}
--
2.17.1

2022-03-09 23:37:02

by Alexandre Belloni

Subject: Re: [RFC PATCH 1/1] rtc: bq32k: update years calculation according century bit

Hello,

On 07/02/2022 09:01:56+0200, Hanna Hawa wrote:
> tm_year filed hold the number of years since 1900, in case the century
> was changed the driver will return invalid year, as it will not
> increment the years field by 200.
>
> This change update the years calculation in bq32k_rtc_read_time() and
> bq32k_rtc_write_time(). By increasing the years by 100 always, and only
> if the century enable bit (BQ32K_CENT_EN) is set and century bit
> (BQ32K_CENT) is cleared will increase again by 100 to represent the next
> century.
>

I'm not sure I get the issue, currently, the driver considers that
BQ32K_CENT not set is 19YY and BQ32K_CENT set is 20YY and what is done
seems fine to me. Can you elaborate on what you are trying to fix?

> Signed-off-by: Hanna Hawa <[email protected]>
> ---
> drivers/rtc/rtc-bq32k.c | 29 +++++++++++++++++++++++------
> 1 file changed, 23 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c
> index 2235c968842d..09795dd2728b 100644
> --- a/drivers/rtc/rtc-bq32k.c
> +++ b/drivers/rtc/rtc-bq32k.c
> @@ -108,8 +108,20 @@ static int bq32k_rtc_read_time(struct device *dev, struct rtc_time *tm)
> tm->tm_mday = bcd2bin(regs.date);
> tm->tm_wday = bcd2bin(regs.day) - 1;
> tm->tm_mon = bcd2bin(regs.month) - 1;
> - tm->tm_year = bcd2bin(regs.years) +
> - ((regs.cent_hours & BQ32K_CENT) ? 100 : 0);
> + /*
> + * tm_year is number of years since 1900. Need to increase the years by
> + * 100 always assuming we are on 20YY and not 19YY.
> + */
> + tm->tm_year = bcd2bin(regs.years) + 100;
> +
> + /*
> + * If the century enable bit (BQ32K_CENT_EN) is set, and century bit
> + * (BQ32K_CENT) is cleared, that means we are on the next century, which
> + * required to increase by 100.
> + */
> + if ((regs.cent_hours & BQ32K_CENT_EN) &&
> + !(regs.cent_hours & BQ32K_CENT))
> + tm->tm_year += 100;
>
> return 0;
> }
> @@ -117,6 +129,7 @@ static int bq32k_rtc_read_time(struct device *dev, struct rtc_time *tm)
> static int bq32k_rtc_set_time(struct device *dev, struct rtc_time *tm)
> {
> struct bq32k_regs regs;
> + int year;
>
> regs.seconds = bin2bcd(tm->tm_sec);
> regs.minutes = bin2bcd(tm->tm_min);
> @@ -125,11 +138,15 @@ static int bq32k_rtc_set_time(struct device *dev, struct rtc_time *tm)
> regs.date = bin2bcd(tm->tm_mday);
> regs.month = bin2bcd(tm->tm_mon + 1);
>
> - if (tm->tm_year >= 100) {
> + /* Assume we are on 20YY and not 19YY */
> + year = tm->tm_year - 100;
> +
> + if (year < 100) {
> regs.cent_hours |= BQ32K_CENT;
> - regs.years = bin2bcd(tm->tm_year - 100);
> - } else
> - regs.years = bin2bcd(tm->tm_year);
> + regs.years = bin2bcd(year);
> + } else {
> + regs.years = bin2bcd(year - 100);
> + }
>
> return bq32k_write(dev, &regs, 0, sizeof(regs));
> }
> --
> 2.17.1
>

--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com