All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] gpio: raspberrypi-ext: fix firmware dependency
@ 2018-02-28 13:48 Arnd Bergmann
  2018-02-28 21:47 ` Baruch Siach
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Arnd Bergmann @ 2018-02-28 13:48 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Stefan Wahren, Dave Stevenson, Baruch Siach, Arnd Bergmann,
	linux-gpio, linux-kernel

When the firmware driver is a loadable module, the gpio driver cannot be
built-in:

drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_set':
gpio-raspberrypi-exp.c:(.text+0xb4): undefined reference to `rpi_firmware_property'
drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get':
gpio-raspberrypi-exp.c:(.text+0x1ec): undefined reference to `rpi_firmware_property'
drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get_direction':
gpio-raspberrypi-exp.c:(.text+0x360): undefined reference to `rpi_firmware_property'
drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get_polarity':
gpio-raspberrypi-exp.c:(.text+0x4d4): undefined reference to `rpi_firmware_property'
drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_dir_out':
gpio-raspberrypi-exp.c:(.text+0x670): undefined reference to `rpi_firmware_property'
drivers/gpio/gpio-raspberrypi-exp.o:gpio-raspberrypi-exp.c:(.text+0x7fc): more undefined references to `rpi_firmware_property' follow
drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_dir_in':
drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_probe':
gpio-raspberrypi-exp.c:(.text+0x93c): undefined reference to `rpi_firmware_get'

We already have a Kconfig dependency for it, but when compile-testing, it
is disregarded.

This changes the dependency so that compile-testing is only done when the
firmware driver is completely disabled.

Fixes: a98d90e7d588 ("gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpio/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 2ecd2adbaec6..52a8b0a6f4e1 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -126,7 +126,7 @@ config GPIO_RASPBERRYPI_EXP
 	tristate "Raspberry Pi 3 GPIO Expander"
 	default RASPBERRYPI_FIRMWARE
 	depends on OF_GPIO
-	depends on (ARCH_BCM2835 && RASPBERRYPI_FIRMWARE) || COMPILE_TEST
+	depends on (ARCH_BCM2835 && RASPBERRYPI_FIRMWARE) || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE)
 	help
 	  Turn on GPIO support for the expander on Raspberry Pi 3 boards, using
 	  the firmware mailbox to communicate with VideoCore on BCM283x chips.
-- 
2.9.0

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

* Re: [PATCH] gpio: raspberrypi-ext: fix firmware dependency
  2018-02-28 13:48 [PATCH] gpio: raspberrypi-ext: fix firmware dependency Arnd Bergmann
@ 2018-02-28 21:47 ` Baruch Siach
  2018-02-28 22:08   ` Arnd Bergmann
  2018-03-01  9:28 ` Linus Walleij
  2018-03-02  8:47 ` Linus Walleij
  2 siblings, 1 reply; 8+ messages in thread
From: Baruch Siach @ 2018-02-28 21:47 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linus Walleij, Stefan Wahren, Dave Stevenson, linux-gpio, linux-kernel

Hi Arnd,

Thanks for the fix.

