All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rtc: rzn1: Fix RTC_RD_TIME: Invalid argument
@ 2022-07-06 12:07 Biju Das
  2022-07-26 15:33 ` Alexandre Belloni
  0 siblings, 1 reply; 3+ messages in thread
From: Biju Das @ 2022-07-06 12:07 UTC (permalink / raw)
  To: Miquel Raynal, Alessandro Zummo, Alexandre Belloni
  Cc: Biju Das, Michel Pollet, linux-rtc, linux-renesas-soc,
	Geert Uytterhoeven, Chris Paterson, Biju Das

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;
+	/* epoch == 1900 */
+	tm->tm_year = bcd2bin(tm->tm_year) + 100;
+}
+
+static int tm2bcd(struct rtc_time *tm)
+{
+	if (rtc_valid_tm(tm) != 0)
+		return -EINVAL;
+
+	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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] rtc: rzn1: Fix RTC_RD_TIME: Invalid argument
  2022-07-06 12:07 [PATCH] rtc: rzn1: Fix RTC_RD_TIME: Invalid argument Biju Das
@ 2022-07-26 15:33 ` Alexandre Belloni
  2022-07-26 15:54   ` Biju Das
  0 siblings, 1 reply; 3+ messages in thread
From: Alexandre Belloni @ 2022-07-26 15:33 UTC (permalink / raw)
  To: Biju Das
  Cc: Miquel Raynal, Alessandro Zummo, Michel Pollet, linux-rtc,
	linux-renesas-soc, Geert Uytterhoeven, Chris Paterson, Biju Das

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH] rtc: rzn1: Fix RTC_RD_TIME: Invalid argument
  2022-07-26 15:33 ` Alexandre Belloni
@ 2022-07-26 15:54   ` Biju Das
  0 siblings, 0 replies; 3+ messages in thread
From: Biju Das @ 2022-07-26 15:54 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: Miquel Raynal, Alessandro Zummo, Michel Pollet, linux-rtc,
	linux-renesas-soc, Geert Uytterhoeven, Chris Paterson, Biju Das

Hi Alexandre Belloni,

> Subject: Re: [PATCH] rtc: rzn1: Fix RTC_RD_TIME: Invalid argument
> 
> 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]
> > ---
> >  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?

I guess so When I debugged last time, With the current code,
HW registers are entering into a wrong state where day =32, when it reaches 23:59:59
and month and year are not propagated to next level.

With the patch I submitted, it never entered to this wrong state.

> 
> > +}
> > +
> > +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.

Currently I don't have a board to recheck this.

Cheers,
Biju

> 
> > +	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
> >
> 
> --


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-07-26 15:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-06 12:07 [PATCH] rtc: rzn1: Fix RTC_RD_TIME: Invalid argument Biju Das
2022-07-26 15:33 ` Alexandre Belloni
2022-07-26 15:54   ` Biju Das

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.