w1: w1_therm: fix locking behavior in convert_t
authorStefan Wahren <stefan.wahren@i2se.com>
Thu, 27 Apr 2023 11:21:52 +0000 (13:21 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 19 Jul 2023 14:21:48 +0000 (16:21 +0200)
[ Upstream commit dca5480ab7b77a889088ab7cac81934604510ac7 ]

The commit 67b392f7b8ed ("w1_therm: optimizing temperature read timings")
accidentially inverted the logic for lock handling of the bus mutex.

Before:
  pullup -> release lock before sleep
  no pullup -> release lock after sleep

After:
  pullup -> release lock after sleep
  no pullup -> release lock before sleep

This cause spurious measurements of 85 degree (powerup value) on the
Tarragon board with connected 1-w temperature sensor
(w1_therm.w1_strong_pull=0).

In the meantime a new feature for polling the conversion
completion has been integrated in these branches with
commit 021da53e65fd ("w1: w1_therm: Add sysfs entries to control
conversion time and driver features"). But this feature isn't
available for parasite power mode, so handle this separately.

Link: https://lore.kernel.org/regressions/2023042645-attentive-amends-7b0b@gregkh/T/
Fixes: 67b392f7b8ed ("w1_therm: optimizing temperature read timings")
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
Link: https://lore.kernel.org/r/20230427112152.12313-1-stefan.wahren@i2se.com
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
drivers/w1/slaves/w1_therm.c

index 0676926..99c58bd 100644 (file)
@@ -1159,29 +1159,26 @@ static int convert_t(struct w1_slave *sl, struct therm_info *info)
 
                        w1_write_8(dev_master, W1_CONVERT_TEMP);
 
-                       if (strong_pullup) { /*some device need pullup */
+                       if (SLAVE_FEATURES(sl) & W1_THERM_POLL_COMPLETION) {
+                               ret = w1_poll_completion(dev_master, W1_POLL_CONVERT_TEMP);
+                               if (ret) {
+                                       dev_dbg(&sl->dev, "%s: Timeout\n", __func__);
+                                       goto mt_unlock;
+                               }
+                               mutex_unlock(&dev_master->bus_mutex);
+                       } else if (!strong_pullup) { /*no device need pullup */
                                sleep_rem = msleep_interruptible(t_conv);
                                if (sleep_rem != 0) {
                                        ret = -EINTR;
                                        goto mt_unlock;
                                }
                                mutex_unlock(&dev_master->bus_mutex);
-                       } else { /*no device need pullup */
-                               if (SLAVE_FEATURES(sl) & W1_THERM_POLL_COMPLETION) {
-                                       ret = w1_poll_completion(dev_master, W1_POLL_CONVERT_TEMP);
-                                       if (ret) {
-                                               dev_dbg(&sl->dev, "%s: Timeout\n", __func__);
-                                               goto mt_unlock;
-                                       }
-                                       mutex_unlock(&dev_master->bus_mutex);
-                               } else {
-                                       /* Fixed delay */
-                                       mutex_unlock(&dev_master->bus_mutex);
-                                       sleep_rem = msleep_interruptible(t_conv);
-                                       if (sleep_rem != 0) {
-                                               ret = -EINTR;
-                                               goto dec_refcnt;
-                                       }
+                       } else { /*some device need pullup */
+                               mutex_unlock(&dev_master->bus_mutex);
+                               sleep_rem = msleep_interruptible(t_conv);
+                               if (sleep_rem != 0) {
+                                       ret = -EINTR;
+                                       goto dec_refcnt;
                                }
                        }
                        ret = read_scratchpad(sl, info);