lkml.org 
[lkml]   [2016]   [Nov]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/3] hwmon: Add Texas Instruments TMP108 temperature sensor driver.
On Mon, Nov 28, 2016 at 03:10:38PM -0800, John Muir wrote:
> On Nov 28, 2016, at 2:19 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > 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.
>
> Note that the TMP102 code is looking at ‘config_orig’ which was the initial device state, whereas in my proposed driver the code looks at the current configuration.
>
Ah, yes. Your code also uses tmp108->config in tmp108_restore_config(),
which I guess confused me.

Not sure I understand why you would want to add the wait time even if M1
was set before (and the tmp102 driver doesn't do that). Really, if that
is what you want, it would be much simpler if you just unconditionally
set ready_time to 30 ms after the current time after setting the M1 bit,
no matter if it was set before or not. I personally don't think that would
be such a good idea, but it would at least be less obfuscating than the
current code. Also, I would suggest to either drop tmp108_restore_config(),
or to have it restore the real original state.

Thanks,
Guenter

\
 
 \ /
  Last update: 2016-11-29 00:34    [W:0.033 / U:1.268 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site