Re: [PATCH 1/3] hwmon: Add Texas Instruments TMP108 temperature sensor driver.

From	John Muir <john@jmuir.com>
Newsgroups	linux.kernel
Subject	Re: [PATCH 1/3] hwmon: Add Texas Instruments TMP108 temperature sensor driver.
Date	2016-11-29 00:20 +0100
Message-ID	<sIDnj-aD-17@gated-at.bofh.it> (permalink)
References	(1 earlier) <sI6KJ-4eR-1@gated-at.bofh.it> <sIA66-6pe-27@gated-at.bofh.it> <sIApt-6LC-57@gated-at.bofh.it> <sIBES-7sc-21@gated-at.bofh.it> <sICrf-82A-5@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

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.

The update_ready_time function was only intended to be called AFTER the device has moved to a new mode. For the probe case I simply ignored the fact that it was already in continuous mode at startup and lazily re-used the update_ready_time function causing the code to run through a few extra instructions. I was attempting to re-use the logic in multiple cases.

I’ll update the code in my next patch series and you can re-review.

> 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.
> 

OK. Noted.

Thanks,

John.

Thread

[PATCH 1/3] hwmon: Add Texas Instruments TMP108 temperature sensor driver. John Muir <john@jmuir.com> - 2016-11-27 06:20 +0100
  [PATCH 2/3] hwmon: tmp108: Use devm variants of registration interfaces. John Muir <john@jmuir.com> - 2016-11-27 06:20 +0100
    Re: [PATCH 2/3] hwmon: tmp108: Use devm variants of registration  interfaces. Guenter Roeck <linux@roeck-us.net> - 2016-11-27 13:30 +0100
  Re: [PATCH 1/3] hwmon: Add Texas Instruments TMP108 temperature  sensor driver. Guenter Roeck <linux@roeck-us.net> - 2016-11-27 13:30 +0100
    Re: [PATCH 1/3] hwmon: Add Texas Instruments TMP108 temperature  sensor driver. John Muir <john@jmuir.com> - 2016-11-28 20:50 +0100
      Re: [PATCH 1/3] hwmon: Add Texas Instruments TMP108 temperature  sensor driver. Guenter Roeck <linux@roeck-us.net> - 2016-11-28 21:10 +0100
        Re: [PATCH 1/3] hwmon: Add Texas Instruments TMP108 temperature  sensor driver. John Muir <john@jmuir.com> - 2016-11-28 22:30 +0100
          Re: [PATCH 1/3] hwmon: Add Texas Instruments TMP108 temperature  sensor driver. Guenter Roeck <linux@roeck-us.net> - 2016-11-28 23:20 +0100
            Re: [PATCH 1/3] hwmon: Add Texas Instruments TMP108 temperature  sensor driver. John Muir <john@jmuir.com> - 2016-11-29 00:20 +0100
              Re: [PATCH 1/3] hwmon: Add Texas Instruments TMP108 temperature  sensor driver. Guenter Roeck <linux@roeck-us.net> - 2016-11-29 00:40 +0100

csiph-web