2010-08-12 21:27:34

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: rtc/m41t80: use rtc_valid_tm() to check returned tm

On Wed, Aug 11, 2010 at 18:59, Linux Kernel Mailing List
<[email protected]> wrote:
> Gitweb:     http://git.kernel.org/linus/b485fe5ea1008db02abff9ef15be4f31b52df4f7

>    rtc/m41t80: use rtc_valid_tm() to check returned tm
>
>    Use rtc_valid_tm() to check returned struct rtc_time *tm - it can avoid
>    returning wrong tm value.

> 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

> @@ -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);

warning: passing argument 1 of ‘rtc_valid_tm’ from incompatible pointer type

`t' is of type `struct rtc_wkalrm *', not `struct rtc_time *'.
Probably the intention was:

return rtc_valid_tm(&t->time);

??

>  }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds


2010-08-13 01:43:41

by Wan ZongShun

[permalink] [raw]
Subject: Re: rtc/m41t80: use rtc_valid_tm() to check returned tm

2010/8/13 Geert Uytterhoeven <[email protected]>:
> On Wed, Aug 11, 2010 at 18:59, Linux Kernel Mailing List
> <[email protected]> wrote:
>> Gitweb:     http://git.kernel.org/linus/b485fe5ea1008db02abff9ef15be4f31b52df4f7
>
>>    rtc/m41t80: use rtc_valid_tm() to check returned tm
>>
>>    Use rtc_valid_tm() to check returned struct rtc_time *tm - it can avoid
>>    returning wrong tm value.
>
>> 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
>
>> @@ -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);
>
> warning: passing argument 1 of ‘rtc_valid_tm’ from incompatible pointer type
>
> `t' is of type `struct rtc_wkalrm *', not `struct rtc_time *'.
> Probably the intention was:
>
>        return rtc_valid_tm(&t->time);

Okay, thank you point this warning.

Andrew ,

Could you please fix this warning directly for me?
'return rtc_valid_tm(t)' should be insteaded by 'return
rtc_valid_tm(&t->time);'.

thanks!


>
> ??
>
>>  }
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
>



--
*linux-arm-kernel mailing list
mail addr:[email protected]
you can subscribe by:
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

* linux-arm-NUC900 mailing list
mail addr:[email protected]
main web: https://groups.google.com/group/NUC900
you can subscribe it by sending me mail:
[email protected]

2010-08-17 14:50:59

by Atsushi Nemoto

[permalink] [raw]
Subject: Re: rtc/m41t80: use rtc_valid_tm() to check returned tm

On Fri, 13 Aug 2010 09:43:38 +0800, Wan ZongShun <[email protected]> wrote:
> 2010/8/13 Geert Uytterhoeven <[email protected]>:
> >> -       return 0;
> >> +       return rtc_valid_tm(t);
> >
> > warning: passing argument 1 of ‘rtc_valid_tm’ from incompatible pointer type
> >
> > `t' is of type `struct rtc_wkalrm *', not `struct rtc_time *'.
> > Probably the intention was:
> >
> >        return rtc_valid_tm(&t->time);
>
> Okay, thank you point this warning.
>
> Andrew ,
>
> Could you please fix this warning directly for me?
> 'return rtc_valid_tm(t)' should be insteaded by 'return
> rtc_valid_tm(&t->time);'.

No, t->time does not contain complete date/time. It just holds some
alarm settings and -1 for some fields.

So rtc_valid_tm(&t->time) will always returns -EINVAL.
The correct fix would be just reverting rtc_valid_tm here.

------------------------------------------------------
From: Atsushi Nemoto <[email protected]>
Subject: [PATCH] rtc: m41t80: do not use rtc_valid_tm in m41t80_rtc_read_alarm

The commit b485fe5ea ("rtc/m41t80: use rtc_valid_tm() to check
returned tm") added rtc_valid_tm to m41t80_rtc_read_alarm() but it was
wrong while the t->time does not contain complete date/time.

This patch also fix a warning:
warning: passing argument 1 of ‘rtc_valid_tm’ from incompatible pointer type

Signed-off-by: Atsushi Nemoto <[email protected]>
---
drivers/rtc/rtc-m41t80.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/rtc/rtc-m41t80.c b/drivers/rtc/rtc-m41t80.c
index 66377f3..d60557c 100644
--- a/drivers/rtc/rtc-m41t80.c
+++ b/drivers/rtc/rtc-m41t80.c
@@ -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 rtc_valid_tm(t);
+ return 0;
}

static struct rtc_class_ops m41t80_rtc_ops = {
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?