lkml.org 
[lkml]   [2008]   [May]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/3] i2c : use class_for_each_device
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
>


\
 
 \ /
  Last update: 2008-05-21 03:33    [W:0.199 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site