add missing class_device_del to transport classes

PATCH In review Mike Christie 2 participants 3 messages Jan 26, 2005 – Jan 30, 2005

Subsystems (2)

SCSI Driver core

lore

Share & tools

lore

Mike Christie

author

2005-01-26 23:59 UTC #0 lore raw

PATCH 1/1 add missing class_device_del to transport classes

James,

It appears there is a missing class_device_del. The comments for transport_remove_device indicate that transport_remove_classdev should call it (which the attached patch does), but the comment in attribute_container_remove_device:

If you want a
* two phase release: remove from visibility and then delete the
* device, then you should use this routine with a fn that calls
* class_device_del() and then use
* attribute_container_device_trigger() to do the final put on the
* classdev.

Indicate that maybe transport_destroy_classdev and transport_remove_device are incorrect. However, attribute_container_remove_device will do a list_del(&ic->node); so that later calls to attribute_container_device_trigger would not be able to do the final put on that classdev. I assumed the comments in attribute_container_remove_device are incorrect, so the attached patch made against scsi-rc-fixes-2.6 adds a call to class_device_del to transport_remove_classdev.

I did not fix the comments in attribute_container_remove_device becuase I was not 100% certain what is the correct behavior or usage.

Mike

drivers/base/transport_class.c +6 -0

@@ -216,10 +216,16 @@ static int transport_remove_classdev(str
 				     struct class_device *classdev)
 {
 	struct transport_class *tclass = class_to_transport_class(cont->class);
+	struct class_device_attribute **attrs = cont->attrs;
+	int i;
 
 	if (tclass->remove)
 		tclass->remove(dev);
 
+	for (i = 0; attrs[i]; i++)
+		class_device_remove_file(classdev, attrs[i]);
+	class_device_del(classdev);
+
 	return 0;
 }

James Bottomley

2005-01-27 21:33 UTC #1 lore raw

Quoted 1 line · Mike Christie

It appears there is a missing class_device_del.

Yes, my fault for converting it incorrectly. What it's supposed to use is the (nonexistent) equivalent of attribute_container_class_device_del and attribute_container_add_class_attrs, but basically I forgot and put them into the transport class where they shouldn't really be.

I'll fix it.

James

James Bottomley

2005-01-30 01:48 UTC #2 lore raw

In reply to Mike Christie

Quoted 5 lines · Mike Christie

It appears there is a missing class_device_del. The comments for transport_remove_device indicate that transport_remove_classdev should call it (which the attached patch does), but the comment in attribute_container_remove_device:

OK, try this. I think it corrects my architectural cockup and puts the attribute additions and removals in the container class where they should have been in the first place.

James

===== drivers/base/attribute_container.c 1.1 vs edited =====

drivers/base/attribute_container.c +105 -3

@@ -150,7 +150,7 @@
 		if (fn)
 			fn(cont, dev, &ic->classdev);
 		else
-			class_device_add(&ic->classdev);
+			attribute_container_add_class_device(&ic->classdev);
 		list_add_tail(&ic->node, &cont->containers);
 	}
 	up(&attribute_container_mutex);

@@ -195,8 +195,10 @@
 			list_del(&ic->node);
 			if (fn)
 				fn(cont, dev, &ic->classdev);
-			else
+			else {
+				attribute_container_remove_attrs(&ic->classdev);
 				class_device_unregister(&ic->classdev);
+			}
 		}
 	}
 	up(&attribute_container_mutex);

@@ -264,7 +266,107 @@
 	up(&attribute_container_mutex);
 }
 EXPORT_SYMBOL_GPL(attribute_container_trigger);
