Message ID | 1516018126-16119-1-git-send-email-hofrat@osadl.org |
---|---|
State | New |
Headers | show |
Series | Documentation: i2c: drop unnecessary .owner field in examples | expand |
On Mon, Jan 15, 2018 at 2:08 PM, Nicholas Mc Guire <hofrat@osadl.org> wrote: > From: Nicholas Mc Guire <hofrat@osadl.at> > > Currently there are a few drivers that still set the .owner > in the i2c_driver structure - all of which are reported by > coccinelle (scripts/coccinelle/api/platform_no_drv_owner.cocci) > and there are no cases that set the .onwer and do not call any > of the functions that set the .owner field anyway in any of the > drivers (checked by a modified coccinelle script based on the > above) so it seems that the examples are no longer valid and > .owner = THIS_MODULE, can be removed here. > > While at it an obvious typo (new new) was also fixed. AFAIU It is right only in case when someone does this, e.g. module_i2c_driver() macro. Otherwise the field is pretty valid and must be filled.
On Mon, Jan 15, 2018 at 10:24:52PM +0200, Andy Shevchenko wrote: > On Mon, Jan 15, 2018 at 2:08 PM, Nicholas Mc Guire <hofrat@osadl.org> wrote: > > From: Nicholas Mc Guire <hofrat@osadl.at> > > > > Currently there are a few drivers that still set the .owner > > in the i2c_driver structure - all of which are reported by > > coccinelle (scripts/coccinelle/api/platform_no_drv_owner.cocci) > > and there are no cases that set the .onwer and do not call any > > of the functions that set the .owner field anyway in any of the > > drivers (checked by a modified coccinelle script based on the > > above) so it seems that the examples are no longer valid and > > .owner = THIS_MODULE, can be removed here. > > > > While at it an obvious typo (new new) was also fixed. > > AFAIU It is right only in case when someone does this, e.g. > module_i2c_driver() macro. Otherwise the field is pretty valid and > must be filled. It gets filled with i2c_add_driver. module_i2c_driver uses i2c_add_driver. I was about to suggest to keep the field in the old driver and describe that it can be removed when using one of i2c_add_driver or module_i2c_driver. But then I realised that the kernel tree does not have any such old drivers anymore and I couldn't even find out-of-tree code via some search engines (I tried looking for "I2C_CLIENT_INSMOD"). I consider this obsolete and irrelevant these days. It might be good to simply remove it to not confuse users. Thoughts?
On Mon, Jan 15, 2018 at 09:28:47PM +0100, Wolfram Sang wrote: > On Mon, Jan 15, 2018 at 10:24:52PM +0200, Andy Shevchenko wrote: > > On Mon, Jan 15, 2018 at 2:08 PM, Nicholas Mc Guire <hofrat@osadl.org> wrote: > > > From: Nicholas Mc Guire <hofrat@osadl.at> > > > > > > Currently there are a few drivers that still set the .owner > > > in the i2c_driver structure - all of which are reported by > > > coccinelle (scripts/coccinelle/api/platform_no_drv_owner.cocci) > > > and there are no cases that set the .onwer and do not call any > > > of the functions that set the .owner field anyway in any of the > > > drivers (checked by a modified coccinelle script based on the > > > above) so it seems that the examples are no longer valid and > > > .owner = THIS_MODULE, can be removed here. > > > > > > While at it an obvious typo (new new) was also fixed. > > > > AFAIU It is right only in case when someone does this, e.g. > > module_i2c_driver() macro. Otherwise the field is pretty valid and > > must be filled. > > It gets filled with i2c_add_driver. module_i2c_driver uses > i2c_add_driver. I was about to suggest to keep the field in the old > driver and describe that it can be removed when using one of > i2c_add_driver or module_i2c_driver. > > But then I realised that the kernel tree does not have any such old > drivers anymore and I couldn't even find out-of-tree code via some > search engines (I tried looking for "I2C_CLIENT_INSMOD"). > > I consider this obsolete and irrelevant these days. It might be good to > simply remove it to not confuse users. > Not sure what the status of this is now - but I would want to clean up some of the coccinelle findings - as a pre-requisite it would make sense to either drop the examples inclusion of .owner = THIS_MODULE or add a note in the documentation making clear that this is only needed in case where the appropriate module initialization helpers are not used. Is there any good reason *not* to use these initialization helpers when upgrading a driver ? If not (and I could not find one) then it might simply be the right way to recommend using the initialization helpers and drop the .owner = THIS_MODULE from the examples. Anyway - cleaning up coccinelle findings would seem futile if the documentation may be the cause. thx! hofrat
diff --git a/Documentation/i2c/upgrading-clients b/Documentation/i2c/upgrading-clients index ccba3ff..d5e51ae 100644 --- a/Documentation/i2c/upgrading-clients +++ b/Documentation/i2c/upgrading-clients @@ -7,7 +7,7 @@ Introduction ------------ This guide outlines how to alter existing Linux 2.6 client drivers from -the old to the new new binding methods. +the old to the new binding methods. Example old-style driver @@ -77,7 +77,6 @@ static int example_attach_adapter(struct i2c_adapter *adap) static struct i2c_driver example_driver = { .driver = { - .owner = THIS_MODULE, .name = "example", .pm = &example_pm_ops, }, @@ -217,7 +216,6 @@ and other utilities: static struct i2c_driver example_driver = { .driver = { - .owner = THIS_MODULE, .name = "example", }, + .id_table = example_ids, @@ -269,7 +267,6 @@ MODULE_DEVICE_TABLE(i2c, example_idtable); static struct i2c_driver example_driver = { .driver = { - .owner = THIS_MODULE, .name = "example", .pm = &example_pm_ops, },