On Wed, Feb 28, 2018 at 02:48:08PM +0100, Arnd Bergmann wrote:
> When the firmware driver is a loadable module, the gpio driver cannot be
> built-in:
> 
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_set':
> gpio-raspberrypi-exp.c:(.text+0xb4): undefined reference to `rpi_firmware_property'
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get':
> gpio-raspberrypi-exp.c:(.text+0x1ec): undefined reference to `rpi_firmware_property'
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get_direction':
> gpio-raspberrypi-exp.c:(.text+0x360): undefined reference to `rpi_firmware_property'
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get_polarity':
> gpio-raspberrypi-exp.c:(.text+0x4d4): undefined reference to `rpi_firmware_property'
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_dir_out':
> gpio-raspberrypi-exp.c:(.text+0x670): undefined reference to `rpi_firmware_property'
> drivers/gpio/gpio-raspberrypi-exp.o:gpio-raspberrypi-exp.c:(.text+0x7fc): more undefined references to `rpi_firmware_property' follow
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_dir_in':
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_probe':
> gpio-raspberrypi-exp.c:(.text+0x93c): undefined reference to `rpi_firmware_get'
> 
> We already have a Kconfig dependency for it, but when compile-testing, it
> is disregarded.
> 
> This changes the dependency so that compile-testing is only done when the
> firmware driver is completely disabled.

What about the CONFIG_ARCH_BCM2835=y case? The combination of 
CONFIG_GPIO_RASPBERRYPI_EXP=y and CONFIG_RASPBERRYPI_FIRMWARE=m is still 
valid. Wouldn't that break the build?

Isn't there a way in Kconfig to force CONFIG_GPIO_RASPBERRYPI_EXP=m when 
CONFIG_RASPBERRYPI_FIRMWARE=m?

What about 'depends on m || RASPBERRYPI_FIRMWARE=y'?

Grepping around I also found this:

drivers/power/supply/Kconfig:   depends on USB_GADGET || !USB_GADGET # if USB_GADGET=m, this can't be 'y'

And this:

drivers/infiniband/Kconfig:     depends on m || IPV6 != m

> Fixes: a98d90e7d588 ("gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/gpio/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 2ecd2adbaec6..52a8b0a6f4e1 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -126,7 +126,7 @@ config GPIO_RASPBERRYPI_EXP
>  	tristate "Raspberry Pi 3 GPIO Expander"
>  	default RASPBERRYPI_FIRMWARE
>  	depends on OF_GPIO
> -	depends on (ARCH_BCM2835 && RASPBERRYPI_FIRMWARE) || COMPILE_TEST
> +	depends on (ARCH_BCM2835 && RASPBERRYPI_FIRMWARE) || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE)

This is really non-obvious. An inline comment here might help, IMO.

>  	help
>  	  Turn on GPIO support for the expander on Raspberry Pi 3 boards, using
>  	  the firmware mailbox to communicate with VideoCore on BCM283x chips.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

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

* Re: [PATCH] gpio: raspberrypi-ext: fix firmware dependency
  2018-02-28 21:47 ` Baruch Siach
@ 2018-02-28 22:08   ` Arnd Bergmann
  2018-03-01  9:20     ` Baruch Siach
  0 siblings, 1 reply; 8+ messages in thread
From: Arnd Bergmann @ 2018-02-28 22:08 UTC (permalink / raw)
  To: Baruch Siach
  Cc: Linus Walleij, Stefan Wahren, Dave Stevenson, linux-gpio,
	Linux Kernel Mailing List

On Wed, Feb 28, 2018 at 10:47 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> Hi Arnd,
>
> Thanks for the fix.
>
> On Wed, Feb 28, 2018 at 02:48:08PM +0100, Arnd Bergmann wrote:
>> When the firmware driver is a loadable module, the gpio driver cannot be
>> built-in:
>>
>> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_set':
>> gpio-raspberrypi-exp.c:(.text+0xb4): undefined reference to `rpi_firmware_property'
>> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get':
>> gpio-raspberrypi-exp.c:(.text+0x1ec): undefined reference to `rpi_firmware_property'
>> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get_direction':
>> gpio-raspberrypi-exp.c:(.text+0x360): undefined reference to `rpi_firmware_property'
>> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get_polarity':
>> gpio-raspberrypi-exp.c:(.text+0x4d4): undefined reference to `rpi_firmware_property'
>> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_dir_out':
>> gpio-raspberrypi-exp.c:(.text+0x670): undefined reference to `rpi_firmware_property'
>> drivers/gpio/gpio-raspberrypi-exp.o:gpio-raspberrypi-exp.c:(.text+0x7fc): more undefined references to `rpi_firmware_property' follow
>> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_dir_in':
>> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_probe':
>> gpio-raspberrypi-exp.c:(.text+0x93c): undefined reference to `rpi_firmware_get'
>>
>> We already have a Kconfig dependency for it, but when compile-testing, it
>> is disregarded.
>>
>> This changes the dependency so that compile-testing is only done when the
>> firmware driver is completely disabled.
>
> What about the CONFIG_ARCH_BCM2835=y case? The combination of
> CONFIG_GPIO_RASPBERRYPI_EXP=y and CONFIG_RASPBERRYPI_FIRMWARE=m is still
> valid. Wouldn't that break the build?
>
> Isn't there a way in Kconfig to force CONFIG_GPIO_RASPBERRYPI_EXP=m when
> CONFIG_RASPBERRYPI_FIRMWARE=m?

The problem I ran into only happens with CONFIG_ARCH_BCM2835=y to
start with. My fix handles that case correctly, it forces
CONFIG_GPIO_RASPBERRYPI_EXP to be either 'n' or 'm'
when CONFIG_RASPBERRYPI_FIRMWARE=m.

> What about 'depends on m || RASPBERRYPI_FIRMWARE=y'?

That would be (slightly) wrong, it would force CONFIG_GPIO_RASPBERRYPI_EXP
to be 'm' even if RASPBERRYPI_FIRMWARE=n.

> Grepping around I also found this:
>
> drivers/power/supply/Kconfig:   depends on USB_GADGET || !USB_GADGET # if USB_GADGET=m, this can't be 'y'

That is what I did here as well, except the !RASPBERRYPI_FIRMWARE
only applies for COMPILE_TEST.

> And this:
>
> drivers/infiniband/Kconfig:     depends on m || IPV6 != m

This is a less common way to express it. The idiomatic
Kconfig expression here would be 'depends on IPV6 || !IPV6'.

>> Fixes: a98d90e7d588 ("gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>>  drivers/gpio/Kconfig | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
>> index 2ecd2adbaec6..52a8b0a6f4e1 100644
>> --- a/drivers/gpio/Kconfig
>> +++ b/drivers/gpio/Kconfig
>> @@ -126,7 +126,7 @@ config GPIO_RASPBERRYPI_EXP
>>       tristate "Raspberry Pi 3 GPIO Expander"
>>       default RASPBERRYPI_FIRMWARE
>>       depends on OF_GPIO
>> -     depends on (ARCH_BCM2835 && RASPBERRYPI_FIRMWARE) || COMPILE_TEST
>> +     depends on (ARCH_BCM2835 && RASPBERRYPI_FIRMWARE) || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE)
>
> This is really non-obvious. An inline comment here might help, IMO.

How about

       # RASPBERRYPI_FIRMWARE is only available for ARCH_BCM2835, but we want to
       # allow compile-testing when it is disabled

?

       Arnd

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

* Re: [PATCH] gpio: raspberrypi-ext: fix firmware dependency
  2018-02-28 22:08   ` Arnd Bergmann
