public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Valentin Rothberg <valentinrothberg@gmail.com>
To: Rob Clark <robdclark@gmail.com>
Cc: Paul Bolle <pebolle@tiscali.nl>,
	Greg KH <gregkh@linuxfoundation.org>,
	Hai Li <hali@codeaurora.org>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	David Airlie <airlied@linux.ie>,
	rupran@einserver.de, stefan.hengelein@fau.de
Subject: Re: drm/msm/mdp5: undefined CONFIG_MSM_BUS_SCALING
Date: Fri, 10 Apr 2015 08:04:30 +0200	[thread overview]
Message-ID: <20150410060430.GA11962@station> (raw)
In-Reply-To: <CAF6AEGsnFqJ9CSSv-tEPW1cqaTSp90GohkvO=9+06tzgMx9o8A@mail.gmail.com>

On Thu, Apr 09, 2015 at 04:20:45PM -0400, Rob Clark wrote:
> On Thu, Apr 9, 2015 at 3:44 PM, Valentin Rothberg
> <valentinrothberg@gmail.com> wrote:
> > On Thu, Apr 09, 2015 at 02:54:29PM -0400, Rob Clark wrote:
> >> On Thu, Apr 9, 2015 at 2:12 PM, Paul Bolle <pebolle@tiscali.nl> wrote:
> >> > On Thu, 2015-04-09 at 19:07 +0200, Greg KH wrote:
> >> >> I really don't understand.  Why is this code in the kernel tree if it
> >> >> can't be built?  How does anyone use this?  By taking it and copying it
> >> >> where?  If it can't be built, and no one can update it, and of course
> >> >> not run it, why is it here?  What good is this code doing sitting here?
> >> >
> >> > The Erlangen bot (courtesy of Valentin, Stefan, and Andreas) has taken
> >> > over what I've been doing for quite some time, but doing it much more
> >> > thoroughly. And my experience tells me that the reports they'll send in
> >> > will trigger more discussions like this one.
> >> >
> >> > A lesson I learned from my daily checks for Kconfig oddities is that
> >> > people go to great lengths defending unbuildable code. (Do a web search
> >> > for ATHEROS_AR231X to find a discussion that dragged on for over three
> >> > years!) Personally I stopped caring after someone insisted on having a
> >> > file in the tree that was in no way connected to the build system: not a
> >> > single line in any of the Makefiles pointed at it. So, as far as I'm
> >> > concerned, if people can't point at a patch pending, somehow, somewhere,
> >> > that would make their code buildable one might as well delete the code.
> >> >
> >> > I really think it's as simple as that.
> >> >
> >>
> >> In the example you reference, sure it is as simple as that.  But here
> >> we are not talking about files that aren't even referenced by build
> >> system.  We are talking about a driver which does build and run on
> >> upstream kernel, and which has a few small #ifdef blocks to simplify
> >> backporting to downstream kernels (which we still do need to use for
> >> some generations and some devices)
> >>
> >> Sure, I'd love never to have to deal with a downstream kernel.  But
> >> really.. I didn't create the downstream mess in the arm/android
> >> ecosystem, I'm just trying to cope with it as best as possible.. don't
> >> hate the player, hate the game :-P
> >
> > I really understand your point.  But I also see conflicting interests.
> >
> > The goal of static analysis tools such as Paul's scripts, undertaker or
> > scripts/checkkconfigsymbols.py is to detect and ideally avoid certain
> > kind of bugs.  Having to deal with intentional dead code or entirely
> > dead files makes such analysis quite challenging.  The main issue for
> > the tools is that as soon as there is a CONFIG_ prefixed identifier, it
> > will be treated as a Kconfig variable.  Strictly speaking, it's
> > violating the Kconfig naming convention for the upstream case.
> >
> > Then there is another issue maintaining the code, studying the code,
> > making any kind of analysis.  How should people know which code is meant
> > for upstream, downstream or other streams?  Currently I am working on
> > detecting deprecated functions, data types, etc.  If there were too many
> > of such downstream #ifdefs, it would inherently complicate affords.
> 
> Hmm, admittedly, I hadn't really considered the static analysis case
> before today..
> 
> If at all possible, I would like to keep those, at least for the time
> being, since it is one less thing for me to mess up on backports.
> 
> Not sure if a comment tag could help make things clear (for humans and
> tools), ie.
> 
> #ifdef CONFIG_FOO
> /* downstream bonghits */
> ...
> #endif

The main problem for those analyzers is the 'CONFIG_' prefix.  This
prefix is reserved for Kconfig options only.  Using #ifdef FOO instead
avoids tools to run into this trap.  A comment would also be very
helpful.  The tools would be happy then : )

Kind regards,
 Valentin

> no idea if that would be trivial or difficult to implement?  If the
> latter, I can drop those parts of the code.  But if at all possible,
> I'm always a fan of giving myself less things to screw up.
> 
> > So I try to discourage such cases for the aforementioned reasons.  But
> > that's just my humble opinion and for sure my own interests : )
> >
> > In any case, thank you a lot for taking the time explain everything in
> > such nice detail.  I learned a lot!
> 
> No problem, and thanks for your work
> 
> BR,
> -R
> 
> > Kind regards,
> >  Valentin
> >
> >>
> >> BR,
> >> -R
> >>
> >> >
> >> > Paul Bolle
> >> >

      reply	other threads:[~2015-04-10  6:04 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-09 11:22 drm/msm/mdp5: undefined CONFIG_MSM_BUS_SCALING Valentin Rothberg
2015-04-09 13:49 ` Rob Clark
2015-04-09 14:20   ` Greg KH
2015-04-09 14:50     ` Rob Clark
2015-04-09 17:07       ` Greg KH
2015-04-09 18:02         ` Rob Clark
2015-04-09 18:12         ` Paul Bolle
2015-04-09 18:54           ` Rob Clark
2015-04-09 19:38             ` Paul Bolle
2015-04-09 19:44             ` Valentin Rothberg
2015-04-09 20:20               ` Rob Clark
2015-04-10  6:04                 ` Valentin Rothberg [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=20150410060430.GA11962@station \
    --to=valentinrothberg@gmail.com \
    --cc=airlied@linux.ie \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hali@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pebolle@tiscali.nl \
    --cc=robdclark@gmail.com \
    --cc=rupran@einserver.de \
    --cc=stefan.hengelein@fau.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