Date: Thu, 17 Nov 2016 17:18:07 +0100
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
Message-ID: <20161117161807.GA11775@wunner.de>
References: <20161115155908.650902-1-arnd@arndb.de>
 <3694785.gp6vtA09eN@wuerfel>
 <20161117151207.GA11741@wunner.de>
 <6569629.n4CqvS5mLI@wuerfel>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <6569629.n4CqvS5mLI@wuerfel>
User-Agent: Mutt/1.6.1 (2016-04-27)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2666
Lines: 59

On Thu, Nov 17, 2016 at 04:29:05PM +0100, Arnd Bergmann wrote:
> On Thursday, November 17, 2016 4:12:07 PM CET Lukas Wunner wrote:
> > 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.
> 
> It would take care of ARM, but not ARM64: ARM64 has ACPI
> and EFI_STUB, but cannot build APPLE_PROPERTIES. Adding
> the ACPI dependency by itself would not be sufficient.
> 
> > > > 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.
> 
> Ok.
> 
> > > 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.
> 
> Ok, sounds good. That should also fix ARM64 then.

Are you going to submit a fix for this or would you prefer me to do it?
Given the effort you've already put into it you might as well claim
credit for the fix, but I'll be happy to do it if that is preferred.

Thanks,

Lukas