@ 2018-03-01  9:20     ` Baruch Siach
  0 siblings, 0 replies; 8+ messages in thread
From: Baruch Siach @ 2018-03-01  9:20 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linus Walleij, Stefan Wahren, Dave Stevenson, linux-gpio,
	Linux Kernel Mailing List

Hi Arnd,

On Wed, Feb 28, 2018 at 11:08:54PM +0100, Arnd Bergmann wrote:
> On Wed, Feb 28, 2018 at 10:47 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> > Thanks for the fix.
> >
> > On Wed, Feb 28, 2018 at 02:48:08PM +0100, Arnd Bergmann wrote:
> >> When the firmware driver is a loadable module, the gpio driver cannot be
> >> built-in:
> >>
> >> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_set':
> >> gpio-raspberrypi-exp.c:(.text+0xb4): undefined reference to `rpi_firmware_property'
> >> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get':
> >> gpio-raspberrypi-exp.c:(.text+0x1ec): undefined reference to `rpi_firmware_property'
> >> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get_direction':
> >> gpio-raspberrypi-exp.c:(.text+0x360): undefined reference to `rpi_firmware_property'
> >> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get_polarity':
> >> gpio-raspberrypi-exp.c:(.text+0x4d4): undefined reference to `rpi_firmware_property'
> >> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_dir_out':
> >> gpio-raspberrypi-exp.c:(.text+0x670): undefined reference to `rpi_firmware_property'
> >> drivers/gpio/gpio-raspberrypi-exp.o:gpio-raspberrypi-exp.c:(.text+0x7fc): more undefined references to `rpi_firmware_property' follow
> >> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_dir_in':
> >> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_probe':
> >> gpio-raspberrypi-exp.c:(.text+0x93c): undefined reference to `rpi_firmware_get'
> >>
> >> We already have a Kconfig dependency for it, but when compile-testing, it
> >> is disregarded.
> >>
> >> This changes the dependency so that compile-testing is only done when the
> >> firmware driver is completely disabled.
> >
> > What about the CONFIG_ARCH_BCM2835=y case? The combination of
> > CONFIG_GPIO_RASPBERRYPI_EXP=y and CONFIG_RASPBERRYPI_FIRMWARE=m is still
> > valid. Wouldn't that break the build?
> >
> > Isn't there a way in Kconfig to force CONFIG_GPIO_RASPBERRYPI_EXP=m when
> > CONFIG_RASPBERRYPI_FIRMWARE=m?
> 
> The problem I ran into only happens with CONFIG_ARCH_BCM2835=y to
> start with. My fix handles that case correctly, it forces
> CONFIG_GPIO_RASPBERRYPI_EXP to be either 'n' or 'm'
> when CONFIG_RASPBERRYPI_FIRMWARE=m.

