| 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: <a8e1da0805201829n439f251uef438bca0b1d006@mail.gmail.com>
Date: Wed, 21 May 2008 09:29:51 +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 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.
>
>> + 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