LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] i2c : use class_for_each_device
@ 2008-05-09  7:20 Dave Young
  2008-05-20 17:31 ` Jean Delvare
  0 siblings, 1 reply; 5+ messages in thread
From: Dave Young @ 2008-05-09  7:20 UTC (permalink / raw
  To: khali; +Cc: gregkh, i2c, linux-kernel

Use class_for_each_device for iteration.

Signed-off-by: Dave Young <hidave.darkstar@gmail.com>

---
 drivers/i2c/i2c-core.c |   96 ++++++++++++++++++++++++++-----------------------
 1 file changed, 52 insertions(+), 44 deletions(-)

diff -upr a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
--- a/drivers/i2c/i2c-core.c	2008-05-09 13:55:41.000000000 +0800
+++ b/drivers/i2c/i2c-core.c	2008-05-09 14:24:07.000000000 +0800
@@ -35,7 +35,6 @@
 #include <linux/completion.h>
 #include <linux/hardirq.h>
 #include <linux/irqflags.h>
-#include <linux/semaphore.h>
 #include <asm/uaccess.h>
 
 #include "i2c-core.h"
@@ -655,6 +654,16 @@ EXPORT_SYMBOL(i2c_del_adapter);
 
 
 /* ------------------------------------------------------------------------- */
+static int __attach_adapter(struct device *dev, void *data)
+{
+	struct i2c_adapter *adapter;
+	struct i2c_driver *driver = data;
+
+	adapter = container_of(dev, struct i2c_adapter, dev);
+	driver->attach_adapter(adapter);
+
+	return 0;
+}
 
 /*
  * An i2c_driver is used with one or more i2c_client (device) nodes to access
@@ -696,21 +705,52 @@ int i2c_register_driver(struct module *o
 	pr_debug("i2c-core: driver [%s] registered\n", driver->driver.name);
 
 	/* legacy drivers scan i2c busses directly */
-	if (driver->attach_adapter) {
-		struct i2c_adapter *adapter;
+	if (driver->attach_adapter)
+		class_for_each_device(&i2c_adapter_class, driver,
+					__attach_adapter);
+
+	mutex_unlock(&core_lock);
+	return 0;
+}
+EXPORT_SYMBOL(i2c_register_driver);
+
+static int __detach_adapter(struct device *dev, void *data)
+{
+	struct list_head   *item2, *_n;
+	struct i2c_client  *client;
+	struct i2c_adapter *adap;
+	struct i2c_driver *driver = data;
 
-		down(&i2c_adapter_class.sem);
-		list_for_each_entry(adapter, &i2c_adapter_class.devices,
-				    dev.node) {
-			driver->attach_adapter(adapter);
+	/* Have a look at each adapter, if clients of this driver
+	 * are still attached. If so, detach them to be able to
+	 * kill the driver afterwards.
+	 */
+	adap = container_of(dev, struct i2c_adapter, dev);
+	if (driver->detach_adapter) {
+		if (driver->attach_adapter(adap))
+			dev_err(&adap->dev, "detach_adapter failed "
+					"for driver [%s]\n",
+					driver->driver.name);
+	} else {
+		list_for_each_safe(item2, _n, &adap->clients) {
+			client = list_entry(item2, struct i2c_client,
+					list);
+			if (client->driver != driver)
+				continue;
+			dev_dbg(&adap->dev, "detaching client [%s] "
+				"at 0x%02x\n", client->name,
+				client->addr);
+			if (driver->detach_client(client)) {
+				dev_err(&adap->dev, "detach_client "
+					"failed for client [%s] at "
+					"0x%02x\n", client->name,
+					client->addr);
+			}
 		}
-		up(&i2c_adapter_class.sem);
 	}
 
-	mutex_unlock(&core_lock);
 	return 0;
 }
-EXPORT_SYMBOL(i2c_register_driver);
 
 /**
  * i2c_del_driver - unregister I2C driver
@@ -719,46 +759,14 @@ EXPORT_SYMBOL(i2c_register_driver);
  */
 void i2c_del_driver(struct i2c_driver *driver)
 {
-	struct list_head   *item2, *_n;
-	struct i2c_client  *client;
-	struct i2c_adapter *adap;
-
 	mutex_lock(&core_lock);
 
 	/* new-style driver? */
 	if (is_newstyle_driver(driver))
 		goto unregister;
 
-	/* Have a look at each adapter, if clients of this driver are still
-	 * attached. If so, detach them to be able to kill the driver
-	 * afterwards.
-	 */
-	down(&i2c_adapter_class.sem);
-	list_for_each_entry(adap, &i2c_adapter_class.devices, dev.node) {
-		if (driver->detach_adapter) {
-			if (driver->detach_adapter(adap)) {
-				dev_err(&adap->dev, "detach_adapter failed "
-					"for driver [%s]\n",
-					driver->driver.name);
-			}
-		} else {
-			list_for_each_safe(item2, _n, &adap->clients) {
-				client = list_entry(item2, struct i2c_client, list);
-				if (client->driver != driver)
-					continue;
-				dev_dbg(&adap->dev, "detaching client [%s] "
-					"at 0x%02x\n", client->name,
-					client->addr);
-				if (driver->detach_client(client)) {
-					dev_err(&adap->dev, "detach_client "
-						"failed for client [%s] at "
-						"0x%02x\n", client->name,
-						client->addr);
-				}
-			}
-		}
-	}
-	up(&i2c_adapter_class.sem);
+	class_for_each_device(&i2c_adapter_class, driver,
+				__detach_adapter);
 
  unregister:
 	driver_unregister(&driver->driver);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] i2c : use class_for_each_device
  2008-05-09  7:20 [PATCH 1/3] i2c : use class_for_each_device Dave Young
@ 2008-05-20 17:31 ` Jean Delvare
  2008-05-21  1:29   ` Dave Young
  0 siblings, 1 reply; 5+ messages in thread
From: Jean Delvare @ 2008-05-20 17:31 UTC (permalink / raw
  To: Dave Young; +Cc: gregkh, i2c, linux-kernel

Hi Dave,

On Fri, 9 May 2008 15:20:29 +0800, Dave Young wrote:
> Use class_for_each_device for iteration.
> 
> Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
> 
> ---
>  drivers/i2c/i2c-core.c |   96 ++++++++++++++++++++++++++-----------------------
>  1 file changed, 52 insertions(+), 44 deletions(-)
> 
> diff -upr a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
> --- a/drivers/i2c/i2c-core.c	2008-05-09 13:55:41.000000000 +0800
> +++ b/drivers/i2c/i2c-core.c	2008-05-09 14:24:07.000000000 +0800
> @@ -35,7 +35,6 @@
>  #include <linux/completion.h>
>  #include <linux/hardirq.h>
>  #include <linux/irqflags.h>
> -#include <linux/semaphore.h>
>  #include <asm/uaccess.h>
>  
>  #include "i2c-core.h"
> @@ -655,6 +654,16 @@ EXPORT_SYMBOL(i2c_del_adapter);
>  
>  
>  /* ------------------------------------------------------------------------- */
> +static int __attach_adapter(struct device *dev, void *data)
> +{
> +	struct i2c_adapter *adapter;
> +	struct i2c_driver *driver = data;
> +
> +	adapter = container_of(dev, struct i2c_adapter, dev);

You could use to_i2c_adapter(dev).

> +	driver->attach_adapter(adapter);
> +
> +	return 0;
> +}
>  
>  /*
>   * An i2c_driver is used with one or more i2c_client (device) nodes to access
> @@ -696,21 +705,52 @@ int i2c_register_driver(struct module *o
>  	pr_debug("i2c-core: driver [%s] registered\n", driver->driver.name);
>  
>  	/* legacy drivers scan i2c busses directly */
> -	if (driver->attach_adapter) {
> -		struct i2c_adapter *adapter;
> +	if (driver->attach_adapter)
> +		class_for_each_device(&i2c_adapter_class, driver,
> +					__attach_adapter);
> +
> +	mutex_unlock(&core_lock);
> +	return 0;
> +}
> +EXPORT_SYMBOL(i2c_register_driver);
> +
> +static int __detach_adapter(struct device *dev, void *data)
> +{
> +	struct list_head   *item2, *_n;
> +	struct i2c_client  *client;
> +	struct i2c_adapter *adap;
> +	struct i2c_driver *driver = data;
>  
> -		down(&i2c_adapter_class.sem);
> -		list_for_each_entry(adapter, &i2c_adapter_class.devices,
> -				    dev.node) {
> -			driver->attach_adapter(adapter);
> +	/* Have a look at each adapter, if clients of this driver
> +	 * are still attached. If so, detach them to be able to
> +	 * kill the driver afterwards.
> +	 */
> +	adap = container_of(dev, struct i2c_adapter, dev);

You could use to_i2c_adapter(dev).

> +	if (driver->detach_adapter) {

Testing for a method...

> +		if (driver->attach_adapter(adap))

... but calling another one? Can't be correct. Which raises a question:
you didn't test your patch, did you? I'm also surprised how you managed
to mess this up, given that all you had to do was to move already
existing code around.

> +			dev_err(&adap->dev, "detach_adapter failed "
> +					"for driver [%s]\n",
> +					driver->driver.name);
> +	} else {
> +		list_for_each_safe(item2, _n, &adap->clients) {
> +			client = list_entry(item2, struct i2c_client,
> +					list);
> +			if (client->driver != driver)
> +				continue;
> +			dev_dbg(&adap->dev, "detaching client [%s] "
> +				"at 0x%02x\n", client->name,
> +				client->addr);
> +			if (driver->detach_client(client)) {
> +				dev_err(&adap->dev, "detach_client "
> +					"failed for client [%s] at "
> +					"0x%02x\n", client->name,
> +					client->addr);
> +			}
>  		}
> -		up(&i2c_adapter_class.sem);
>  	}
>  
> -	mutex_unlock(&core_lock);
>  	return 0;
>  }
> -EXPORT_SYMBOL(i2c_register_driver);
>  
>  /**
>   * i2c_del_driver - unregister I2C driver
> @@ -719,46 +759,14 @@ EXPORT_SYMBOL(i2c_register_driver);
>   */
>  void i2c_del_driver(struct i2c_driver *driver)
>  {
> -	struct list_head   *item2, *_n;
> -	struct i2c_client  *client;
> -	struct i2c_adapter *adap;
> -
>  	mutex_lock(&core_lock);
>  
>  	/* new-style driver? */
>  	if (is_newstyle_driver(driver))
>  		goto unregister;
>  
> -	/* Have a look at each adapter, if clients of this driver are still
> -	 * attached. If so, detach them to be able to kill the driver
> -	 * afterwards.
> -	 */
> -	down(&i2c_adapter_class.sem);
> -	list_for_each_entry(adap, &i2c_adapter_class.devices, dev.node) {
> -		if (driver->detach_adapter) {
> -			if (driver->detach_adapter(adap)) {
> -				dev_err(&adap->dev, "detach_adapter failed "
> -					"for driver [%s]\n",
> -					driver->driver.name);
> -			}
> -		} else {
> -			list_for_each_safe(item2, _n, &adap->clients) {
> -				client = list_entry(item2, struct i2c_client, list);
> -				if (client->driver != driver)
> -					continue;
> -				dev_dbg(&adap->dev, "detaching client [%s] "
> -					"at 0x%02x\n", client->name,
> -					client->addr);
> -				if (driver->detach_client(client)) {
> -					dev_err(&adap->dev, "detach_client "
> -						"failed for client [%s] at "
> -						"0x%02x\n", client->name,
> -						client->addr);
> -				}
> -			}
> -		}
> -	}
> -	up(&i2c_adapter_class.sem);
> +	class_for_each_device(&i2c_adapter_class, driver,
> +				__detach_adapter);
>  
>   unregister:
>  	driver_unregister(&driver->driver);

The rest looks OK. I'll fix the patch myself and add it to my i2c tree
now.

-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] i2c : use class_for_each_device
  2008-05-20 17:31 ` Jean Delvare
