public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation•org>
To: Jiri Slaby <jslaby@suse•cz>
Cc: Jeff Mahoney <jeffm@suse•com>,
	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>,
	Michal Marek <mmarek@suse•cz>,
	tomi.valkeinen@ti•com
Subject: Re: [PATCH] build some drivers only when compile-testing
Date: Tue, 18 Jun 2013 09:04:27 -0700	[thread overview]
Message-ID: <20130618160427.GE28961@kroah.com> (raw)
In-Reply-To: <51BF6BFF.7050705@suse.cz>

On Mon, Jun 17, 2013 at 10:05:19PM +0200, Jiri Slaby wrote:
> 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 ;).
> 
> [1] http://comments.gmane.org/gmane.linux.kbuild.devel/9829
> 
> The last point I inclined to the Greg's argument to remove the EXPERT
> dependency.
> 
> So currently I have what is attached... Comments?

Looks good to me, want me to queue it up through my char/misc driver
tree for 3.11?

thanks,

greg k-h

  parent reply	other threads:[~2013-06-18 16:04 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
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 [this message]
     [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=20130618160427.GE28961@kroah.com \
    --to=gregkh@linuxfoundation$(echo .)org \
    --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=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