Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1614965 > unrolled thread

Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table' Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-04-03 10:20 +0200
    Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table' Arnd Bergmann <arnd@arndb.de> - 2017-04-03 11:40 +0200
      Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table' Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-04-03 11:40 +0200
        Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table' Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-04-03 11:50 +0200
          Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to  `gpiod_add_lookup_table' Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-04-04 17:30 +0200
            Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table' Linus Walleij <linus.walleij@linaro.org> - 2017-04-07 10:20 +0200
              Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table' Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-04-07 11:00 +0200
                Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table' Linus Walleij <linus.walleij@linaro.org> - 2017-04-07 12:40 +0200
        Re: [kbuild-all] [tip:x86/platform 1/1] platform_bt.c:undefined  reference to `gpiod_add_lookup_table' Fengguang Wu <fengguang.wu@intel.com> - 2017-04-03 13:00 +0200

#1614965 — Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'

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

[toc] | [next] | [standalone]

#1615031

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

[toc] | [prev] | [next] | [standalone]

#1615032

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

[toc] | [prev] | [next] | [standalone]

#1615040

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

[toc] | [prev] | [next] | [standalone]

#1616099 — Re: [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'

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

[toc] | [prev] | [next] | [standalone]

#1618567

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

[toc] | [prev] | [next] | [standalone]

#1618611

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

[toc] | [prev] | [next] | [standalone]

#1618670

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

[toc] | [prev] | [next] | [standalone]

#1615092 — Re: [kbuild-all] [tip:x86/platform 1/1] platform_bt.c:undefined reference to `gpiod_add_lookup_table'

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web