From: Alexandre Belloni <alexandre.belloni@bootlin.com>
To: Biju Das <biju.das.jz@bp.renesas.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>,
Alessandro Zummo <a.zummo@towertech.it>,
Michel Pollet <michel.pollet@bp.renesas.com>,
linux-rtc@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
Geert Uytterhoeven <geert+renesas@glider.be>,
Chris Paterson <Chris.Paterson2@renesas.com>,
Biju Das <biju.das@bp.renesas.com>
Subject: Re: [PATCH] rtc: rzn1: Fix RTC_RD_TIME: Invalid argument
Date: Tue, 26 Jul 2022 17:33:02 +0200 [thread overview]
Message-ID: <YuAJLk8UIA0omk3N@mail.local> (raw)
In-Reply-To: <20220706120756.777378-1-biju.das.jz@bp.renesas.com>
Hello,
On 06/07/2022 13:07:56+0100, Biju Das wrote:
> This patch fixes the issue RTC_RD_TIME: Invalid argument with the below
> use case.
>
> Steps to reproduce:
> ------------------
> date -s "2022-12-31 23:59:55";hwclock -w
> hwclock; sleep 10; hwclock
>
> Original result:
> ---------------
> Sat Dec 31 23:59:55 UTC 2022
> Sat Dec 31 23:59:56 2022 0.000000 seconds
> hwclock: RTC_RD_TIME: Invalid argument
>
> Result after the fix:
> --------------------
> Sat Dec 31 23:59:55 UTC 2022
> Sat Dec 31 23:59:56 2022 0.000000 seconds
> Sun Jan 1 00:00:06 2023 0.000000 seconds
>
> Fixes: deeb4b5393e1 ("rtc: rzn1: Add new RTC driver")
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> This patch fix is based on [1]
> [1] https://github.com/renesas-rz/rzn1_linux/blob/rzn1-stable-v4.19/drivers/rtc/rtc-rzn1.c
> ---
> drivers/rtc/rtc-rzn1.c | 47 ++++++++++++++++++++++++++++--------------
> 1 file changed, 32 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/rtc/rtc-rzn1.c b/drivers/rtc/rtc-rzn1.c
> index ac788799c8e3..0f99b4fd3c4b 100644
> --- a/drivers/rtc/rtc-rzn1.c
> +++ b/drivers/rtc/rtc-rzn1.c
> @@ -88,6 +88,35 @@ static unsigned int rzn1_rtc_tm_to_wday(struct rtc_time *tm)
> return (days + 4) % 7;
> }
>
> +static void bcd2tm(struct rtc_time *tm)
> +{
> + tm->tm_sec = bcd2bin(tm->tm_sec);
> + tm->tm_min = bcd2bin(tm->tm_min);
> + 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) - 1;
I guess this is the actual fix, I'm not sure creating the bcd2tm and
tm2bcd functions is useful, it obfuscates more than it helps.
> + /* epoch == 1900 */
> + tm->tm_year = bcd2bin(tm->tm_year) + 100;
Is it really the epoch of the RTC?
> +}
> +
> +static int tm2bcd(struct rtc_time *tm)
> +{
> + if (rtc_valid_tm(tm) != 0)
> + return -EINVAL;
> +
This will never fail, I'm not sure why you need to check here.
> + tm->tm_sec = bin2bcd(tm->tm_sec);
> + tm->tm_min = bin2bcd(tm->tm_min);
> + 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 + 1);
> + /* epoch == 1900 */
> + tm->tm_year = bin2bcd(tm->tm_year - 100);
> +
> + return 0;
> +}
> +
> static int rzn1_rtc_read_time(struct device *dev, struct rtc_time *tm)
> {
> struct rzn1_rtc *rtc = dev_get_drvdata(dev);
> @@ -106,14 +135,7 @@ static int rzn1_rtc_read_time(struct device *dev, struct rtc_time *tm)
> if (tm->tm_sec != secs)
> rzn1_rtc_get_time_snapshot(rtc, tm);
>
> - tm->tm_sec = bcd2bin(tm->tm_sec);
> - tm->tm_min = bcd2bin(tm->tm_min);
> - 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);
> -
> + bcd2tm(tm);
> return 0;
> }
>
> @@ -123,13 +145,8 @@ static int rzn1_rtc_set_time(struct device *dev, struct rtc_time *tm)
> u32 val;
> int ret;
>
> - tm->tm_sec = bin2bcd(tm->tm_sec);
> - tm->tm_min = bin2bcd(tm->tm_min);
> - 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);
> + if (tm2bcd(tm) < 0)
> + return -EINVAL;
>
> val = readl(rtc->base + RZN1_RTC_CTL2);
> if (!(val & RZN1_RTC_CTL2_STOPPED)) {
> --
> 2.25.1
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
next prev parent reply other threads:[~2022-07-26 15:33 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-06 12:07 [PATCH] rtc: rzn1: Fix RTC_RD_TIME: Invalid argument Biju Das
2022-07-26 15:33 ` Alexandre Belloni [this message]
2022-07-26 15:54 ` Biju Das
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YuAJLk8UIA0omk3N@mail.local \
--to=alexandre.belloni@bootlin.com \
--cc=Chris.Paterson2@renesas.com \
--cc=a.zummo@towertech.it \
--cc=biju.das.jz@bp.renesas.com \
--cc=biju.das@bp.renesas.com \
--cc=geert+renesas@glider.be \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=linux-rtc@vger.kernel.org \
--cc=michel.pollet@bp.renesas.com \
--cc=miquel.raynal@bootlin.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox