Re: [PATCH]I2C device - release cleanup
On Tue, 23 Mar 2010 19:11:26 +, jhautb...@gmail.com wrote: > > Yes, that would be better. If you do that, please make sure to run your > > > patch through scripts/checkpatch.pl before sending it. > > I will take the time to do it. I can't say when :p. > I did run checkpatch.pl, and I can't remove the 3 "trailing whitespace" > errors... You'll have to, otherwise I won't apply your patches (nobody will). > Plus the fact that gmail is changing tabs and spaces... Check the options. At least attaching the patch should work. Or you can use a "real" e-mail client for sending patches. -- Jean Delvare -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH]I2C device - release cleanup
On Tue, 23 Mar 2010 13:02:41 +, jhautb...@gmail.com wrote: > Hi Jean, > > < snip > > > Did you test your patch? I am very skeptical that calling > > > single_release() out of the blue is the right thing to do. My instinct > > > tells me that single_release() is only meant for callers of > > > single_open(). > > Well, using this call works fine with my hardware. > I would say, as before :). > Looking at the source code of single_release, this is very similar to what > is done today. > > But yes, it would also be interesting to use single_open in the open() > syscall. > I think this would be nicer to use only single_*() functions. > > Maybe is it interesting to submit a patch that does a cleanup for all the > i2c-dev file ? And not only the release function ? Yes, that would be better. If you do that, please make sure to run your patch through scripts/checkpatch.pl before sending it. -- Jean Delvare -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH]I2C device - release cleanup
Hi Jean-Michel,
On Tue, 23 Mar 2010 11:01:28 +0100, Jean-Michel Hautbois wrote:
> Hi there,
>
> Here is a little patch which aims to cleanup the release function in
> i2c-dev.c.
> This is only a call to single_release, instead of kfree and several things.
>
> Signed-off-by: Jean-Michel Hautbois
> ---
> drivers/i2c/i2c-dev.c
>
> --- linux-2.6.34-rc2/drivers/i2c/i2c-dev.c.orig 2010-03-23
> 10:19:26.0 +0100
> +++ linux-2.6.34-rc2/drivers/i2c/i2c-dev.c2010-03-23 10:55:54.0
> +0100
> @@ -36,6 +36,7 @@
> #include
> #include
> #include
> +#include
>
> static struct i2c_driver i2cdev_driver;
>
> @@ -477,12 +478,10 @@ static int i2cdev_open(struct inode *ino
> static int i2cdev_release(struct inode *inode, struct file *file)
> {
> struct i2c_client *client = file->private_data;
> -
> +
This is adding trailing white-space. Obviously you did not review your
own patch before sending it. And you did not run it through
scripts/checkpatch.pl either.
> i2c_put_adapter(client->adapter);
> - kfree(client);
> - file->private_data = NULL;
> -
> - return 0;
> +
> + return single_release(inode, file);
> }
>
> static const struct file_operations i2cdev_fops = {
Did you test your patch? I am very skeptical that calling
single_release() out of the blue is the right thing to do. My instinct
tells me that single_release() is only meant for callers of
single_open().
--
Jean Delvare
http://khali.linux-fr.org/wishlist.html
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
