kobj_to_dev ?

kobj_to_dev ?

[Hollis Blanchard]

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

Re: kobj_to_dev ?

[Greg KH]

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

Re: kobj_to_dev ?

[Hollis Blanchard]

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

Re: kobj_to_dev ?

[Greg KH]

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

Re: kobj_to_dev ?

[Hollis Blanchard]

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

Re: kobj_to_dev ?

[Greg KH]

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

Re: kobj_to_dev ?

[Hollis Blanchard]

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);