@ 2008-05-21  1:29   ` Dave Young
  2008-05-21  1:30     ` Dave Young
  0 siblings, 1 reply; 5+ messages in thread
From: Dave Young @ 2008-05-21  1:29 UTC (permalink / raw
  To: Jean Delvare; +Cc: gregkh, i2c, linux-kernel

On Wed, May 21, 2008 at 1:31 AM, Jean Delvare <khali@linux-fr.org> wrote:
> Hi Dave,
>
> On Fri, 9 May 2008 15:20:29 +0800, Dave Young wrote:
>> Use class_for_each_device for iteration.
>>
>> Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
>>
>> ---
>>  drivers/i2c/i2c-core.c |   96 ++++++++++++++++++++++++++-----------------------
>>  1 file changed, 52 insertions(+), 44 deletions(-)
>>
>> diff -upr a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
>> --- a/drivers/i2c/i2c-core.c  2008-05-09 13:55:41.000000000 +0800
>> +++ b/drivers/i2c/i2c-core.c  2008-05-09 14:24:07.000000000 +0800
>> @@ -35,7 +35,6 @@
>>  #include <linux/completion.h>
>>  #include <linux/hardirq.h>
>>  #include <linux/irqflags.h>
>> -#include <linux/semaphore.h>
>>  #include <asm/uaccess.h>
>>
>>  #include "i2c-core.h"
>> @@ -655,6 +654,16 @@ EXPORT_SYMBOL(i2c_del_adapter);
>>
>>
>>  /* ------------------------------------------------------------------------- */
>> +static int __attach_adapter(struct device *dev, void *data)
>> +{
>> +     struct i2c_adapter *adapter;
>> +     struct i2c_driver *driver = data;
>> +
>> +     adapter = container_of(dev, struct i2c_adapter, dev);
>
> You could use to_i2c_adapter(dev).
>
>> +     driver->attach_adapter(adapter);
>> +
>> +     return 0;
>> +}
>>
>>  /*
>>   * An i2c_driver is used with one or more i2c_client (device) nodes to access
>> @@ -696,21 +705,52 @@ int i2c_register_driver(struct module *o
>>       pr_debug("i2c-core: driver [%s] registered\n", driver->driver.name);
>>
>>       /* legacy drivers scan i2c busses directly */
>> -     if (driver->attach_adapter) {
>> -             struct i2c_adapter *adapter;
>> +     if (driver->attach_adapter)
>> +             class_for_each_device(&i2c_adapter_class, driver,
>> +                                     __attach_adapter);
>> +
>> +     mutex_unlock(&core_lock);
>> +     return 0;
>> +}
>> +EXPORT_SYMBOL(i2c_register_driver);
>> +
>> +static int __detach_adapter(struct device *dev, void *data)
>> +{
>> +     struct list_head   *item2, *_n;
>> +     struct i2c_client  *client;
>> +     struct i2c_adapter *adap;
>> +     struct i2c_driver *driver = data;
>>
>> -             down(&i2c_adapter_class.sem);
>> -             list_for_each_entry(adapter, &i2c_adapter_class.devices,
>> -                                 dev.node) {
>> -                     driver->attach_adapter(adapter);
>> +     /* Have a look at each adapter, if clients of this driver
>> +      * are still attached. If so, detach them to be able to
>> +      * kill the driver afterwards.
>> +      */
>> +     adap = container_of(dev, struct i2c_adapter, dev);
>
> You could use to_i2c_adapter(dev).
>
>> +     if (driver->detach_adapter) {
>
> Testing for a method...
>
>> +             if (driver->attach_adapter(adap))
>
> ... but calling another one? Can't be correct. Which raises a question:
> you didn't test your patch, did you? I'm also surprised how you managed
> to mess this up, given that all you had to do was to move already
> existing code around.

Thanks for review, It's only boot-tested with i2c part as built-in by
me because I have the i2c device to test.

>
>> +                     dev_err(&adap->dev, "detach_adapter failed "
>> +                                     "for driver [%s]\n",
>> +                                     driver->driver.name);
>> +     } else {
>> +             list_for_each_safe(item2, _n, &adap->clients) {
>> +                     client = list_entry(item2, struct i2c_client,
>> +                                     list);
>> +                     if (client->driver != driver)
>> +                             continue;
>> +                     dev_dbg(&adap->dev, "detaching client [%s] "
>> +                             "at 0x%02x\n", client->name,
>> +                             client->addr);
>> +                     if (driver->detach_client(client)) {
>> +                             dev_err(&adap->dev, "detach_client "
>> +                                     "failed for client [%s] at "
>> +                                     "0x%02x\n", client->name,
>> +                                     client->addr);
>> +                     }
>>               }
>> -             up(&i2c_adapter_class.sem);
>>       }
>>
>> -     mutex_unlock(&core_lock);
>>       return 0;
>>  }
>> -EXPORT_SYMBOL(i2c_register_driver);
>>
>>  /**
>>   * i2c_del_driver - unregister I2C driver
>> @@ -719,46 +759,14 @@ EXPORT_SYMBOL(i2c_register_driver);
>>   */
>>  void i2c_del_driver(struct i2c_driver *driver)
>>  {
>> -     struct list_head   *item2, *_n;
>> -     struct i2c_client  *client;
>> -     struct i2c_adapter *adap;
>> -
>>       mutex_lock(&core_lock);
>>
>>       /* new-style driver? */
>>       if (is_newstyle_driver(driver))
>>               goto unregister;
>>
>> -     /* Have a look at each adapter, if clients of this driver are still
>> -      * attached. If so, detach them to be able to kill the driver
>> -      * afterwards.
>> -      */
>> -     down(&i2c_adapter_class.sem);
>> -     list_for_each_entry(adap, &i2c_adapter_class.devices, dev.node) {
>> -             if (driver->detach_adapter) {
>> -                     if (driver->detach_adapter(adap)) {
>> -                             dev_err(&adap->dev, "detach_adapter failed "
>> -                                     "for driver [%s]\n",
>> -                                     driver->driver.name);
>> -                     }
>> -             } else {
>> -                     list_for_each_safe(item2, _n, &adap->clients) {
>> -                             client = list_entry(item2, struct i2c_client, list);
>> -                             if (client->driver != driver)
>> -                                     continue;
>> -                             dev_dbg(&adap->dev, "detaching client [%s] "
>> -                                     "at 0x%02x\n", client->name,
>> -                                     client->addr);
>> -                             if (driver->detach_client(client)) {
>> -                                     dev_err(&adap->dev, "detach_client "
>> -                                             "failed for client [%s] at "
>> -                                             "0x%02x\n", client->name,
>> -                                             client->addr);
>> -                             }
>> -                     }
>> -             }
>> -     }
>> -     up(&i2c_adapter_class.sem);
>> +     class_for_each_device(&i2c_adapter_class, driver,
>> +                             __detach_adapter);
>>
>>   unregister:
>>       driver_unregister(&driver->driver);
>
> The rest looks OK. I'll fix the patch myself and add it to my i2c tree
> now.

Glad to see that, thanks.

Regards
dave

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] i2c : use class_for_each_device
  2008-05-21  1:29   ` Dave Young
@ 2008-05-21  1:30     ` Dave Young
  2008-05-21  6:00       ` Jean Delvare
  0 siblings, 1 reply; 5+ messages in thread
From: Dave Young @ 2008-05-21  1:30 UTC (permalink / raw
  To: Jean Delvare; +Cc: gregkh, i2c, linux-kernel

On Wed, May 21, 2008 at 9:29 AM, Dave Young <hidave.darkstar@gmail.com> wrote:
> On Wed, May 21, 2008 at 1:31 AM, Jean Delvare <khali@linux-fr.org> wrote:
>> Hi Dave,
>>
>> On Fri, 9 May 2008 15:20:29 +0800, Dave Young wrote:
>>> Use class_for_each_device for iteration.
>>>
>>> Signed-off-by: Dave Young <hidave.darkstar@gmail.com>
>>>
>>> ---
>>>  drivers/i2c/i2c-core.c |   96 ++++++++++++++++++++++++++-----------------------
>>>  1 file changed, 52 insertions(+), 44 deletions(-)
>>>
>>> diff -upr a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
>>> --- a/drivers/i2c/i2c-core.c  2008-05-09 13:55:41.000000000 +0800
>>> +++ b/drivers/i2c/i2c-core.c  2008-05-09 14:24:07.000000000 +0800
>>> @@ -35,7 +35,6 @@
>>>  #include <linux/completion.h>
>>>  #include <linux/hardirq.h>
>>>  #include <linux/irqflags.h>
>>> -#include <linux/semaphore.h>
>>>  #include <asm/uaccess.h>
>>>
>>>  #include "i2c-core.h"
>>> @@ -655,6 +654,16 @@ EXPORT_SYMBOL(i2c_del_adapter);
>>>
>>>
>>>  /* ------------------------------------------------------------------------- */
>>> +static int __attach_adapter(struct device *dev, void *data)
>>> +{
>>> +     struct i2c_adapter *adapter;
>>> +     struct i2c_driver *driver = data;
>>> +
>>> +     adapter = container_of(dev, struct i2c_adapter, dev);
>>
>> You could use to_i2c_adapter(dev).
>>
>>> +     driver->attach_adapter(adapter);
>>> +
>>> +     return 0;
>>> +}
>>>
>>>  /*
>>>   * An i2c_driver is used with one or more i2c_client (device) nodes to access
>>> @@ -696,21 +705,52 @@ int i2c_register_driver(struct module *o
>>>       pr_debug("i2c-core: driver [%s] registered\n", driver->driver.name);
>>>
>>>       /* legacy drivers scan i2c busses directly */
>>> -     if (driver->attach_adapter) {
>>> -             struct i2c_adapter *adapter;
>>> +     if (driver->attach_adapter)
>>> +             class_for_each_device(&i2c_adapter_class, driver,
>>> +                                     __attach_adapter);
>>> +
>>> +     mutex_unlock(&core_lock);
>>> +     return 0;
>>> +}
>>> +EXPORT_SYMBOL(i2c_register_driver);
>>> +
>>> +static int __detach_adapter(struct device *dev, void *data)
>>> +{
>>> +     struct list_head   *item2, *_n;
>>> +     struct i2c_client  *client;
>>> +     struct i2c_adapter *adap;
>>> +     struct i2c_driver *driver = data;
>>>
>>> -             down(&i2c_adapter_class.sem);
>>> -             list_for_each_entry(adapter, &i2c_adapter_class.devices,
>>> -                                 dev.node) {
>>> -                     driver->attach_adapter(adapter);
>>> +     /* Have a look at each adapter, if clients of this driver
>>> +      * are still attached. If so, detach them to be able to
>>> +      * kill the driver afterwards.
>>> +      */
>>> +     adap = container_of(dev, struct i2c_adapter, dev);
>>
>> You could use to_i2c_adapter(dev).
>>
>>> +     if (driver->detach_adapter) {
>>
>> Testing for a method...
>>
>>> +             if (driver->attach_adapter(adap))
>>
>> ... but calling another one? Can't be correct. Which raises a question:
>> you didn't test your patch, did you? I'm also surprised how you managed
>> to mess this up, given that all you had to do was to move already
>> existing code around.
>
> Thanks for review, It's only boot-tested with i2c part as built-in by
> me because I have the i2c device to test.

should be "have no the i2c device to test" ;)

>
>>
>>> +                     dev_err(&adap->dev, "detach_adapter failed "
>>> +                                     "for driver [%s]\n",
>>> +                                     driver->driver.name);
>>> +     } else {
>>> +             list_for_each_safe(item2, _n, &adap->clients) {
>>> +                     client = list_entry(item2, struct i2c_client,
>>> +                                     list);
>>> +                     if (client->driver != driver)
>>> +                             continue;
>>> +                     dev_dbg(&adap->dev, "detaching client [%s] "
>>> +                             "at 0x%02x\n", client->name,
>>> +                             client->addr);
>>> +                     if (driver->detach_client(client)) {
>>> +                             dev_err(&adap->dev, "detach_client "
>>> +                                     "failed for client [%s] at "
>>> +                                     "0x%02x\n", client->name,
>>> +                                     client->addr);
>>> +                     }
>>>               }
>>> -             up(&i2c_adapter_class.sem);
>>>       }
>>>
>>> -     mutex_unlock(&core_lock);
>>>       return 0;
>>>  }
>>> -EXPORT_SYMBOL(i2c_register_driver);
>>>
>>>  /**
>>>   * i2c_del_driver - unregister I2C driver
>>> @@ -719,46 +759,14 @@ EXPORT_SYMBOL(i2c_register_driver);
>>>   */
>>>  void i2c_del_driver(struct i2c_driver *driver)
>>>  {
>>> -     struct list_head   *item2, *_n;
>>> -     struct i2c_client  *client;
>>> -     struct i2c_adapter *adap;
>>> -
>>>       mutex_lock(&core_lock);
>>>
>>>       /* new-style driver? */
>>>       if (is_newstyle_driver(driver))
>>>               goto unregister;
>>>
>>> -     /* Have a look at each adapter, if clients of this driver are still
>>> -      * attached. If so, detach them to be able to kill the driver
>>> -      * afterwards.
>>> -      */
>>> -     down(&i2c_adapter_class.sem);
>>> -     list_for_each_entry(adap, &i2c_adapter_class.devices, dev.node) {
>>> -             if (driver->detach_adapter) {
>>> -                     if (driver->detach_adapter(adap)) {
>>> -                             dev_err(&adap->dev, "detach_adapter failed "
>>> -                                     "for driver [%s]\n",
>>> -                                     driver->driver.name);
>>> -                     }
>>> -             } else {
>>> -                     list_for_each_safe(item2, _n, &adap->clients) {
>>> -                             client = list_entry(item2, struct i2c_client, list);
>>> -                             if (client->driver != driver)
>>> -                                     continue;
>>> -                             dev_dbg(&adap->dev, "detaching client [%s] "
>>> -                                     "at 0x%02x\n", client->name,
>>> -                                     client->addr);
>>> -                             if (driver->detach_client(client)) {
>>> -                                     dev_err(&adap->dev, "detach_client "
>>> -                                             "failed for client [%s] at "
>>> -                                             "0x%02x\n", client->name,
>>> -                                             client->addr);
>>> -                             }
>>> -                     }
>>> -             }
>>> -     }
>>> -     up(&i2c_adapter_class.sem);
>>> +     class_for_each_device(&i2c_adapter_class, driver,
>>> +                             __detach_adapter);
>>>
>>>   unregister:
>>>       driver_unregister(&driver->driver);
>>
>> The rest looks OK. I'll fix the patch myself and add it to my i2c tree
>> now.
>
> Glad to see that, thanks.
>
> Regards
> dave
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/3] i2c : use class_for_each_device
  2008-05-21  1:30     ` Dave Young
@ 2008-05-21  6:00       ` Jean Delvare
  0 siblings, 0 replies; 5+ messages in thread
From: Jean Delvare @ 2008-05-21  6:00 UTC (permalink / raw
  To: Dave Young; +Cc: gregkh, i2c, linux-kernel

On Wed, 21 May 2008 09:30:59 +0800, Dave Young wrote:
> On Wed, May 21, 2008 at 9:29 AM, Dave Young <hidave.darkstar@gmail.com> wrote:
> > On Wed, May 21, 2008 at 1:31 AM, Jean Delvare <khali@linux-fr.org> wrote:
> >> ... but calling another one? Can't be correct. Which raises a question:
> >> you didn't test your patch, did you? I'm also surprised how you managed
> >> to mess this up, given that all you had to do was to move already
> >> existing code around.
> >
> > Thanks for review, It's only boot-tested with i2c part as built-in by
> > me because I have the i2c device to test.
> 
> should be "have no the i2c device to test" ;)

Too late for this time, but for next time, you can emulate an i2c
device using the i2c-stub driver, that's very convenient when you want
to test a change that affects i2c and don't have any physical device to
test it.

-- 
Jean Delvare

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2008-05-21  6:01 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-05-09  7:20 [PATCH 1/3] i2c : use class_for_each_device Dave Young
2008-05-20 17:31 ` Jean Delvare
2008-05-21  1:29   ` Dave Young
2008-05-21  1:30     ` Dave Young
2008-05-21  6:00       ` Jean Delvare

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).