mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jean Delvare <jdelvare@suse.de>
To: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Cc: Douglas Anderson <dianders@chromium.org>,
	dri-devel@lists.freedesktop.org,
	LKML <linux-kernel@vger.kernel.org>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH RESEND] drm/display: Drop obsolete dependency on COMPILE_TEST
Date: Mon, 17 Jun 2024 20:18:14 +0200	[thread overview]
Message-ID: <20240617201814.73a07702@endymion.delvare> (raw)
In-Reply-To: <vsrsvmrkqnmxs3ncqv5m2gevzefiq55tr2iolxlmoehsvgcfkn@hyx37vax6r5e>

On Mon, 17 Jun 2024 14:55:22 +0300, Dmitry Baryshkov wrote:
> On Mon, Jun 17, 2024 at 01:23:48PM GMT, Jean Delvare wrote:
> > Hi Dmitry,
> > 
> > Thanks for your feedback.
> > 
> > On Mon, 17 Jun 2024 12:57:19 +0300, Dmitry Baryshkov wrote:  
> > > On Mon, Jun 17, 2024 at 10:30:30AM GMT, Jean Delvare wrote:  
> > > > Since commit 0166dc11be91 ("of: make CONFIG_OF user selectable"), it
> > > > is possible to test-build any driver which depends on OF on any
> > > > architecture by explicitly selecting OF. Therefore depending on
> > > > COMPILE_TEST as an alternative is no longer needed.    
> > > 
> > > The goal of this clause is to allow build-testing the driver with OF
> > > being disabled (meaning that some of OF functions are stubbed and some
> > > might disappear). I don't see how user-selectable OF provides the same
> > > result.  
> > 
> > Historically, the goal of this clause *was* to allow build-testing the
> > driver on architectures which did not support OF, and that did make
> > sense back then. As I understand it, building the driver without OF
> > support was never a goal per se (if it was, then the driver wouldn't be
> > set to depend on OF in the first place).
> > 
> > Some of my other submissions include the following explanation which
> > you might find useful (I ended up stripping it on resubmission after
> > being told I was being too verbose, but maybe it was needed after all):
> > 
> > It is actually better to always build such drivers with OF enabled,
> > so that the test builds are closer to how each driver will actually be
> > built on its intended target. Building them without OF may not test
> > much as the compiler will optimize out potentially large parts of the
> > code. In the worst case, this could even pop false positive warnings.
> > Dropping COMPILE_TEST here improves the quality of our testing and
> > avoids wasting time on non-existent issues.  
> 
> This doesn't seem to match the COMPILE_TEST usage that I observe in
> other places. For example, we frequently use 'depends on ARCH_QCOM ||
> COMPILE_TEST'. Which means that the driver itself doesn't make sense
> without ARCH_QCOM, but we want for it to be tested on non-ARCH_QCOM
> cases. I think the same logic applies to 'depends on OF ||
> COMPILE_TEST' clauses. The driver (DP AUX bus) depends on OF to be fully
> functional, but it should be compilable even without OF case.

The major difference is that one can't possibly enable ARCH_QCOM if
building on X86 for example. Therefore COMPILE_TEST is the only way to
let everyone (including randconfig/allmodconfig build farms) test-build
your code.

On the other hand, if you want to test-build drm_dp_aux_bus, you can
simply enable OF, because it is available on all architectures and
doesn't depend on anything. No need for COMPILE_TEST.

For clarity, I'm not advocating against the use of COMPILE_TEST,
actually if you check the history of my kernel contributions 10 years
back, you'll find commits from me adding COMPILE_TEST in addition to
arch-specific dependencies to many drivers. All I'm saying is that it
should only be used when it is the only way to enable the build.

-- 
Jean Delvare
SUSE L3 Support

  reply	other threads:[~2024-06-17 18:18 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-17  8:30 Jean Delvare
2024-06-17  9:57 ` Dmitry Baryshkov
2024-06-17 11:23   ` Jean Delvare
2024-06-17 11:55     ` Dmitry Baryshkov
2024-06-17 18:18       ` Jean Delvare [this message]
2024-06-17 22:26         ` Dmitry Baryshkov
2024-06-18  2:12           ` Doug Anderson
2024-06-18  7:20             ` Jean Delvare
2024-06-18  8:53             ` Dmitry Baryshkov
  -- strict thread matches above, loose matches on Subject: below --
2023-12-02 20:32 Jean Delvare
2023-12-04  9:17 ` Javier Martinez Canillas
2023-01-27 10:59 Jean Delvare

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=20240617201814.73a07702@endymion.delvare \
    --to=jdelvare@suse.de \
    --cc=airlied@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=daniel@ffwll.ch \
    --cc=dianders@chromium.org \
    --cc=dmitry.baryshkov@linaro.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=tzimmermann@suse.de \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®