[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>

drivers/w1/Kconfig

@@ -30,7 +30,7 @@ config W1_DS9490
 	  This support is also available as a module.  If so, the module
 	  will be called ds9490r.ko.
 
-config W1_DS9490_BRIDGE
+config W1_DS9490R_BRIDGE
 	tristate "DS9490R USB <-> W1 transport layer for 1-wire"
 	depends on W1_DS9490
 	help

drivers/w1/w1.c

@@ -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,24 +136,6 @@ 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);
@@ -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;
@@ -472,7 +442,7 @@ 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;
@@ -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;
 		}
@@ -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;
@@ -620,37 +588,10 @@ void w1_search(struct w1_master *dev)
 	}
 }
 
-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,20 +634,12 @@ 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_for_each_entry_safe(sl, sln, &dev->slist, w1_slave_entry) {
 				list_del(&sl->w1_slave_entry);
 
 				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);
-
-			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 @@ int w1_init(void)
 	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);

drivers/w1/w1.h

@@ -137,7 +137,6 @@ struct w1_master
 };
 
 int w1_create_master_attributes(struct w1_master *);
-void w1_destroy_master_attributes(struct w1_master *);
 void w1_search(struct w1_master *dev);
 
 #endif /* __KERNEL__ */

drivers/w1/w1_int.c

@@ -39,8 +39,9 @@ extern spinlock_t w1_mlock;
 
 extern int w1_process(void *);
 
-struct w1_master * w1_alloc_dev(u32 id, int slave_count, int slave_ttl,
-	      struct device_driver *driver, struct device *device)
+static struct w1_master * w1_alloc_dev(u32 id, int slave_count, int slave_ttl,
+				       struct device_driver *driver,
+				       struct device *device)
 {
 	struct w1_master *dev;
 	int err;
@@ -105,7 +106,7 @@ struct w1_master * w1_alloc_dev(u32 id, int slave_count, int slave_ttl,
 	return dev;
 }
 
-void w1_free_dev(struct w1_master *dev)
+static void w1_free_dev(struct w1_master *dev)
 {
 	device_unregister(&dev->dev);
 	if (dev->nls && dev->nls->sk_socket)
@@ -197,10 +198,8 @@ void __w1_remove_master_device(struct w1_master *dev)
 void w1_remove_master_device(struct w1_bus_master *bm)
 {
 	struct w1_master *dev = NULL;
-	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) {
 		if (!dev->initialized)
 			continue;
 

drivers/w1/w1_int.h

@@ -27,8 +27,6 @@
 
 #include "w1.h"
 
-struct w1_master * w1_alloc_dev(u32, int, int, struct device_driver *, struct device *);
-void w1_free_dev(struct w1_master *dev);
 int w1_add_master_device(struct w1_bus_master *);
 void w1_remove_master_device(struct w1_bus_master *);
 void __w1_remove_master_device(struct w1_master *);