From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Jean Delvare <jdelvare@suse.de>, Ohad Ben-Cohen <ohad@wizery.com>,
Peter Griffin <peter.griffin@linaro.org>,
Vinod Koul <vinod.koul@intel.com>,
Loic Pallardy <loic.pallardy@st.com>, Pavel Machek <pavel@ucw.cz>,
linux-remoteproc@vger.kernel.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [RESEND PATCH] remoteproc: qcom: fix QCOM_SMD dependencies
Date: Mon, 20 Mar 2017 15:22:08 -0700 [thread overview]
Message-ID: <20170320222208.GE20094@minitux> (raw)
In-Reply-To: <CAK8P3a0ZY1BBD8ChB7Qd0v4aEqiw+ifQ_nspOgJeve7wssbO9Q@mail.gmail.com>
On Tue 14 Mar 04:01 PDT 2017, Arnd Bergmann wrote:
> On Tue, Mar 14, 2017 at 10:05 AM, Jean Delvare <jdelvare@suse.de> wrote:
[..]
> > I don't think the COMPILE_TEST adds any value here. The whole set of
> > drivers is architecture specific anyway so you won't gain much build
> > test coverage. It may even prevent a legitimate combination of options
> > on the intended target, if the feature provided by QCOM_SMD and
> > RPMSG_QCOM_SMD is optional (if not, I would suggest to drop all the
> > stubs and simply depend on RPMSG_QCOM_SMD || QCOM_SMD, for the sake of
> > simplicity.)
> >
> > Don't get me wrong, I am a big fan of COMPILE_TEST, but only when we
> > can use it to get build testing coverage for free. If you have to change
> > the code itself in order to be able to get the extra build testing
> > coverage, I don't think it is a good idea. The kernel and the Kconfig
> > dependencies can be complex enough as is.
>
> I think the problem is the ARCH_QCOM dependency here, which
> clearly gets in the way of COMPILE_TEST having any real effect.
>
> I think generally speaking we either want to run the code and need both
> ARCH_QCOM and (RPMSG_QCOM_SMD || QCOM_SMD), or we do
> build-testing and need (COMPILE_TEST && RPMSG_QCOM_SMD=n
> && QCOM_SMD=n). We could add another Kconfig symbol that
> captures the dependency and then just add a simple dependency
> here.
The drivers doesn't depend on SMD for loading and booting the
remoteprocs, the dependency is just to get the appropriate
functions/stubs - although currently all firmware I've seen requires SMD
to be in place.
I'll push forward with purging QCOM_SMD and I'll have a proper review of
the dependencies as part of/following that process.
I believe it makes more sense to check for ARCH_QCOM || COMPILE_TEST and
just have the others to check if we have each dependency build-in or
module.
I forwarded your patch to Linus for v4.11, thanks.
Regards,
Bjorn
prev parent reply other threads:[~2017-03-20 22:22 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-13 16:36 [RESEND PATCH] remoteproc: qcom: fix QCOM_SMD dependencies Arnd Bergmann
2017-03-14 9:05 ` Jean Delvare
2017-03-14 11:01 ` Arnd Bergmann
2017-03-20 22:22 ` Bjorn Andersson [this message]
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=20170320222208.GE20094@minitux \
--to=bjorn.andersson@linaro.org \
--cc=arnd@arndb.de \
--cc=jdelvare@suse.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-remoteproc@vger.kernel.org \
--cc=loic.pallardy@st.com \
--cc=ohad@wizery.com \
--cc=pavel@ucw.cz \
--cc=peter.griffin@linaro.org \
--cc=vinod.koul@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.