Messages in this thread | | | Date | Wed, 27 May 2015 20:50:22 -0400 | From | Paul Gortmaker <> | Subject | Re: [PATCH 1/7] platform_device: better support builtin boilerplate avoidance |
| |
[Re: [PATCH 1/7] platform_device: better support builtin boilerplate avoidance] On 12/05/2015 (Tue 13:46) Linus Walleij wrote:
> On Sun, May 10, 2015 at 9:49 PM, Paul Gortmaker > <paul.gortmaker@windriver.com> wrote: > > > We have macros that help reduce the boilerplate for modules > > that register with no extra init/exit complexity other than the > > most standard use case. However we see an increasing number of > > non-modular drivers using these modular_driver() type register > > functions. > > > > There are several downsides to this: > > 1) The code can appear modular to a reader of the code, and they > > won't know if the code really is modular without checking the > > Makefile and Kconfig to see if compilation is governed by a > > bool or tristate. > > 2) Coders of drivers may be tempted to code up an __exit function > > that is never used, just in order to satisfy the required three > > args of the modular registration function. > > 3) Non-modular code ends up including the <module.h> which increases > > CPP overhead that they don't need. > > 4) It hinders us from performing better separation of the module > > init code and the generic init code. > > > > Here we introduce similar macros, with the mapping from module_driver > > to builtin_driver and similar, so that simple changes of: > > > > module_platform_driver() ---> builtin_platform_driver() > > module_platform_driver_probe() ---> builtin_platform_driver_probe(). > > > > can help us avoid #3 above, without having to code up the same > > __init functions and device_initcall() boilerplate. > > > > For non modular code, module_init becomes __initcall. But direct use > > of __initcall is discouraged, vs. one of the priority categorized > > subgroups. As __initcall gets mapped onto device_initcall, our > > use of device_initcall directly in this change means that the > > runtime impact is zero -- drivers will remain at level 6 in the > > initcall ordering. > > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> > > This does not inhibit probe() and remove() to be > triggered from sysfs does it? > > What is needed on builtin drivers is to set > .suppress_bind_attrs = true on the struct device_driver > so that we inhibit the creation of sysfs files to probe > and remove the driver by operator intervention.
Is this needed? I think we will break existing use cases if we do this.
For example, I have IGB as built-in, but I can still unbind one of the four devices and make it available for PCI pass through to KVM with:
echo "0000:0a:00.1" > /sys/bus/pci/drivers/igb/unbind echo "0000:0a:00.1" > /sys/bus/pci/drivers/pci-stub/bind
> > I don't know if there is a simple way do address > this though since you don't seem to operate on > the struct device_driver, just pass it on. > > Maybe it's possible to inhibit compilation of > builtin_platform_driver's if .suppress_bind_attrs == 0?
If we wanted to do this, I think we could simply do something like:
int __platform_driver_register(struct platform_driver *drv, struct module *owner) { drv->driver.owner = owner; drv->driver.bus = &platform_bus_type; + if (!owner) /* built in */ + drv->driver.suppress_bind_attrs = true; if (drv->probe) drv->driver.probe = platform_drv_probe; if (drv->remove)
...but again, I'm thinking that will break things for people, unless I'm missing something here.
Paul. --
> > Yours, > Linus Walleij
| |