Add the following class iteration functions for driver use:
class_for_each_device
class_find_device
class_for_each_child
class_find_child
Signed-off-by: Dave Young <[email protected]>
---
drivers/base/class.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/device.h | 8 ++
2 files changed, 167 insertions(+)
diff -upr linux/drivers/base/class.c linux.new/drivers/base/class.c
--- linux/drivers/base/class.c 2008-01-12 14:42:24.000000000 +0800
+++ linux.new/drivers/base/class.c 2008-01-12 14:42:24.000000000 +0800
@@ -798,6 +798,165 @@ void class_device_put(struct class_devic
kobject_put(&class_dev->kobj);
}
+/**
+ * class_for_each_device - device iterator
+ * @class: the class we're iterating
+ * @data: data for the callback
+ * @fn: function to be called for each device
+ *
+ * Iterate over @class's list of devices, and call @fn for each,
+ * passing it @data.
+ *
+ * We check the return of @fn each time. If it returns anything
+ * other than 0, we break out and return that value.
+ */
+int class_for_each_device(struct class *class, void *data,
+ int (*fn)(struct device *, void *))
+{
+ struct device *dev;
+ int error = 0;
+
+ if (!class)
+ return -EINVAL;
+ down(&class->sem);
+ list_for_each_entry(dev, &class->devices, node) {
+ dev = get_device(dev);
+ if (dev) {
+ error = fn(dev, data);
+ put_device(dev);
+ } else
+ error = -ENODEV;
+ if (error)
+ break;
+ }
+ up(&class->sem);
+
+ return error;
+}
+EXPORT_SYMBOL_GPL(class_for_each_device);
+
+/**
+ * class_find_device - device iterator for locating a particular device
+ * @class: the class we're iterating
+ * @data: data for the match function
+ * @match: function to check device
+ *
+ * This is similar to the class_for_each_dev() function above, but it
+ * returns a reference to a device that is 'found' for later use, as
+ * determined by the @match callback.
+ *
+ * The callback should return 0 if the device doesn't match and non-zero
+ * if it does. If the callback returns non-zero, this function will
+ * return to the caller and not iterate over any more devices.
+ */
+struct device *class_find_device(struct class *class, void *data,
+ int (*match)(struct device *, void *))
+{
+ struct device *dev;
+ int error = 1;
+
+ if (!class)
+ return NULL;
+
+ down(&class->sem);
+ list_for_each_entry(dev, &class->devices, node) {
+ dev = get_device(dev);
+ if (dev) {
+ if (match(dev, data)) {
+ error = 0;
+ break;
+ } else
+ put_device(dev);
+ } else
+ break;
+ }
+ up(&class->sem);
+
+ if (error)
+ return NULL;
+ return dev;
+}
+EXPORT_SYMBOL_GPL(class_find_device);
+
+/**
+ * class_for_each_child - class child iterator
+ * @class: the class we're iterating
+ * @data: data for the callback
+ * @fn: function to be called for each child of the class
+ *
+ * Iterate over @class's list of children, and call @fn for each,
+ * passing it @data.
+ *
+ * We check the return of @fn each time. If it returns anything
+ * other than 0, we break out and return that value.
+ */
+int class_for_each_child(struct class *class, void *data,
+ int (*fn)(struct class_device *, void *))
+{
+ struct class_device *dev;
+ int error = 0;
+
+ if (!class)
+ return -EINVAL;
+ down(&class->sem);
+ list_for_each_entry(dev, &class->children, node) {
+ dev = class_device_get(dev);
+ if (dev) {
+ error = fn(dev, data);
+ class_device_put(dev);
+ } else
+ error = -ENODEV;
+ if (error)
+ break;
+ }
+ up(&class->sem);
+
+ return error;
+}
+EXPORT_SYMBOL_GPL(class_for_each_child);
+
+/**
+ * class_find_child - device iterator for locating a particular class_device
+ * @class: the class we're iterating
+ * @data: data for the match function
+ * @match: function to check class_device
+ *
+ * This is similar to the class_for_each_child() function above, but it
+ * returns a reference to a class_device that is 'found' for later use, as
+ * determined by the @match callback.
+ *
+ * The callback should return 0 if the class_device doesn't match and non-zero
+ * if it does. If the callback returns non-zero, this function will
+ * return to the caller and not iterate over any more class_devices.
+ */
+struct class_device *class_find_child(struct class *class, void *data,
+ int (*match)(struct class_device *, void *))
+{
+ struct class_device *dev;
+ int error = 1;
+
+ if (!class)
+ return NULL;
+
+ down(&class->sem);
+ list_for_each_entry(dev, &class->children, node) {
+ dev = class_device_get(dev);
+ if (dev) {
+ if (match(dev, data)) {
+ error = 0;
+ break;
+ } else
+ class_device_put(dev);
+ } else
+ break;
+ }
+ up(&class->sem);
+
+ if (error)
+ return NULL;
+ return dev;
+}
+EXPORT_SYMBOL_GPL(class_find_child);
int class_interface_register(struct class_interface *class_intf)
{
diff -upr linux/include/linux/device.h linux.new/include/linux/device.h
--- linux/include/linux/device.h 2008-01-12 14:42:24.000000000 +0800
+++ linux.new/include/linux/device.h 2008-01-12 14:42:24.000000000 +0800
@@ -199,6 +199,14 @@ struct class {
extern int __must_check class_register(struct class *);
extern void class_unregister(struct class *);
+extern int class_for_each_device(struct class *class, void *data,
+ int (*fn)(struct device *dev, void *data));
+extern struct device *class_find_device(struct class *class, void *data,
+ int (*match)(struct device *, void *));
+extern int class_for_each_child(struct class *class, void *data,
+ int (*fn)(struct class_device *, void *));
+extern struct class_device *class_find_child(struct class *class, void *data,
+ int (*match)(struct class_device *, void *));
struct class_attribute {
Dave Young wrote:
> Add the following class iteration functions for driver use:
Thanks Dave. I will check the ieee1394 part in detail later.
...
> +/**
> + * class_find_device - device iterator for locating a particular device
> + * @class: the class we're iterating
> + * @data: data for the match function
> + * @match: function to check device
> + *
> + * This is similar to the class_for_each_dev() function above, but it
> + * returns a reference to a device that is 'found' for later use, as
> + * determined by the @match callback.
Maybe add "Drop the reference with put_device() after use." for the
really slow driver programmers like me?
> + *
> + * The callback should return 0 if the device doesn't match and non-zero
> + * if it does. If the callback returns non-zero, this function will
> + * return to the caller and not iterate over any more devices.
> + */
> +struct device *class_find_device(struct class *class, void *data,
> + int (*match)(struct device *, void *))
> +{
A general comment on the linux/device.h API (not a direct comment on
your patch):
The match argument in bus_find_device(), driver_find_device(),
device_find_child(), class_find_device(), class_find_child() could be
changed to
bool (*match)(struct device *, void *)).
Then the semantics are IMO a little bit clearer. Ditto for the
dr_match_t type and the struct bus_type.match member.
I don't know though whether the churn of doing such a change everywhere
would be justified by the result.
A comment on patch 2/7...6/7:
You can bring most or all of the various __match implementations into a
slightly terser but IMO easy to read form, like this:
static int __match_ne(struct device *dev, void *data)
{
struct unit_directory *ud;
struct node_entry *ne = (struct node_entry *)data;
ud = container_of(dev, struct unit_directory, unit_dev);
- if (ud->ne == ne)
- return 1;
- return 0;
+ return ud->ne == ne;
}
Here it is also easy to see that readability would improve if the return
type was bool rather than int.
--
Stefan Richter
-=====-==--- ---= -==--
http://arcgraph.de/sr/
On Sat, Jan 12, 2008 at 05:47:54PM +0800, Dave Young wrote:
> Add the following class iteration functions for driver use:
> class_for_each_device
> class_find_device
> class_for_each_child
> class_find_child
>
> Signed-off-by: Dave Young <[email protected]>
>
> ---
> drivers/base/class.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/device.h | 8 ++
> 2 files changed, 167 insertions(+)
>
> diff -upr linux/drivers/base/class.c linux.new/drivers/base/class.c
> --- linux/drivers/base/class.c 2008-01-12 14:42:24.000000000 +0800
> +++ linux.new/drivers/base/class.c 2008-01-12 14:42:24.000000000 +0800
> @@ -798,6 +798,165 @@ void class_device_put(struct class_devic
> kobject_put(&class_dev->kobj);
> }
>
> +/**
> + * class_for_each_device - device iterator
> + * @class: the class we're iterating
> + * @data: data for the callback
> + * @fn: function to be called for each device
> + *
> + * Iterate over @class's list of devices, and call @fn for each,
> + * passing it @data.
> + *
> + * We check the return of @fn each time. If it returns anything
> + * other than 0, we break out and return that value.
> + */
> +int class_for_each_device(struct class *class, void *data,
> + int (*fn)(struct device *, void *))
> +{
> + struct device *dev;
> + int error = 0;
> +
> + if (!class)
> + return -EINVAL;
> + down(&class->sem);
> + list_for_each_entry(dev, &class->devices, node) {
Probably some tiny oversight, but I see this comment to struct class
doesn't mention devices list, so maybe this needs to be updated BTW?:
(from include/linux/device.h)
" struct semaphore sem; /* locks both the children and interfaces lists */"
Regards,
Jarek P.
On Jan 12, 2008 6:50 PM, Stefan Richter <[email protected]> wrote:
> Dave Young wrote:
> > Add the following class iteration functions for driver use:
>
> Thanks Dave. I will check the ieee1394 part in detail later.
>
> ...
> > +/**
> > + * class_find_device - device iterator for locating a particular device
> > + * @class: the class we're iterating
> > + * @data: data for the match function
> > + * @match: function to check device
> > + *
> > + * This is similar to the class_for_each_dev() function above, but it
> > + * returns a reference to a device that is 'found' for later use, as
> > + * determined by the @match callback.
>
> Maybe add "Drop the reference with put_device() after use." for the
> really slow driver programmers like me?
Sounds good, thanks.
>
> > + *
> > + * The callback should return 0 if the device doesn't match and non-zero
> > + * if it does. If the callback returns non-zero, this function will
> > + * return to the caller and not iterate over any more devices.
> > + */
> > +struct device *class_find_device(struct class *class, void *data,
> > + int (*match)(struct device *, void *))
> > +{
>
> A general comment on the linux/device.h API (not a direct comment on
> your patch):
>
> The match argument in bus_find_device(), driver_find_device(),
> device_find_child(), class_find_device(), class_find_child() could be
> changed to
>
> bool (*match)(struct device *, void *)).
>
> Then the semantics are IMO a little bit clearer. Ditto for the
> dr_match_t type and the struct bus_type.match member.
Yes, from semantics side it's better.
But IMHO int is good as well, although it need a little bit more
understanding of the api.
>
> I don't know though whether the churn of doing such a change everywhere
> would be justified by the result.
>
>
> A comment on patch 2/7...6/7:
>
> You can bring most or all of the various __match implementations into a
> slightly terser but IMO easy to read form, like this:
>
> static int __match_ne(struct device *dev, void *data)
> {
> struct unit_directory *ud;
> struct node_entry *ne = (struct node_entry *)data;
>
> ud = container_of(dev, struct unit_directory, unit_dev);
> - if (ud->ne == ne)
> - return 1;
> - return 0;
> + return ud->ne == ne;
> }
>
> Here it is also easy to see that readability would improve if the return
> type was bool rather than int.
Ok, thanks.
> --
> Stefan Richter
> -=====-==--- ---= -==--
> http://arcgraph.de/sr/
>
On Jan 13, 2008 4:11 AM, Jarek Poplawski <[email protected]> wrote:
>
> On Sat, Jan 12, 2008 at 05:47:54PM +0800, Dave Young wrote:
> > Add the following class iteration functions for driver use:
> > class_for_each_device
> > class_find_device
> > class_for_each_child
> > class_find_child
> >
> > Signed-off-by: Dave Young <[email protected]>
> >
> > ---
> > drivers/base/class.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++
> > include/linux/device.h | 8 ++
> > 2 files changed, 167 insertions(+)
> >
> > diff -upr linux/drivers/base/class.c linux.new/drivers/base/class.c
> > --- linux/drivers/base/class.c 2008-01-12 14:42:24.000000000 +0800
> > +++ linux.new/drivers/base/class.c 2008-01-12 14:42:24.000000000 +0800
> > @@ -798,6 +798,165 @@ void class_device_put(struct class_devic
> > kobject_put(&class_dev->kobj);
> > }
> >
> > +/**
> > + * class_for_each_device - device iterator
> > + * @class: the class we're iterating
> > + * @data: data for the callback
> > + * @fn: function to be called for each device
> > + *
> > + * Iterate over @class's list of devices, and call @fn for each,
> > + * passing it @data.
> > + *
> > + * We check the return of @fn each time. If it returns anything
> > + * other than 0, we break out and return that value.
> > + */
> > +int class_for_each_device(struct class *class, void *data,
> > + int (*fn)(struct device *, void *))
> > +{
> > + struct device *dev;
> > + int error = 0;
> > +
> > + if (!class)
> > + return -EINVAL;
> > + down(&class->sem);
> > + list_for_each_entry(dev, &class->devices, node) {
>
> Probably some tiny oversight, but I see this comment to struct class
> doesn't mention devices list, so maybe this needs to be updated BTW?:
>
> (from include/linux/device.h)
> " struct semaphore sem; /* locks both the children and interfaces lists */"
Sorry for my lazy, I think so too.
IMHO, it should be updated after the comments.
>
> Regards,
> Jarek P.
>
On Mon, Jan 14, 2008 at 09:36:04AM +0800, Dave Young wrote:
> On Jan 13, 2008 4:11 AM, Jarek Poplawski <[email protected]> wrote:
...
> > Probably some tiny oversight, but I see this comment to struct class
> > doesn't mention devices list, so maybe this needs to be updated BTW?:
> >
> > (from include/linux/device.h)
> > " struct semaphore sem; /* locks both the children and interfaces lists */"
>
> Sorry for my lazy, I think so too.
> IMHO, it should be updated after the comments.
As a matter of fact, only later I've found this question is 'fixed' in
7/7. But, actually, I was more concerned if this patch changed the
'official' policy of this sem (then it would be nice to mention about
this in a changelog) or this comment was simply incomplete.
Thanks,
Jarek P.
On Jan 14, 2008 2:58 PM, Jarek Poplawski <[email protected]> wrote:
> On Mon, Jan 14, 2008 at 09:36:04AM +0800, Dave Young wrote:
> > On Jan 13, 2008 4:11 AM, Jarek Poplawski <[email protected]> wrote:
> ...
> > > Probably some tiny oversight, but I see this comment to struct class
> > > doesn't mention devices list, so maybe this needs to be updated BTW?:
> > >
> > > (from include/linux/device.h)
> > > " struct semaphore sem; /* locks both the children and interfaces lists */"
> >
> > Sorry for my lazy, I think so too.
> > IMHO, it should be updated after the comments.
>
> As a matter of fact, only later I've found this question is 'fixed' in
> 7/7. But, actually, I was more concerned if this patch changed the
> 'official' policy of this sem (then it would be nice to mention about
> this in a changelog) or this comment was simply incomplete.
It was just "killed" because I thought it's not true.
I means I will update the correct comment after comments.
>
> Thanks,
> Jarek P.
>
On Sat, 12 Jan 2008 17:47:54 +0800,
Dave Young <[email protected]> wrote:
Minor style suggestion (same for class_find_child):
> +struct device *class_find_device(struct class *class, void *data,
> + int (*match)(struct device *, void *))
> +{
> + struct device *dev;
> + int error = 1;
How about using inverse logic here (e.g., start with int found = 0)...
> +
> + if (!class)
> + return NULL;
> +
> + down(&class->sem);
> + list_for_each_entry(dev, &class->devices, node) {
> + dev = get_device(dev);
> + if (dev) {
> + if (match(dev, data)) {
> + error = 0;
...and set found = 1 here...
> + break;
> + } else
> + put_device(dev);
> + } else
> + break;
> + }
> + up(&class->sem);
> +
> + if (error)
> + return NULL;
> + return dev;
...and do
return found ? dev : NULL;
in the end?
Especially since not finding the device is not really an error.
> +}
Otherwise this looks fine to me.
On Jan 14, 2008 8:13 PM, Cornelia Huck <[email protected]> wrote:
> On Sat, 12 Jan 2008 17:47:54 +0800,
> Dave Young <[email protected]> wrote:
>
> Minor style suggestion (same for class_find_child):
>
> > +struct device *class_find_device(struct class *class, void *data,
> > + int (*match)(struct device *, void *))
> > +{
> > + struct device *dev;
> > + int error = 1;
>
> How about using inverse logic here (e.g., start with int found = 0)...
Sounds good, will do. Thanks.
>
> > +
> > + if (!class)
> > + return NULL;
> > +
> > + down(&class->sem);
> > + list_for_each_entry(dev, &class->devices, node) {
> > + dev = get_device(dev);
> > + if (dev) {
> > + if (match(dev, data)) {
> > + error = 0;
>
> ...and set found = 1 here...
>
> > + break;
> > + } else
> > + put_device(dev);
> > + } else
> > + break;
> > + }
> > + up(&class->sem);
> > +
> > + if (error)
> > + return NULL;
> > + return dev;
>
> ...and do
> return found ? dev : NULL;
> in the end?
>
> Especially since not finding the device is not really an error.
>
> > +}
>
> Otherwise this looks fine to me.
>
Add the following class iteration functions for driver use:
class_for_each_device
class_find_device
class_for_each_child
class_find_child
Signed-off-by: Dave Young <[email protected]>
---
drivers/base/class.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/device.h | 11 ++-
2 files changed, 168 insertions(+), 2 deletions(-)
diff -upr linux/drivers/base/class.c linux.new/drivers/base/class.c
--- linux/drivers/base/class.c 2008-01-15 11:12:29.000000000 +0800
+++ linux.new/drivers/base/class.c 2008-01-15 11:12:29.000000000 +0800
@@ -798,6 +798,165 @@ void class_device_put(struct class_devic
kobject_put(&class_dev->kobj);
}
+/**
+ * class_for_each_device - device iterator
+ * @class: the class we're iterating
+ * @data: data for the callback
+ * @fn: function to be called for each device
+ *
+ * Iterate over @class's list of devices, and call @fn for each,
+ * passing it @data.
+ *
+ * We check the return of @fn each time. If it returns anything
+ * other than 0, we break out and return that value.
+ */
+int class_for_each_device(struct class *class, void *data,
+ int (*fn)(struct device *, void *))
+{
+ struct device *dev;
+ int error = 0;
+
+ if (!class)
+ return -EINVAL;
+ down(&class->sem);
+ list_for_each_entry(dev, &class->devices, node) {
+ dev = get_device(dev);
+ if (dev) {
+ error = fn(dev, data);
+ put_device(dev);
+ } else
+ error = -ENODEV;
+ if (error)
+ break;
+ }
+ up(&class->sem);
+
+ return error;
+}
+EXPORT_SYMBOL_GPL(class_for_each_device);
+
+/**
+ * class_find_device - device iterator for locating a particular device
+ * @class: the class we're iterating
+ * @data: data for the match function
+ * @match: function to check device
+ *
+ * This is similar to the class_for_each_dev() function above, but it
+ * returns a reference to a device that is 'found' for later use, as
+ * determined by the @match callback.
+ *
+ * The callback should return 0 if the device doesn't match and non-zero
+ * if it does. If the callback returns non-zero, this function will
+ * return to the caller and not iterate over any more devices.
+
+ * Note, you will need to drop the reference with put_device() after use.
+ */
+struct device *class_find_device(struct class *class, void *data,
+ int (*match)(struct device *, void *))
+{
+ struct device *dev;
+ int found = 0;
+
+ if (!class)
+ return NULL;
+
+ down(&class->sem);
+ list_for_each_entry(dev, &class->devices, node) {
+ dev = get_device(dev);
+ if (dev) {
+ if (match(dev, data)) {
+ found = 1;
+ break;
+ } else
+ put_device(dev);
+ } else
+ break;
+ }
+ up(&class->sem);
+
+ return found ? dev : NULL;
+}
+EXPORT_SYMBOL_GPL(class_find_device);
+
+/**
+ * class_for_each_child - class child iterator
+ * @class: the class we're iterating
+ * @data: data for the callback
+ * @fn: function to be called for each child of the class
+ *
+ * Iterate over @class's list of children, and call @fn for each,
+ * passing it @data.
+ *
+ * We check the return of @fn each time. If it returns anything
+ * other than 0, we break out and return that value.
+ */
+int class_for_each_child(struct class *class, void *data,
+ int (*fn)(struct class_device *, void *))
+{
+ struct class_device *dev;
+ int error = 0;
+
+ if (!class)
+ return -EINVAL;
+ down(&class->sem);
+ list_for_each_entry(dev, &class->children, node) {
+ dev = class_device_get(dev);
+ if (dev) {
+ error = fn(dev, data);
+ class_device_put(dev);
+ } else
+ error = -ENODEV;
+ if (error)
+ break;
+ }
+ up(&class->sem);
+
+ return error;
+}
+EXPORT_SYMBOL_GPL(class_for_each_child);
+
+/**
+ * class_find_child - device iterator for locating a particular class_device
+ * @class: the class we're iterating
+ * @data: data for the match function
+ * @match: function to check class_device
+ *
+ * This is similar to the class_for_each_child() function above, but it
+ * returns a reference to a class_device that is 'found' for later use, as
+ * determined by the @match callback.
+ *
+ * The callback should return 0 if the class_device doesn't match and non-zero
+ * if it does. If the callback returns non-zero, this function will
+ * return to the caller and not iterate over any more class_devices.
+
+ * Note, you will need to drop the reference with class_device_put() after use.
+ */
+struct class_device *class_find_child(struct class *class, void *data,
+ int (*match)(struct class_device *, void *))
+{
+ struct class_device *dev;
+ int found = 0;
+
+ if (!class)
+ return NULL;
+
+ down(&class->sem);
+ list_for_each_entry(dev, &class->children, node) {
+ dev = class_device_get(dev);
+ if (dev) {
+ if (match(dev, data)) {
+ found = 1;
+ break;
+ } else
+ class_device_put(dev);
+ } else
+ break;
+ }
+ up(&class->sem);
+
+ return found ? dev : NULL;
+}
+EXPORT_SYMBOL_GPL(class_find_child);
int class_interface_register(struct class_interface *class_intf)
{
diff -upr linux/include/linux/device.h linux.new/include/linux/device.h
--- linux/include/linux/device.h 2008-01-15 11:12:29.000000000 +0800
+++ linux.new/include/linux/device.h 2008-01-15 11:12:29.000000000 +0800
@@ -180,8 +180,7 @@ struct class {
struct list_head devices;
struct list_head interfaces;
struct kset class_dirs;
- struct semaphore sem; /* locks both the children and interfaces lists */
-
+ struct semaphore sem; /* locks children, devices, interfaces */
struct class_attribute * class_attrs;
struct class_device_attribute * class_dev_attrs;
struct device_attribute * dev_attrs;
@@ -199,6 +198,14 @@ struct class {
extern int __must_check class_register(struct class *);
extern void class_unregister(struct class *);
+extern int class_for_each_device(struct class *class, void *data,
+ int (*fn)(struct device *dev, void *data));
+extern struct device *class_find_device(struct class *class, void *data,
+ int (*match)(struct device *, void *));
+extern int class_for_each_child(struct class *class, void *data,
+ int (*fn)(struct class_device *, void *));
+extern struct class_device *class_find_child(struct class *class, void *data,
+ int (*match)(struct class_device *, void *));
struct class_attribute {
On Tue, 15 Jan 2008 17:13:22 +0800,
Dave Young <[email protected]> wrote:
>
> Add the following class iteration functions for driver use:
> class_for_each_device
> class_find_device
> class_for_each_child
> class_find_child
>
> Signed-off-by: Dave Young <[email protected]>
Acked-by: Cornelia Huck <[email protected]>
>
> ---
> drivers/base/class.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/device.h | 11 ++-
> 2 files changed, 168 insertions(+), 2 deletions(-)
Add the following class iteration functions for driver use:
class_for_each_device
class_find_device
class_for_each_child
class_find_child
Signed-off-by: Dave Young <[email protected]>
---
drivers/base/class.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/device.h | 11 ++-
2 files changed, 168 insertions(+), 2 deletions(-)
diff -upr linux/drivers/base/class.c linux.new/drivers/base/class.c
--- linux/drivers/base/class.c 2008-01-15 11:12:29.000000000 +0800
+++ linux.new/drivers/base/class.c 2008-01-15 11:12:29.000000000 +0800
@@ -798,6 +798,165 @@ void class_device_put(struct class_devic
kobject_put(&class_dev->kobj);
}
+/**
+ * class_for_each_device - device iterator
+ * @class: the class we're iterating
+ * @data: data for the callback
+ * @fn: function to be called for each device
+ *
+ * Iterate over @class's list of devices, and call @fn for each,
+ * passing it @data.
+ *
+ * We check the return of @fn each time. If it returns anything
+ * other than 0, we break out and return that value.
+ */
+int class_for_each_device(struct class *class, void *data,
+ int (*fn)(struct device *, void *))
+{
+ struct device *dev;
+ int error = 0;
+
+ if (!class)
+ return -EINVAL;
+ down(&class->sem);
+ list_for_each_entry(dev, &class->devices, node) {
+ dev = get_device(dev);
+ if (dev) {
+ error = fn(dev, data);
+ put_device(dev);
+ } else
+ error = -ENODEV;
+ if (error)
+ break;
+ }
+ up(&class->sem);
+
+ return error;
+}
+EXPORT_SYMBOL_GPL(class_for_each_device);
+
+/**
+ * class_find_device - device iterator for locating a particular device
+ * @class: the class we're iterating
+ * @data: data for the match function
+ * @match: function to check device
+ *
+ * This is similar to the class_for_each_dev() function above, but it
+ * returns a reference to a device that is 'found' for later use, as
+ * determined by the @match callback.
+ *
+ * The callback should return 0 if the device doesn't match and non-zero
+ * if it does. If the callback returns non-zero, this function will
+ * return to the caller and not iterate over any more devices.
+
+ * Note, you will need to drop the reference with put_device() after use.
+ */
+struct device *class_find_device(struct class *class, void *data,
+ int (*match)(struct device *, void *))
+{
+ struct device *dev;
+ int found = 0;
+
+ if (!class)
+ return NULL;
+
+ down(&class->sem);
+ list_for_each_entry(dev, &class->devices, node) {
+ dev = get_device(dev);
+ if (dev) {
+ if (match(dev, data)) {
+ found = 1;
+ break;
+ } else
+ put_device(dev);
+ } else
+ break;
+ }
+ up(&class->sem);
+
+ return found ? dev : NULL;
+}
+EXPORT_SYMBOL_GPL(class_find_device);
+
+/**
+ * class_for_each_child - class child iterator
+ * @class: the class we're iterating
+ * @data: data for the callback
+ * @fn: function to be called for each child of the class
+ *
+ * Iterate over @class's list of children, and call @fn for each,
+ * passing it @data.
+ *
+ * We check the return of @fn each time. If it returns anything
+ * other than 0, we break out and return that value.
+ */
+int class_for_each_child(struct class *class, void *data,
+ int (*fn)(struct class_device *, void *))
+{
+ struct class_device *dev;
+ int error = 0;
+
+ if (!class)
+ return -EINVAL;
+ down(&class->sem);
+ list_for_each_entry(dev, &class->children, node) {
+ dev = class_device_get(dev);
+ if (dev) {
+ error = fn(dev, data);
+ class_device_put(dev);
+ } else
+ error = -ENODEV;
+ if (error)
+ break;
+ }
+ up(&class->sem);
+
+ return error;
+}
+EXPORT_SYMBOL_GPL(class_for_each_child);
+
+/**
+ * class_find_child - device iterator for locating a particular class_device
+ * @class: the class we're iterating
+ * @data: data for the match function
+ * @match: function to check class_device
+ *
+ * This is similar to the class_for_each_child() function above, but it
+ * returns a reference to a class_device that is 'found' for later use, as
+ * determined by the @match callback.
+ *
+ * The callback should return 0 if the class_device doesn't match and non-zero
+ * if it does. If the callback returns non-zero, this function will
+ * return to the caller and not iterate over any more class_devices.
+
+ * Note, you will need to drop the reference with class_device_put() after use.
+ */
+struct class_device *class_find_child(struct class *class, void *data,
+ int (*match)(struct class_device *, void *))
+{
+ struct class_device *dev;
+ int found = 0;
+
+ if (!class)
+ return NULL;
+
+ down(&class->sem);
+ list_for_each_entry(dev, &class->children, node) {
+ dev = class_device_get(dev);
+ if (dev) {
+ if (match(dev, data)) {
+ found = 1;
+ break;
+ } else
+ class_device_put(dev);
+ } else
+ break;
+ }
+ up(&class->sem);
+
+ return found ? dev : NULL;
+}
+EXPORT_SYMBOL_GPL(class_find_child);
int class_interface_register(struct class_interface *class_intf)
{
diff -upr linux/include/linux/device.h linux.new/include/linux/device.h
--- linux/include/linux/device.h 2008-01-15 11:12:29.000000000 +0800
+++ linux.new/include/linux/device.h 2008-01-15 11:12:29.000000000 +0800
@@ -180,8 +180,7 @@ struct class {
struct list_head devices;
struct list_head interfaces;
struct kset class_dirs;
- struct semaphore sem; /* locks both the children and interfaces lists */
-
+ struct semaphore sem; /* locks children, devices, interfaces */
struct class_attribute * class_attrs;
struct class_device_attribute * class_dev_attrs;
struct device_attribute * dev_attrs;
@@ -199,6 +198,14 @@ struct class {
extern int __must_check class_register(struct class *);
extern void class_unregister(struct class *);
+extern int class_for_each_device(struct class *class, void *data,
+ int (*fn)(struct device *dev, void *data));
+extern struct device *class_find_device(struct class *class, void *data,
+ int (*match)(struct device *, void *));
+extern int class_for_each_child(struct class *class, void *data,
+ int (*fn)(struct class_device *, void *));
+extern struct class_device *class_find_child(struct class *class, void *data,
+ int (*match)(struct class_device *, void *));
struct class_attribute {
On Jan 22, 2008 1:54 PM, Dave Young <[email protected]> wrote:
>
> Add the following class iteration functions for driver use:
> class_for_each_device
> class_find_device
> class_for_each_child
> class_find_child
>
> Signed-off-by: Dave Young <[email protected]>
>
Acked-by: Cornelia Huck <[email protected]>
On Monday 21 January 2008, Dave Young wrote:
>
> +/**
> + * class_for_each_device - device iterator
> + * @class: the class we're iterating
> + * @data: data for the callback
> + * @fn: function to be called for each device
> + *
> + * Iterate over @class's list of devices, and call @fn for each,
> + * passing it @data.
> + *
> + * We check the return of @fn each time. If it returns anything
> + * other than 0, we break out and return that value.
I have a suggestion for better documentation, which
applies to all these utilities:
> + */
> +int class_for_each_device(struct class *class, void *data,
> + int (*fn)(struct device *, void *))
> +{
> + struct device *dev;
> + int error = 0;
> +
> + if (!class)
> + return -EINVAL;
> + down(&class->sem);
> + list_for_each_entry(dev, &class->devices, node) {
> + dev = get_device(dev);
> + if (dev) {
> + error = fn(dev, data);
This is called with class->sem held. So fn() has a
constraint to not re-acquire that ... else it'd be
self-deadlocking. I'd like to see docs at least
mention that; calls to add or remove class members
would be verboten, for example, which isn't an issue
with most other driver model iterators.
> + put_device(dev);
> + } else
> + error = -ENODEV;
> + if (error)
> + break;
> + }
> + up(&class->sem);
> +
> + return error;
> +}
> +EXPORT_SYMBOL_GPL(class_for_each_device);
On Jan 22, 2008 2:24 PM, David Brownell <[email protected]> wrote:
> On Monday 21 January 2008, Dave Young wrote:
> >
> > +/**
> > + * class_for_each_device - device iterator
> > + * @class: the class we're iterating
> > + * @data: data for the callback
> > + * @fn: function to be called for each device
> > + *
> > + * Iterate over @class's list of devices, and call @fn for each,
> > + * passing it @data.
> > + *
> > + * We check the return of @fn each time. If it returns anything
> > + * other than 0, we break out and return that value.
>
> I have a suggestion for better documentation, which
> applies to all these utilities:
>
>
> > + */
> > +int class_for_each_device(struct class *class, void *data,
> > + int (*fn)(struct device *, void *))
> > +{
> > + struct device *dev;
> > + int error = 0;
> > +
> > + if (!class)
> > + return -EINVAL;
> > + down(&class->sem);
> > + list_for_each_entry(dev, &class->devices, node) {
> > + dev = get_device(dev);
> > + if (dev) {
> > + error = fn(dev, data);
>
> This is called with class->sem held. So fn() has a
> constraint to not re-acquire that ... else it'd be
> self-deadlocking. I'd like to see docs at least
> mention that; calls to add or remove class members
> would be verboten, for example, which isn't an issue
> with most other driver model iterators.
Very good comment, thanks david. I will update after a while.
>
>
>
> > + put_device(dev);
> > + } else
> > + error = -ENODEV;
> > + if (error)
> > + break;
> > + }
> > + up(&class->sem);
> > +
> > + return error;
> > +}
> > +EXPORT_SYMBOL_GPL(class_for_each_device);
>
On Mon, Jan 21, 2008 at 10:24:17PM -0800, David Brownell wrote:
> On Monday 21 January 2008, Dave Young wrote:
> >
> > +/**
> > + * class_for_each_device - device iterator
> > + * @class: the class we're iterating
> > + * @data: data for the callback
> > + * @fn: function to be called for each device
> > + *
> > + * Iterate over @class's list of devices, and call @fn for each,
> > + * passing it @data.
> > + *
> > + * We check the return of @fn each time. If it returns anything
> > + * other than 0, we break out and return that value.
>
> I have a suggestion for better documentation, which
> applies to all these utilities:
>
>
> > + */
> > +int class_for_each_device(struct class *class, void *data,
> > + int (*fn)(struct device *, void *))
> > +{
> > + struct device *dev;
> > + int error = 0;
> > +
> > + if (!class)
> > + return -EINVAL;
> > + down(&class->sem);
> > + list_for_each_entry(dev, &class->devices, node) {
> > + dev = get_device(dev);
> > + if (dev) {
> > + error = fn(dev, data);
>
> This is called with class->sem held. So fn() has a
> constraint to not re-acquire that ... else it'd be
> self-deadlocking. I'd like to see docs at least
> mention that; calls to add or remove class members
> would be verboten, for example, which isn't an issue
> with most other driver model iterators.
>
>
> > + put_device(dev);
> > + } else
> > + error = -ENODEV;
> > + if (error)
> > + break;
> > + }
> > + up(&class->sem);
> > +
> > + return error;
> > +}
> > +EXPORT_SYMBOL_GPL(class_for_each_device);
Update kerneldoc as david brownell's sugestion.
Is it right for me add Cornelia Huck's ack after this change?
---
Add the following class iteration functions for driver use:
class_for_each_device
class_find_device
class_for_each_child
class_find_child
Signed-off-by: Dave Young <[email protected]>
Acked-by: Cornelia Huck <[email protected]>
---
drivers/base/class.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/device.h | 11 ++-
2 files changed, 184 insertions(+), 2 deletions(-)
diff -upr linux/drivers/base/class.c linux.new/drivers/base/class.c
--- linux/drivers/base/class.c 2008-01-22 15:06:55.000000000 +0800
+++ linux.new/drivers/base/class.c 2008-01-22 15:06:55.000000000 +0800
@@ -798,6 +798,181 @@ void class_device_put(struct class_devic
kobject_put(&class_dev->kobj);
}
+/**
+ * class_for_each_device - device iterator
+ * @class: the class we're iterating
+ * @data: data for the callback
+ * @fn: function to be called for each device
+ *
+ * Iterate over @class's list of devices, and call @fn for each,
+ * passing it @data.
+ *
+ * We check the return of @fn each time. If it returns anything
+ * other than 0, we break out and return that value.
+ *
+ * Note, we hold class->sem in this function, so it can not be
+ * re-acquired in @fn, otherwise it will self-deadlocking. For
+ * example, calls to add or remove class members would be verboten.
+ */
+int class_for_each_device(struct class *class, void *data,
+ int (*fn)(struct device *, void *))
+{
+ struct device *dev;
+ int error = 0;
+
+ if (!class)
+ return -EINVAL;
+ down(&class->sem);
+ list_for_each_entry(dev, &class->devices, node) {
+ dev = get_device(dev);
+ if (dev) {
+ error = fn(dev, data);
+ put_device(dev);
+ } else
+ error = -ENODEV;
+ if (error)
+ break;
+ }
+ up(&class->sem);
+
+ return error;
+}
+EXPORT_SYMBOL_GPL(class_for_each_device);
+
+/**
+ * class_find_device - device iterator for locating a particular device
+ * @class: the class we're iterating
+ * @data: data for the match function
+ * @match: function to check device
+ *
+ * This is similar to the class_for_each_dev() function above, but it
+ * returns a reference to a device that is 'found' for later use, as
+ * determined by the @match callback.
+ *
+ * The callback should return 0 if the device doesn't match and non-zero
+ * if it does. If the callback returns non-zero, this function will
+ * return to the caller and not iterate over any more devices.
+
+ * Note, you will need to drop the reference with put_device() after use.
+ *
+ * We hold class->sem in this function, so it can not be
+ * re-acquired in @match, otherwise it will self-deadlocking. For
+ * example, calls to add or remove class members would be verboten.
+ */
+struct device *class_find_device(struct class *class, void *data,
+ int (*match)(struct device *, void *))
+{
+ struct device *dev;
+ int found = 0;
+
+ if (!class)
+ return NULL;
+
+ down(&class->sem);
+ list_for_each_entry(dev, &class->devices, node) {
+ dev = get_device(dev);
+ if (dev) {
+ if (match(dev, data)) {
+ found = 1;
+ break;
+ } else
+ put_device(dev);
+ } else
+ break;
+ }
+ up(&class->sem);
+
+ return found ? dev : NULL;
+}
+EXPORT_SYMBOL_GPL(class_find_device);
+
+/**
+ * class_for_each_child - class child iterator
+ * @class: the class we're iterating
+ * @data: data for the callback
+ * @fn: function to be called for each child of the class
+ *
+ * Iterate over @class's list of children, and call @fn for each,
+ * passing it @data.
+ *
+ * We check the return of @fn each time. If it returns anything
+ * other than 0, we break out and return that value.
+ *
+ * Note, we hold class->sem in this function, so it can not be
+ * re-acquired in @fn, otherwise it will self-deadlocking. For
+ * example, calls to add or remove class members would be verboten.
+ */
+int class_for_each_child(struct class *class, void *data,
+ int (*fn)(struct class_device *, void *))
+{
+ struct class_device *dev;
+ int error = 0;
+
+ if (!class)
+ return -EINVAL;
+ down(&class->sem);
+ list_for_each_entry(dev, &class->children, node) {
+ dev = class_device_get(dev);
+ if (dev) {
+ error = fn(dev, data);
+ class_device_put(dev);
+ } else
+ error = -ENODEV;
+ if (error)
+ break;
+ }
+ up(&class->sem);
+
+ return error;
+}
+EXPORT_SYMBOL_GPL(class_for_each_child);
+
+/**
+ * class_find_child - device iterator for locating a particular class_device
+ * @class: the class we're iterating
+ * @data: data for the match function
+ * @match: function to check class_device
+ *
+ * This is similar to the class_for_each_child() function above, but it
+ * returns a reference to a class_device that is 'found' for later use, as
+ * determined by the @match callback.
+ *
+ * The callback should return 0 if the class_device doesn't match and non-zero
+ * if it does. If the callback returns non-zero, this function will
+ * return to the caller and not iterate over any more class_devices.
+ *
+ * Note, you will need to drop the reference with class_device_put() after use.
+ *
+ * We hold class->sem in this function, so it can not be
+ * re-acquired in @match, otherwise it will self-deadlocking. For
+ * example, calls to add or remove class members would be verboten.
+ */
+struct class_device *class_find_child(struct class *class, void *data,
+ int (*match)(struct class_device *, void *))
+{
+ struct class_device *dev;
+ int found = 0;
+
+ if (!class)
+ return NULL;
+
+ down(&class->sem);
+ list_for_each_entry(dev, &class->children, node) {
+ dev = class_device_get(dev);
+ if (dev) {
+ if (match(dev, data)) {
+ found = 1;
+ break;
+ } else
+ class_device_put(dev);
+ } else
+ break;
+ }
+ up(&class->sem);
+
+ return found ? dev : NULL;
+}
+EXPORT_SYMBOL_GPL(class_find_child);
int class_interface_register(struct class_interface *class_intf)
{
diff -upr linux/include/linux/device.h linux.new/include/linux/device.h
--- linux/include/linux/device.h 2008-01-22 15:06:55.000000000 +0800
+++ linux.new/include/linux/device.h 2008-01-22 15:06:55.000000000 +0800
@@ -180,8 +180,7 @@ struct class {
struct list_head devices;
struct list_head interfaces;
struct kset class_dirs;
- struct semaphore sem; /* locks both the children and interfaces lists */
-
+ struct semaphore sem; /* locks children, devices, interfaces */
struct class_attribute * class_attrs;
struct class_device_attribute * class_dev_attrs;
struct device_attribute * dev_attrs;
@@ -199,6 +198,14 @@ struct class {
extern int __must_check class_register(struct class *);
extern void class_unregister(struct class *);
+extern int class_for_each_device(struct class *class, void *data,
+ int (*fn)(struct device *dev, void *data));
+extern struct device *class_find_device(struct class *class, void *data,
+ int (*match)(struct device *, void *));
+extern int class_for_each_child(struct class *class, void *data,
+ int (*fn)(struct class_device *, void *));
+extern struct class_device *class_find_child(struct class *class, void *data,
+ int (*match)(struct class_device *, void *));
struct class_attribute {
On Tue, 22 Jan 2008 15:27:08 +0800,
Dave Young <[email protected]> wrote:
> On Mon, Jan 21, 2008 at 10:24:17PM -0800, David Brownell wrote:
> > This is called with class->sem held. So fn() has a
> > constraint to not re-acquire that ... else it'd be
> > self-deadlocking. I'd like to see docs at least
> > mention that; calls to add or remove class members
> > would be verboten, for example, which isn't an issue
> > with most other driver model iterators.
Indeed, it's a good idea to point this out.
> Update kerneldoc as david brownell's sugestion.
> Is it right for me add Cornelia Huck's ack after this change?
Fine with me.
> ---
>
> Add the following class iteration functions for driver use:
> class_for_each_device
> class_find_device
> class_for_each_child
> class_find_child
>
> Signed-off-by: Dave Young <[email protected]>
> Acked-by: Cornelia Huck <[email protected]>
>
> ---
> drivers/base/class.c | 175 +++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/device.h | 11 ++-
> 2 files changed, 184 insertions(+), 2 deletions(-)
On Tue, Jan 22, 2008 at 03:27:08PM +0800, Dave Young wrote:
>
> Add the following class iteration functions for driver use:
> class_for_each_device
> class_find_device
> class_for_each_child
> class_find_child
As class_for_each_child() is not used by anyone in this patch series,
and we want to heavily discourage the use of class_device (only scsi and
IB are the last remaining users), I'll cut out this portion of the
patch.
Any objection?
thanks,
greg k-h
On Jan 23, 2008 6:25 AM, Greg KH <[email protected]> wrote:
> On Tue, Jan 22, 2008 at 03:27:08PM +0800, Dave Young wrote:
> >
> > Add the following class iteration functions for driver use:
> > class_for_each_device
> > class_find_device
> > class_for_each_child
> > class_find_child
>
> As class_for_each_child() is not used by anyone in this patch series,
> and we want to heavily discourage the use of class_device (only scsi and
> IB are the last remaining users), I'll cut out this portion of the
> patch.
>
> Any objection?
Looks good to me.
>
> thanks,
>
> greg k-h
>