2004-01-16 23:32:33

by Hollis Blanchard

[permalink] [raw]
Subject: kobj_to_dev ?

Hi Greg, could this be added to device.h:

--- 1.112/include/linux/device.h Wed Jan 7 23:58:16 2004
+++ edited/include/linux/device.h Fri Jan 16 17:35:04 2004
@@ -279,6 +279,8 @@
void (*release)(struct device * dev);
};

+#define kobj_to_dev(k) container_of((k), struct device, kobj)
+
static inline struct device *
list_to_dev(struct list_head *node)
{

I'm using it as the following (inspired by find_bus), and it seems like
it would make sense to put in device.h.

struct vio_dev *vio_find_device(const char *name)
{
struct kobject *kobj;

kobj = kset_find_obj(&vio_bus_type.devices, name);
if (!kobj)
return NULL;

return to_vio_dev(kobj_to_dev(kobj));
}

As a side node, since those #defines don't to type-checking, would it
make sense to name them with both types? E.g. "kobj_to_dev" instead of
just "to_dev"?

--
Hollis Blanchard
IBM Linux Technology Center


2004-01-17 00:17:39

by Greg KH

[permalink] [raw]
Subject: Re: kobj_to_dev ?

On Fri, Jan 16, 2004 at 05:32:29PM -0600, Hollis Blanchard wrote:
> Hi Greg, could this be added to device.h:
>
> --- 1.112/include/linux/device.h Wed Jan 7 23:58:16 2004
> +++ edited/include/linux/device.h Fri Jan 16 17:35:04 2004
> @@ -279,6 +279,8 @@
> void (*release)(struct device * dev);
> };
>
> +#define kobj_to_dev(k) container_of((k), struct device, kobj)
> +
> static inline struct device *
> list_to_dev(struct list_head *node)
> {
>
> I'm using it as the following (inspired by find_bus), and it seems like
> it would make sense to put in device.h.

How about just adding a find_device() function to the driver core, where
you pass in a name and a type, so that others can use it?

> As a side node, since those #defines don't to type-checking, would it
> make sense to name them with both types? E.g. "kobj_to_dev" instead of
> just "to_dev"?

The compiler does the typechecking for you :)

thanks,

greg k-h

2004-01-19 20:27:38

by Hollis Blanchard

[permalink] [raw]
Subject: Re: kobj_to_dev ?

Greg KH wrote:
>
> How about just adding a find_device() function to the driver core, where
> you pass in a name and a type, so that others can use it?

Something like this?

===== include/linux/device.h 1.111 vs edited =====
--- 1.111/include/linux/device.h Mon Dec 29 15:38:10 2003
+++ edited/include/linux/device.h Mon Jan 19 14:25:26 2004
@@ -354,6 +354,7 @@
*/
extern struct device * get_device(struct device * dev);
extern void put_device(struct device * dev);
+extern struct device *find_device(const char *name, struct bus_type *bus);


/* drivers/base/platform.c */
===== drivers/base/core.c 1.78 vs edited =====
--- 1.78/drivers/base/core.c Mon Sep 29 16:20:44 2003
+++ edited/drivers/base/core.c Mon Jan 19 14:33:42 2004
@@ -400,6 +400,14 @@
return error;
}

