Message-ID: <1743f479-68c8-5f27-8d35-e17d5c96b60a@linaro.org>
Date:   Wed, 2 Sep 2020 06:46:01 -0500
From:   Alex Elder <elder@...aro.org>
To:     Randy Dunlap <rdunlap@...radead.org>,
        Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>
Cc:     Networking <netdev@...r.kernel.org>
Subject: Re: COMPILE_TEST

On 9/1/20 7:52 PM, Randy Dunlap wrote:
> On 9/1/20 5:17 PM, Jakub Kicinski wrote:
>> On Tue, 1 Sep 2020 23:48:52 +0200 Andrew Lunn wrote:
>>> On Tue, Sep 01, 2020 at 03:22:31PM -0500, Alex Elder wrote:
>>>> Jakub, you suggested/requested that the Qualcomm IPA driver get
>>>> built when the COMPILE_TEST config option is enabled.  I started
>>>> working on this a few months ago but didn't finish, and picked
>>>> it up again today.  I'd really like to get this done soon.
>>>>
>>>> The QCOM_IPA config option depends on and selects other things,
>>>> and those other things depend on and select still more config
>>>> options.  I've worked through some of these, but now question
>>>> whether this is even the right approach.  Should I try to ensure
>>>> all the code the IPA driver depends on and selects *also* gets
>>>> built when COMPILE_TEST is enabled?  Or should I try to minimize
>>>> the impact on other code by making IPA config dependencies and
>>>> selections also depend on the value of COMPILE_TEST?
>>>>
>>>> Is there anything you know of that describes best practice for
>>>> enabling a config option when COMPILE_TEST is enabled?  
>>>
>>> Hi Alex
>>>
>>> In general everything which can be build with COMPILE_TEST should be
>>> built with COMPILE_TEST. So generally it just works, because
>>> everything selected should already be selected because they already
>>> have COMPILE_TEST.

This makes sense.  And it sounds to me that you are saying I should
ensure every dependency and selected option is also built (or maybe
tolerates) having COMPILE_TEST enabled.  It's basically the approach
I was following, but the chain of dependencies means it affects a
lot more code than I wanted, including the ARM SCM (e.g. Trust
Zone) entry points.  I agree things should "just work" but before
I went too much further I wanted to see if there was a better way.

>>> Correctly written drivers should compile for just about any
>>> architecture. If they don't it suggests they are not using the APIs
>>> correctly, and should be fixed.
>>>
>>> If the dependencies have not had COMPILE_TEST before, you are probably
>>> in for some work, but in the end all the drivers will be of better
>>> quality, and get build tested a lot more.

Yes I understand this, and agree with it.

>> Nothing to add :) I'm not aware of any codified best practices.
>>
> 
> I have a little to add, but maybe some can complete this more
> than I can.
> 
> Several subsystem header files add stubs for when that subsystem is not
> enabled.  I know <linux/of.h> works with CONFIG_OF=y or =n, with lots of stubs.
> 
> Same is true for <linux/gpio.h> and CONFIG_GPIOLIB.

Yes I saw this in some places.  In other places I saw things
addressed by (for example) something like:
  config FOO
    ...
    select BAR if !COMPILE_TEST

I didn't chase down whether that meant code enabled by FOO
created its own stubs, or if something else was going on.

> It would be good to know which other CONFIG symbols and header files
> are known to work (or expected to work) like this.
> 
> Having these stubs allows us to always either omit e.g.
> 	depends on GPIOLIB

The above could only be done if the dependency is simply for
linkage and not functionality.  Maybe that makes sense in
some cases but it seems like a mistake.

> or make it say
> 	depends on GPIOLIB || COMPILE_TEST
> 
> 
> Also, there are $ARCH conditions whose drivers also usually
> could benefit from COMPILE_TEST, so we often see for a driver:
> 
> 	depends on MIPS || COMPILE_TEST
> or
> 	depends on ARCH_some_arm_derivative || COMPILE_TEST
> or
> 	depends on *PLATFORM* || COMPILE_TEST
> or
> 	depends on ARCH_TEGRA || COMPILE_TEST
> 
> So if a driver is destined (or designed) for one h/w environment,
> we very much want to build it with COMPILE_TEST for other h/w platforms.

Yes, that's really my purpose here.  Thanks a lot for your
comments.  I'll continue with what I've been doing and will
incorporate this input where possible.

					-Alex

> HTH.
> 

Powered by blists - more mailing lists