From dca5480ab7b77a889088ab7cac81934604510ac7 Mon Sep 17 00:00:00 2001 From: Stefan Wahren Date: Thu, 27 Apr 2023 13:21:52 +0200 Subject: w1: w1_therm: fix locking behavior in convert_t 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 Link: https://lore.kernel.org/r/20230427112152.12313-1-stefan.wahren@i2se.com Signed-off-by: Krzysztof Kozlowski --- drivers/w1/slaves/w1_therm.c | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c index 067692626cf0..99c58bd9d2df 100644 --- a/drivers/w1/slaves/w1_therm.c +++ b/drivers/w1/slaves/w1_therm.c @@ -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); -- cgit v1.2.3 From 388f22fe5d91d707352b4b743368b30e21d9d9bf Mon Sep 17 00:00:00 2001 From: Lizhe Date: Sun, 19 Mar 2023 12:41:07 +0800 Subject: w1: Remove driver match function If there is no driver match function, the driver core assumes that each candidate pair (driver, device) matches, see driver_match_device(). Drop the bus's match function that always returned 1 and so implements the same behaviour as when there is no match function Signed-off-by: Lizhe Link: https://lore.kernel.org/r/20230319044107.311555-1-sensor1010@163.com Signed-off-by: Krzysztof Kozlowski --- drivers/w1/w1.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index 9d199fed9628..e7e42f9dabf4 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -58,11 +58,6 @@ MODULE_PARM_DESC(slave_ttl, DEFINE_MUTEX(w1_mlock); LIST_HEAD(w1_masters); -static int w1_master_match(struct device *dev, struct device_driver *drv) -{ - return 1; -} - static int w1_master_probe(struct device *dev) { return -ENODEV; @@ -174,7 +169,6 @@ static int w1_uevent(const struct device *dev, struct kobj_uevent_env *env); static struct bus_type w1_bus_type = { .name = "w1", - .match = w1_master_match, .uevent = w1_uevent, }; -- cgit v1.2.3 From 5dfd3c73ff81618fee0ef682b6fd7779863f41e4 Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Thu, 18 Aug 2022 23:01:21 +0200 Subject: w1: sgi: move from strlcpy with unused retval to strscpy Follow the advice of the below link and prefer 'strscpy' in this subsystem. Conversion is 1:1 because the return value is not used. Generated by a coccinelle script. Link: https://lore.kernel.org/r/CAHk-=wgfRnXz0W3D37d01q3JFkr_i_uTL=V6A6G1oUZcprmknw@mail.gmail.com/ Signed-off-by: Wolfram Sang Link: https://lore.kernel.org/r/20220818210121.7589-1-wsa+renesas@sang-engineering.com Signed-off-by: Krzysztof Kozlowski --- drivers/w1/masters/sgi_w1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/w1/masters/sgi_w1.c b/drivers/w1/masters/sgi_w1.c index e8c7fa68d3cc..d7fbc3c146e1 100644 --- a/drivers/w1/masters/sgi_w1.c +++ b/drivers/w1/masters/sgi_w1.c @@ -93,7 +93,7 @@ static int sgi_w1_probe(struct platform_device *pdev) pdata = dev_get_platdata(&pdev->dev); if (pdata) { - strlcpy(sdev->dev_id, pdata->dev_id, sizeof(sdev->dev_id)); + strscpy(sdev->dev_id, pdata->dev_id, sizeof(sdev->dev_id)); sdev->bus_master.dev_id = sdev->dev_id; } -- cgit v1.2.3 From 4f5a5badb4eee46e43dc45be5e6058bff767eb80 Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Wed, 20 Nov 2019 21:38:26 +0800 Subject: w1: Fix Kconfig indentation Adjust indentation from spaces to tab (+optional two spaces) as in coding style with command like: $ sed -e 's/^ /\t/' -i */Kconfig Signed-off-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20191120133826.12964-1-krzk@kernel.org Signed-off-by: Krzysztof Kozlowski --- drivers/w1/slaves/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/w1/slaves/Kconfig b/drivers/w1/slaves/Kconfig index 687753889c34..32b8a757744e 100644 --- a/drivers/w1/slaves/Kconfig +++ b/drivers/w1/slaves/Kconfig @@ -71,8 +71,8 @@ config W1_SLAVE_DS2805 help Say Y here if you want to use a 1-wire is a 112-byte user-programmable EEPROM is - organized as 7 pages of 16 bytes each with 64bit - unique number. Requires OverDrive Speed to talk to. + organized as 7 pages of 16 bytes each with 64bit + unique number. Requires OverDrive Speed to talk to. config W1_SLAVE_DS2430 tristate "256b EEPROM family support (DS2430)" -- cgit v1.2.3 From a8c4dda94115c4079d3aaa35ba238f2376b6aa53 Mon Sep 17 00:00:00 2001 From: zuoqilin Date: Fri, 18 Jun 2021 17:24:18 +0800 Subject: w1: Simplify the atribute show There is no necessary to define variable assignment, return directly. Signed-off-by: zuoqilin Link: https://lore.kernel.org/r/20210618092418.1424-1-zuoqilin1@163.com Signed-off-by: Krzysztof Kozlowski --- drivers/w1/w1.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index e7e42f9dabf4..0da9f528b6c1 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -295,17 +295,13 @@ static ssize_t w1_master_attribute_show_pointer(struct device *dev, struct devic static ssize_t w1_master_attribute_show_timeout(struct device *dev, struct device_attribute *attr, char *buf) { - ssize_t count; - count = sprintf(buf, "%d\n", w1_timeout); - return count; + return sprintf(buf, "%d\n", w1_timeout); } static ssize_t w1_master_attribute_show_timeout_us(struct device *dev, struct device_attribute *attr, char *buf) { - ssize_t count; - count = sprintf(buf, "%d\n", w1_timeout_us); - return count; + return sprintf(buf, "%d\n", w1_timeout_us); } static ssize_t w1_master_attribute_store_max_slave_count(struct device *dev, -- cgit v1.2.3 From 9033ff4c0fc65f3f168ee029b7e302a999c152ca Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Wed, 21 Jul 2021 11:34:51 +0100 Subject: w1: remove redundant initialization to variable result The variable result is being initialized with a value that is never read, it is being updated later on. The assignment is redundant and can be removed. Addresses-Coverity: ("Unused value") Signed-off-by: Colin Ian King Link: https://lore.kernel.org/r/20210721103451.43026-1-colin.king@canonical.com Signed-off-by: Krzysztof Kozlowski --- drivers/w1/w1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index 0da9f528b6c1..074bbdb67f0e 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -491,7 +491,7 @@ static ssize_t w1_master_attribute_store_remove(struct device *dev, struct w1_master *md = dev_to_w1_master(dev); struct w1_reg_num rn; struct w1_slave *sl; - ssize_t result = count; + ssize_t result; if (w1_atoreg_num(dev, buf, count, &rn)) return -EINVAL; -- cgit v1.2.3 From 83f3fcf96fcc7e5405b37d9424c7ef26bfa203f8 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 19 May 2021 17:17:45 +0300 Subject: w1: fix loop in w1_fini() The __w1_remove_master_device() function calls: list_del(&dev->w1_master_entry); So presumably this can cause an endless loop. Fixes: 7785925dd8e0 ("[PATCH] w1: cleanups.") Signed-off-by: Dan Carpenter Signed-off-by: Krzysztof Kozlowski --- drivers/w1/w1.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index 074bbdb67f0e..b651e42161e6 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -1253,10 +1253,10 @@ err_out_exit_init: static void __exit w1_fini(void) { - struct w1_master *dev; + struct w1_master *dev, *n; /* Set netlink removal messages and some cleanup */ - list_for_each_entry(dev, &w1_masters, w1_master_entry) + list_for_each_entry_safe(dev, n, &w1_masters, w1_master_entry) __w1_remove_master_device(dev); w1_fini_netlink(); -- cgit v1.2.3 From 1aa75bf5174c8243505a27476422060fce630cb1 Mon Sep 17 00:00:00 2001 From: Haowen Bai Date: Thu, 10 Mar 2022 18:29:29 +0800 Subject: w1: w1_therm: Use max() instead of doing it manually Fix following coccicheck warning: drivers/w1/slaves/w1_therm.c:1452:18-19: WARNING opportunity for max() Signed-off-by: Haowen Bai Link: https://lore.kernel.org/r/1646908169-8050-1-git-send-email-baihaowen@meizu.com Signed-off-by: Krzysztof Kozlowski --- drivers/w1/slaves/w1_therm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c index 99c58bd9d2df..fa7c162c661f 100644 --- a/drivers/w1/slaves/w1_therm.c +++ b/drivers/w1/slaves/w1_therm.c @@ -1512,7 +1512,7 @@ static int trigger_bulk_read(struct w1_master *dev_master) if (bulk_read_support(sl)) { int t_cur = conversion_time(sl); - t_conv = t_cur > t_conv ? t_cur : t_conv; + t_conv = max(t_cur, t_conv); strong_pullup = strong_pullup || (w1_strong_pullup == 2 || (!SLAVE_POWERMODE(sl) && -- cgit v1.2.3 From 7f25058c96a0631f4a1ccb4c70831c52e71decf2 Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Sat, 21 May 2022 13:10:16 +0200 Subject: w1: w1_therm: fix typo in comment Spelling mistake (triple letters) in comment. Detected with the help of Coccinelle. Signed-off-by: Julia Lawall Link: https://lore.kernel.org/r/20220521111145.81697-6-Julia.Lawall@inria.fr Signed-off-by: Krzysztof Kozlowski --- drivers/w1/slaves/w1_therm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c index fa7c162c661f..ab7a411578f8 100644 --- a/drivers/w1/slaves/w1_therm.c +++ b/drivers/w1/slaves/w1_therm.c @@ -284,7 +284,7 @@ static int read_powermode(struct w1_slave *sl); * trigger_bulk_read() - function to trigger a bulk read on the bus * @dev_master: the device master of the bus * - * Send a SKIP ROM follow by a CONVERT T commmand on the bus. + * Send a SKIP ROM follow by a CONVERT T command on the bus. * It also set the status flag in each slave &struct w1_therm_family_data * to signal that a conversion is in progress. * -- cgit v1.2.3 From ee896c5bf21cbac3bed8f958507a449168e965d3 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Sun, 22 May 2022 20:46:22 +0100 Subject: w1: ds2438: remove redundant initialization of variable crc Variable crc is being initialized with a value that is never read, it is being re-assigned later on. The initialization is redundant and can be removed. Cleans up clang scan build warning: warning: Value stored to 'crc' is never read [deadcode.DeadStores] Signed-off-by: Colin Ian King Link: https://lore.kernel.org/r/20220522194622.13277-1-colin.i.king@gmail.com Signed-off-by: Krzysztof Kozlowski --- drivers/w1/slaves/w1_ds2438.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c index ca64f99c8f3d..e008c27b3db9 100644 --- a/drivers/w1/slaves/w1_ds2438.c +++ b/drivers/w1/slaves/w1_ds2438.c @@ -66,8 +66,6 @@ static int w1_ds2438_get_page(struct w1_slave *sl, int pageno, u8 *buf) size_t count; while (retries--) { - crc = 0; - if (w1_reset_select_slave(sl)) continue; w1_buf[0] = W1_DS2438_RECALL_MEMORY; -- cgit v1.2.3 From ecaed1a26f7215f48420a9c02e229b84b5fbb882 Mon Sep 17 00:00:00 2001 From: Jason Wang Date: Sun, 8 May 2022 10:34:00 +0800 Subject: w1: no need to initialise statics to 0 Static variables do not need to be initialised to 0, because compiler will initialise all uninitialised statics to 0. Thus, remove the unneeded initializations. Signed-off-by: Jason Wang Link: https://lore.kernel.org/r/20220508023400.102244-1-wangborong@cdjrlc.com Signed-off-by: Krzysztof Kozlowski --- drivers/w1/w1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index b651e42161e6..bc11cdd2e4f2 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -32,7 +32,7 @@ static int w1_timeout = 10; module_param_named(timeout, w1_timeout, int, 0); MODULE_PARM_DESC(timeout, "time in seconds between automatic slave searches"); -static int w1_timeout_us = 0; +static int w1_timeout_us; module_param_named(timeout_us, w1_timeout_us, int, 0); MODULE_PARM_DESC(timeout_us, "time in microseconds between automatic slave searches"); -- cgit v1.2.3 From 51cbbcd6469b2a32e222ec220039af20a16f2769 Mon Sep 17 00:00:00 2001 From: Liang He Date: Wed, 15 Jun 2022 20:51:05 +0800 Subject: w1: Add missing of_node_put() in w1.c In __w1_attach_slave_device, we really need not to use of_node_put in normal path as the reference is escaped by sl. However, we need of_node_put in the fail path before put_device. Signed-off-by: Liang He Link: https://lore.kernel.org/r/20220615125105.3966317-1-windhl@126.com [krzysztof: fix whitespace / checkpatch] Signed-off-by: Krzysztof Kozlowski --- drivers/w1/w1.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index bc11cdd2e4f2..e16a60872226 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -692,6 +692,7 @@ static int __w1_attach_slave_device(struct w1_slave *sl) dev_err(&sl->dev, "Device registration [%s] failed. err=%d\n", dev_name(&sl->dev), err); + of_node_put(sl->dev.of_node); put_device(&sl->dev); return err; } -- cgit v1.2.3 From 0df2a5e99d0cb10a3da93fd71dd6753af5adc79f Mon Sep 17 00:00:00 2001 From: Krzysztof Kozlowski Date: Fri, 7 Apr 2023 17:01:21 +0200 Subject: w1: therm: constify pointers to hwmon_channel_info Statically allocated array of pointed to hwmon_channel_info can be made const for safety. Link: https://lore.kernel.org/r/20230407150121.79887-1-krzysztof.kozlowski@linaro.org Signed-off-by: Krzysztof Kozlowski --- drivers/w1/slaves/w1_therm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c index ab7a411578f8..c85e80c7e130 100644 --- a/drivers/w1/slaves/w1_therm.c +++ b/drivers/w1/slaves/w1_therm.c @@ -454,7 +454,7 @@ static const struct hwmon_channel_info w1_temp = { .config = w1_temp_config, }; -static const struct hwmon_channel_info *w1_info[] = { +static const struct hwmon_channel_info * const w1_info[] = { &w1_temp, NULL }; -- cgit v1.2.3 From b332af5398a3aa1a2fdd69bb6968a8f866cc39aa Mon Sep 17 00:00:00 2001 From: Jakob Koschel Date: Fri, 12 May 2023 13:36:10 +0200 Subject: w1: Replace usage of found with dedicated list iterator variable To move the list iterator variable into the list_for_each_entry_*() macro in the future it should be avoided to use the list iterator variable after the loop body. To *never* use the list iterator variable after the loop it was concluded to use a separate iterator variable instead of a found boolean [1]. This removes the need to use a found variable and simply checking if the variable was set, can determine if the break/goto was hit. Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1] Signed-off-by: Jakob Koschel Link: https://lore.kernel.org/r/20230509-w1-replace-usage-of-found-with-tmp-list-iterator-variable-v3-1-e07c9603fd9d@gmail.com Signed-off-by: Krzysztof Kozlowski --- drivers/w1/w1.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c index e16a60872226..5353cbd75126 100644 --- a/drivers/w1/w1.c +++ b/drivers/w1/w1.c @@ -821,49 +821,47 @@ int w1_slave_detach(struct w1_slave *sl) struct w1_master *w1_search_master_id(u32 id) { - struct w1_master *dev; - int found = 0; + struct w1_master *dev = NULL, *iter; mutex_lock(&w1_mlock); - list_for_each_entry(dev, &w1_masters, w1_master_entry) { - if (dev->id == id) { - found = 1; - atomic_inc(&dev->refcnt); + list_for_each_entry(iter, &w1_masters, w1_master_entry) { + if (iter->id == id) { + dev = iter; + atomic_inc(&iter->refcnt); break; } } mutex_unlock(&w1_mlock); - return (found)?dev:NULL; + return dev; } struct w1_slave *w1_search_slave(struct w1_reg_num *id) { struct w1_master *dev; - struct w1_slave *sl = NULL; - int found = 0; + struct w1_slave *sl = NULL, *iter; mutex_lock(&w1_mlock); list_for_each_entry(dev, &w1_masters, w1_master_entry) { mutex_lock(&dev->list_mutex); - list_for_each_entry(sl, &dev->slist, w1_slave_entry) { - if (sl->reg_num.family == id->family && - sl->reg_num.id == id->id && - sl->reg_num.crc == id->crc) { - found = 1; + list_for_each_entry(iter, &dev->slist, w1_slave_entry) { + if (iter->reg_num.family == id->family && + iter->reg_num.id == id->id && + iter->reg_num.crc == id->crc) { + sl = iter; atomic_inc(&dev->refcnt); - atomic_inc(&sl->refcnt); + atomic_inc(&iter->refcnt); break; } } mutex_unlock(&dev->list_mutex); - if (found) + if (sl) break; } mutex_unlock(&w1_mlock); - return (found)?sl:NULL; + return sl; } void w1_reconnect_slaves(struct w1_family *f, int attach) -- cgit v1.2.3