-     
+
+/**
+ * attribute_container_add_attrs - add attributes
+ *
+ * @classdev: The class device
+ *
+ * This simply creates all the class device sysfs files from the
+ * attributes listed in the container
+ */
+int
+attribute_container_add_attrs(struct class_device *classdev)
+{
+	struct attribute_container *cont =
+		attribute_container_classdev_to_container(classdev);
+	struct class_device_attribute **attrs =	cont->attrs;
+	int i, error;
+
+	if (!attrs)
+		return 0;
+
+	for (i = 0; attrs[i]; i++) {
+		error = class_device_create_file(classdev, attrs[i]);
+		if (error)
+			return error;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(attribute_container_add_attrs);
+
+/**
+ * attribute_container_add_class_device - same function as class_device_add
+ *
+ * @classdev:	the class device to add
+ *
+ * This performs essentially the same function as class_device_add except for
+ * attribute containers, namely add the classdev to the system and then
+ * create the attribute files
+ */
+int
+attribute_container_add_class_device(struct class_device *classdev)
+{
+	int error = class_device_add(classdev);
+	if (error)
+		return error;
+	return attribute_container_add_attrs(classdev);
+}
+EXPORT_SYMBOL_GPL(attribute_container_add_class_device);
+
+/**
+ * attribute_container_add_class_device_adapter - simple adapter for triggers
+ *
+ * This function is identical to attribute_container_add_class_device except
+ * that it is designed to be called from the triggers
+ */
+int
+attribute_container_add_class_device_adapter(struct attribute_container *cont,
+					     struct device *dev,
+					     struct class_device *classdev)
+{
+	return attribute_container_add_class_device(classdev);
+}
+EXPORT_SYMBOL_GPL(attribute_container_add_class_device_adapter);
+
+/**
+ * attribute_container_remove_attrs - remove any attribute files
+ *
+ * @classdev: The class device to remove the files from
+ *
+ */
+void
+attribute_container_remove_attrs(struct class_device *classdev)
+{
+	struct attribute_container *cont =
+		attribute_container_classdev_to_container(classdev);
+	struct class_device_attribute **attrs =	cont->attrs;
+	int i;
+
+	if (!attrs)
+		return;
+
+	for (i = 0; attrs[i]; i++)
+		class_device_remove_file(classdev, attrs[i]);
+}
+EXPORT_SYMBOL_GPL(attribute_container_remove_attrs);
+
+/**
+ * attribute_container_class_device_del - equivalent of class_device_del
+ *
+ * @classdev: the class device
+ *
+ * This function simply removes all the attribute files and then calls
+ * class_device_del.
+ */
+void
+attribute_container_class_device_del(struct class_device *classdev)
+{
+	attribute_container_remove_attrs(classdev);
+	class_device_del(classdev);
+}
+EXPORT_SYMBOL_GPL(attribute_container_class_device_del);
 
 int __init
 attribute_container_init(void)

===== drivers/base/transport_class.c 1.1 vs edited =====

drivers/base/transport_class.c +5 ~~-20~~

@@ -146,25 +146,6 @@
 EXPORT_SYMBOL_GPL(transport_setup_device);
 
 
-static int transport_add_classdev(struct attribute_container *cont,
-				  struct device *dev,
-				  struct class_device *classdev)
-{
-	struct class_device_attribute **attrs =	cont->attrs;
-	int i, error;
-
-	error = class_device_add(classdev);
-	if (error)
-		return error;
-	for (i = 0; attrs[i]; i++) {
-		error = class_device_create_file(classdev, attrs[i]);
-		if (error)
-			return error;
-	}
-
-	return 0;
-}
-
 /**
  * transport_add_device - declare a new dev for transport class association
  *

@@ -178,7 +159,8 @@
 
 void transport_add_device(struct device *dev)
 {
-	attribute_container_device_trigger(dev, transport_add_classdev);
+	attribute_container_device_trigger(dev,
+			   attribute_container_add_class_device_adapter);
 }
 EXPORT_SYMBOL_GPL(transport_add_device);

@@ -219,6 +201,9 @@
 
 	if (tclass->remove)
 		tclass->remove(dev);
+
+	if (tclass->remove != anon_transport_dummy_function)
+		attribute_container_class_device_del(classdev);
 
 	return 0;
 }

===== include/linux/attribute_container.h 1.1 vs edited =====

include/linux/attribute_container.h +12 -0

@@ -55,6 +55,18 @@
 void attribute_container_trigger(struct device *dev, 
 				 int (*fn)(struct attribute_container *,
 					   struct device *));
+int attribute_container_add_attrs(struct class_device *classdev);
+int attribute_container_add_class_device(struct class_device *classdev);
+int attribute_container_add_class_device_adapter(struct attribute_container *cont,
+						 struct device *dev,
+						 struct class_device *classdev);
+void attribute_container_remove_attrs(struct class_device *classdev);
+void attribute_container_class_device_del(struct class_device *classdev);
+
+
+
+
+
 
 struct class_device_attribute **attribute_container_classdev_to_attrs(const struct class_device *classdev);