Patchwork RTC/m41t80: use rtc_valid_tm to check returning tm

login
register
mail settings
Submitter Wan ZongShun
Date June 22, 2010, 8:35 a.m.
Message ID <4C2075B5.3040700@gmail.com>
Download mbox | patch
Permalink /patch/56413/
State New
Headers show

Comments

Wan ZongShun - June 22, 2010, 8:35 a.m.
This patch is to use rtc_valid_tm for checking
returning struct rtc_time *tm, it can avoid returning
wrong tm value.

Signed-off-by: Wan ZongShun <mcuos.com@gmail.com>

---
 drivers/rtc/rtc-m41t80.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
Andrew Morton - June 22, 2010, 9:17 p.m.
On Tue, 22 Jun 2010 16:35:01 +0800
Wan ZongShun <mcuos.com@gmail.com> wrote:

> This patch is to use rtc_valid_tm for checking
> returning struct rtc_time *tm, it can avoid returning
> wrong tm value.
> 
> Signed-off-by: Wan ZongShun <mcuos.com@gmail.com>
> 
> ---
>  drivers/rtc/rtc-m41t80.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
> index 6dc4e62..66377f3 100644
> --- a/drivers/rtc/rtc-m41t80.c
> +++ b/drivers/rtc/rtc-m41t80.c
> @@ -121,7 +121,7 @@ static int m41t80_get_datetime(struct i2c_client *client,
> 
>  	/* assume 20YY not 19YY, and ignore the Century Bit */
>  	tm->tm_year = bcd2bin(buf[M41T80_REG_YEAR]) + 100;
> -	return 0;
> +	return rtc_valid_tm(tm);
>  }

OK.  But there's a not a lot of point in returning an error code if
the callers forget to check it.  And m41t80_probe() ignores the
m41t80_get_datetime() return value.


>  /* Sets the given date and time to the real time clock. */
> @@ -364,7 +364,7 @@ static int m41t80_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *t)
>  	t->time.tm_isdst = -1;
>  	t->enabled = !!(reg[M41T80_REG_ALARM_MON] & M41T80_ALMON_AFE);
>  	t->pending = !!(reg[M41T80_REG_FLAGS] & M41T80_FLAGS_AF);
> -	return 0;
> +	return rtc_valid_tm(t);
>  }
> 
>  static struct rtc_class_ops m41t80_rtc_ops = {
Wan ZongShun - June 23, 2010, 2:51 a.m.
2010/6/23 Andrew Morton <akpm@linux-foundation.org>:
> On Tue, 22 Jun 2010 16:35:01 +0800
> Wan ZongShun <mcuos.com@gmail.com> wrote:
>
>> This patch is to use rtc_valid_tm for checking
>> returning struct rtc_time *tm, it can avoid returning
>> wrong tm value.
>>
>> Signed-off-by: Wan ZongShun <mcuos.com@gmail.com>
>>
>> ---
>>  drivers/rtc/rtc-m41t80.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
>> index 6dc4e62..66377f3 100644
>> --- a/drivers/rtc/rtc-m41t80.c
>> +++ b/drivers/rtc/rtc-m41t80.c
>> @@ -121,7 +121,7 @@ static int m41t80_get_datetime(struct i2c_client *client,
>>
>>       /* assume 20YY not 19YY, and ignore the Century Bit */
>>       tm->tm_year = bcd2bin(buf[M41T80_REG_YEAR]) + 100;
>> -     return 0;
>> +     return rtc_valid_tm(tm);
>>  }
>
> OK.  But there's a not a lot of point in returning an error code if
> the callers forget to check it.  And m41t80_probe() ignores the
> m41t80_get_datetime() return value.
>

Also, I notice that, seriously, it should use 'm41t80_rtc_read_time'
instead of 'm41t80_get_datetime()',
but I think this function here only for ' dev_info' use. It seems not
to make any sense to check returning value, right?

>
>>  /* Sets the given date and time to the real time clock. */
>> @@ -364,7 +364,7 @@ static int m41t80_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *t)
>>       t->time.tm_isdst = -1;
>>       t->enabled = !!(reg[M41T80_REG_ALARM_MON] & M41T80_ALMON_AFE);
>>       t->pending = !!(reg[M41T80_REG_FLAGS] & M41T80_FLAGS_AF);
>> -     return 0;
>> +     return rtc_valid_tm(t);
>>  }
>>
>>  static struct rtc_class_ops m41t80_rtc_ops = {
>
>

Patch

diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
index 6dc4e62..66377f3 100644
--- a/drivers/rtc/rtc-m41t80.c
+++ b/drivers/rtc/rtc-m41t80.c
@@ -121,7 +121,7 @@  static int m41t80_get_datetime(struct i2c_client *client,

 	/* assume 20YY not 19YY, and ignore the Century Bit */
 	tm->tm_year = bcd2bin(buf[M41T80_REG_YEAR]) + 100;
-	return 0;
+	return rtc_valid_tm(tm);
 }

 /* Sets the given date and time to the real time clock. */
@@ -364,7 +364,7 @@  static int m41t80_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *t)
 	t->time.tm_isdst = -1;
 	t->enabled = !!(reg[M41T80_REG_ALARM_MON] & M41T80_ALMON_AFE);
 	t->pending = !!(reg[M41T80_REG_FLAGS] & M41T80_FLAGS_AF);
-	return 0;
+	return rtc_valid_tm(t);
 }

 static struct rtc_class_ops m41t80_rtc_ops = {