+struct device *find_device(const char *name, struct bus_type *bus)
+{
+ struct kobject *k = kset_find_obj(&bus->devices, name);
+ if (k)
+ return to_dev(k);
+ return NULL;
+}
+
int __init devices_init(void)
{
return subsystem_register(&devices_subsys);
@@ -416,6 +424,7 @@
EXPORT_SYMBOL(device_unregister_wait);
EXPORT_SYMBOL(get_device);
EXPORT_SYMBOL(put_device);
+EXPORT_SYMBOL(find_device);

EXPORT_SYMBOL(device_create_file);
EXPORT_SYMBOL(device_remove_file);


--
Hollis Blanchard
IBM Linux Technology Center

2004-01-20 00:29:38

by Hollis Blanchard

[permalink] [raw]
Subject: Re: kobj_to_dev ?


On Jan 19, 2004, at 6:04 PM, Greg KH wrote:

> On Mon, Jan 19, 2004 at 02:26:47PM -0600, Hollis Blanchard wrote:
>> Greg KH wrote:
>>>
>>> How about just adding a find_device() function to the driver core,
>>> where
>>> you pass in a name and a type, so that others can use it?
>>
>> Something like this?
>
> Very nice, yes. But I'll rename it to device_find() to keep the
> namespace sane. Sound ok?

Sure. I'm having a problem inside kset_find_obj() when actually using
it though, and I'm not sure if it's my fault or not. It seems there are
kobjects present for which kobject_name() returns NULL.

kset_find_obj:
list_for_each(entry,&kset->list) {
struct kobject * k = to_kobj(entry);
if (!strcmp(kobject_name(k),name)) {
ret = k;
break;
}
}

where "kset" above is "&my_bus_type.devices". strcmp doesn't like NULL
and panics. I've registered 11 devices in my_bus_type, and all of them
have names (device_add() makes sure of that).

Does this sound like my fault?

--
Hollis Blanchard
IBM Linux Technology Center

2004-01-20 00:33:28

by Greg KH

[permalink] [raw]
Subject: Re: kobj_to_dev ?

On Mon, Jan 19, 2004 at 02:26:47PM -0600, Hollis Blanchard wrote:
> Greg KH wrote:
> >
> >How about just adding a find_device() function to the driver core, where
> >you pass in a name and a type, so that others can use it?
>
> Something like this?

Very nice, yes. But I'll rename it to device_find() to keep the
namespace sane. Sound ok?

thanks,

greg k-h

2004-01-20 00:54:14

by Greg KH

[permalink] [raw]
Subject: Re: kobj_to_dev ?

On Mon, Jan 19, 2004 at 06:25:14PM -0600, Hollis Blanchard wrote:
>
> On Jan 19, 2004, at 6:04 PM, Greg KH wrote:
>
> >On Mon, Jan 19, 2004 at 02:26:47PM -0600, Hollis Blanchard wrote:
> >>Greg KH wrote:
> >>>
> >>>How about just adding a find_device() function to the driver core,
> >>>where
> >>>you pass in a name and a type, so that others can use it?
> >>
> >>Something like this?
> >
> >Very nice, yes. But I'll rename it to device_find() to keep the
> >namespace sane. Sound ok?
>
> Sure. I'm having a problem inside kset_find_obj() when actually using
> it though, and I'm not sure if it's my fault or not. It seems there are
> kobjects present for which kobject_name() returns NULL.

Hm, we should probably fix that oops up too. I'll go do that.

> kset_find_obj:
> list_for_each(entry,&kset->list) {
> struct kobject * k = to_kobj(entry);
> if (!strcmp(kobject_name(k),name)) {
> ret = k;
> break;
> }
> }
>
> where "kset" above is "&my_bus_type.devices". strcmp doesn't like NULL
> and panics. I've registered 11 devices in my_bus_type, and all of them
> have names (device_add() makes sure of that).
>
> Does this sound like my fault?

I don't know. If you enable debugging for kobjects (in kobject.c) do
you see any kobjects getting added to your bus with no name?

thanks,

greg k-h

2004-01-20 03:18:57

by Hollis Blanchard

[permalink] [raw]
Subject: Re: kobj_to_dev ?

On Jan 19, 2004, at 6:53 PM, Greg KH wrote:

> I don't know. If you enable debugging for kobjects (in kobject.c) do
> you see any kobjects getting added to your bus with no name?

Sigh, that took too long.

c000000001d29828 -- address of the struct device I register
c000000001d29818 -- address present in vio_bus_type.devices.list

I think the problem is that bus_add_device() adds a struct device to
bus_type.devices.list, but that's a a kset! So it contains a list of
kobjects, not devices. The obvious fix doesn't work because
driver_attach() got it wrong too. However it seems bus_for_each_dev()
got it right.

This patch compiles but I haven't tested the driver_attach() part yet.
Oh also to_dev is different in bus.c than in core.c, so I don't think it
will work here. I'm going home... I have a vague feeling that some other
language might work better for all this list stuff..

--
Hollis Blanchard
IBM Linux Technology Center

===== drivers/base/bus.c 1.52 vs edited =====
--- 1.52/drivers/base/bus.c Tue Sep 30 10:59:35 2003
+++ edited/drivers/base/bus.c Mon Jan 19 21:19:18 2004
@@ -337,7 +337,12 @@
return;

list_for_each(entry,&bus->devices.list) {
- struct device * dev = container_of(entry,struct
device,bus_list);
+ struct kobject * kobj = container_of(entry,struct
kobject,entry);
+ struct device * dev;
+
+ if (!kobj)
+ return;
+ dev = container_of(kobj,struct device,kobj);
if (!dev->driver) {
error = bus_match(dev,drv);
if (error && (error != -ENODEV))
@@ -405,7 +410,7 @@
if (bus) {
down_write(&dev->bus->subsys.rwsem);
pr_debug("bus %s: add device %s\n",bus->name,dev->bus_id);
- list_add_tail(&dev->bus_list,&dev->bus->devices.list);
+ list_add_tail(&dev->kobj.entry,&dev->bus->devices.list);
device_attach(dev);
up_write(&dev->bus->subsys.rwsem);

sysfs_create_link(&bus->devices.kobj,&dev->kobj,dev->bus_id);