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

From: Andy Shevchenko
Date: Fri Apr 07 2017 - 04:58:09 EST


On Fri, Apr 7, 2017 at 11:16 AM, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
> On Tue, Apr 4, 2017 at 5:27 PM, Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> 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