Re: [PATCH 000/100] rtc: remove cargo culted code

Alexandre Belloni
2018-03-02T08:57:01Z

On 02/03/2018 at 08:46:48 +0000, Steve Twiss wrote:
> On Wed, Feb 21, 2018 at 8:54 PM, Alexandre Belloni wrote:
>
> > subject: [PATCH 000/100] rtc: remove cargo culted code
> > mailing list: linux-kernel@vger.kernel.org Filter messages from this mailing list
> >
> > Hello,
> >
> > This series:
> > - removes useless calls to rtc_valid_tm in .read_time, .set_time and
> > .set_alarm
> > - removes code setting default values for RTCs (and lets the core
> > handle it)
> > - removes useless "time is invalid" messages at probe time
> > - removes useless indirect calls
> >
> > Those were mostly copy pasted from other drivers
>
> Hi Alexandre,
>
> Acked for:
> rtc: da9063: stop validating rtc_time in .read_time
> rtc: da9052: stop validating rtc_time in .read_time
> rtc: da9055: stop validating rtc_time in .read_time
>
> Acked-by: Steve Twiss <stwiss.opensource@diasemi.com>
>
> Agreed -- rtc_valid_tm() call is cargo cult for the above.
>
> (By definition) for DA9063 I was trying to be rigorous.
> The .read_time function is slightly different here because I can make a copy the alarm time into the RTC time
> structure to solve an RTC synchronisation problem internally to the DA9063.
> https://elixir.bootlin.com/linux/v4.5.6/source/drivers/rtc/rtc-da9063.c#L253
>
> But after some further looking, I have not got any explicit case of how the time read directly from the DA9063
> registers can be incorrectly represented. So there should be no need to check this.
>

My point is that it is checked later in the core anyway so you end up
doing:

da9063_rtc_read_time()
return rtc_valid_tm(tm);

__rtc_read_time()
if (err < 0)
return err;
err = rtc_valid_tm(tm);

return err;

So the check in da9063_rtc_read_time is always pointless.

--
Alexandre Belloni, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering