w1_therm: adding code comments and code reordering
authorAkira Shimahara <akira215corp@gmail.com>
Mon, 11 May 2020 20:35:35 +0000 (22:35 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Fri, 15 May 2020 14:28:58 +0000 (16:28 +0200)
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 <akira215corp@gmail.com>
Link: https://lore.kernel.org/r/20200511203535.409599-1-akira215corp@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/w1/slaves/w1_therm.c

index e028e00927991cd6aa1e91dcca6ce7fb9d5b79a1..1234916daaa8f5438e1846ece182be78ab532ca9 100644 (file)
@@ -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.
 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;