On Mon, Nov 28, 2016 at 01:25:37PM -0800, John Muir wrote:
>
> > On Nov 28, 2016, at 11:58 AM, Guenter Roeck <li...@roeck-us.net> wrote:
> >
> > On Mon, Nov 28, 2016 at 11:40:42AM -0800, John Muir wrote:
> >>>> +static void tmp108_update_ready_time(struct tmp108 *tmp108)
> >>>> +{
> >>>> + tmp108->ready_time = jiffies;
> >>>> + if ((tmp108->config & TMP108_CONF_MODE_MASK)
> >>>> + == TMP108_MODE_CONTINUOUS) {
> >>>
> >>> Don't you want a "!" here ? Presumably the delay is only needed
> >>> if the original configuration was not for continuous mode.
> >>>
> >>>> + tmp108->ready_time +=
> >>>> + msecs_to_jiffies(TMP108_CONVERSION_TIME_MS);
> >>>> + }
> >>>> +}
> >> The delay is required for both really. When the device is set into
> >> continuous mode it starts converting and the first temperature is ready
> >> (just under) 30ms later.
> >>
> >
> > The datasheet states, though, that the chip will start converting as soon
> > as the device powers up. The kernel would have to start really fast after
> > that
> > to have to wait another 30 ms.
> >
> > The current code ends up waiting if it doesn't have to (because it waits
> > if the chip was originally configured for continuous mode), and not waiting
> > if it has to (because the chip was not configured for continuous mode),
> > which doesn't seem to be such a good idea.
>
> You are referring to the statement under “Conversion Rate” in the data sheet?
>
> "After power-up or a general-call reset, the TMP108 immediately starts a
> conversion, as shown in Figure 9. The first result is available after 27 ms
> (typical).”
>
> The datasheet also states that in ’shutdown’ mode, the device is only
> powering the serial interface (see "Mode Bits”). So, I assumed that the
> conversions weren’t happening during in that state, and that there would be
> the delay after moving from the ‘shutdown' state (as is described by the
> one-shot mode as well where the device state returns to ’shutdown’ when the
> conversion has taken place).
The device is, after power-on, not in shutdown mode.
>
> My intention was that the delay is enforced after PM resume, and for
> convenience I re-used the same function during probe to initialize the
> ready_time. I was assuming that the device could be in the shutdown state -
> for example if the module had previously been unloaded. I agree that this is
> not required at system startup time. Perhaps I should check to see if the
> previous configuration had the device in the ‘shutdown’ state, and in that
> instance apply the delay? In my opinion it doesn’t really matter either way.
>
> FYI, this same logic exists in the TMP102 device driver. The TMP102 device is
> very similar.
>
The tmp102 driver adds the timeout if the device was in shutdown mode (SD=1).
if (tmp102->config_orig & TMP102_CONF_SD) {
...
tmp102->ready_time += msecs_to_jiffies(CONVERSION_TIME_MS);
}
Your code adds the timeout if the device was in continuous operation mode
(M1=1).
if ((tmp108->config & TMP108_CONF_MODE_MASK)
== TMP108_MODE_CONTINUOUS) {
tmp108->ready_time +=
msecs_to_jiffies(TMP108_CONVERSION_TIME_MS);
}
Unless I am missing something, that is exactly the opposite.
Side note: Per datasheet, M0 is irrelevant if M1=1. The above check does not
match M1=1, M0=1, but that condition would still reflect continuous mode.
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
