[PATCH] rtc: rzn1: fix BCD to rtc_time conversion errors

[PATCH] rtc: rzn1: fix BCD to rtc_time conversion errors

[Wolfram Sang]

tm_mon describes months from 0 to 11, but the register contains BCD from
1 to 12. tm_year contains years since 1900, but the BCD contains 20XX.
Apply the offsets when converting these numbers.

Fixes: deeb4b5393e1 ("rtc: rzn1: Add new RTC driver")
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

We had this kind of fix in two different BSPs, time to get it upstream.
While I am here, I have one question: Isn't it better to replace

	tm->tm_mon = bin2bcd(tm->tm_mon + 1);
	...
	writel(tm->tm_mon, rtc->base + RZN1_RTC_MONTH);

with
	writel(bin2bcd(tm->tm_mon + 1), rtc->base + RZN1_RTC_MONTH);

I'd say it is even more readable and I also think it is better coding
style if 'tm' always contains defined values.

Happy hacking,

   Wolfram

 drivers/rtc/rtc-rzn1.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
index 56ebbd4d0481..8570c8e63d70 100644
--- a/drivers/rtc/rtc-rzn1.c
+++ b/drivers/rtc/rtc-rzn1.c
@@ -111,8 +111,8 @@ static int rzn1_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	tm->tm_hour = bcd2bin(tm->tm_hour);
 	tm->tm_wday = bcd2bin(tm->tm_wday);
 	tm->tm_mday = bcd2bin(tm->tm_mday);
-	tm->tm_mon = bcd2bin(tm->tm_mon);
-	tm->tm_year = bcd2bin(tm->tm_year);
+	tm->tm_mon = bcd2bin(tm->tm_mon) - 1;
+	tm->tm_year = bcd2bin(tm->tm_year) + 100;
 
 	return 0;
 }
@@ -128,8 +128,8 @@ static int rzn1_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	tm->tm_hour = bin2bcd(tm->tm_hour);
 	tm->tm_wday = bin2bcd(rzn1_rtc_tm_to_wday(tm));
 	tm->tm_mday = bin2bcd(tm->tm_mday);
-	tm->tm_mon = bin2bcd(tm->tm_mon);
-	tm->tm_year = bin2bcd(tm->tm_year);
+	tm->tm_mon = bin2bcd(tm->tm_mon + 1);
+	tm->tm_year = bin2bcd(tm->tm_year - 100);
 
 	val = readl(rtc->base + RZN1_RTC_CTL2);
 	if (!(val & RZN1_RTC_CTL2_STOPPED)) {
-- 
2.39.2

Re: [PATCH] rtc: rzn1: fix BCD to rtc_time conversion errors

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]

On Wed, Nov 13, 2024 at 12:30:32PM +0100, Wolfram Sang wrote:
> tm_mon describes months from 0 to 11, but the register contains BCD from
> 1 to 12. tm_year contains years since 1900, but the BCD contains 20XX.
> Apply the offsets when converting these numbers.
>
> Fixes: deeb4b5393e1 ("rtc: rzn1: Add new RTC driver")
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Forgot to add that Biju Das had a nice test case for the months:

    Steps to reproduce:
    ------------------
    date -s "2016-12-31 23:59:55";hwclock -w
    hwclock; sleep 10; hwclock

    Original result:
    ---------------
    Sat Dec 31 23:59:55 UTC 2016
    Sat Dec 31 23:59:55 2016  0.000000 seconds
    hwclock: RTC_RD_TIME: Invalid argument

    Result after the fix:
    --------------------
    Sat Dec 31 23:59:55 2016  0.000000 seconds
    Sun Jan  1 00:00:05 2017  0.000000 seconds


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] rtc: rzn1: fix BCD to rtc_time conversion errors

[Miquel Raynal]

Hello Wolfram,

On 13/11/2024 at 12:30:32 +01, Wolfram Sang <wsa+renesas@sang-engineering.com> wrote:

> tm_mon describes months from 0 to 11, but the register contains BCD from
> 1 to 12. tm_year contains years since 1900, but the BCD contains 20XX.
> Apply the offsets when converting these numbers.
>
> Fixes: deeb4b5393e1 ("rtc: rzn1: Add new RTC driver")

Should probably be Cc'd to stable?

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

Re: [PATCH] rtc: rzn1: fix BCD to rtc_time conversion errors

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 617 bytes --]

On Fri, Nov 15, 2024 at 08:36:05PM +0100, Miquel Raynal wrote:
> Hello Wolfram,
> 
> On 13/11/2024 at 12:30:32 +01, Wolfram Sang <wsa+renesas@sang-engineering.com> wrote:
> 
> > tm_mon describes months from 0 to 11, but the register contains BCD from
> > 1 to 12. tm_year contains years since 1900, but the BCD contains 20XX.
> > Apply the offsets when converting these numbers.
> >
> > Fixes: deeb4b5393e1 ("rtc: rzn1: Add new RTC driver")
> 
> Should probably be Cc'd to stable?
> 
> Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks, Alexandre do you want me to resend CCing stable?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] rtc: rzn1: fix BCD to rtc_time conversion errors

[Alexandre Belloni]

On Wed, 13 Nov 2024 12:30:32 +0100, Wolfram Sang wrote:
> tm_mon describes months from 0 to 11, but the register contains BCD from
> 1 to 12. tm_year contains years since 1900, but the BCD contains 20XX.
> Apply the offsets when converting these numbers.
> 
> 

Applied, thanks!

[1/1] rtc: rzn1: fix BCD to rtc_time conversion errors
      https://git.kernel.org/abelloni/c/55727188dfa3

Best regards,

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

Re: [PATCH] rtc: rzn1: fix BCD to rtc_time conversion errors

[Alexandre Belloni]

On 18/11/2024 10:30:40+0100, Wolfram Sang wrote:
> On Fri, Nov 15, 2024 at 08:36:05PM +0100, Miquel Raynal wrote:
> > Hello Wolfram,
> > 
> > On 13/11/2024 at 12:30:32 +01, Wolfram Sang <wsa+renesas@sang-engineering.com> wrote:
> > 
> > > tm_mon describes months from 0 to 11, but the register contains BCD from
> > > 1 to 12. tm_year contains years since 1900, but the BCD contains 20XX.
> > > Apply the offsets when converting these numbers.
> > >
> > > Fixes: deeb4b5393e1 ("rtc: rzn1: Add new RTC driver")
> > 
> > Should probably be Cc'd to stable?
> > 
> > Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
> 
> Thanks, Alexandre do you want me to resend CCing stable?
> 

No, this is fine, I believe it is going to be picked up anyway.


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