[PATCH] w1: cleanups.

 - white space changes.
 - list_for_each_entry/list_for_each_entry_safe and reverse changes.
 - small coding style changes.
 - removed redundant NULL checks.
 - use attribute group and macros instead of direct device attributes.
Patch is havily based on work from Adrian Bunk and Dmitry Torokhov,
thanks guys.

Signed-off-by: Evgeniy Polyakov <johnpol@2ka.mipt.ru>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

1 files changed, 96 insertions, 177 deletions

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 24a192e3b8b4..5b49c9a937f0 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -1,8 +1,8 @@
 /*
- *      w1.c
+ *      w1.c
  *
  * Copyright (c) 2004 Evgeniy Polyakov <johnpol@2ka.mipt.ru>
- * 
+ *
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -99,6 +99,23 @@ static ssize_t w1_default_read_bin(struct kobject *kobj, char *buf, loff_t off,
         return sprintf(buf, "No family registered.\n");
 }
 
+static struct device_attribute w1_slave_attribute =
+        __ATTR(name, S_IRUGO, w1_default_read_name, NULL);
+
+static struct device_attribute w1_slave_attribute_val =
+        __ATTR(value, S_IRUGO, w1_default_read_name, NULL);
+
+static struct bin_attribute w1_slave_bin_attribute = {
+        .attr = {
+                .name = "w1_slave",
+                .mode = S_IRUGO,
+                .owner = THIS_MODULE,
+        },
+        .size = W1_SLAVE_DATA_SIZE,
+        .read = &w1_default_read_bin,
+};
+
+
 static struct bus_type w1_bus_type = {
         .name = "w1",
         .match = w1_master_match,
@@ -119,34 +136,16 @@ struct device w1_device = {
         .release = &w1_master_release
 };
 
-static struct device_attribute w1_slave_attribute = {
-        .attr = {
-                        .name = "name",
-                        .mode = S_IRUGO,
-                        .owner = THIS_MODULE
-        },
-        .show = &w1_default_read_name,
-};
-
-static struct device_attribute w1_slave_attribute_val = {
-        .attr = {
-                        .name = "value",
-                        .mode = S_IRUGO,
-                        .owner = THIS_MODULE
-        },
-        .show = &w1_default_read_name,
-};
-
 static ssize_t w1_master_attribute_show_name(struct device *dev, struct device_attribute *attr, char *buf)
 {
         struct w1_master *md = container_of (dev, struct w1_master, dev);
         ssize_t count;
-        
+
         if (down_interruptible (&md->mutex))
                 return -EBUSY;
 
         count = sprintf(buf, "%s\n", md->name);
-        
+
         up(&md->mutex);
 
         return count;
@@ -156,12 +155,12 @@ static ssize_t w1_master_attribute_show_pointer(struct device *dev, struct devic
 {
         struct w1_master *md = container_of(dev, struct w1_master, dev);
         ssize_t count;
-        
+
         if (down_interruptible(&md->mutex))
                 return -EBUSY;
 
         count = sprintf(buf, "0x%p\n", md->bus_master);
-        
+
         up(&md->mutex);
         return count;
 }
@@ -177,12 +176,12 @@ static ssize_t w1_master_attribute_show_max_slave_count(struct device *dev, stru
 {
         struct w1_master *md = container_of(dev, struct w1_master, dev);
         ssize_t count;
-        
+
         if (down_interruptible(&md->mutex))
                 return -EBUSY;
 
         count = sprintf(buf, "%d\n", md->max_slave_count);
-        
+
         up(&md->mutex);
         return count;
 }
@@ -191,12 +190,12 @@ static ssize_t w1_master_attribute_show_attempts(struct device *dev, struct devi
 {
         struct w1_master *md = container_of(dev, struct w1_master, dev);
         ssize_t count;
-        
+
         if (down_interruptible(&md->mutex))
                 return -EBUSY;
 
         count = sprintf(buf, "%lu\n", md->attempts);
-        
+
         up(&md->mutex);
         return count;
 }
@@ -205,12 +204,12 @@ static ssize_t w1_master_attribute_show_slave_count(struct device *dev, struct d
 {
         struct w1_master *md = container_of(dev, struct w1_master, dev);
         ssize_t count;
-        
+
         if (down_interruptible(&md->mutex))
                 return -EBUSY;
 
         count = sprintf(buf, "%d\n", md->slave_count);
-        
+
         up(&md->mutex);
         return count;
 }
@@ -233,7 +232,7 @@ static ssize_t w1_master_attribute_show_slaves(struct device *dev, struct device
                 list_for_each_safe(ent, n, &md->slist) {
                         sl = list_entry(ent, struct w1_slave, w1_slave_entry);
 
-                        c -= snprintf(buf + PAGE_SIZE - c, c, "%s\n", sl->name);
+                        c -= snprintf(buf + PAGE_SIZE - c, c, "%s\n", sl->name);
                 }
         }
 
@@ -242,73 +241,44 @@ static ssize_t w1_master_attribute_show_slaves(struct device *dev, struct device
         return PAGE_SIZE - c;
 }
 
-static struct device_attribute w1_master_attribute_slaves = {
-        .attr = {
-                        .name = "w1_master_slaves",
-                        .mode = S_IRUGO,
-                        .owner = THIS_MODULE,
-        },
-        .show = &w1_master_attribute_show_slaves,
-};
-static struct device_attribute w1_master_attribute_slave_count = {
-        .attr = {
-                        .name = "w1_master_slave_count",
-                        .mode = S_IRUGO,
-                        .owner = THIS_MODULE
-        },
-        .show = &w1_master_attribute_show_slave_count,
-};
-static struct device_attribute w1_master_attribute_attempts = {
-        .attr = {
-                        .name = "w1_master_attempts",
-                        .mode = S_IRUGO,
-                        .owner = THIS_MODULE
-        },
-        .show = &w1_master_attribute_show_attempts,
-};
-static struct device_attribute w1_master_attribute_max_slave_count = {
-        .attr = {
-                        .name = "w1_master_max_slave_count",
-                        .mode = S_IRUGO,
-                        .owner = THIS_MODULE
-        },
-        .show = &w1_master_attribute_show_max_slave_count,
-};
-static struct device_attribute w1_master_attribute_timeout = {
-        .attr = {
-                        .name = "w1_master_timeout",
-                        .mode = S_IRUGO,
-                        .owner = THIS_MODULE
-        },
-        .show = &w1_master_attribute_show_timeout,
-};
-static struct device_attribute w1_master_attribute_pointer = {
-        .attr = {
-                        .name = "w1_master_pointer",
-                        .mode = S_IRUGO,
-                        .owner = THIS_MODULE
-        },
-        .show = &w1_master_attribute_show_pointer,
-};
-static struct device_attribute w1_master_attribute_name = {
-        .attr = {
-                        .name = "w1_master_name",
-                        .mode = S_IRUGO,
-                        .owner = THIS_MODULE
-        },
-        .show = &w1_master_attribute_show_name,
+#define W1_MASTER_ATTR_RO(_name, _mode)                         \
+        struct device_attribute w1_master_attribute_##_name =   \
+                __ATTR(w1_master_##_name, _mode,                \
+                       w1_master_attribute_show_##_name, NULL)
+
+static W1_MASTER_ATTR_RO(name, S_IRUGO);
+static W1_MASTER_ATTR_RO(slaves, S_IRUGO);
+static W1_MASTER_ATTR_RO(slave_count, S_IRUGO);
+static W1_MASTER_ATTR_RO(max_slave_count, S_IRUGO);
+static W1_MASTER_ATTR_RO(attempts, S_IRUGO);
+static W1_MASTER_ATTR_RO(timeout, S_IRUGO);
+static W1_MASTER_ATTR_RO(pointer, S_IRUGO);
+
+static struct attribute *w1_master_default_attrs[] = {
+        &w1_master_attribute_name.attr,
+        &w1_master_attribute_slaves.attr,
+        &w1_master_attribute_slave_count.attr,
+        &w1_master_attribute_max_slave_count.attr,
+        &w1_master_attribute_attempts.attr,
+        &w1_master_attribute_timeout.attr,
+        &w1_master_attribute_pointer.attr,
+        NULL
 };
 
-static struct bin_attribute w1_slave_bin_attribute = {
-        .attr = {
-                        .name = "w1_slave",
-                        .mode = S_IRUGO,
-                        .owner = THIS_MODULE,
-        },
-        .size = W1_SLAVE_DATA_SIZE,
-        .read = &w1_default_read_bin,
+static struct attribute_group w1_master_defattr_group = {
+        .attrs = w1_master_default_attrs,
 };
 
+int w1_create_master_attributes(struct w1_master *master)
+{
+        return sysfs_create_group(&master->dev.kobj, &w1_master_defattr_group);
+}
+
+void w1_destroy_master_attributes(struct w1_master *master)
+{
+        sysfs_remove_group(&master->dev.kobj, &w1_master_defattr_group);
+}
+
 static int __w1_attach_slave_device(struct w1_slave *sl)
 {
         int err;
@@ -341,7 +311,7 @@ static int __w1_attach_slave_device(struct w1_slave *sl)
         memcpy(&sl->attr_bin, &w1_slave_bin_attribute, sizeof(sl->attr_bin));
         memcpy(&sl->attr_name, &w1_slave_attribute, sizeof(sl->attr_name));
         memcpy(&sl->attr_val, &w1_slave_attribute_val, sizeof(sl->attr_val));
-        
+
         sl->attr_bin.read = sl->family->fops->rbin;
         sl->attr_name.show = sl->family->fops->rname;
         sl->attr_val.show = sl->family->fops->rval;
@@ -445,7 +415,7 @@ static int w1_attach_slave_device(struct w1_master *dev, struct w1_reg_num *rn)
 static void w1_slave_detach(struct w1_slave *sl)
 {
         struct w1_netlink_msg msg;
-        
+
         dev_info(&sl->dev, "%s: detaching %s.\n", __func__, sl->name);
 
         while (atomic_read(&sl->refcnt)) {
@@ -471,8 +441,8 @@ static struct w1_master *w1_search_master(unsigned long data)
 {
         struct w1_master *dev;
         int found = 0;
-        
-        spin_lock_irq(&w1_mlock);
+
+        spin_lock_bh(&w1_mlock);
         list_for_each_entry(dev, &w1_masters, w1_master_entry) {
                 if (dev->bus_master->data == data) {
                         found = 1;
@@ -480,12 +450,12 @@ static struct w1_master *w1_search_master(unsigned long data)
                         break;
                 }
         }
-        spin_unlock_irq(&w1_mlock);
+        spin_unlock_bh(&w1_mlock);
 
         return (found)?dev:NULL;
 }
 
-void w1_slave_found(unsigned long data, u64 rn)
+static void w1_slave_found(unsigned long data, u64 rn)
 {
         int slave_count;
         struct w1_slave *sl;
@@ -500,7 +470,7 @@ void w1_slave_found(unsigned long data, u64 rn)
                                 data);
                 return;
         }
-        
+
         tmp = (struct w1_reg_num *) &rn;
 
         slave_count = 0;
@@ -513,8 +483,7 @@ void w1_slave_found(unsigned long data, u64 rn)
                     sl->reg_num.crc == tmp->crc) {
                         set_bit(W1_SLAVE_ACTIVE, (long *)&sl->flags);
                         break;
-                }
-                else if (sl->reg_num.family == tmp->family) {
+                } else if (sl->reg_num.family == tmp->family) {
                         family_found = 1;
                         break;
                 }
@@ -528,7 +497,7 @@ void w1_slave_found(unsigned long data, u64 rn)
                 rn && ((le64_to_cpu(rn) >> 56) & 0xff) == w1_calc_crc8((u8 *)&rn, 7)) {
                 w1_attach_slave_device(dev, tmp);
         }
-                        
+
         atomic_dec(&dev->refcnt);
 }
 
@@ -545,8 +514,8 @@ void w1_search(struct w1_master *dev)
 
         desc_bit = 64;
 
-        while (!(id_bit && comp_bit) && !last_device
-                && count++ < dev->max_slave_count) {
+        while (!(id_bit && comp_bit) && !last_device &&
+               count++ < dev->max_slave_count) {
                 last = rn;
                 rn = 0;
 
@@ -591,8 +560,7 @@ void w1_search(struct w1_master *dev)
                                                 last_family_desc = last_zero;
                                 }
 
-                        }
-                        else
+                        } else
                                 search_bit = id_bit;
 
                         tmp = search_bit;
@@ -615,42 +583,15 @@ void w1_search(struct w1_master *dev)
                         last_device = 1;
 
                 desc_bit = last_zero;
-        
+
                 w1_slave_found(dev->bus_master->data, rn);
         }
 }
 
-int w1_create_master_attributes(struct w1_master *dev)
-{
-        if (    device_create_file(&dev->dev, &w1_master_attribute_slaves) < 0 ||
-                device_create_file(&dev->dev, &w1_master_attribute_slave_count) < 0 ||
-                device_create_file(&dev->dev, &w1_master_attribute_attempts) < 0 ||
-                device_create_file(&dev->dev, &w1_master_attribute_max_slave_count) < 0 ||
-                device_create_file(&dev->dev, &w1_master_attribute_timeout) < 0||
-                device_create_file(&dev->dev, &w1_master_attribute_pointer) < 0||
-                device_create_file(&dev->dev, &w1_master_attribute_name) < 0)
-                return -EINVAL;
-
-        return 0;
-}
-
-void w1_destroy_master_attributes(struct w1_master *dev)
-{
-        device_remove_file(&dev->dev, &w1_master_attribute_slaves);
-        device_remove_file(&dev->dev, &w1_master_attribute_slave_count);
-        device_remove_file(&dev->dev, &w1_master_attribute_attempts);
-        device_remove_file(&dev->dev, &w1_master_attribute_max_slave_count);
-        device_remove_file(&dev->dev, &w1_master_attribute_timeout);
-        device_remove_file(&dev->dev, &w1_master_attribute_pointer);
-        device_remove_file(&dev->dev, &w1_master_attribute_name);
-}
-
-
-int w1_control(void *data)
+static int w1_control(void *data)
 {
-        struct w1_slave *sl;
-        struct w1_master *dev;
-        struct list_head *ent, *ment, *n, *mn;
+        struct w1_slave *sl, *sln;
+        struct w1_master *dev, *n;
         int err, have_to_wait = 0;
 
         daemonize("w1_control");
@@ -665,9 +606,7 @@ int w1_control(void *data)
                 if (signal_pending(current))
                         flush_signals(current);
 
-                list_for_each_safe(ment, mn, &w1_masters) {
-                        dev = list_entry(ment, struct w1_master, w1_master_entry);
-
+                list_for_each_entry_safe(dev, n, &w1_masters, w1_master_entry) {
                         if (!control_needs_exit && !dev->need_exit)
                                 continue;
                         /*
@@ -679,9 +618,9 @@ int w1_control(void *data)
                                 continue;
                         }
 
-                        spin_lock(&w1_mlock);
+                        spin_lock_bh(&w1_mlock);
                         list_del(&dev->w1_master_entry);
-                        spin_unlock(&w1_mlock);
+                        spin_unlock_bh(&w1_mlock);
 
                         if (control_needs_exit) {
                                 dev->need_exit = 1;
@@ -695,19 +634,11 @@ int w1_control(void *data)
 
                         wait_for_completion(&dev->dev_exited);
 
-                        list_for_each_safe(ent, n, &dev->slist) {
-                                sl = list_entry(ent, struct w1_slave, w1_slave_entry);
-
-                                if (!sl)
-                                        dev_warn(&dev->dev,
-                                                  "%s: slave entry is NULL.\n",
-                                                  __func__);
-                                else {
-                                        list_del(&sl->w1_slave_entry);
+                        list_for_each_entry_safe(sl, sln, &dev->slist, w1_slave_entry) {
+                                list_del(&sl->w1_slave_entry);
 
-                                        w1_slave_detach(sl);
-                                        kfree(sl);
-                                }
+                                w1_slave_detach(sl);
+                                kfree(sl);
                         }
                         w1_destroy_master_attributes(dev);
                         atomic_dec(&dev->refcnt);
@@ -720,8 +651,7 @@ int w1_control(void *data)
 int w1_process(void *data)
 {
         struct w1_master *dev = (struct w1_master *) data;
-        struct list_head *ent, *n;
-        struct w1_slave *sl;
+        struct w1_slave *sl, *sln;
 
         daemonize("%s", dev->name);
         allow_signal(SIGTERM);
@@ -742,27 +672,20 @@ int w1_process(void *data)
                 if (down_interruptible(&dev->mutex))
                         continue;
 
-                list_for_each_safe(ent, n, &dev->slist) {
-                        sl = list_entry(ent, struct w1_slave, w1_slave_entry);
-
-                        if (sl)
+                list_for_each_entry(sl, &dev->slist, w1_slave_entry)
                                 clear_bit(W1_SLAVE_ACTIVE, (long *)&sl->flags);
-                }
-                
-                w1_search_devices(dev, w1_slave_found);
 
-                list_for_each_safe(ent, n, &dev->slist) {
-                        sl = list_entry(ent, struct w1_slave, w1_slave_entry);
+                w1_search_devices(dev, w1_slave_found);
 
-                        if (sl && !test_bit(W1_SLAVE_ACTIVE, (unsigned long *)&sl->flags) && !--sl->ttl) {
+                list_for_each_entry_safe(sl, sln, &dev->slist, w1_slave_entry) {
+                        if (!test_bit(W1_SLAVE_ACTIVE, (unsigned long *)&sl->flags) && !--sl->ttl) {
                                 list_del (&sl->w1_slave_entry);
 
                                 w1_slave_detach (sl);
                                 kfree (sl);
 
                                 dev->slave_count--;
-                        }
-                        else if (test_bit(W1_SLAVE_ACTIVE, (unsigned long *)&sl->flags))
+                        } else if (test_bit(W1_SLAVE_ACTIVE, (unsigned long *)&sl->flags))
                                 sl->ttl = dev->slave_ttl;
                 }
                 up(&dev->mutex);
@@ -774,7 +697,7 @@ int w1_process(void *data)
         return 0;
 }
 
-int w1_init(void)
+static int w1_init(void)
 {
         int retval;
 
@@ -814,18 +737,14 @@ err_out_exit_init:
         return retval;
 }
 
-void w1_fini(void)
+static void w1_fini(void)
 {
         struct w1_master *dev;
-        struct list_head *ent, *n;
 
-        list_for_each_safe(ent, n, &w1_masters) {
-                dev = list_entry(ent, struct w1_master, w1_master_entry);
+        list_for_each_entry(dev, &w1_masters, w1_master_entry)
                 __w1_remove_master_device(dev);
-        }
 
         control_needs_exit = 1;
-
         wait_for_completion(&w1_control_complete);
 
         driver_unregister(&w1_driver);