Thanks for the explanation.

> > What about 'depends on m || RASPBERRYPI_FIRMWARE=y'?
> 
> That would be (slightly) wrong, it would force CONFIG_GPIO_RASPBERRYPI_EXP
> to be 'm' even if RASPBERRYPI_FIRMWARE=n.
> 
> > Grepping around I also found this:
> >
> > drivers/power/supply/Kconfig:   depends on USB_GADGET || !USB_GADGET # if USB_GADGET=m, this can't be 'y'
> 
> That is what I did here as well, except the !RASPBERRYPI_FIRMWARE
> only applies for COMPILE_TEST.
> 
> > And this:
> >
> > drivers/infiniband/Kconfig:     depends on m || IPV6 != m
> 
> This is a less common way to express it. The idiomatic
> Kconfig expression here would be 'depends on IPV6 || !IPV6'.

I find this way much easier to understand. Without the comment I would never 
have guessed what 'USB_GADGET || !USB_GADGET' actually means. And indeed no 
comment is needed in the infiniband case.

> >> Fixes: a98d90e7d588 ("gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service")
> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >> ---
> >>  drivers/gpio/Kconfig | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> >> index 2ecd2adbaec6..52a8b0a6f4e1 100644
> >> --- a/drivers/gpio/Kconfig
> >> +++ b/drivers/gpio/Kconfig
> >> @@ -126,7 +126,7 @@ config GPIO_RASPBERRYPI_EXP
> >>       tristate "Raspberry Pi 3 GPIO Expander"
> >>       default RASPBERRYPI_FIRMWARE
> >>       depends on OF_GPIO
> >> -     depends on (ARCH_BCM2835 && RASPBERRYPI_FIRMWARE) || COMPILE_TEST
> >> +     depends on (ARCH_BCM2835 && RASPBERRYPI_FIRMWARE) || (COMPILE_TEST && !RASPBERRYPI_FIRMWARE)
> >
> > This is really non-obvious. An inline comment here might help, IMO.
> 
> How about
> 
>        # RASPBERRYPI_FIRMWARE is only available for ARCH_BCM2835, but we want to
>        # allow compile-testing when it is disabled
> 
> ?

The module vs built-in aspect is missing. That is the non-obvious part.

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

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

* Re: [PATCH] gpio: raspberrypi-ext: fix firmware dependency
  2018-02-28 13:48 [PATCH] gpio: raspberrypi-ext: fix firmware dependency Arnd Bergmann
  2018-02-28 21:47 ` Baruch Siach
@ 2018-03-01  9:28 ` Linus Walleij
  2018-03-01 23:33   ` Baruch Siach
  2018-03-02  8:47 ` Linus Walleij
  2 siblings, 1 reply; 8+ messages in thread
From: Linus Walleij @ 2018-03-01  9:28 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Stefan Wahren, Dave Stevenson, Baruch Siach,
	open list:GPIO SUBSYSTEM, linux-kernel

On Wed, Feb 28, 2018 at 2:48 PM, Arnd Bergmann <arnd@arndb.de> wrote:

> When the firmware driver is a loadable module, the gpio driver cannot be
> built-in:
>
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_set':
> gpio-raspberrypi-exp.c:(.text+0xb4): undefined reference to `rpi_firmware_property'
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get':
> gpio-raspberrypi-exp.c:(.text+0x1ec): undefined reference to `rpi_firmware_property'
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get_direction':
> gpio-raspberrypi-exp.c:(.text+0x360): undefined reference to `rpi_firmware_property'
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get_polarity':
> gpio-raspberrypi-exp.c:(.text+0x4d4): undefined reference to `rpi_firmware_property'
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_dir_out':
> gpio-raspberrypi-exp.c:(.text+0x670): undefined reference to `rpi_firmware_property'
> drivers/gpio/gpio-raspberrypi-exp.o:gpio-raspberrypi-exp.c:(.text+0x7fc): more undefined references to `rpi_firmware_property' follow
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_dir_in':
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_probe':
> gpio-raspberrypi-exp.c:(.text+0x93c): undefined reference to `rpi_firmware_get'
>
> We already have a Kconfig dependency for it, but when compile-testing, it
> is disregarded.
>
> This changes the dependency so that compile-testing is only done when the
> firmware driver is completely disabled.
>
> Fixes: a98d90e7d588 ("gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Baruch, are you waiting for a fixed fix or should I apply this?

It's a bit unclear from the mail chain what action I should take...

Yours,
Linus Walleij

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

* Re: [PATCH] gpio: raspberrypi-ext: fix firmware dependency
  2018-03-01  9:28 ` Linus Walleij
@ 2018-03-01 23:33   ` Baruch Siach
  2018-03-02  8:49     ` Linus Walleij
  0 siblings, 1 reply; 8+ messages in thread
From: Baruch Siach @ 2018-03-01 23:33 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Arnd Bergmann, Stefan Wahren, Dave Stevenson,
	open list:GPIO SUBSYSTEM, linux-kernel

Hi Linus,

On Thu, Mar 01, 2018 at 10:28:52AM +0100, Linus Walleij wrote:
> On Wed, Feb 28, 2018 at 2:48 PM, Arnd Bergmann <arnd@arndb.de> wrote: 
> > When the firmware driver is a loadable module, the gpio driver cannot be
> > built-in:
> >
> > drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_set':
> > gpio-raspberrypi-exp.c:(.text+0xb4): undefined reference to `rpi_firmware_property'
> > drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get':
> > gpio-raspberrypi-exp.c:(.text+0x1ec): undefined reference to `rpi_firmware_property'
> > drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get_direction':
> > gpio-raspberrypi-exp.c:(.text+0x360): undefined reference to `rpi_firmware_property'
> > drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get_polarity':
> > gpio-raspberrypi-exp.c:(.text+0x4d4): undefined reference to `rpi_firmware_property'
> > drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_dir_out':
> > gpio-raspberrypi-exp.c:(.text+0x670): undefined reference to `rpi_firmware_property'
> > drivers/gpio/gpio-raspberrypi-exp.o:gpio-raspberrypi-exp.c:(.text+0x7fc): more undefined references to `rpi_firmware_property' follow
> > drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_dir_in':
> > drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_probe':
> > gpio-raspberrypi-exp.c:(.text+0x93c): undefined reference to `rpi_firmware_get'
> >
> > We already have a Kconfig dependency for it, but when compile-testing, it
> > is disregarded.
> >
> > This changes the dependency so that compile-testing is only done when the
> > firmware driver is completely disabled.
> >
> > Fixes: a98d90e7d588 ("gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Baruch, are you waiting for a fixed fix or should I apply this?
> 
> It's a bit unclear from the mail chain what action I should take...

This patch fixes the issue. I think that an inline comment should be added at 
least, because otherwise the dependency in incomprehensible. I also prefer the

  depends on m || DEPENDENCY != m 

style to express this kind of dependencies.

What do you think?

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

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

* Re: [PATCH] gpio: raspberrypi-ext: fix firmware dependency
  2018-02-28 13:48 [PATCH] gpio: raspberrypi-ext: fix firmware dependency Arnd Bergmann
  2018-02-28 21:47 ` Baruch Siach
  2018-03-01  9:28 ` Linus Walleij
@ 2018-03-02  8:47 ` Linus Walleij
  2 siblings, 0 replies; 8+ messages in thread
From: Linus Walleij @ 2018-03-02  8:47 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Stefan Wahren, Dave Stevenson, Baruch Siach,
	open list:GPIO SUBSYSTEM, linux-kernel

On Wed, Feb 28, 2018 at 2:48 PM, Arnd Bergmann <arnd@arndb.de> wrote:

