Messages in this thread | | | Date | Fri, 28 Dec 2018 09:42:36 +0000 | From | Charles Keepax <> | Subject | Re: [PATCH 18/18] mfd: wm8400-core: Make it explicitly non-modular |
| |
On Fri, Dec 21, 2018 at 10:55:16AM -0500, Paul Gortmaker wrote: > [Re: [PATCH 18/18] mfd: wm8400-core: Make it explicitly non-modular] On 19/12/2018 (Wed 09:17) Charles Keepax wrote: > > > On Mon, Dec 17, 2018 at 03:31:28PM -0500, Paul Gortmaker wrote: > > > -MODULE_DEVICE_TABLE(i2c, wm8400_i2c_id); > > > > > > static struct i2c_driver wm8400_i2c_driver = { > > > .driver = { > > > @@ -161,7 +160,7 @@ static struct i2c_driver wm8400_i2c_driver = { > > > }; > > > #endif > > > > Do we not want to add suppress_bind_attrs into the i2c_driver > > struct here? > > We can add one if you/maintainers want one, but if you look at the > original patch, this driver was using the more classic/legacy case of > subsys_init() vs. platform_driver_register() used in other drivers. > > Not adding a suppress_bind_attrs here was intentional, since I'd decided > to put in the unbind entries for code that used platform_driver_register() > where the author had created the .remove code, on the assumption that they > had put some thought into the process of unbind/remove - to make it > explicit that unbind is now disabled. > > To be clear, using the subsys_init() doesn't implicitly disable unbind. > However, there are lots of non-modular drivers out there; ones I've not > even touched, and to start a project to add an unbind disable to them > all is beyond the scope of the goals I've listed in the 00/18 preamble. > > I'd hope maybe we can revisit the global default setting for non-modular > code someday - to make non-modules opt-in instead of opt-out, and > achieve better consistency from one driver to the next, without having > to add a new .driver sub-struct to each file for the suppress entry. > > I think LinusW hinted at in an earlier email in this ongoing review, > that the default setting didn't quite make sense to him either. But in > any case, that is a separate discussion for another time and place. > > Let me know if you explicitly want one added, otherwise I'll just leave > the .remove + .suppress_bind_attrs pairing as described above. >
Nah its ok, if you are specifically not doing this one I think I am ok with it. Just seemed out of place compared to the the others:
Acked-by: Charles Keepax <ckeepax@opensource.cirrus.com>
Thanks, Charles
| |