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

Groups > linux.kernel > #1625334 > 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 <andriy.shevchenko@linux.intel.com> - 2017-04-18 15:50 +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-19 11:10 +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-19 14:10 +0200

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

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

[toc] | [next] | [standalone]

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

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

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

#1626049

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web