From: Felipe Balbi <balbi@ti•com>
To: Michal Marek <mmarek@suse•cz>
Cc: Jiri Slaby <jslaby@suse•cz>, Jeff Mahoney <jeffm@suse•com>,
Greg Kroah-Hartman <gregkh@linuxfoundation•org>,
<jirislaby@gmail•com>, <linux-kernel@vger•kernel.org>,
Andrew Morton <akpm@linux-foundation•org>,
Linus Torvalds <torvalds@linux-foundation•org>,
Alexander Shishkin <alexander.shishkin@linux•intel.com>,
<linux-usb@vger•kernel.org>,
Florian Tobias Schandinat <FlorianSchandinat@gmx•de>,
<linux-geode@lists•infradead.org>, <linux-fbdev@vger•kernel.org>,
Richard Cochran <richardcochran@gmail•com>,
<netdev@vger•kernel.org>, Ben Hutchings <ben@decadent•org.uk>,
"Keller, Jacob E" <jacob.e.keller@intel•com>,
<tomi.valkeinen@ti•com>
Subject: Re: [PATCH] build some drivers only when compile-testing
Date: Tue, 18 Jun 2013 11:18:58 +0300 [thread overview]
Message-ID: <20130618081858.GD5461@arwen.pp.htv.fi> (raw)
In-Reply-To: <51BFE754.5080301@suse.cz>
[-- Attachment #1: Type: text/plain, Size: 4500 bytes --]
Hi,
On Tue, Jun 18, 2013 at 06:51:32AM +0200, Michal Marek wrote:
> Dne 17.6.2013 22:05, Jiri Slaby napsal(a):
> > On 05/23/2013 05:09 AM, Jeff Mahoney wrote:
> >> On 5/22/13 10:23 PM, Greg Kroah-Hartman wrote:
> >>> On Wed, May 22, 2013 at 11:18:46AM +0200, Jiri Slaby wrote:
> >>>> Some drivers can be built on more platforms than they run on. This
> >>>> causes users and distributors packaging burden when they have to
> >>>> manually deselect some drivers from their allmodconfigs. Or sometimes
> >>>> it is even impossible to disable the drivers without patching the
> >>>> kernel.
> >>>>
> >>>> Introduce a new config option COMPILE_TEST and make all those drivers
> >>>> to depend on the platform they run on, or on the COMPILE_TEST option.
> >>>> Now, when users/distributors choose COMPILE_TEST=n they will not have
> >>>> the drivers in their allmodconfig setups, but developers still can
> >>>> compile-test them with COMPILE_TEST=y.
> >>>
> >>> I understand the urge, and it's getting hard for distros to handle these
> >>> drivers that just don't work on other architectures, but it's really
> >>> valuable to ensure that they build properly, for those of us that don't
> >>> have many/any cross compilers set up.
> >
> > But this is exactly what COMPILE_TEST will give us when set to "y", or
> > am I missing something?
> >
> >>>> Now the drivers where we use this new option:
> >>>> * PTP_1588_CLOCK_PCH: The PCH EG20T is only compatible with Intel Atom
> >>>> processors so it should depend on x86.
> >>>> * FB_GEODE: Geode is 32-bit only so only enable it for X86_32.
> >>>> * USB_CHIPIDEA_IMX: The OF_DEVICE dependency will be met on powerpc
> >>>> systems -- which do not actually support the hardware via that
> >>>> method.
> >>>
> >>> This seems ripe to start to get really messy, really quickly. Shouldn't
> >>> "default configs" handle if this "should" be enabled for a platform or
> >>> not, and let the rest of us just build them with no problems?
> >>
> >> If every time a new Kconfig option is added, corresponding default
> >> config updates come with it, sure. I just don't see that happening,
> >> especially when it can be done much more clearly in the Kconfig while
> >> the developer is writing the driver.
> >>
> >>> What problems is this causing you? Are you running out of space in
> >>> kernel packages with drivers that will never be actually used?
> >>
> >> Wasted build resources. Wasted disk space on /every/ system the kernel
> >> package is installed on. We're all trying to pare down the kernel
> >> packages to eliminate wasted space and doing it manually means a bunch
> >> of research, sometimes with incorrect assumptions about the results,
> >> needs to be done by someone not usually associated with that code. That
> >> research gets repeated by people maintaining kernel packages for pretty
> >> much every distro.
> >
> > I second all the above.
> >
> >>>> +config COMPILE_TEST
> >>>> + bool "Compile also drivers which will not load" if EXPERT
> >>>
> >>> EXPERT is getting to be the "let's hide it here" option, isn't it...
> >>>
> >>> I don't know, if no one else strongly objects, I can be convinced that
> >>> this is needed, but so far, I don't see why it really is, or what this
> >>> is going to help with.
> >>
> >> I'm not convinced adding a || COMPILE_TEST option to every driver that
> >> may be arch specific is the best way to go either. Perhaps adding a new
> >> Kconfig verb called "archdepends on" or something that will evaluate as
> >> true if COMPILE_TEST is enabled but will evaluate the conditional if
> >> not. *waves hands*
> >
> > Sam Ravnborg (the kconfig ex-maintainer) once wrote that he doesn't want
> > to extend the kconfig language for this purpose (which I support). That
> > a config option is fine and sufficient in this case [1]. Except he
> > called the config option "SHOW_ALL_DRIVERS". Adding the current
> > maintainer to CCs ;).
>
> I agree with Sam. 'depends on XY || COMPILE_TEST' is quite
> self-explanatory. And even if it's not, you can look up the help text
> for COMPILE_TEST. With "archdepends on" or "available on", you need to
> know what to look for to override the dependency.
you will still end up with:
depends on (ARCH_OMAP || ARCH_EXYNOS || ARCH_DAVINCI || ARCH_PPC || ...)
And every now and again that particular line will be updated to add
another arch dependency.
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
next prev parent reply other threads:[~2013-06-18 8:21 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-05-22 9:18 [PATCH] build some drivers only when compile-testing Jiri Slaby
2013-05-23 2:23 ` Greg Kroah-Hartman
[not found] ` <20130523022327.GB6159-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2013-05-23 3:09 ` Jeff Mahoney
2013-06-17 20:05 ` Jiri Slaby
2013-06-18 4:51 ` Michal Marek
2013-06-18 8:18 ` Felipe Balbi [this message]
2013-06-18 8:24 ` Jiri Slaby
[not found] ` <51C01948.9060708-AlSwsSmVLrQ@public.gmane.org>
2013-06-18 8:34 ` Felipe Balbi
2013-06-18 8:44 ` Michal Marek
[not found] ` <51C01E04.2050908-AlSwsSmVLrQ@public.gmane.org>
2013-06-18 8:51 ` Felipe Balbi
2013-06-18 9:21 ` Jiri Slaby
2013-06-19 16:38 ` Mark Brown
2013-06-18 8:35 ` Tomi Valkeinen
2013-06-18 16:04 ` Greg Kroah-Hartman
[not found] ` <20130618160427.GE28961-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2013-06-19 6:50 ` Jiri Slaby
2013-06-24 23:42 ` Greg Kroah-Hartman
2013-06-25 8:16 ` Jiri Slaby
2013-06-19 7:10 ` Tomi Valkeinen
2013-06-19 7:12 ` Jiri Slaby
2013-06-19 7:19 ` Tomi Valkeinen
[not found] ` <51C159F8.8090604-AlSwsSmVLrQ@public.gmane.org>
2013-06-19 14:27 ` Greg Kroah-Hartman
2013-06-21 11:11 ` [PATCH v3] " Jiri Slaby
2013-05-23 14:01 ` [PATCH] " Ben Hutchings
2013-05-24 4:50 ` Rob Landley
2013-05-23 7:46 ` Tomi Valkeinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130618081858.GD5461@arwen.pp.htv.fi \
--to=balbi@ti$(echo .)com \
--cc=FlorianSchandinat@gmx$(echo .)de \
--cc=akpm@linux-foundation$(echo .)org \
--cc=alexander.shishkin@linux$(echo .)intel.com \
--cc=ben@decadent$(echo .)org.uk \
--cc=gregkh@linuxfoundation$(echo .)org \
--cc=jacob.e.keller@intel$(echo .)com \
--cc=jeffm@suse$(echo .)com \
--cc=jirislaby@gmail$(echo .)com \
--cc=jslaby@suse$(echo .)cz \
--cc=linux-fbdev@vger$(echo .)kernel.org \
--cc=linux-geode@lists$(echo .)infradead.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-usb@vger$(echo .)kernel.org \
--cc=mmarek@suse$(echo .)cz \
--cc=netdev@vger$(echo .)kernel.org \
--cc=richardcochran@gmail$(echo .)com \
--cc=tomi.valkeinen@ti$(echo .)com \
--cc=torvalds@linux-foundation$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox