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.
>
