* Re: device interface api
2002-12-18 0:40 device interface api Martin Waitz
@ 2002-12-19 19:31 ` Patrick Mochel
0 siblings, 0 replies; 2+ messages in thread
From: Patrick Mochel @ 2002-12-19 19:31 UTC (permalink / raw)
To: Martin Waitz; +Cc: linux-kernel
> 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?
The former. The appended patch should fix that. Sorry about that.
> another small glitch i noticed while diggin in the code:
>
> ================================================================
> --- linux-2.5/lib/kobject.c Tue Dec 10 12:59:02 2002
> +++ linux/lib/kobject.c Tue Dec 17 13:15:19 2002
> @@ -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)
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 =====
--- 1.9/drivers/base/intf.c Tue Dec 3 12:40:21 2002
+++ edited/drivers/base/intf.c Thu Dec 19 13:04:56 2002
@@ -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 =====
--- 1.8/include/linux/kobject.h Thu Nov 21 17:01:53 2002
+++ edited/include/linux/kobject.h Thu Dec 19 13:05:41 2002
@@ -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 =====
--- 1.9/lib/kobject.c Sun Dec 1 23:24:33 2002
+++ edited/lib/kobject.c Thu Dec 19 13:00:05 2002
@@ -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)
^ permalink raw reply [flat|nested] 2+ messages in thread