Skip to contentSkip to conversation

device interface api

PATCH Martin Waitz 2 participants 2 messages Dec 18, 2002 – Dec 19, 2002

Subsystems (2)
Library code Driver core
lore
Share & tools
lore

Martin Waitz

author
#0 lore raw

hi :)

please have a look at interface_add_data:

int interface_add_data(struct intf_data * data)

{
	struct device_interface * intf = intf_from_data(data);

	data->intf_num = data->intf->devnum++;
	data->kobj.subsys = &intf->subsys;
	kobject_register(&data->kobj);
	[...]

data->kobj.subsys is initialized here, but intf_from_data is using this data->kobj.subsys to get intf, instead of data->intf. do you want to remove data->intf?
either way, interface_add_data should change.

are interface users supposed to set data->intf or data->kobj.subsys?

another small glitch i noticed while diggin in the code:

================================================================

lib/kobject.c Tue Dec 17 13:15:19 2002 renamed from lib/kobject.c Tue Dec 10 12:59:02 2002 +1 -1
@@ -218,7 +218,7 @@ int subsystem_register(struct subsystem 
                s->kobj.parent = &s->parent->kobj;
        pr_debug("subsystem %s: registering, parent: %s\n",
                 s->kobj.name,s->parent ? s->parent->kobj.name : "<none>");
-       return kobject_register(&s->kobj);
+       return kobject_add(&s->kobj);
 }
 
 void subsystem_unregister(struct subsystem * s)

================================================================

subsystem_register first calls subsystem_init, which already does kobject_init, so a full kobject_register is not needed (and is in fact bad, as it again increases the refcounter for kobj.subsys)

Signature · 10 lines
CU,		  / Friedrich-Alexander University Erlangen, Germany
Martin Waitz	//  [Tali on IRCnet]  [tali.home.pages.de] _________
______________/// - - - - - - - - - - - - - - - - - - - - ///
dies ist eine manuell generierte mail, sie beinhaltet    //
tippfehler und ist auch ohne grossbuchstaben gueltig.   /
			    -
Wer bereit ist, grundlegende Freiheiten aufzugeben, um sich 
kurzfristige Sicherheit zu verschaffen, der hat weder Freiheit 
noch Sicherheit verdient.
			Benjamin Franklin  (1706 - 1790)
#1 lore raw
Quoted 11 lines

int interface_add_data(struct intf_data * data)

{
	struct device_interface * intf = intf_from_data(data);

	data->intf_num = data->intf->devnum++;
	data->kobj.subsys = &intf->subsys;
	kobject_register(&data->kobj);
	[...]
Quoted 6 lines

data->kobj.subsys is initialized here, but intf_from_data is using this data->kobj.subsys to get intf, instead of data->intf. do you want to remove data->intf?
either way, interface_add_data should change.

are interface users supposed to set data->intf or data->kobj.subsys?

The former. The appended patch should fix that. Sorry about that.

Quoted 16 lines

another small glitch i noticed while diggin in the code:

================================================================

lib/kobject.c Tue Dec 17 13:15:19 2002 renamed from lib/kobject.c Tue Dec 10 12:59:02 2002 +1 -1
@@ -218,7 +218,7 @@ int subsystem_register(struct subsystem 
                s->kobj.parent = &s->parent->kobj;
        pr_debug("subsystem %s: registering, parent: %s\n",
                 s->kobj.name,s->parent ? s->parent->kobj.name : "<none>");
-       return kobject_register(&s->kobj);
+       return kobject_add(&s->kobj);
 }
 
 void subsystem_unregister(struct subsystem * s)

================================================================

Quoted 3 lines

subsystem_register first calls subsystem_init, which already does kobject_init, so a full kobject_register is not needed (and is in fact bad, as it again increases the refcounter for kobj.subsys)

Nice catch. The internal semantics changed a while back, and I missed that one when fixing up the users. A fix for that is also inluded in the following patch.

-pat

===== drivers/base/intf.c 1.9 vs edited =====

drivers/base/intf.c +13 -13
@@ -14,9 +14,6 @@
 
 #define to_data(e) container_of(e,struct intf_data,kobj.entry)
 
-#define intf_from_data(d) container_of(d->kobj.subsys,struct device_interface, subsys);
-
-
 /**
  *	intf_dev_link - create sysfs symlink for interface.
  *	@data:	interface data descriptor.

@@ -61,15 +58,18 @@
 
 int interface_add_data(struct intf_data * data)
 {
-	struct device_interface * intf = intf_from_data(data);
+	struct device_interface * intf = data->intf;
 
-	data->intf_num = data->intf->devnum++;
-	data->kobj.subsys = &intf->subsys;
-	kobject_register(&data->kobj);
-
-	list_add_tail(&data->dev_entry,&data->dev->intf_list);
-	intf_dev_link(data);
-	return 0;
+	if (intf) {
+		data->intf_num = intf->devnum++;
+		data->kobj.subsys = &intf->subsys;
+		kobject_register(&data->kobj);
+
+		list_add_tail(&data->dev_entry,&data->dev->intf_list);
+		intf_dev_link(data);
+		return 0;
+	}
+	return -EINVAL;
 }
 
 

@@ -121,9 +121,9 @@
 
 static void del(struct intf_data * data)
 {
-	struct device_interface * intf = intf_from_data(data);
+	struct device_interface * intf = data->intf;
 
-	pr_debug(" -> %s ",data->intf->name);
+	pr_debug(" -> %s ",intf->name);
 	interface_remove_data(data);
 	if (intf->remove_device)
 		intf->remove_device(data);

===== include/linux/kobject.h 1.8 vs edited =====

include/linux/kobject.h +1 -1
@@ -52,7 +52,7 @@
 
 static inline struct subsystem * subsys_get(struct subsystem * s)
 {
-	return container_of(kobject_get(&s->kobj),struct subsystem,kobj);
+	return s ? container_of(kobject_get(&s->kobj),struct subsystem,kobj) : NULL;
 }
 
 static inline void subsys_put(struct subsystem * s)

===== lib/kobject.c 1.9 vs edited =====

lib/kobject.c +7 -4
@@ -74,10 +74,13 @@
 {
 	int error = 0;
 	struct subsystem * s = kobj->subsys;
-	struct kobject * parent = kobject_get(kobj->parent);
+	struct kobject * parent;
 
 	if (!(kobj = kobject_get(kobj)))
 		return -ENOENT;
+
+	parent = kobject_get(kobj->parent);
+
 	pr_debug("kobject %s: registering. parent: %s, subsys: %s\n",
 		 kobj->name, parent ? parent->name : "<NULL>", 
 		 kobj->subsys ? kobj->subsys->kobj.name : "<NULL>" );

@@ -93,8 +96,8 @@
 		up_write(&s->rwsem);
 	}
 	error = create_dir(kobj);
-	if (error && kobj->parent)
-		kobject_put(kobj->parent);
+	if (error && parent)
+		kobject_put(parent);
 	return error;
 }
 

@@ -218,7 +221,7 @@
 		s->kobj.parent = &s->parent->kobj;
 	pr_debug("subsystem %s: registering, parent: %s\n",
 		 s->kobj.name,s->parent ? s->parent->kobj.name : "<none>");
-	return kobject_register(&s->kobj);
+	return kobject_add(&s->kobj);
 }
 
 void subsystem_unregister(struct subsystem * s)