From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753521AbbE1Aw2 (ORCPT ); Wed, 27 May 2015 20:52:28 -0400 Received: from mail1.windriver.com ([147.11.146.13]:33139 "EHLO mail1.windriver.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751185AbbE1Aw1 (ORCPT ); Wed, 27 May 2015 20:52:27 -0400 Date: Wed, 27 May 2015 20:50:22 -0400 From: Paul Gortmaker To: Linus Walleij CC: "linux-kernel@vger.kernel.org" , Greg Kroah-Hartman Subject: Re: [PATCH 1/7] platform_device: better support builtin boilerplate avoidance Message-ID: <20150528005022.GA23774@windriver.com> References: <1431287385-1526-1-git-send-email-paul.gortmaker@windriver.com> <1431287385-1526-2-git-send-email-paul.gortmaker@windriver.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org [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 > 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 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 > > Signed-off-by: Paul Gortmaker > > 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