All of lore.kernel.org
 help / color / mirror / Atom feed
* [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'
@ 2017-04-02 23:23 kbuild test robot
  2017-04-03  8:13 ` Andy Shevchenko
  2017-05-22 18:41 ` Andy Shevchenko
  0 siblings, 2 replies; 15+ messages in thread
From: kbuild test robot @ 2017-04-02 23:23 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: kbuild-all, linux-kernel, tipbuild, Ingo Molnar

[-- Attachment #1: Type: text/plain, Size: 857 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/platform
head:   d4d969909bef4c1e103eec0fc2c820773811fb72
commit: d4d969909bef4c1e103eec0fc2c820773811fb72 [1/1] x86/platform/intel-mid: Enable Bluetooth support on Intel Edison
config: x86_64-randconfig-s4-04030434 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        git checkout d4d969909bef4c1e103eec0fc2c820773811fb72
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   arch/x86/built-in.o: In function `tng_bt_sfi_setup':
>> platform_bt.c:(.init.text+0x13a30): undefined reference to `gpiod_add_lookup_table'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 29266 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'
  2017-04-02 23:23 [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table' kbuild test robot
@ 2017-04-03  8:13 ` Andy Shevchenko
  2017-04-03  9:31   ` Arnd Bergmann
  2017-05-22 18:41 ` Andy Shevchenko
  1 sibling, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2017-04-03  8:13 UTC (permalink / raw)
  To: kbuild test robot
  Cc: Andy Shevchenko, kbuild-all, linux-kernel, tipbuild, Ingo Molnar

On Mon, Apr 3, 2017 at 2:23 AM, kbuild test robot
<fengguang.wu@intel.com> wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/platform
> head:   d4d969909bef4c1e103eec0fc2c820773811fb72
> commit: d4d969909bef4c1e103eec0fc2c820773811fb72 [1/1] x86/platform/intel-mid: Enable Bluetooth support on Intel Edison
> config: x86_64-randconfig-s4-04030434 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         git checkout d4d969909bef4c1e103eec0fc2c820773811fb72
>         # save the attached .config to linux build tree
>         make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
>    arch/x86/built-in.o: In function `tng_bt_sfi_setup':
>>> platform_bt.c:(.init.text+0x13a30): undefined reference to `gpiod_add_lookup_table'

Thank you for report.
I'm about to check this.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'
  2017-04-03  8:13 ` Andy Shevchenko
@ 2017-04-03  9:31   ` Arnd Bergmann
  2017-04-03  9:36     ` Andy Shevchenko
  0 siblings, 1 reply; 15+ messages in thread
From: Arnd Bergmann @ 2017-04-03  9:31 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: kbuild test robot, Andy Shevchenko, kbuild-all, linux-kernel,
	tipbuild, Ingo Molnar

On Mon, Apr 3, 2017 at 10:13 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Apr 3, 2017 at 2:23 AM, kbuild test robot
> <fengguang.wu@intel.com> wrote:
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/platform
>> head:   d4d969909bef4c1e103eec0fc2c820773811fb72
>> commit: d4d969909bef4c1e103eec0fc2c820773811fb72 [1/1] x86/platform/intel-mid: Enable Bluetooth support on Intel Edison
>> config: x86_64-randconfig-s4-04030434 (attached as .config)
>> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
>> reproduce:
>>         git checkout d4d969909bef4c1e103eec0fc2c820773811fb72
>>         # save the attached .config to linux build tree
>>         make ARCH=x86_64
>>
>> All errors (new ones prefixed by >>):
>>
>>    arch/x86/built-in.o: In function `tng_bt_sfi_setup':
>>>> platform_bt.c:(.init.text+0x13a30): undefined reference to `gpiod_add_lookup_table'

I'd guess it's a missing dependency on GPIOLIB

     Arnd

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'
  2017-04-03  9:31   ` Arnd Bergmann
@ 2017-04-03  9:36     ` Andy Shevchenko
  2017-04-03  9:44       ` Andy Shevchenko
  2017-04-03 10:53       ` [kbuild-all] " Fengguang Wu
  0 siblings, 2 replies; 15+ messages in thread
From: Andy Shevchenko @ 2017-04-03  9:36 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: kbuild test robot, Andy Shevchenko, kbuild-all, linux-kernel,
	tipbuild, Ingo Molnar

On Mon, Apr 3, 2017 at 12:31 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Mon, Apr 3, 2017 at 10:13 AM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Mon, Apr 3, 2017 at 2:23 AM, kbuild test robot
>> <fengguang.wu@intel.com> wrote:
>>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/platform
>>> head:   d4d969909bef4c1e103eec0fc2c820773811fb72
>>> commit: d4d969909bef4c1e103eec0fc2c820773811fb72 [1/1] x86/platform/intel-mid: Enable Bluetooth support on Intel Edison
>>> config: x86_64-randconfig-s4-04030434 (attached as .config)
>>> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
>>> reproduce:
>>>         git checkout d4d969909bef4c1e103eec0fc2c820773811fb72
>>>         # save the attached .config to linux build tree
>>>         make ARCH=x86_64
>>>
>>> All errors (new ones prefixed by >>):
>>>
>>>    arch/x86/built-in.o: In function `tng_bt_sfi_setup':
>>>>> platform_bt.c:(.init.text+0x13a30): undefined reference to `gpiod_add_lookup_table'
>
> I'd guess it's a missing dependency on GPIOLIB

Apparently not this one.

The module is purely built whenever HCIUART_BCM is defined which is a
consumer of GPIO.
And that one does not have a dependency.

Fengguang, this is second semi-false positive report. Can we do
something about such?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'
  2017-04-03  9:36     ` Andy Shevchenko
@ 2017-04-03  9:44       ` Andy Shevchenko
  2017-04-04 15:27         ` Andy Shevchenko
  2017-04-03 10:53       ` [kbuild-all] " Fengguang Wu
  1 sibling, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2017-04-03  9:44 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: kbuild test robot, Andy Shevchenko, kbuild-all, linux-kernel,
	tipbuild, Ingo Molnar

On Mon, Apr 3, 2017 at 12:36 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Apr 3, 2017 at 12:31 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Mon, Apr 3, 2017 at 10:13 AM, Andy Shevchenko
>> <andy.shevchenko@gmail.com> wrote:
>>> On Mon, Apr 3, 2017 at 2:23 AM, kbuild test robot
>>> <fengguang.wu@intel.com> wrote:

>>>>    arch/x86/built-in.o: In function `tng_bt_sfi_setup':
>>>>>> platform_bt.c:(.init.text+0x13a30): undefined reference to `gpiod_add_lookup_table'
>>
>> I'd guess it's a missing dependency on GPIOLIB
>
> Apparently not this one.
>
> The module is purely built whenever HCIUART_BCM is defined which is a
> consumer of GPIO.

> And that one does not have a dependency.

...and they should not. So, there are two ways of fix this:
- add a stub for gpiod_add_lookup_table() for !GPIOLIB case, or
- add ifdeffery around this code here (with obvious increase of ugliness).

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [kbuild-all] [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'
  2017-04-03  9:36     ` Andy Shevchenko
  2017-04-03  9:44       ` Andy Shevchenko
@ 2017-04-03 10:53       ` Fengguang Wu
  1 sibling, 0 replies; 15+ messages in thread
From: Fengguang Wu @ 2017-04-03 10:53 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Arnd Bergmann, Andy Shevchenko, linux-kernel, kbuild-all,
	tipbuild, Ingo Molnar

On Mon, Apr 03, 2017 at 12:36:25PM +0300, Andy Shevchenko wrote:
>On Mon, Apr 3, 2017 at 12:31 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Mon, Apr 3, 2017 at 10:13 AM, Andy Shevchenko
>> <andy.shevchenko@gmail.com> wrote:
>>> On Mon, Apr 3, 2017 at 2:23 AM, kbuild test robot
>>> <fengguang.wu@intel.com> wrote:
>>>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/platform
>>>> head:   d4d969909bef4c1e103eec0fc2c820773811fb72
>>>> commit: d4d969909bef4c1e103eec0fc2c820773811fb72 [1/1] x86/platform/intel-mid: Enable Bluetooth support on Intel Edison
>>>> config: x86_64-randconfig-s4-04030434 (attached as .config)
>>>> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
>>>> reproduce:
>>>>         git checkout d4d969909bef4c1e103eec0fc2c820773811fb72
>>>>         # save the attached .config to linux build tree
>>>>         make ARCH=x86_64
>>>>
>>>> All errors (new ones prefixed by >>):
>>>>
>>>>    arch/x86/built-in.o: In function `tng_bt_sfi_setup':
>>>>>> platform_bt.c:(.init.text+0x13a30): undefined reference to `gpiod_add_lookup_table'
>>
>> I'd guess it's a missing dependency on GPIOLIB
>
>Apparently not this one.
>
>The module is purely built whenever HCIUART_BCM is defined which is a
>consumer of GPIO.
>And that one does not have a dependency.
>
>Fengguang, this is second semi-false positive report. Can we do
>something about such?

Andy, I didn't look into the logical side of this problem, however I
can reproduce the issue in commit d4d969909. While its parent commit
succeeded in producing the kernel:

=============== commit d4d969909 ===============
/home/wfg/linux
HEAD is now at d4d9699... x86/platform/intel-mid: Enable Bluetooth support on Intel Edison
/home/wfg/linux/obj-compiletest

make ARCH=x86_64

!!! BUILD ERROR !!!
cat /tmp/build-err-d4d969909bef4c1e103eec0fc2c820773811fb72-wfg
arch/x86/built-in.o: In function `tng_bt_sfi_setup':
platform_bt.c:(.init.text+0x13a3a): undefined reference to `gpiod_add_lookup_table'
make[1]: *** [vmlinux] Error 1
make[1]: Target '_all' not remade because of errors.
make: *** [sub-make] Error 2

=============== PREV commit 2f2a033fb5819c393d65da9b6233e095f3690f15 ===============
/home/wfg/linux
Previous HEAD position was d4d9699... x86/platform/intel-mid: Enable Bluetooth support on Intel Edison
HEAD is now at 2f2a033... x86/platform/uv/BAU: Implement uv4_wait_completion with read_status
/home/wfg/linux/obj-compiletest

make ARCH=x86_64

cat /tmp/build-err-2f2a033fb5819c393d65da9b6233e095f3690f15-wfg

Thanks,
Fengguang

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'
  2017-04-03  9:44       ` Andy Shevchenko
@ 2017-04-04 15:27         ` Andy Shevchenko
  2017-04-07  8:16           ` Linus Walleij
  0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2017-04-04 15:27 UTC (permalink / raw)
  To: Andy Shevchenko, Arnd Bergmann, Linus Walleij
  Cc: kbuild test robot, kbuild-all, linux-kernel, tipbuild, Ingo Molnar

On Mon, 2017-04-03 at 12:44 +0300, Andy Shevchenko wrote:
> On Mon, Apr 3, 2017 at 12:36 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Mon, Apr 3, 2017 at 12:31 PM, Arnd Bergmann <arnd@arndb.de>
> > wrote:
> > > On Mon, Apr 3, 2017 at 10:13 AM, Andy Shevchenko
> > > <andy.shevchenko@gmail.com> wrote:
> > > > On Mon, Apr 3, 2017 at 2:23 AM, kbuild test robot
> > > > <fengguang.wu@intel.com> wrote:
> > > > >    arch/x86/built-in.o: In function `tng_bt_sfi_setup':
> > > > > > > platform_bt.c:(.init.text+0x13a30): undefined reference to
> > > > > > > `gpiod_add_lookup_table'
> > > 
> > > I'd guess it's a missing dependency on GPIOLIB
> > 

+Cc: Linus.

> > Apparently not this one.
> > 
> > The module is purely built whenever HCIUART_BCM is defined which is
> > a
> > consumer of GPIO.
> > And that one does not have a dependency.
> 
> ...and they should not. So, there are two ways of fix this:
> - add a stub for gpiod_add_lookup_table() for !GPIOLIB case, or
> - add ifdeffery around this code here (with obvious increase of
> ugliness).

Linus, there is a (minor) issue with one of the user of
gpiod_add_lookup_table(), i.e. we have a code which is either built-in
or not compiled at all and it has some external dependency without
having an explicit Kconfig option. The external dependency considers
GPIOLIB as an optional, and thus some configurations might bring
unresolved symbols.

There are at least two solutions:
- add a stub for gpiod_add_lookup_table() for !GPIOLIB case, or
- add ifdeffery around this code here (with obvious increase of
ugliness).

What is your opinion on the case?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'
  2017-04-04 15:27         ` Andy Shevchenko
@ 2017-04-07  8:16           ` Linus Walleij
  2017-04-07  8:57             ` Andy Shevchenko
  0 siblings, 1 reply; 15+ messages in thread
From: Linus Walleij @ 2017-04-07  8:16 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Arnd Bergmann, kbuild test robot, kbuild-all,
	linux-kernel, tipbuild, Ingo Molnar

On Tue, Apr 4, 2017 at 5:27 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Mon, 2017-04-03 at 12:44 +0300, Andy Shevchenko wrote:

>> ...and they should not. So, there are two ways of fix this:
>> - add a stub for gpiod_add_lookup_table() for !GPIOLIB case, or
>> - add ifdeffery around this code here (with obvious increase of
>> ugliness).
>
> Linus, there is a (minor) issue with one of the user of
> gpiod_add_lookup_table(), i.e. we have a code which is either built-in
> or not compiled at all and it has some external dependency without
> having an explicit Kconfig option. The external dependency considers
> GPIOLIB as an optional, and thus some configurations might bring
> unresolved symbols.
>
> There are at least two solutions:
> - add a stub for gpiod_add_lookup_table() for !GPIOLIB case, or
> - add ifdeffery around this code here (with obvious increase of
> ugliness).
>
> What is your opinion on the case?

I've heard similar things about other places where people want
to have "half GPIOLIB" adding a few lookup stubs for just numbing
gpiolib.

If it is just a stub without much code I guess that is prettier, but
still it is a bit weird, because I guess the table that are passed
to gpiod_add_lookup_table() will still be compiled into the object
so you are anyways carrying cruft, and then what is the point in
not just doing select GPIOLIB.

But we might be looking at the wrong thing.

Looking closer at the file I see it is some kind of translation
of SFI (simple firmware interface) information into platform/board
data.

IMO SFI is just another hardware description language, and
SFI-described GPIO should be in drivers/gpio/gpiolib-sfi.c or
something like this akin to ACPI and OF. Why is it contained as
platform hacks?

Also a lot of the code down there and I mean
arch/x86/platform/intel-mid/device_libs/ is starting to look like the
ARM board files before we started to convert to device tree.
It includes using the old GPIO interface with the global number space
rather than descriptors and such quite extensively.

So I'm a bit worried that we are seeing a symptom of board data
stockpiling in arch/x86 and not really a GPIO compilation problem.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'
  2017-04-07  8:16           ` Linus Walleij
@ 2017-04-07  8:57             ` Andy Shevchenko
  2017-04-07 10:36               ` Linus Walleij
  0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2017-04-07  8:57 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Andy Shevchenko, Arnd Bergmann, kbuild test robot, kbuild-all,
	linux-kernel, tipbuild, Ingo Molnar

On Fri, Apr 7, 2017 at 11:16 AM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Apr 4, 2017 at 5:27 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
>> On Mon, 2017-04-03 at 12:44 +0300, Andy Shevchenko wrote:
>
>>> ...and they should not. So, there are two ways of fix this:
>>> - add a stub for gpiod_add_lookup_table() for !GPIOLIB case, or
>>> - add ifdeffery around this code here (with obvious increase of
>>> ugliness).
>>
>> Linus, there is a (minor) issue with one of the user of
>> gpiod_add_lookup_table(), i.e. we have a code which is either built-in
>> or not compiled at all and it has some external dependency without
>> having an explicit Kconfig option. The external dependency considers
>> GPIOLIB as an optional, and thus some configurations might bring
>> unresolved symbols.
>>
>> There are at least two solutions:
>> - add a stub for gpiod_add_lookup_table() for !GPIOLIB case, or
>> - add ifdeffery around this code here (with obvious increase of
>> ugliness).
>>
>> What is your opinion on the case?
>
> I've heard similar things about other places where people want
> to have "half GPIOLIB" adding a few lookup stubs for just numbing
> gpiolib.
>
> If it is just a stub without much code I guess that is prettier, but
> still it is a bit weird, because I guess the table that are passed
> to gpiod_add_lookup_table() will still be compiled into the object
> so you are anyways carrying cruft, and then what is the point in
> not just doing select GPIOLIB.

Because it's optional to HCIUART_BCM as far as I know. But I didn't
look closer to possibilities there (IIRC there no *_optional() calls
to GPIOLIB).

>
> But we might be looking at the wrong thing.
>
> Looking closer at the file I see it is some kind of translation
> of SFI (simple firmware interface) information into platform/board
> data.
>
> IMO SFI is just another hardware description language, and
> SFI-described GPIO should be in drivers/gpio/gpiolib-sfi.c or
> something like this akin to ACPI and OF. Why is it contained as
> platform hacks?

We have already discussed this a lot when I proposed to create
something like gpiolib-sfi.c. We have only legacy stuff with SFI amd
SFI perse is 99% useless. To not uglify drivers we in any case have to
hardcode mappings between weird SFI names to what drivers are
expecting.
So, it does not worth doing it.

> Also a lot of the code down there and I mean
> arch/x86/platform/intel-mid/device_libs/ is starting to look like the
> ARM board files before we started to convert to device tree.
> It includes using the old GPIO interface with the global number space
> rather than descriptors and such quite extensively.
>
> So I'm a bit worried that we are seeing a symptom of board data
> stockpiling in arch/x86 and not really a GPIO compilation problem.

Don't be. I'm trying to avoid this and my plan is actually to modify
bootloader on that board to provide ACPI tables instead. This will
hide all crappy stuff in bootloader, though we better to support
legacy (stock) bootloader as well and thus platform data.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'
  2017-04-07  8:57             ` Andy Shevchenko
@ 2017-04-07 10:36               ` Linus Walleij
  2017-04-18 13:41                 ` Andy Shevchenko
  0 siblings, 1 reply; 15+ messages in thread
From: Linus Walleij @ 2017-04-07 10:36 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Arnd Bergmann, kbuild test robot, kbuild-all,
	linux-kernel, tipbuild, Ingo Molnar

On Fri, Apr 7, 2017 at 10:57 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Apr 7, 2017 at 11:16 AM, Linus Walleij <linus.walleij@linaro.org> wrote:

>> If it is just a stub without much code I guess that is prettier, but
>> still it is a bit weird, because I guess the table that are passed
>> to gpiod_add_lookup_table() will still be compiled into the object
>> so you are anyways carrying cruft, and then what is the point in
>> not just doing select GPIOLIB.
>
> Because it's optional to HCIUART_BCM as far as I know. But I didn't
> look closer to possibilities there (IIRC there no *_optional() calls
> to GPIOLIB).

Do you mean for adding tables?
We have:
devm_gpiod_get_optional() & friends.

They return NULL if the GPIO is not there, or if the
GPIO library is compiled out (as of HEAD, due to the
patch from Dmitry T.)

>> IMO SFI is just another hardware description language, and
>> SFI-described GPIO should be in drivers/gpio/gpiolib-sfi.c or
>> something like this akin to ACPI and OF. Why is it contained as
>> platform hacks?
>
> We have already discussed this a lot when I proposed to create
> something like gpiolib-sfi.c. We have only legacy stuff with SFI amd
> SFI perse is 99% useless. To not uglify drivers we in any case have to
> hardcode mappings between weird SFI names to what drivers are
> expecting.
> So, it does not worth doing it.

Aha OK. Yeah you know this way better than me so I trust
you on this.


>> So I'm a bit worried that we are seeing a symptom of board data
>> stockpiling in arch/x86 and not really a GPIO compilation problem.
>
> Don't be. I'm trying to avoid this and my plan is actually to modify
> bootloader on that board to provide ACPI tables instead. This will
> hide all crappy stuff in bootloader, though we better to support
> legacy (stock) bootloader as well and thus platform data.

OK sounds reasonable. Kind of like the attached device tree we
do on ARM.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'
  2017-04-07 10:36               ` Linus Walleij
@ 2017-04-18 13:41                 ` Andy Shevchenko
  2017-04-19  9:01                   ` Linus Walleij
  0 siblings, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2017-04-18 13:41 UTC (permalink / raw)
  To: Linus Walleij, Andy Shevchenko
  Cc: Arnd Bergmann, kbuild test robot, kbuild-all, linux-kernel,
	tipbuild, Ingo Molnar

On Fri, 2017-04-07 at 12:36 +0200, Linus Walleij wrote:
> On Fri, Apr 7, 2017 at 10:57 AM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
> > On Fri, Apr 7, 2017 at 11:16 AM, Linus Walleij <linus.walleij@linaro
> > .org> wrote:
> > > If it is just a stub without much code I guess that is prettier,
> > > but
> > > still it is a bit weird, because I guess the table that are passed
> > > to gpiod_add_lookup_table() will still be compiled into the object
> > > so you are anyways carrying cruft, and then what is the point in
> > > not just doing select GPIOLIB.
> > 
> > Because it's optional to HCIUART_BCM as far as I know. But I didn't
> > look closer to possibilities there (IIRC there no *_optional() calls
> > to GPIOLIB).
> 
> Do you mean for adding tables?
> We have:
> devm_gpiod_get_optional() & friends.

> They return NULL if the GPIO is not there, or if the
> GPIO library is compiled out (as of HEAD, due to the
> patch from Dmitry T.)

I'm talking if they are used or not in hci_bcm.c.

Just checked and indeed the driver is using _optional() variants.
This means GPIOLIB is optional to the driver.

> > > So I'm a bit worried that we are seeing a symptom of board data
> > > stockpiling in arch/x86 and not really a GPIO compilation problem.
> > 
> > Don't be. I'm trying to avoid this and my plan is actually to modify
> > boot loader on that board to provide ACPI tables instead. This will
> > hide all crappy stuff in bootloader, though we better to support
> > legacy (stock) bootloader as well and thus platform data.
> 
> OK sounds reasonable. Kind of like the attached device tree we
> do on ARM.

It will time. Meanwhile, what is the best approach to avoid build break?

Taking into consideration above (hci_bcm.c driver uses _optional()
variants) and no separate Kconfig option for platform code, I would go
with a stub for gpiod_add_lookup_table() when !GPIOLIB.

Another option is to make this stub inside that driver. Btw, as far as I
can see this is the only user which has no explicit dependency to
GPIOLIB.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'
  2017-04-18 13:41                 ` Andy Shevchenko
@ 2017-04-19  9:01                   ` Linus Walleij
  2017-04-19 12:01                     ` Andy Shevchenko
  0 siblings, 1 reply; 15+ messages in thread
From: Linus Walleij @ 2017-04-19  9:01 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Arnd Bergmann, kbuild test robot, kbuild-all,
	linux-kernel, tipbuild, Ingo Molnar

On Tue, Apr 18, 2017 at 3:41 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Fri, 2017-04-07 at 12:36 +0200, Linus Walleij wrote:

>> They return NULL if the GPIO is not there, or if the
>> GPIO library is compiled out (as of HEAD, due to the
>> patch from Dmitry T.)
>
> I'm talking if they are used or not in hci_bcm.c.
>
> Just checked and indeed the driver is using _optional() variants.
> This means GPIOLIB is optional to the driver.

GPIOLIB's idea of "optional" should preferrably be the same as
an optional regulator, i.e. "electrically optional" not "software optional".

The regulator example is for example an ADC reference voltage that is
not strictly required, then it is optional, and the ADC can use some
internal reference voltage instead. It does not mean that the regulator
framework is optional.

I'd like to have the same semantice for GPIO but maybe that is a
pipe dream :/

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'
  2017-04-19  9:01                   ` Linus Walleij
@ 2017-04-19 12:01                     ` Andy Shevchenko
  0 siblings, 0 replies; 15+ messages in thread
From: Andy Shevchenko @ 2017-04-19 12:01 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Andy Shevchenko, Arnd Bergmann, kbuild test robot, kbuild-all,
	linux-kernel, tipbuild, Ingo Molnar

On Wed, 2017-04-19 at 11:01 +0200, Linus Walleij wrote:
> On Tue, Apr 18, 2017 at 3:41 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Fri, 2017-04-07 at 12:36 +0200, Linus Walleij wrote:
> > > They return NULL if the GPIO is not there, or if the
> > > GPIO library is compiled out (as of HEAD, due to the
> > > patch from Dmitry T.)
> > 
> > I'm talking if they are used or not in hci_bcm.c.
> > 
> > Just checked and indeed the driver is using _optional() variants.
> > This means GPIOLIB is optional to the driver.
> 
> GPIOLIB's idea of "optional" should preferrably be the same as
> an optional regulator, i.e. "electrically optional" not "software
> optional".
> 
> The regulator example is for example an ADC reference voltage that is
> not strictly required, then it is optional, and the ADC can use some
> internal reference voltage instead. It does not mean that the
> regulator
> framework is optional.

Yeah, my point is that hci_bcm.c by design can work without GPIOs (looks
like it might be hardware without dedicated GPIO lines, though I'm quite
 in doubts there is one exists) which makes GPIOLIB requirement
optional.  Thus, platform code in question (this patch) should cope with
absence of gpiod_add_lookup_table().

The question is what by your opinion is the best approach here (I put
them in order of my preferences):
1) make a stub for gpiod_add_lookup_table() for !GPIOLIB case for all
users;
2) as in 1) but inside this very user;
3) uglify Makefile with something like ifeq $(CONFIG_GPIOLIB,y);
4) create a Kconfig option for this very user (the worst variant I could
even think about).

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'
  2017-04-02 23:23 [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table' kbuild test robot
  2017-04-03  8:13 ` Andy Shevchenko
@ 2017-05-22 18:41 ` Andy Shevchenko
  2017-05-29  8:21   ` Linus Walleij
  1 sibling, 1 reply; 15+ messages in thread
From: Andy Shevchenko @ 2017-05-22 18:41 UTC (permalink / raw)
  To: kbuild test robot, Linus Walleij
  Cc: kbuild-all, linux-kernel, tipbuild, Ingo Molnar

On Mon, 2017-04-03 at 07:23 +0800, kbuild test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
> x86/platform
> head:   d4d969909bef4c1e103eec0fc2c820773811fb72
> commit: d4d969909bef4c1e103eec0fc2c820773811fb72 [1/1]
> x86/platform/intel-mid: Enable Bluetooth support on Intel Edison
> config: x86_64-randconfig-s4-04030434 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         git checkout d4d969909bef4c1e103eec0fc2c820773811fb72
>         # save the attached .config to linux build tree
>         make ARCH=x86_64 
> 
> All errors (new ones prefixed by >>):
> 
>    arch/x86/built-in.o: In function `tng_bt_sfi_setup':
> > > platform_bt.c:(.init.text+0x13a30): undefined reference to
> > > `gpiod_add_lookup_table'
> 
> 

This is going to be fixed by
https://lkml.org/lkml/2017/5/11/774
when Linus applies the patch.

> ---
> 0-DAY kernel test infrastructure                Open Source Technology
> Center
> https://lists.01.org/pipermail/kbuild-all                   Intel
> Corporation

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'
  2017-05-22 18:41 ` Andy Shevchenko
@ 2017-05-29  8:21   ` Linus Walleij
  0 siblings, 0 replies; 15+ messages in thread
From: Linus Walleij @ 2017-05-29  8:21 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: kbuild test robot, kbuild-all, linux-kernel, tipbuild, Ingo Molnar

On Mon, May 22, 2017 at 8:41 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Mon, 2017-04-03 at 07:23 +0800, kbuild test robot wrote:
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git
>> x86/platform
>> head:   d4d969909bef4c1e103eec0fc2c820773811fb72
>> commit: d4d969909bef4c1e103eec0fc2c820773811fb72 [1/1]
>> x86/platform/intel-mid: Enable Bluetooth support on Intel Edison
>> config: x86_64-randconfig-s4-04030434 (attached as .config)
>> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
>> reproduce:
>>         git checkout d4d969909bef4c1e103eec0fc2c820773811fb72
>>         # save the attached .config to linux build tree
>>         make ARCH=x86_64
>>
>> All errors (new ones prefixed by >>):
>>
>>    arch/x86/built-in.o: In function `tng_bt_sfi_setup':
>> > > platform_bt.c:(.init.text+0x13a30): undefined reference to
>> > > `gpiod_add_lookup_table'
>>
>>
>
> This is going to be fixed by
> https://lkml.org/lkml/2017/5/11/774
> when Linus applies the patch.

I've sent a pull request to Torvalds for this patch today.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-05-29  8:21 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-02 23:23 [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table' kbuild test robot
2017-04-03  8:13 ` Andy Shevchenko
2017-04-03  9:31   ` Arnd Bergmann
2017-04-03  9:36     ` Andy Shevchenko
2017-04-03  9:44       ` Andy Shevchenko
2017-04-04 15:27         ` Andy Shevchenko
2017-04-07  8:16           ` Linus Walleij
2017-04-07  8:57             ` Andy Shevchenko
2017-04-07 10:36               ` Linus Walleij
2017-04-18 13:41                 ` Andy Shevchenko
2017-04-19  9:01                   ` Linus Walleij
2017-04-19 12:01                     ` Andy Shevchenko
2017-04-03 10:53       ` [kbuild-all] " Fengguang Wu
2017-05-22 18:41 ` Andy Shevchenko
2017-05-29  8:21   ` Linus Walleij

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.