| Message ID | 20241113113032.27409-1-wsa+renesas@sang-engineering.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | rtc: rzn1: fix BCD to rtc_time conversion errors | expand |
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
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
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)) {
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(-)