| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <a8e1da0805201830q88665afnf00e5f96f832f90@mail.gmail.com>
Date: Wed, 21 May 2008 09:30:59 +0800
From: "Dave Young" <hidave.darkstar@...il.com>
To: "Jean Delvare" <khali@...ux-fr.org>
Cc: gregkh@...e.de, i2c@...sensors.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] i2c : use class_for_each_device
On Wed, May 21, 2008 at 9:29 AM, Dave Young <hidave.darkstar@...il.com> wrote:
> On Wed, May 21, 2008 at 1:31 AM, Jean Delvare <khali@...ux-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@...il.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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists