All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Arnd Bergmann <arnd@arndb.de>
Cc: Andreas Noever <andreas.noever@gmail.com>,
	Matt Fleming <matt@codeblueprint.co.uk>,
	Ingo Molnar <mingo@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] thunderbolt: fix compile-test dependencies
Date: Thu, 17 Nov 2016 16:12:07 +0100	[thread overview]
Message-ID: <20161117151207.GA11741@wunner.de> (raw)
In-Reply-To: <3694785.gp6vtA09eN@wuerfel>

On Thu, Nov 17, 2016 at 03:10:11PM +0100, Arnd Bergmann wrote:
> On Thursday, November 17, 2016 2:53:55 PM CET Lukas Wunner wrote:
> > Hm, so far Thunderbolt is (unfortunately) an Intel proprietary
> > technology that is only available on x86, so compiling anything
> > in drivers/thunderbolt/ on other arches doesn't seem to make much
> > sense.  So maybe a "depends on X86" would be more appropriate?
> 
> I also found that we need "depends on ACPI" because APPLE_PROPERTIES
> does "select EFI_DEV_PATH_PARSER" and that requires APCI...

There's a series coming up to power the Thunderbolt controller
down when nothing is plugged in and this is done via ACPI.
This commit (slated for 4.11) was going to add a dependency on
ACPI anyway:
https://github.com/l1k/linux/commit/c1f379d5dee4

So adding "depends on ACPI" now would be fine I guess.


> > One could argue that compiling on other arches helps avoid x86-isms
> > in case Thunderbolt does become available on other arches one day,
> > then again it seems like an enormous waste of CPU cycles. *shrug*
> > 
> > Opinions?
> 
> We try to avoid adding architecture-specific dependencies that
> prevent build testing, and we are adding '|| COMPILE_TEST' to
> a lot of drivers for this. We could use 'depends on X86 ||
> COMPILE_TEST' here, but that wouldn't help the problem on ARM.

Yes, "depends on X86 || COMPILE_TEST" sounds like the right thing
to do, independently of the build breakage at hand.


> Another option would be to use 'depends on APPLE_PROPERTIES ||
> APPLE_PROPERTIES=n', which would force the thunderbolt driver
> to be a loadable module if APPLE_PROPERTIES is one, but otherwise
> just allow all configurations.

APPLE_PROPERTIES is bool, not tristate, so this would work.
However the solution you proposed earlier ("select APPLE_PROPERTIES if
(X86 && EFI_STUB)") is more explicit and easier to understand,
thus seems preferable to me.

Thanks!

Lukas

  parent reply	other threads:[~2016-11-17 17:37 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-15 15:58 [PATCH] thunderbolt: fix compile-test dependencies Arnd Bergmann
2016-11-15 16:21 ` Lukas Wunner
2016-11-17 10:00   ` Arnd Bergmann
2016-11-17 13:53     ` Lukas Wunner
     [not found]       ` <3694785.gp6vtA09eN@wuerfel>
2016-11-17 15:12         ` Lukas Wunner [this message]
2016-11-17 15:29           ` Arnd Bergmann
2016-11-17 16:18             ` Lukas Wunner
2016-11-17 16:36               ` Arnd Bergmann

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=20161117151207.GA11741@wunner.de \
    --to=lukas@wunner.de \
    --cc=andreas.noever@gmail.com \
    --cc=arnd@arndb.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=matt@codeblueprint.co.uk \
    --cc=mingo@kernel.org \
    /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.