Re: [PATCH] build some drivers only when compile-testing

[Jiri Slaby <jslaby@suse.cz>]

On 06/18/2013 10:18 AM, Felipe Balbi wrote:
> 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.

But that is perfectly fine *when* the driver is supported on those
archs only.

And come on, how much often will this "every now and again" happen? We
don't have that much cases where a driver is augmented to work on
another arch or platform. It either works on all of them => doesn't
need COMPILE_TEST, or work on one or two arches at most.

-- 
js
suse labs