Andrew,
On Wed, Sep 10, 2014 at 3:08 PM, Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Wed, 10 Sep 2014 14:37:13 -0700 Doug Anderson <dianders@xxxxxxxxxxxx> wrote:
>
>> Andrew,
>>
>> On Wed, Sep 10, 2014 at 1:44 PM, Andrew Morton
>> <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>> > On Wed, 10 Sep 2014 09:18:04 +0800 Chris Zhong <zyw@xxxxxxxxxxxxxx> wrote:
>> >
>> >> Adding RTC driver for supporting RTC device present inside RK808 PMIC.
>> >>
>> >> ...
>> >>
>> >> + ret = rtc_valid_tm(&tm);
>> >> + if (ret) {
>> >> + dev_warn(&pdev->dev, "invalid date/time and init time\n");
>> >> + rk808_rtc_set_time(&pdev->dev, &tm_def);
>> >> + }
>> >
>> > This is somewhat unusual. Most drivers will emit a warning and give up
>> > when they find the time is wrong. Why is this driver different and is
>> > this desirable behaviour?
>>
>> When you say "give up", what does that mean? I assume the driver
>> should keep initting, right? Then the user can go in and set a time
>> later...
>
> I think I was misreading current drivers a bit. rtc-cmos.c will go in
> and set a dummy time but regular low-level drivers don't sanity-check
> the time at all at setup time.
>
>>
>> I did test things with just removing this chunk of code. You get some
>> yells at bootup if you put a bogus time in there:
>>
>> [ 2.987590] rk808-rtc rk808-rtc: invalid date/time and init time
>> [ 3.013148] rk808-rtc rk808-rtc: rtc core: registered rk808-rtc as rtc0
>> [ 4.586115] rk808-rtc rk808-rtc: hctosys: invalid date/time
>>
>> ...but if you later set a valid time then everything is fine. That
>> seems reasonable behavior to me, so I guess we could just remove this
>> whole chunk? It appears that after a normal bootup the date/time is
>> something valid.
>
> hm. Having an invalid time is perhaps better than having a valid but
> incorrect time.
OK. It's an easy patch to remove this. Chris: do you want to test
that too and post it, or do you want me to? Basically just remove:
rk808_rtc_set_time(&pdev->dev, &tm_def);
...and of course then remove the tm_def structure and take braces off
the "if". I think everything works if you do that. We need to make
sure to base on the patch Andrew talked about about making the
structure static (the new patch would remove the structure completely,
but nice not to get a merge conflict).
-Doug
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
