Patchwork [4/5] rtc: rtc-s3c: Fix on RTC initialization method

login
register
mail settings
Submitter Kukjin Kim
Date Sept. 7, 2010, 5:29 a.m.
Message ID <1283837381-9575-5-git-send-email-kgene.kim@samsung.com>
Download mbox | patch
Permalink /patch/63971/
State New
Headers show

Comments

Kukjin Kim - Sept. 7, 2010, 5:29 a.m.
From: Changhwan Youn <chaos.youn@samsung.com>

This patch changes RTC initialization method on probe() as
per Wan ZongShun's suggestion. The 'rtc_valid_tm(tm)' can
check whether RTC BCD is valid or not.

And should be changed the method of check because previous
method cannot validate RTC BCD registers properly.

Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
Cc: Ben Dooks <ben-linux@fluff.org>
Cc: Wan ZongShun <mcuos.com@gmail.com>
---
 drivers/rtc/rtc-s3c.c |   16 +++++++++++-----
 1 files changed, 11 insertions(+), 5 deletions(-)
Kyungmin Park - Sept. 7, 2010, 8:56 a.m.
On Tue, Sep 7, 2010 at 2:29 PM, Kukjin Kim <kgene.kim@samsung.com> wrote:
> From: Changhwan Youn <chaos.youn@samsung.com>
>
> This patch changes RTC initialization method on probe() as
> per Wan ZongShun's suggestion. The 'rtc_valid_tm(tm)' can
> check whether RTC BCD is valid or not.
>
> And should be changed the method of check because previous
> method cannot validate RTC BCD registers properly.
>
> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> Cc: Ben Dooks <ben-linux@fluff.org>
> Cc: Wan ZongShun <mcuos.com@gmail.com>
> ---
>  drivers/rtc/rtc-s3c.c |   16 +++++++++++-----
>  1 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
> index c078548..7f15073 100644
> --- a/drivers/rtc/rtc-s3c.c
> +++ b/drivers/rtc/rtc-s3c.c
> @@ -458,8 +458,8 @@ static int __devexit s3c_rtc_remove(struct platform_device *dev)
>  static int __devinit s3c_rtc_probe(struct platform_device *pdev)
>  {
>        struct rtc_device *rtc;
> +       struct rtc_time rtc_tm;
>        struct resource *res;
> -       unsigned int tmp, i;
>        int ret;
>
>        pr_debug("%s: probe=%p\n", __func__, pdev);
> @@ -540,11 +540,17 @@ static int __devinit s3c_rtc_probe(struct platform_device *pdev)
>
>        /* Check RTC Time */
>
> -       for (i = S3C2410_RTCSEC; i <= S3C2410_RTCYEAR; i += 0x4) {
> -               tmp = readb(s3c_rtc_base + i);
> +       s3c_rtc_gettime(NULL, &rtc_tm);
>
> -               if ((tmp & 0xf) > 0x9 || ((tmp >> 4) & 0xf) > 0x9)
> -                       writeb(0, s3c_rtc_base + i);
> +       if (rtc_valid_tm(&rtc_tm)) {
instead of !rtc_valid_tm(&rtc_tm) ???
> +               rtc_tm.tm_year  = 100;
> +               rtc_tm.tm_mon   = 0;
> +               rtc_tm.tm_mday  = 1;
> +               rtc_tm.tm_hour  = 0;
> +               rtc_tm.tm_min   = 0;
> +               rtc_tm.tm_sec   = 0;
> +
> +               s3c_rtc_settime(NULL, &rtc_tm);
>        }
>
>        if (s3c_rtc_cpu_type == TYPE_S3C64XX)
> --
> 1.6.2.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Ben Dooks - Sept. 7, 2010, 11:24 p.m.
On 07/09/10 06:29, Kukjin Kim wrote:
> From: Changhwan Youn <chaos.youn@samsung.com>
> 
> This patch changes RTC initialization method on probe() as
> per Wan ZongShun's suggestion. The 'rtc_valid_tm(tm)' can
> check whether RTC BCD is valid or not.
> 
> And should be changed the method of check because previous
> method cannot validate RTC BCD registers properly.
> 
> Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> Cc: Ben Dooks <ben-linux@fluff.org>
> Cc: Wan ZongShun <mcuos.com@gmail.com>
> ---
>  drivers/rtc/rtc-s3c.c |   16 +++++++++++-----
>  1 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
> index c078548..7f15073 100644
> --- a/drivers/rtc/rtc-s3c.c
> +++ b/drivers/rtc/rtc-s3c.c
> @@ -458,8 +458,8 @@ static int __devexit s3c_rtc_remove(struct platform_device *dev)
>  static int __devinit s3c_rtc_probe(struct platform_device *pdev)
>  {
>  	struct rtc_device *rtc;
> +	struct rtc_time rtc_tm;
>  	struct resource *res;
> -	unsigned int tmp, i;
>  	int ret;
>  
>  	pr_debug("%s: probe=%p\n", __func__, pdev);
> @@ -540,11 +540,17 @@ static int __devinit s3c_rtc_probe(struct platform_device *pdev)
>  
>  	/* Check RTC Time */
>  
> -	for (i = S3C2410_RTCSEC; i <= S3C2410_RTCYEAR; i += 0x4) {
> -		tmp = readb(s3c_rtc_base + i);
> +	s3c_rtc_gettime(NULL, &rtc_tm);
>  
> -		if ((tmp & 0xf) > 0x9 || ((tmp >> 4) & 0xf) > 0x9)
> -			writeb(0, s3c_rtc_base + i);
> +	if (rtc_valid_tm(&rtc_tm)) {
> +		rtc_tm.tm_year	= 100;
> +		rtc_tm.tm_mon	= 0;
> +		rtc_tm.tm_mday	= 1;
> +		rtc_tm.tm_hour	= 0;
> +		rtc_tm.tm_min	= 0;
> +		rtc_tm.tm_sec	= 0;
> +
> +		s3c_rtc_settime(NULL, &rtc_tm);

I think a dev_warn() in this path is good to alert the user
to bad things happening, esp if the system should have a valid
time, it marks he posibility the rtc battery has gone.
Kukjin Kim - Sept. 9, 2010, 1:04 a.m.
Ben Dooks wrote:
> 
> On 07/09/10 06:29, Kukjin Kim wrote:
> > From: Changhwan Youn <chaos.youn@samsung.com>
> >
> > This patch changes RTC initialization method on probe() as
> > per Wan ZongShun's suggestion. The 'rtc_valid_tm(tm)' can
> > check whether RTC BCD is valid or not.
> >
> > And should be changed the method of check because previous
> > method cannot validate RTC BCD registers properly.
> >
> > Signed-off-by: Changhwan Youn <chaos.youn@samsung.com>
> > Signed-off-by: Kukjin Kim <kgene.kim@samsung.com>
> > Cc: Ben Dooks <ben-linux@fluff.org>
> > Cc: Wan ZongShun <mcuos.com@gmail.com>
> > ---
> >  drivers/rtc/rtc-s3c.c |   16 +++++++++++-----
> >  1 files changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
> > index c078548..7f15073 100644
> > --- a/drivers/rtc/rtc-s3c.c
> > +++ b/drivers/rtc/rtc-s3c.c
> > @@ -458,8 +458,8 @@ static int __devexit s3c_rtc_remove(struct
> platform_device *dev)
> >  static int __devinit s3c_rtc_probe(struct platform_device *pdev)
> >  {
> >  	struct rtc_device *rtc;
> > +	struct rtc_time rtc_tm;
> >  	struct resource *res;
> > -	unsigned int tmp, i;
> >  	int ret;
> >
> >  	pr_debug("%s: probe=%p\n", __func__, pdev);
> > @@ -540,11 +540,17 @@ static int __devinit s3c_rtc_probe(struct
> platform_device *pdev)
> >
> >  	/* Check RTC Time */
> >
> > -	for (i = S3C2410_RTCSEC; i <= S3C2410_RTCYEAR; i += 0x4) {
> > -		tmp = readb(s3c_rtc_base + i);
> > +	s3c_rtc_gettime(NULL, &rtc_tm);
> >
> > -		if ((tmp & 0xf) > 0x9 || ((tmp >> 4) & 0xf) > 0x9)
> > -			writeb(0, s3c_rtc_base + i);
> > +	if (rtc_valid_tm(&rtc_tm)) {
> > +		rtc_tm.tm_year	= 100;
> > +		rtc_tm.tm_mon	= 0;
> > +		rtc_tm.tm_mday	= 1;
> > +		rtc_tm.tm_hour	= 0;
> > +		rtc_tm.tm_min	= 0;
> > +		rtc_tm.tm_sec	= 0;
> > +
> > +		s3c_rtc_settime(NULL, &rtc_tm);
> 
> I think a dev_warn() in this path is good to alert the user
> to bad things happening, esp if the system should have a valid
> time, it marks he posibility the rtc battery has gone.

Ok...will address comment from you.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <kgene.kim@samsung.com>, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

Patch

diff --git a/drivers/rtc/rtc-s3c.c b/drivers/rtc/rtc-s3c.c
index c078548..7f15073 100644
--- a/drivers/rtc/rtc-s3c.c
+++ b/drivers/rtc/rtc-s3c.c
@@ -458,8 +458,8 @@  static int __devexit s3c_rtc_remove(struct platform_device *dev)
 static int __devinit s3c_rtc_probe(struct platform_device *pdev)
 {
 	struct rtc_device *rtc;
+	struct rtc_time rtc_tm;
 	struct resource *res;
-	unsigned int tmp, i;
 	int ret;
 
 	pr_debug("%s: probe=%p\n", __func__, pdev);
@@ -540,11 +540,17 @@  static int __devinit s3c_rtc_probe(struct platform_device *pdev)
 
 	/* Check RTC Time */
 
-	for (i = S3C2410_RTCSEC; i <= S3C2410_RTCYEAR; i += 0x4) {
-		tmp = readb(s3c_rtc_base + i);
+	s3c_rtc_gettime(NULL, &rtc_tm);
 
-		if ((tmp & 0xf) > 0x9 || ((tmp >> 4) & 0xf) > 0x9)
-			writeb(0, s3c_rtc_base + i);
+	if (rtc_valid_tm(&rtc_tm)) {
+		rtc_tm.tm_year	= 100;
+		rtc_tm.tm_mon	= 0;
+		rtc_tm.tm_mday	= 1;
+		rtc_tm.tm_hour	= 0;
+		rtc_tm.tm_min	= 0;
+		rtc_tm.tm_sec	= 0;
+
+		s3c_rtc_settime(NULL, &rtc_tm);
 	}
 
 	if (s3c_rtc_cpu_type == TYPE_S3C64XX)