From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.6 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B0BAEC47255 for ; Mon, 11 May 2020 20:35:50 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7BF3C20752 for ; Mon, 11 May 2020 20:35:50 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="X9nVtlSk" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731706AbgEKUft (ORCPT ); Mon, 11 May 2020 16:35:49 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40226 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727873AbgEKUft (ORCPT ); Mon, 11 May 2020 16:35:49 -0400 Received: from mail-wm1-x341.google.com (mail-wm1-x341.google.com [IPv6:2a00:1450:4864:20::341]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 86EEDC061A0C for ; Mon, 11 May 2020 13:35:47 -0700 (PDT) Received: by mail-wm1-x341.google.com with SMTP id k12so19422601wmj.3 for ; Mon, 11 May 2020 13:35:47 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=yB6i/h5KVU7JnGPQA/VgqY96W18AzSVUUQ29F1FV2a8=; b=X9nVtlSkZvZgz8youcLtqdGsnIv6qAcBoBfH8jW/UjVxYX8uiHADQG2VSW/fxTmCEF oquIn1kopovJs2isMM7jSXgxn4tUhhfa6A6htaaKKURPNQuvndGjq3Zniy8b5xigHJDj oxbXGPZI53DYZt1/5ci14ucZ/mXf8BR9Whllw24p0S1RfjqvYNThJuR6sP4YzB5pCE1E NmXKOepfWzZ5fXWOV5eTg2793EmJBD1rZBJ6gZMEvWF3/BcLKlcREA5fni2ano8/5Ovj ox4PM04MPjUpfEqqBKLzatpGcqnMi0Efj/4L/4+7jkkZByfels0gIEsb9sn6g2hHlzrj w35A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=yB6i/h5KVU7JnGPQA/VgqY96W18AzSVUUQ29F1FV2a8=; b=HqwCabwSn66Y78IWvOKB8tpRV3nd7F/E7nDiSNHsaHEuryNxN6th2n2RkhLctalhMv MbhHWI/Rc9EzwIZ4HR9By7tEU0CZz2IpAxiehEwFUnNdXCuR9hFASw7NO9k78F6P329z 4lBloYoNB24lwd9YLl1rRkSoCujuAQcW0NXemK3ch+YUZ3w88aYzd+F28hxS5WqhVw8s sE+Spgz3cbsOgP8bZ9vHzvDTe0C6fRWmbd7XTYm2DGTP8KgqlKedU0BNXDAQglERLV24 vzQnm3R/Ewpu77CoylUkjaMbOycOdhbWZrf9iNeJJB+pERTTBAn35ilaEWrCtT2ZYo3+ I3bQ== X-Gm-Message-State: AGi0PuYf8DrIy7nC5OOxj4JXqakQSEcjJ4eAix3Wij+0YSIzGeWURpwn REN5xx2IbYtf5BcDsvoJdkk= X-Google-Smtp-Source: APiQypLlp7E/kZRRWlZ2lKswpoeB9DfdF9A6j3wgEBD5snJvS3SfXO7c1FP+wj/Daq8u9NZXWkHALQ== X-Received: by 2002:a7b:c413:: with SMTP id k19mr7542508wmi.124.1589229346036; Mon, 11 May 2020 13:35:46 -0700 (PDT) Received: from akira-laptop.home ([2a01:cb19:8b28:7600:a0b9:1c6f:cfba:2b21]) by smtp.googlemail.com with ESMTPSA id g24sm20783102wrb.35.2020.05.11.13.35.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 11 May 2020 13:35:45 -0700 (PDT) From: Akira Shimahara To: greg@kroah.com, rdunlap@infradead.org Cc: zbr@ioremap.net, linux-kernel@vger.kernel.org, Akira Shimahara Subject: [PATCH v7 1/9] w1_therm: adding code comments and code reordering Date: Mon, 11 May 2020 22:35:35 +0200 Message-Id: <20200511203535.409599-1-akira215corp@gmail.com> X-Mailer: git-send-email 2.26.2 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Adding code comments to split code in dedicated parts. After the global declarations (defines, macros and function declarations), code is organized as follow : - Device and family dependent structures and functions - Interfaces functions - Helpers functions - Hardware functions - Sysfs interface functions Signed-off-by: Akira Shimahara --- Main motivation on the first patch of this serie is to clean up the code, document it and reorder it to prepare the next patches, which are clearer after this. One main point is to keep all device/family dependent code gather at the beginning to ease adding new devices if required. Changes in v5: - All patch serie in one .c file - Correcting some comments - adding include Changes in v6: - Formatting code comments according to kernel-doc requirements Changes in v7: - Formatting code comments and correcting comments mistakes drivers/w1/slaves/w1_therm.c | 427 +++++++++++++++++++++-------------- 1 file changed, 259 insertions(+), 168 deletions(-) diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c index 18f08d7..67204b3 100644 --- a/drivers/w1/slaves/w1_therm.c +++ b/drivers/w1/slaves/w1_therm.c @@ -25,7 +25,8 @@ #define W1_THERM_DS1825 0x3B #define W1_THERM_DS28EA00 0x42 -/* Allow the strong pullup to be disabled, but default to enabled. +/* + * Allow the strong pullup to be disabled, but default to enabled. * If it was disabled a parasite powered device might not get the require * current to do a temperature conversion. If it is enabled parasite powered * devices have a better chance of getting the current required. @@ -41,42 +42,55 @@ static int w1_strong_pullup = 1; module_param_named(strong_pullup, w1_strong_pullup, int, 0); +/* Helpers Macros */ + +/* return the address of the refcnt in the family data */ +#define THERM_REFCNT(family_data) \ + (&((struct w1_therm_family_data *)family_data)->refcnt) + +/* Structs definition */ + +/** + * struct w1_therm_family_converter - bind device specific functions + * @broken: flag for non-registred families + * @reserved: not used here + * @f: pointer to the device binding structure + * @convert: pointer to the device conversion function + * @precision: pointer to the device precision function + * @eeprom: pointer to eeprom function + */ +struct w1_therm_family_converter { + u8 broken; + u16 reserved; + struct w1_family *f; + int (*convert)(u8 rom[9]); + int (*precision)(struct device *device, int val); + int (*eeprom)(struct device *device); +}; + +/** + * struct w1_therm_family_data - device data + * @rom: ROM device id (64bit Lasered ROM code + 1 CRC byte) + * @refcnt: ref count + */ struct w1_therm_family_data { uint8_t rom[9]; atomic_t refcnt; }; +/** + * struct therm_info - store temperature reading + * @rom: read device data (8 data bytes + 1 CRC byte) + * @crc: computed crc from rom + * @verdict: 1 crc checked, 0 crc not matching + */ struct therm_info { u8 rom[9]; u8 crc; u8 verdict; }; -/* return the address of the refcnt in the family data */ -#define THERM_REFCNT(family_data) \ - (&((struct w1_therm_family_data *)family_data)->refcnt) - -static int w1_therm_add_slave(struct w1_slave *sl) -{ - sl->family_data = kzalloc(sizeof(struct w1_therm_family_data), - GFP_KERNEL); - if (!sl->family_data) - return -ENOMEM; - atomic_set(THERM_REFCNT(sl->family_data), 1); - return 0; -} - -static void w1_therm_remove_slave(struct w1_slave *sl) -{ - int refcnt = atomic_sub_return(1, THERM_REFCNT(sl->family_data)); - - while (refcnt) { - msleep(1000); - refcnt = atomic_read(THERM_REFCNT(sl->family_data)); - } - kfree(sl->family_data); - sl->family_data = NULL; -} +/* Sysfs interface declaration */ static ssize_t w1_slave_show(struct device *device, struct device_attribute *attr, char *buf); @@ -87,9 +101,35 @@ static ssize_t w1_slave_store(struct device *device, static ssize_t w1_seq_show(struct device *device, struct device_attribute *attr, char *buf); +/* Attributes declarations */ + static DEVICE_ATTR_RW(w1_slave); static DEVICE_ATTR_RO(w1_seq); +/* Interface Functions declaration */ + +/** + * w1_therm_add_slave() - Called when a new slave is discovered + * @sl: slave just discovered by the master. + * + * Called by the master when the slave is discovered on the bus. Used to + * initialize slave state before the beginning of any communication. + * + * Return: 0 - If success, negative kernel code otherwise + */ +static int w1_therm_add_slave(struct w1_slave *sl); + +/** + * w1_therm_remove_slave() - Called when a slave is removed + * @sl: slave to be removed. + * + * Called by the master when the slave is considered not to be on the bus + * anymore. Used to free memory. + */ +static void w1_therm_remove_slave(struct w1_slave *sl); + +/* Family attributes */ + static struct attribute *w1_therm_attrs[] = { &dev_attr_w1_slave.attr, NULL, @@ -101,6 +141,8 @@ static struct attribute *w1_ds28ea00_attrs[] = { NULL, }; +/* Attribute groups */ + ATTRIBUTE_GROUPS(w1_therm); ATTRIBUTE_GROUPS(w1_ds28ea00); @@ -154,6 +196,8 @@ static const struct hwmon_chip_info w1_chip_info = { #define W1_CHIPINFO NULL #endif +/* Family operations */ + static struct w1_family_ops w1_therm_fops = { .add_slave = w1_therm_add_slave, .remove_slave = w1_therm_remove_slave, @@ -168,6 +212,8 @@ static struct w1_family_ops w1_ds28ea00_fops = { .chip_info = W1_CHIPINFO, }; +/* Family binding operations struct */ + static struct w1_family w1_therm_family_DS18S20 = { .fid = W1_THERM_DS18S20, .fops = &w1_therm_fops, @@ -193,138 +239,18 @@ static struct w1_family w1_therm_family_DS1825 = { .fops = &w1_therm_fops, }; -struct w1_therm_family_converter { - u8 broken; - u16 reserved; - struct w1_family *f; - int (*convert)(u8 rom[9]); - int (*precision)(struct device *device, int val); - int (*eeprom)(struct device *device); -}; +/* Device dependent func */ /* write configuration to eeprom */ static inline int w1_therm_eeprom(struct device *device); -/* Set precision for conversion */ -static inline int w1_DS18B20_precision(struct device *device, int val); -static inline int w1_DS18S20_precision(struct device *device, int val); - -/* The return value is millidegrees Centigrade. */ -static inline int w1_DS18B20_convert_temp(u8 rom[9]); -static inline int w1_DS18S20_convert_temp(u8 rom[9]); - -static struct w1_therm_family_converter w1_therm_families[] = { - { - .f = &w1_therm_family_DS18S20, - .convert = w1_DS18S20_convert_temp, - .precision = w1_DS18S20_precision, - .eeprom = w1_therm_eeprom - }, - { - .f = &w1_therm_family_DS1822, - .convert = w1_DS18B20_convert_temp, - .precision = w1_DS18S20_precision, - .eeprom = w1_therm_eeprom - }, - { - .f = &w1_therm_family_DS18B20, - .convert = w1_DS18B20_convert_temp, - .precision = w1_DS18B20_precision, - .eeprom = w1_therm_eeprom - }, - { - .f = &w1_therm_family_DS28EA00, - .convert = w1_DS18B20_convert_temp, - .precision = w1_DS18S20_precision, - .eeprom = w1_therm_eeprom - }, - { - .f = &w1_therm_family_DS1825, - .convert = w1_DS18B20_convert_temp, - .precision = w1_DS18S20_precision, - .eeprom = w1_therm_eeprom - } -}; - -static inline int w1_therm_eeprom(struct device *device) -{ - struct w1_slave *sl = dev_to_w1_slave(device); - struct w1_master *dev = sl->master; - u8 rom[9], external_power; - int ret, max_trying = 10; - u8 *family_data = sl->family_data; - - if (!sl->family_data) { - ret = -ENODEV; - goto error; - } - - /* prevent the slave from going away in sleep */ - atomic_inc(THERM_REFCNT(family_data)); - - ret = mutex_lock_interruptible(&dev->bus_mutex); - if (ret != 0) - goto dec_refcnt; - - memset(rom, 0, sizeof(rom)); - - while (max_trying--) { - if (!w1_reset_select_slave(sl)) { - unsigned int tm = 10; - unsigned long sleep_rem; - - /* check if in parasite mode */ - w1_write_8(dev, W1_READ_PSUPPLY); - external_power = w1_read_8(dev); - - if (w1_reset_select_slave(sl)) - continue; - - /* 10ms strong pullup/delay after the copy command */ - if (w1_strong_pullup == 2 || - (!external_power && w1_strong_pullup)) - w1_next_pullup(dev, tm); - - w1_write_8(dev, W1_COPY_SCRATCHPAD); - - if (external_power) { - mutex_unlock(&dev->bus_mutex); - - sleep_rem = msleep_interruptible(tm); - if (sleep_rem != 0) { - ret = -EINTR; - goto dec_refcnt; - } - - ret = mutex_lock_interruptible(&dev->bus_mutex); - if (ret != 0) - goto dec_refcnt; - } else if (!w1_strong_pullup) { - sleep_rem = msleep_interruptible(tm); - if (sleep_rem != 0) { - ret = -EINTR; - goto mt_unlock; - } - } - - break; - } - } - -mt_unlock: - mutex_unlock(&dev->bus_mutex); -dec_refcnt: - atomic_dec(THERM_REFCNT(family_data)); -error: - return ret; -} - /* DS18S20 does not feature configuration register */ static inline int w1_DS18S20_precision(struct device *device, int val) { return 0; } +/* Set precision for conversion */ static inline int w1_DS18B20_precision(struct device *device, int val) { struct w1_slave *sl = dev_to_w1_slave(device); @@ -407,6 +333,14 @@ error: return ret; } +/** + * w1_DS18B20_convert_temp() - temperature computation for DS18B20 + * @rom: data read from device RAM (8 data bytes + 1 CRC byte) + * + * Can be called for any DS18B20 compliant device. + * + * Return: value in millidegrees Celsius. + */ static inline int w1_DS18B20_convert_temp(u8 rom[9]) { s16 t = le16_to_cpup((__le16 *)rom); @@ -414,6 +348,14 @@ static inline int w1_DS18B20_convert_temp(u8 rom[9]) return t*1000/16; } +/** + * w1_DS18S20_convert_temp() - temperature computation for DS18S20 + * @rom: data read from device RAM (8 data bytes + 1 CRC byte) + * + * Can be called for any DS18S20 compliant device. + * + * Return: value in millidegrees Celsius. + */ static inline int w1_DS18S20_convert_temp(u8 rom[9]) { int t, h; @@ -434,6 +376,53 @@ static inline int w1_DS18S20_convert_temp(u8 rom[9]) return t; } +/* Device capability description */ + +static struct w1_therm_family_converter w1_therm_families[] = { + { + .f = &w1_therm_family_DS18S20, + .convert = w1_DS18S20_convert_temp, + .precision = w1_DS18S20_precision, + .eeprom = w1_therm_eeprom + }, + { + .f = &w1_therm_family_DS1822, + .convert = w1_DS18B20_convert_temp, + .precision = w1_DS18S20_precision, + .eeprom = w1_therm_eeprom + }, + { + .f = &w1_therm_family_DS18B20, + .convert = w1_DS18B20_convert_temp, + .precision = w1_DS18B20_precision, + .eeprom = w1_therm_eeprom + }, + { + .f = &w1_therm_family_DS28EA00, + .convert = w1_DS18B20_convert_temp, + .precision = w1_DS18S20_precision, + .eeprom = w1_therm_eeprom + }, + { + .f = &w1_therm_family_DS1825, + .convert = w1_DS18B20_convert_temp, + .precision = w1_DS18S20_precision, + .eeprom = w1_therm_eeprom + } +}; + +/* Helpers Functions */ + +/** + * w1_convert_temp() - temperature conversion binding function + * @rom: data read from device RAM (8 data bytes + 1 CRC byte) + * @fid: device family id + * + * The function call the temperature computation function according to + * device family. + * + * Return: value in millidegrees Celsius. + */ static inline int w1_convert_temp(u8 rom[9], u8 fid) { int i; @@ -445,31 +434,32 @@ static inline int w1_convert_temp(u8 rom[9], u8 fid) return 0; } -static ssize_t w1_slave_store(struct device *device, - struct device_attribute *attr, const char *buf, - size_t size) +/* Interface Functions */ + +static int w1_therm_add_slave(struct w1_slave *sl) { - int val, ret; - struct w1_slave *sl = dev_to_w1_slave(device); - int i; + sl->family_data = kzalloc(sizeof(struct w1_therm_family_data), + GFP_KERNEL); + if (!sl->family_data) + return -ENOMEM; + atomic_set(THERM_REFCNT(sl->family_data), 1); + return 0; +} - ret = kstrtoint(buf, 0, &val); - if (ret) - return ret; +static void w1_therm_remove_slave(struct w1_slave *sl) +{ + int refcnt = atomic_sub_return(1, THERM_REFCNT(sl->family_data)); - for (i = 0; i < ARRAY_SIZE(w1_therm_families); ++i) { - if (w1_therm_families[i].f->fid == sl->family->fid) { - /* zero value indicates to write current configuration to eeprom */ - if (val == 0) - ret = w1_therm_families[i].eeprom(device); - else - ret = w1_therm_families[i].precision(device, val); - break; - } + while (refcnt) { + msleep(1000); + refcnt = atomic_read(THERM_REFCNT(sl->family_data)); } - return ret ? : size; + kfree(sl->family_data); + sl->family_data = NULL; } +/* Hardware Functions */ + static ssize_t read_therm(struct device *device, struct w1_slave *sl, struct therm_info *info) { @@ -564,6 +554,81 @@ error: return ret; } +static inline int w1_therm_eeprom(struct device *device) +{ + struct w1_slave *sl = dev_to_w1_slave(device); + struct w1_master *dev = sl->master; + u8 rom[9], external_power; + int ret, max_trying = 10; + u8 *family_data = sl->family_data; + + if (!sl->family_data) { + ret = -ENODEV; + goto error; + } + + /* prevent the slave from going away in sleep */ + atomic_inc(THERM_REFCNT(family_data)); + + ret = mutex_lock_interruptible(&dev->bus_mutex); + if (ret != 0) + goto dec_refcnt; + + memset(rom, 0, sizeof(rom)); + + while (max_trying--) { + if (!w1_reset_select_slave(sl)) { + unsigned int tm = 10; + unsigned long sleep_rem; + + /* check if in parasite mode */ + w1_write_8(dev, W1_READ_PSUPPLY); + external_power = w1_read_8(dev); + + if (w1_reset_select_slave(sl)) + continue; + + /* 10ms strong pullup/delay after the copy command */ + if (w1_strong_pullup == 2 || + (!external_power && w1_strong_pullup)) + w1_next_pullup(dev, tm); + + w1_write_8(dev, W1_COPY_SCRATCHPAD); + + if (external_power) { + mutex_unlock(&dev->bus_mutex); + + sleep_rem = msleep_interruptible(tm); + if (sleep_rem != 0) { + ret = -EINTR; + goto dec_refcnt; + } + + ret = mutex_lock_interruptible(&dev->bus_mutex); + if (ret != 0) + goto dec_refcnt; + } else if (!w1_strong_pullup) { + sleep_rem = msleep_interruptible(tm); + if (sleep_rem != 0) { + ret = -EINTR; + goto mt_unlock; + } + } + + break; + } + } + +mt_unlock: + mutex_unlock(&dev->bus_mutex); +dec_refcnt: + atomic_dec(THERM_REFCNT(family_data)); +error: + return ret; +} + +/* Sysfs Interface definition */ + static ssize_t w1_slave_show(struct device *device, struct device_attribute *attr, char *buf) { @@ -597,6 +662,32 @@ static ssize_t w1_slave_show(struct device *device, return ret; } +static ssize_t w1_slave_store(struct device *device, + struct device_attribute *attr, const char *buf, + size_t size) +{ + int val, ret; + struct w1_slave *sl = dev_to_w1_slave(device); + int i; + + ret = kstrtoint(buf, 0, &val); + if (ret) + return ret; + + for (i = 0; i < ARRAY_SIZE(w1_therm_families); ++i) { + if (w1_therm_families[i].f->fid == sl->family->fid) { + /* zero value indicates to write current configuration to eeprom */ + if (val == 0) + ret = w1_therm_families[i].eeprom(device); + else + ret = w1_therm_families[i].precision(device, + val); + break; + } + } + return ret ? : size; +} + #if IS_REACHABLE(CONFIG_HWMON) static int w1_read_temp(struct device *device, u32 attr, int channel, long *val) @@ -666,7 +757,7 @@ static ssize_t w1_seq_show(struct device *device, if (ack != W1_42_SUCCESS_CONFIRM_BYTE) goto error; - /* In case the bus fails to send 0xFF, limit*/ + /* In case the bus fails to send 0xFF, limit */ for (i = 0; i <= 64; i++) { if (w1_reset_bus(sl->master)) goto error; -- 2.26.2