> When the firmware driver is a loadable module, the gpio driver cannot be
> built-in:
>
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_set':
> gpio-raspberrypi-exp.c:(.text+0xb4): undefined reference to `rpi_firmware_property'
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get':
> gpio-raspberrypi-exp.c:(.text+0x1ec): undefined reference to `rpi_firmware_property'
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get_direction':
> gpio-raspberrypi-exp.c:(.text+0x360): undefined reference to `rpi_firmware_property'
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get_polarity':
> gpio-raspberrypi-exp.c:(.text+0x4d4): undefined reference to `rpi_firmware_property'
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_dir_out':
> gpio-raspberrypi-exp.c:(.text+0x670): undefined reference to `rpi_firmware_property'
> drivers/gpio/gpio-raspberrypi-exp.o:gpio-raspberrypi-exp.c:(.text+0x7fc): more undefined references to `rpi_firmware_property' follow
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_dir_in':
> drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_probe':
> gpio-raspberrypi-exp.c:(.text+0x93c): undefined reference to `rpi_firmware_get'
>
> We already have a Kconfig dependency for it, but when compile-testing, it
> is disregarded.
>
> This changes the dependency so that compile-testing is only done when the
> firmware driver is completely disabled.
>
> Fixes: a98d90e7d588 ("gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Patch applied.

Yours,
Linus Walleij

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

* Re: [PATCH] gpio: raspberrypi-ext: fix firmware dependency
  2018-03-01 23:33   ` Baruch Siach
@ 2018-03-02  8:49     ` Linus Walleij
  0 siblings, 0 replies; 8+ messages in thread
From: Linus Walleij @ 2018-03-02  8:49 UTC (permalink / raw)
  To: Baruch Siach
  Cc: Arnd Bergmann, Stefan Wahren, Dave Stevenson,
	open list:GPIO SUBSYSTEM, linux-kernel

On Fri, Mar 2, 2018 at 12:33 AM, Baruch Siach <baruch@tkos.co.il> wrote:
> On Thu, Mar 01, 2018 at 10:28:52AM +0100, Linus Walleij wrote:
>> On Wed, Feb 28, 2018 at 2:48 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> > When the firmware driver is a loadable module, the gpio driver cannot be
>> > built-in:
>> >
>> > drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_set':
>> > gpio-raspberrypi-exp.c:(.text+0xb4): undefined reference to `rpi_firmware_property'
>> > drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get':
>> > gpio-raspberrypi-exp.c:(.text+0x1ec): undefined reference to `rpi_firmware_property'
>> > drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get_direction':
>> > gpio-raspberrypi-exp.c:(.text+0x360): undefined reference to `rpi_firmware_property'
>> > drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_get_polarity':
>> > gpio-raspberrypi-exp.c:(.text+0x4d4): undefined reference to `rpi_firmware_property'
>> > drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_dir_out':
>> > gpio-raspberrypi-exp.c:(.text+0x670): undefined reference to `rpi_firmware_property'
>> > drivers/gpio/gpio-raspberrypi-exp.o:gpio-raspberrypi-exp.c:(.text+0x7fc): more undefined references to `rpi_firmware_property' follow
>> > drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_dir_in':
>> > drivers/gpio/gpio-raspberrypi-exp.o: In function `rpi_exp_gpio_probe':
>> > gpio-raspberrypi-exp.c:(.text+0x93c): undefined reference to `rpi_firmware_get'
>> >
>> > We already have a Kconfig dependency for it, but when compile-testing, it
>> > is disregarded.
>> >
>> > This changes the dependency so that compile-testing is only done when the
>> > firmware driver is completely disabled.
>> >
>> > Fixes: a98d90e7d588 ("gpio: raspberrypi-exp: Driver for RPi3 GPIO expander via mailbox service")
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>
>> Baruch, are you waiting for a fixed fix or should I apply this?
>>
>> It's a bit unclear from the mail chain what action I should take...
>
> This patch fixes the issue. I think that an inline comment should be added at
> least, because otherwise the dependency in incomprehensible. I also prefer the
>
>   depends on m || DEPENDENCY != m
>
> style to express this kind of dependencies.
>
> What do you think?

I am hopelessly ignorant about Kconfig and the whole thing just
bites me and scares me all the time, like it's an angry dog.

Just patch on top of Arnds fix if you prefer some other solution,
I bet you know this better than me.

Yours,
Linus Walleij

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

end of thread, other threads:[~2018-03-02  8:49 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-28 13:48 [PATCH] gpio: raspberrypi-ext: fix firmware dependency Arnd Bergmann
2018-02-28 21:47 ` Baruch Siach
2018-02-28 22:08   ` Arnd Bergmann
2018-03-01  9:20     ` Baruch Siach
2018-03-01  9:28 ` Linus Walleij
2018-03-01 23:33   ` Baruch Siach
2018-03-02  8:49     ` Linus Walleij
2018-03-02  8:47 ` 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.