LinuxLists.cc - [PATCH] rtc: Add support for century bits to m41t62 (rv4162) RTC devices

[permalink] [raw]

Subject: Re: [PATCH] rtc: Add support for century bits to m41t62 (rv4162) RTC devices

Hi Alessandro, Alexandre,

> This change adds support for 'century bits' on 4162 family of RTC
> devices (from ST or microcrystal), which allow storing time beyond
> year 2099.
>
> For rv4162 century bits - CB1[7]:CB0[6] are stored in reg6 - 0x6
> (MONTH): CB1 CB0
> 0 0 (year 2000 - 2099)
> 0 1 (year 2100 - 2199)
> 1 0 (year 2200 - 2299)
> 1 1 (year 2300 - 2399)
>
> The driver has been also adjusted to allow setting time up to year
> 2399 if the M41T80_FEATURE_CB is set in its DTS/I2C data.
>
> There shall be no functional changes for devices not supporting this
> feature. However, other devices - like m41t80 - have different
> approaches to handle century information.

Gentle ping on this patch - it has been almost 3 weeks without any
reply ...

Thanks in advance.

>
> Signed-off-by: Lukasz Majewski <[email protected]>
> ---
> drivers/rtc/rtc-m41t80.c | 56
> +++++++++++++++++++++++++++++++--------- 1 file changed, 44
> insertions(+), 12 deletions(-)
>
> diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
> index 9fdc284c943b..5452ab693568 100644
> --- a/drivers/rtc/rtc-m41t80.c
> +++ b/drivers/rtc/rtc-m41t80.c
> @@ -68,9 +68,22 @@
> #define M41T80_FEATURE_SQ BIT(2) /* Squarewave feature
> */ #define M41T80_FEATURE_WD BIT(3) /* Extra watchdog
> resolution */ #define M41T80_FEATURE_SQ_ALT BIT(4) /*
> RSx bits are in reg 4 */ +#define M41T80_FEATURE_CB
> BIT(5) /* Century Bits[CB1:CB0] are in reg 6 */ +
> +/*
> + * Century bits - CB1[7]:CB0[6] in reg6 (MONTH):
> + * CB1 CB0
> + * 0 0 (year 2000 - 2099)
> + * 0 1 (year 2100 - 2199)
> + * 1 0 (year 2200 - 2299)
> + * 1 1 (year 2300 - 2399)
> + */
> +#define M41T80_CB_SHIFT 6 /* CB[0] bit position */
> +#define M41T80_CB_MASK 0xc0 /* Century bits mask */
>
> static const struct i2c_device_id m41t80_id[] = {
> - { "m41t62", M41T80_FEATURE_SQ | M41T80_FEATURE_SQ_ALT },
> + { "m41t62", M41T80_FEATURE_SQ | M41T80_FEATURE_SQ_ALT |
> + M41T80_FEATURE_CB},
> { "m41t65", M41T80_FEATURE_HT | M41T80_FEATURE_WD },
> { "m41t80", M41T80_FEATURE_SQ },
> { "m41t81", M41T80_FEATURE_HT | M41T80_FEATURE_SQ},
> @@ -80,7 +93,8 @@ static const struct i2c_device_id m41t80_id[] = {
> { "m41st84", M41T80_FEATURE_HT | M41T80_FEATURE_BL |
> M41T80_FEATURE_SQ }, { "m41st85", M41T80_FEATURE_HT |
> M41T80_FEATURE_BL | M41T80_FEATURE_SQ }, { "m41st87",
> M41T80_FEATURE_HT | M41T80_FEATURE_BL | M41T80_FEATURE_SQ },
> - { "rv4162", M41T80_FEATURE_SQ | M41T80_FEATURE_WD |
> M41T80_FEATURE_SQ_ALT },
> + { "rv4162", M41T80_FEATURE_SQ | M41T80_FEATURE_WD |
> + M41T80_FEATURE_SQ_ALT | M41T80_FEATURE_CB},
> { }
> };
> MODULE_DEVICE_TABLE(i2c, m41t80_id);
> @@ -88,7 +102,8 @@ MODULE_DEVICE_TABLE(i2c, m41t80_id);
> static const struct of_device_id m41t80_of_match[] = {
> {
> .compatible = "st,m41t62",
> - .data = (void *)(M41T80_FEATURE_SQ |
> M41T80_FEATURE_SQ_ALT)
> + .data = (void *)(M41T80_FEATURE_SQ |
> M41T80_FEATURE_SQ_ALT |
> + M41T80_FEATURE_CB)
> },
> {
> .compatible = "st,m41t65",
> @@ -128,16 +143,19 @@ static const struct of_device_id
> m41t80_of_match[] = { },
> {
> .compatible = "microcrystal,rv4162",
> - .data = (void *)(M41T80_FEATURE_SQ |
> M41T80_FEATURE_WD | M41T80_FEATURE_SQ_ALT)
> + .data = (void *)(M41T80_FEATURE_SQ |
> M41T80_FEATURE_WD |
> + M41T80_FEATURE_SQ_ALT |
> M41T80_FEATURE_CB) },
> /* DT compatibility only, do not use compatibles below: */
> {
> .compatible = "st,rv4162",
> - .data = (void *)(M41T80_FEATURE_SQ |
> M41T80_FEATURE_WD | M41T80_FEATURE_SQ_ALT)
> + .data = (void *)(M41T80_FEATURE_SQ |
> M41T80_FEATURE_WD |
> + M41T80_FEATURE_SQ_ALT |
> M41T80_FEATURE_CB) },
> {
> .compatible = "rv4162",
> - .data = (void *)(M41T80_FEATURE_SQ |
> M41T80_FEATURE_WD | M41T80_FEATURE_SQ_ALT)
> + .data = (void *)(M41T80_FEATURE_SQ |
> M41T80_FEATURE_WD |
> + M41T80_FEATURE_SQ_ALT |
> M41T80_FEATURE_CB) },
> { }
> };
> @@ -197,6 +215,7 @@ static irqreturn_t m41t80_handle_irq(int irq,
> void *dev_id) static int m41t80_rtc_read_time(struct device *dev,
> struct rtc_time *tm) {
> struct i2c_client *client = to_i2c_client(dev);
> + struct m41t80_data *clientdata = i2c_get_clientdata(client);
> unsigned char buf[8];
> int err, flags;
>
> @@ -222,9 +241,13 @@ static int m41t80_rtc_read_time(struct device
> *dev, struct rtc_time *tm) tm->tm_mday = bcd2bin(buf[M41T80_REG_DAY]
> & 0x3f); tm->tm_wday = buf[M41T80_REG_WDAY] & 0x07;
> tm->tm_mon = bcd2bin(buf[M41T80_REG_MON] & 0x1f) - 1;
> -
> - /* assume 20YY not 19YY, and ignore the Century Bit */
> + /* assume 20YY not 19YY */
> tm->tm_year = bcd2bin(buf[M41T80_REG_YEAR]) + 100;
> +
> + if (clientdata->features & M41T80_FEATURE_CB)
> + tm->tm_year += ((buf[M41T80_REG_MON] &
> M41T80_CB_MASK)
> + >> M41T80_CB_SHIFT) * 100;
> +
> return 0;
> }
>
> @@ -232,10 +255,13 @@ static int m41t80_rtc_set_time(struct device
> *dev, struct rtc_time *tm) {
> struct i2c_client *client = to_i2c_client(dev);
> struct m41t80_data *clientdata = i2c_get_clientdata(client);
> + int err, flags, max_year = 199;
> unsigned char buf[8];
> - int err, flags;
>
> - if (tm->tm_year < 100 || tm->tm_year > 199)
> + if (clientdata->features & M41T80_FEATURE_CB)
> + max_year = 499;
> +
> + if (tm->tm_year < 100 || tm->tm_year > max_year)
> return -EINVAL;
>
> buf[M41T80_REG_SSEC] = 0;
> @@ -243,8 +269,14 @@ static int m41t80_rtc_set_time(struct device
> *dev, struct rtc_time *tm) buf[M41T80_REG_MIN] = bin2bcd(tm->tm_min);
> buf[M41T80_REG_HOUR] = bin2bcd(tm->tm_hour);
> buf[M41T80_REG_DAY] = bin2bcd(tm->tm_mday);
> - buf[M41T80_REG_MON] = bin2bcd(tm->tm_mon + 1);
> - buf[M41T80_REG_YEAR] = bin2bcd(tm->tm_year - 100);
> + if (clientdata->features & M41T80_FEATURE_CB) {
> + buf[M41T80_REG_YEAR] = bin2bcd((tm->tm_year - 100) %
> 100);
> + buf[M41T80_REG_MON] = bin2bcd(tm->tm_mon + 1) |
> + (((tm->tm_year - 100) / 100) <<
> M41T80_CB_SHIFT);
> + } else {
> + buf[M41T80_REG_MON] = bin2bcd(tm->tm_mon + 1);
> + buf[M41T80_REG_YEAR] = bin2bcd(tm->tm_year - 100);
> + }
> buf[M41T80_REG_WDAY] = tm->tm_wday;
>
> /* If the square wave output is controlled in the weekday
> register */

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: [email protected]

Attachments:

2019-10-03 11:49:52

[permalink] [raw]

Subject: Re: [PATCH] rtc: Add support for century bits to m41t62 (rv4162) RTC devices

Hello,

On 11/09/2019 17:48:03+0200, Lukasz Majewski wrote:
> This change adds support for 'century bits' on 4162 family of RTC devices
> (from ST or microcrystal), which allow storing time beyond year 2099.
>
> For rv4162 century bits - CB1[7]:CB0[6] are stored in reg6 - 0x6 (MONTH):
> CB1 CB0
> 0 0 (year 2000 - 2099)
> 0 1 (year 2100 - 2199)
> 1 0 (year 2200 - 2299)
> 1 1 (year 2300 - 2399)
>
> The driver has been also adjusted to allow setting time up to year 2399
> if the M41T80_FEATURE_CB is set in its DTS/I2C data.
>
> There shall be no functional changes for devices not supporting this
> feature. However, other devices - like m41t80 - have different approaches
> to handle century information.
>

This does not work because the RTC doesn't handle leap years on century
properly. This means that if you do that, then there is no guarantee the
date will be the correct after 2099. As far as the m41t62 and rv4162
are concerned, there is no way to make the century bits useful.

See the datasheet:

"During any year which is a multiple of 4, the RV-4162 RTC will
automatically insert leap day, February 29. Therefore, the application
software must correct for this during the exception years (2100, 2200,
etc.) as noted above."

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

2019-10-03 12:25:08

[permalink] [raw]

Subject: Re: [PATCH] rtc: Add support for century bits to m41t62 (rv4162) RTC devices

Hi Alexandre,

> Hello,
>
> On 11/09/2019 17:48:03+0200, Lukasz Majewski wrote:
> > This change adds support for 'century bits' on 4162 family of RTC
> > devices (from ST or microcrystal), which allow storing time beyond
> > year 2099.
> >
> > For rv4162 century bits - CB1[7]:CB0[6] are stored in reg6 - 0x6
> > (MONTH): CB1 CB0
> > 0 0 (year 2000 - 2099)
> > 0 1 (year 2100 - 2199)
> > 1 0 (year 2200 - 2299)
> > 1 1 (year 2300 - 2399)
> >
> > The driver has been also adjusted to allow setting time up to year
> > 2399 if the M41T80_FEATURE_CB is set in its DTS/I2C data.
> >
> > There shall be no functional changes for devices not supporting this
> > feature. However, other devices - like m41t80 - have different
> > approaches to handle century information.
> >
>
> This does not work because the RTC doesn't handle leap years on
> century properly. This means that if you do that, then there is no
> guarantee the date will be the correct after 2099. As far as the
> m41t62 and rv4162 are concerned, there is no way to make the century
> bits useful.

The code as is now doesn't allow to setup dates after year 2100.
In my application - I do some tests which depend on setting this time.

>
> See the datasheet:
>
> "During any year which is a multiple of 4, the RV-4162 RTC will
> automatically insert leap day, February 29. Therefore, the
> application software must correct for this during the exception years
> (2100, 2200, etc.) as noted above."

I'm wondering what the phrase "application software" means here?

If it is the userland SW, then we shall at least be able to set 2099 in
this device and then count on software correction.

If the "application software" is the kernel driver - the date
correction shall be done there (maybe some lookup table?).

Personally, I do prefer the first option - this means that with this
patch we can set the time to e.g. 2234 year and then rely on userland
software (or libc) to do the correction.

>

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: [email protected]

Attachments:

2019-10-03 12:36:17

[permalink] [raw]

Subject: Re: [PATCH] rtc: Add support for century bits to m41t62 (rv4162) RTC devices

On 03/10/2019 14:21:50+0200, Lukasz Majewski wrote:
> >
> > See the datasheet:
> >
> > "During any year which is a multiple of 4, the RV-4162 RTC will
> > automatically insert leap day, February 29. Therefore, the
> > application software must correct for this during the exception years
> > (2100, 2200, etc.) as noted above."
>
> I'm wondering what the phrase "application software" means here?
>
> If it is the userland SW, then we shall at least be able to set 2099 in
> this device and then count on software correction.
>
> If the "application software" is the kernel driver - the date
> correction shall be done there (maybe some lookup table?).
>
> Personally, I do prefer the first option - this means that with this
> patch we can set the time to e.g. 2234 year and then rely on userland
> software (or libc) to do the correction.
>

It is not possible to ensure this correction is properly done in
software, there is no point in letting the user set those bits.

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

2019-10-03 14:33:11

[permalink] [raw]

Subject: Re: [PATCH] rtc: Add support for century bits to m41t62 (rv4162) RTC devices

Hi Alexandre,

> On 03/10/2019 14:21:50+0200, Lukasz Majewski wrote:
> > >
> > > See the datasheet:
> > >
> > > "During any year which is a multiple of 4, the RV-4162 RTC will
> > > automatically insert leap day, February 29. Therefore, the
> > > application software must correct for this during the exception
> > > years (2100, 2200, etc.) as noted above."
> >
> > I'm wondering what the phrase "application software" means here?
> >
> > If it is the userland SW, then we shall at least be able to set
> > 2099 in this device and then count on software correction.
> >
> > If the "application software" is the kernel driver - the date
> > correction shall be done there (maybe some lookup table?).
> >
> > Personally, I do prefer the first option - this means that with this
> > patch we can set the time to e.g. 2234 year and then rely on
> > userland software (or libc) to do the correction.
> >
>
> It is not possible to ensure this correction is properly done in
> software, there is no point in letting the user set those bits.
>
>

I see your point.

However, could you share your idea on testing setting RTC time to year
2100 on this particular IC (by using hctosys and friends)?

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: [email protected]

Attachments:

2019-10-03 14:44:26

[permalink] [raw]

Subject: Re: [PATCH] rtc: Add support for century bits to m41t62 (rv4162) RTC devices

On 03/10/2019 15:14:34+0200, Lukasz Majewski wrote:
> Hi Alexandre,
>
> > On 03/10/2019 14:21:50+0200, Lukasz Majewski wrote:
> > > >
> > > > See the datasheet:
> > > >
> > > > "During any year which is a multiple of 4, the RV-4162 RTC will
> > > > automatically insert leap day, February 29. Therefore, the
> > > > application software must correct for this during the exception
> > > > years (2100, 2200, etc.) as noted above."
> > >
> > > I'm wondering what the phrase "application software" means here?
> > >
> > > If it is the userland SW, then we shall at least be able to set
> > > 2099 in this device and then count on software correction.
> > >
> > > If the "application software" is the kernel driver - the date
> > > correction shall be done there (maybe some lookup table?).
> > >
> > > Personally, I do prefer the first option - this means that with this
> > > patch we can set the time to e.g. 2234 year and then rely on
> > > userland software (or libc) to do the correction.
> > >
> >
> > It is not possible to ensure this correction is properly done in
> > software, there is no point in letting the user set those bits.
> >
> >
>
> I see your point.
>
> However, could you share your idea on testing setting RTC time to year
> 2100 on this particular IC (by using hctosys and friends)?
>

You can use rtc from
https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/rtc-tools.git/

You can also use rtc-range with your patch to observe that it fails in
2100.

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

2019-10-03 14:53:25

[permalink] [raw]

Subject: Re: [PATCH] rtc: Add support for century bits to m41t62 (rv4162) RTC devices

Hi Alexandre,

> On 03/10/2019 15:14:34+0200, Lukasz Majewski wrote:
> > Hi Alexandre,
> >
> > > On 03/10/2019 14:21:50+0200, Lukasz Majewski wrote:
> > > > >
> > > > > See the datasheet:
> > > > >
> > > > > "During any year which is a multiple of 4, the RV-4162 RTC
> > > > > will automatically insert leap day, February 29. Therefore,
> > > > > the application software must correct for this during the
> > > > > exception years (2100, 2200, etc.) as noted above."
> > > >
> > > > I'm wondering what the phrase "application software" means here?
> > > >
> > > > If it is the userland SW, then we shall at least be able to set
> > > > 2099 in this device and then count on software correction.
> > > >
> > > > If the "application software" is the kernel driver - the date
> > > > correction shall be done there (maybe some lookup table?).
> > > >
> > > > Personally, I do prefer the first option - this means that with
> > > > this patch we can set the time to e.g. 2234 year and then rely
> > > > on userland software (or libc) to do the correction.
> > > >
> > >
> > > It is not possible to ensure this correction is properly done in
> > > software, there is no point in letting the user set those bits.

Sorry, but I do see some inconsistency here.

The application note [1] says that the correction shall be done in
application SW.

The rtc-range.c program [2] sets and reads the time via ioctl (e.g.
RTC_SET_TIME, RTC_RD_TIME).

To pass your tests one needs to do the correction in linux kernel
driver for drivers/rtc/rtc-m41t80.c.

Please correct me if I'm wrong, but IMHO it shall be enough to adjust
2100, 2200, 2300, years in this driver (the submitted patch shall be
adjusted to support it - I can prepare proper v2).

> > >
> > >
> >
> > I see your point.
> >
> > However, could you share your idea on testing setting RTC time to
> > year 2100 on this particular IC (by using hctosys and friends)?
> >
>
> You can use rtc from
> https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/rtc-tools.git/
>
> You can also use rtc-range with your patch to observe that it fails in
> 2100.

Thanks for sharing useful links.

>

Note:

[1] -
https://www.microcrystal.com/fileadmin/Media/Products/RTC/App.Manual/RV-4162-C7_App-Manual.pdf
(point 4.5 and 4.6).

[2] -
https://git.kernel.org/pub/scm/linux/kernel/git/abelloni/rtc-tools.git/tree/rtc-range.c

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: [email protected]

Attachments:

2019-10-03 14:54:18

[permalink] [raw]

Subject: Re: [PATCH] rtc: Add support for century bits to m41t62 (rv4162) RTC devices

On 03/10/2019 16:10:53+0200, Lukasz Majewski wrote:
> Sorry, but I do see some inconsistency here.
>
> The application note [1] says that the correction shall be done in
> application SW.
>
> The rtc-range.c program [2] sets and reads the time via ioctl (e.g.
> RTC_SET_TIME, RTC_RD_TIME).
>
> To pass your tests one needs to do the correction in linux kernel
> driver for drivers/rtc/rtc-m41t80.c.
>
> Please correct me if I'm wrong, but IMHO it shall be enough to adjust
> 2100, 2200, 2300, years in this driver (the submitted patch shall be
> adjusted to support it - I can prepare proper v2).
>

There is no way you will be able to know when to adjust the date because
Linux may or may not be running when the boundary is crossed.

The only useful range for an RTC is its fully contiguous range. If it
needs software to run to support an extended range, it can't be used in
the context of Linux.

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

2019-10-03 15:00:57

[permalink] [raw]

Subject: Re: [PATCH] rtc: Add support for century bits to m41t62 (rv4162) RTC devices

Hi Alexandre,

> On 03/10/2019 16:10:53+0200, Lukasz Majewski wrote:
> > Sorry, but I do see some inconsistency here.
> >
> > The application note [1] says that the correction shall be done in
> > application SW.
> >
> > The rtc-range.c program [2] sets and reads the time via ioctl (e.g.
> > RTC_SET_TIME, RTC_RD_TIME).
> >
> > To pass your tests one needs to do the correction in linux kernel
> > driver for drivers/rtc/rtc-m41t80.c.
> >
> > Please correct me if I'm wrong, but IMHO it shall be enough to
> > adjust 2100, 2200, 2300, years in this driver (the submitted patch
> > shall be adjusted to support it - I can prepare proper v2).
> >
>
> There is no way you will be able to know when to adjust the date
> because Linux may or may not be running when the boundary is crossed.
>

I'm rather thinking about following use cases:

I. Adjusting time:

1. I start with time < 01.01.2099

2. I issue ioctl to set the time to e.g. 2100

- When driver receives such request I setup century bits

- and also perform in kernel driver time correction (and store
corrected time in RTC)

3. Subsequent reads from rtc use century bits to provide the time
(after year 2100). Century bits are set, so the correction may be
performed if needed.

II. The system is started at year 2098 and is supposed to run for e.g. 3
years:

1. The time is read from the rtc - the "passing" of centuries need to
be detected.

From the documentation [1] (point 4.5):

"The two century bits, CB1 and CB0, are bits 7 and 6, respectively, in
the Month / Century register at address 06h. Together, they comprise a
2 - bit counter which increments at the turn of each century. CB1 is
the most significant bit."

If those bits increment when we pass century boundaries, we can detect
this fact and correct time when ioctl is issued.

> The only useful range for an RTC is its fully contiguous range.

Does the automatic increment of century bits count to "contiguous
range" ?

> If it
> needs software to run to support an extended range, it can't be used
> in the context of Linux.
>

[1] -
https://www.microcrystal.com/fileadmin/Media/Products/RTC/App.Manual/RV-4162-C7_App-Manual.pdf

Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: [email protected]

Attachments:

2019-10-03 21:36:23