Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1281179 > unrolled thread

Re: [PATCH 6/9] rtc-pcf2123: avoid resetting the clock if possible

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 6/9] rtc-pcf2123: avoid resetting the clock if possible Joshua Clayton <stillcompiling@gmail.com> - 2015-12-01 21:30 +0100
    Re: [PATCH 6/9] rtc-pcf2123: avoid resetting the clock if possible Alexandre Belloni <alexandre.belloni@free-electrons.com> - 2015-12-01 22:10 +0100

#1281179 — Re: [PATCH 6/9] rtc-pcf2123: avoid resetting the clock if possible

On Wed, 25 Nov 2015 00:25:12 +0100
Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:

> On 04/11/2015 at 07:36:37 -0800, Joshua Clayton wrote :
> > +	ret = pcf2123_rtc_read_time(dev, &tm);
> > +	if (ret < 0)
> > +		return false;
> > +
> > +	if (rtc_valid_tm(&tm) < 0) {
> > +		dev_err(dev, "retrieved date/time is not
> > valid.\n");
> > +		return false;
> > +	}
> > +
> 
> I would remove that test as basically, the date/time will only be
> valid when OSC_HAS_STOPPED is not set.
> 
OSC_HAS_STOPPED really only protects us in case everything else
looks good, but we've lost power.
There are other reasons to check for a valid time.
Specifically, if there is no communication with the device, the spi
operation will succeed and return all zeros or all ones.
Since either of these results in an invalid time, it is a nice way
to probe whether we really have a pcf2123 compatible device.

I don't think I should remove this test, but I can add a comment
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1281219

On 01/12/2015 at 12:23:51 -0800, Joshua Clayton wrote :
> On Wed, 25 Nov 2015 00:25:12 +0100
> Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote:
> 
> > On 04/11/2015 at 07:36:37 -0800, Joshua Clayton wrote :
> > > +	ret = pcf2123_rtc_read_time(dev, &tm);
> > > +	if (ret < 0)
> > > +		return false;
> > > +
> > > +	if (rtc_valid_tm(&tm) < 0) {
> > > +		dev_err(dev, "retrieved date/time is not
> > > valid.\n");
> > > +		return false;
> > > +	}
> > > +
> > 
> > I would remove that test as basically, the date/time will only be
> > valid when OSC_HAS_STOPPED is not set.
> > 
> OSC_HAS_STOPPED really only protects us in case everything else
> looks good, but we've lost power.
> There are other reasons to check for a valid time.
> Specifically, if there is no communication with the device, the spi
> operation will succeed and return all zeros or all ones.
> Since either of these results in an invalid time, it is a nice way
> to probe whether we really have a pcf2123 compatible device.
> 
> I don't think I should remove this test, but I can add a comment

OK but then pcf2123_rtc_read_time actually returns rtc_valid_tm(tm) ;)

The proper course of action is probably to do the OSC_HAS_STOPPED check
in pcf2123_rtc_read_time then you don't even need pcf2123_time_valid().

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web