linux-kernel - Re: [PATCH v10 1/3] RTC: RK808: add RTC driver for RK808

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAD=FV=VPgU9YwoAecGAVcEe8H2YaA=vPZJUwnjmzhFCZxBXuqw@mail.gmail.com>
Date:	Wed, 10 Sep 2014 17:10:36 -0700
From:	Doug Anderson <dianders@...omium.org>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Chris Zhong <zyw@...k-chips.com>,
	Heiko Stübner <heiko@...ech.de>,
	Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Samuel Ortiz <sameo@...ux.intel.com>,
	Lee Jones <lee.jones@...aro.org>,
	Liam Girdwood <lgirdwood@...il.com>,
	Alessandro Zummo <a.zummo@...ertech.it>,
	Mike Turquette <mturquette@...aro.org>,
	"broonie@...nel.org" <broonie@...nel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	rtc-linux@...glegroups.com, Grant Likely <grant.likely@...aro.org>,
	Lin Huang <hl@...k-chips.com>,
	Tao Huang <huangtao@...k-chips.com>,
	Eddie Cai <cf@...k-chips.com>,
	zhangqing <zhangqing@...k-chips.com>, xxx <xxx@...k-chips.com>,
	Olof Johansson <olof@...om.net>,
	Sonny Rao <sonnyrao@...omium.org>,
	Dmitry Torokhov <dtor@...omium.org>,
	Javier Martinez Canillas <javier.martinez@...labora.co.uk>,
	Kever Yang <kever.yang@...k-chips.com>
Subject: Re: [PATCH v10 1/3] RTC: RK808: add RTC driver for RK808

Andrew,

On Wed, Sep 10, 2014 at 3:08 PM, Andrew Morton
<akpm@...ux-foundation.org> wrote:
> On Wed, 10 Sep 2014 14:37:13 -0700 Doug Anderson <dianders@...omium.org> wrote:
>
>> Andrew,
>>
>> On Wed, Sep 10, 2014 at 1:44 PM, Andrew Morton
>> <akpm@...ux-foundation.org> wrote:
>> > On Wed, 10 Sep 2014 09:18:04 +0800 Chris Zhong <zyw@...k-chips.com> 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 linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/