* [PATCH] thunderbolt: fix compile-test dependencies
@ 2016-11-15 15:58 Arnd Bergmann
2016-11-15 16:21 ` Lukas Wunner
0 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2016-11-15 15:58 UTC (permalink / raw)
To: Andreas Noever
Cc: Arnd Bergmann, Lukas Wunner, Matt Fleming, Ingo Molnar, linux-kernel
Building the Apple thunderbolt driver on non-x86 machines now produces
a harmless warning:
warning: (THUNDERBOLT) selects APPLE_PROPERTIES which has unmet direct dependencies (EFI && EFI_STUB && X86)
As there is no compile-time dependency to the Apple properties support,
we can make that 'select' statement conditional on the dependencies
of that driver.
Fixes: c9cc3aaa0281 ("thunderbolt: Use Device ROM retrieved from EFI")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/thunderbolt/Kconfig | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig
index 0056df7f3c09..4e7d92193b65 100644
--- a/drivers/thunderbolt/Kconfig
+++ b/drivers/thunderbolt/Kconfig
@@ -1,7 +1,8 @@
menuconfig THUNDERBOLT
tristate "Thunderbolt support for Apple devices"
+ depends on (EFI_STUB && X86) || COMPILE_TEST
depends on PCI
- select APPLE_PROPERTIES
+ select APPLE_PROPERTIES if (X86 && EFI_STUB)
select CRC32
help
Cactus Ridge Thunderbolt Controller driver
--
2.9.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] thunderbolt: fix compile-test dependencies 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 0 siblings, 1 reply; 8+ messages in thread From: Lukas Wunner @ 2016-11-15 16:21 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Andreas Noever, Matt Fleming, Ingo Molnar, linux-kernel On Tue, Nov 15, 2016 at 04:58:53PM +0100, Arnd Bergmann wrote: > Building the Apple thunderbolt driver on non-x86 machines now produces > a harmless warning: > > warning: (THUNDERBOLT) selects APPLE_PROPERTIES which has unmet direct dependencies (EFI && EFI_STUB && X86) > > As there is no compile-time dependency to the Apple properties support, > we can make that 'select' statement conditional on the dependencies > of that driver. Thanks Arnd, a commit fixing this is already on the tip.git efi/core branch since this morning and will hopefully have landed in linux-next by tomorrow: http://git.kernel.org/tip/79f9cd35b05e3e91ccf9b4038a8b74b9362b5da7 Previous discussion on LKML: https://lkml.org/lkml/2016/11/14/456 Another comment below... > > Fixes: c9cc3aaa0281 ("thunderbolt: Use Device ROM retrieved from EFI") > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > drivers/thunderbolt/Kconfig | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig > index 0056df7f3c09..4e7d92193b65 100644 > --- a/drivers/thunderbolt/Kconfig > +++ b/drivers/thunderbolt/Kconfig > @@ -1,7 +1,8 @@ > menuconfig THUNDERBOLT > tristate "Thunderbolt support for Apple devices" > + depends on (EFI_STUB && X86) || COMPILE_TEST This addition isn't correct, the thunderbolt driver works without the EFI stub, it just falls back to a hardcoded Device ROM. In other words, using the Device ROM retrieved by the EFI stub is an optional feature that is supposed to be enabled by default in the config if the EFI stub is also enabled. That is what the patch now in tip.git does. Nevertheless, thanks for your efforts and sorry for having caused you extra work. Lukas > depends on PCI > - select APPLE_PROPERTIES > + select APPLE_PROPERTIES if (X86 && EFI_STUB) > select CRC32 > help > Cactus Ridge Thunderbolt Controller driver > -- > 2.9.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] thunderbolt: fix compile-test dependencies 2016-11-15 16:21 ` Lukas Wunner @ 2016-11-17 10:00 ` Arnd Bergmann 2016-11-17 13:53 ` Lukas Wunner 0 siblings, 1 reply; 8+ messages in thread From: Arnd Bergmann @ 2016-11-17 10:00 UTC (permalink / raw) To: Lukas Wunner; +Cc: Andreas Noever, Matt Fleming, Ingo Molnar, linux-kernel On Tuesday, November 15, 2016 5:21:34 PM CET Lukas Wunner wrote: > On Tue, Nov 15, 2016 at 04:58:53PM +0100, Arnd Bergmann wrote: > > diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig > > index 0056df7f3c09..4e7d92193b65 100644 > > --- a/drivers/thunderbolt/Kconfig > > +++ b/drivers/thunderbolt/Kconfig > > @@ -1,7 +1,8 @@ > > menuconfig THUNDERBOLT > > tristate "Thunderbolt support for Apple devices" > > + depends on (EFI_STUB && X86) || COMPILE_TEST > > This addition isn't correct, the thunderbolt driver works without > the EFI stub, it just falls back to a hardcoded Device ROM. Ok, makes sense. I also needed this one though: diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig index bb0318ceaf93..de5d27ec67d6 100644 --- a/drivers/thunderbolt/Kconfig +++ b/drivers/thunderbolt/Kconfig @@ -1,7 +1,7 @@ menuconfig THUNDERBOLT tristate "Thunderbolt support for Apple devices" depends on PCI - select APPLE_PROPERTIES if EFI_STUB + select APPLE_PROPERTIES if EFI_STUB && X86 select CRC32 help Cactus Ridge Thunderbolt Controller driver Without that change, building on ARM with EFI_STUB enabled still gives me a build failure. Arnd ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] thunderbolt: fix compile-test dependencies 2016-11-17 10:00 ` Arnd Bergmann @ 2016-11-17 13:53 ` Lukas Wunner [not found] ` <3694785.gp6vtA09eN@wuerfel> 0 siblings, 1 reply; 8+ messages in thread From: Lukas Wunner @ 2016-11-17 13:53 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Andreas Noever, Matt Fleming, Ingo Molnar, linux-kernel On Thu, Nov 17, 2016 at 11:00:31AM +0100, Arnd Bergmann wrote: > On Tuesday, November 15, 2016 5:21:34 PM CET Lukas Wunner wrote: > > On Tue, Nov 15, 2016 at 04:58:53PM +0100, Arnd Bergmann wrote: > > > diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig > > > index 0056df7f3c09..4e7d92193b65 100644 > > > --- a/drivers/thunderbolt/Kconfig > > > +++ b/drivers/thunderbolt/Kconfig > > > @@ -1,7 +1,8 @@ > > > menuconfig THUNDERBOLT > > > tristate "Thunderbolt support for Apple devices" > > > + depends on (EFI_STUB && X86) || COMPILE_TEST > > > > This addition isn't correct, the thunderbolt driver works without > > the EFI stub, it just falls back to a hardcoded Device ROM. > > Ok, makes sense. > > I also needed this one though: > > diff --git a/drivers/thunderbolt/Kconfig b/drivers/thunderbolt/Kconfig > index bb0318ceaf93..de5d27ec67d6 100644 > --- a/drivers/thunderbolt/Kconfig > +++ b/drivers/thunderbolt/Kconfig > @@ -1,7 +1,7 @@ > menuconfig THUNDERBOLT > tristate "Thunderbolt support for Apple devices" > depends on PCI > - select APPLE_PROPERTIES if EFI_STUB > + select APPLE_PROPERTIES if EFI_STUB && X86 > select CRC32 > help > Cactus Ridge Thunderbolt Controller driver > > > Without that change, building on ARM with EFI_STUB enabled still > gives me a build failure. 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? 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? Thanks, Lukas ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <3694785.gp6vtA09eN@wuerfel>]
* Re: [PATCH] thunderbolt: fix compile-test dependencies [not found] ` <3694785.gp6vtA09eN@wuerfel> @ 2016-11-17 15:12 ` Lukas Wunner 2016-11-17 15:29 ` Arnd Bergmann 0 siblings, 1 reply; 8+ messages in thread From: Lukas Wunner @ 2016-11-17 15:12 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Andreas Noever, Matt Fleming, Ingo Molnar, linux-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] thunderbolt: fix compile-test dependencies 2016-11-17 15:12 ` Lukas Wunner @ 2016-11-17 15:29 ` Arnd Bergmann 2016-11-17 16:18 ` Lukas Wunner 0 siblings, 1 reply; 8+ messages in thread From: Arnd Bergmann @ 2016-11-17 15:29 UTC (permalink / raw) To: Lukas Wunner; +Cc: Andreas Noever, Matt Fleming, Ingo Molnar, linux-kernel 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. Arnd ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] thunderbolt: fix compile-test dependencies 2016-11-17 15:29 ` Arnd Bergmann @ 2016-11-17 16:18 ` Lukas Wunner 2016-11-17 16:36 ` Arnd Bergmann 0 siblings, 1 reply; 8+ messages in thread From: Lukas Wunner @ 2016-11-17 16:18 UTC (permalink / raw) To: Arnd Bergmann; +Cc: Andreas Noever, Matt Fleming, Ingo Molnar, linux-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] thunderbolt: fix compile-test dependencies 2016-11-17 16:18 ` Lukas Wunner @ 2016-11-17 16:36 ` Arnd Bergmann 0 siblings, 0 replies; 8+ messages in thread From: Arnd Bergmann @ 2016-11-17 16:36 UTC (permalink / raw) To: Lukas Wunner; +Cc: Andreas Noever, Matt Fleming, Ingo Molnar, linux-kernel On Thursday, November 17, 2016 5:18:07 PM CET Lukas Wunner wrote: > > > 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. I'd rather have you do it, less work for me that way, just add Suggested-by: Arnd Bergmann <arnd@arndb.de> Thanks, Arnd ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-11-17 17:37 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
2016-11-17 15:29 ` Arnd Bergmann
2016-11-17 16:18 ` Lukas Wunner
2016-11-17 16:36 ` Arnd Bergmann
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.