Re: [Nouveau] [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration

[Karol Herbst <karolherbst@gmail.com>]

2016-11-08 16:46 GMT+01:00 Ilia Mirkin <imirkin@alum.mit.edu>:
> On Tue, Nov 8, 2016 at 8:56 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> The newly introduced LED handling for nouveau fails to link when the
>> driver is built-in but the LED subsystem is a loadable module:
>>
>> drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_do_suspend':
>> tvnv17.c:(.text.nouveau_do_suspend+0x10): undefined reference to `nouveau_led_suspend'
>> drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_do_resume':
>> tvnv17.c:(.text.nouveau_do_resume+0xf0): undefined reference to `nouveau_led_resume'
>> drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_drm_unload':
>> tvnv17.c:(.text.nouveau_drm_unload+0x34): undefined reference to `nouveau_led_fini'
>> drivers/gpu/drm/nouveau/nouveau.o: In function `nouveau_drm_load':
>> tvnv17.c:(.text.nouveau_drm_load+0x7d0): undefined reference to `nouveau_led_init'
>>
>> This adds a separate Kconfig symbol for the LED support that
>> correctly tracks the dependencies.
>>
>> Fixes: 8d021d71b324 ("drm/nouveau/drm/nouveau: add a LED driver for the NVIDIA logo")
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>>  drivers/gpu/drm/nouveau/Kbuild        | 2 +-
>>  drivers/gpu/drm/nouveau/Kconfig       | 8 ++++++++
>>  drivers/gpu/drm/nouveau/nouveau_led.h | 2 +-
>>  3 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/nouveau/Kbuild b/drivers/gpu/drm/nouveau/Kbuild
>> index fde6e3656636..5e00e911daa6 100644
>> --- a/drivers/gpu/drm/nouveau/Kbuild
>> +++ b/drivers/gpu/drm/nouveau/Kbuild
>> @@ -22,7 +22,7 @@ nouveau-$(CONFIG_DEBUG_FS) += nouveau_debugfs.o
>>  nouveau-y += nouveau_drm.o
>>  nouveau-y += nouveau_hwmon.o
>>  nouveau-$(CONFIG_COMPAT) += nouveau_ioc32.o
>> -nouveau-$(CONFIG_LEDS_CLASS) += nouveau_led.o
>> +nouveau-$(CONFIG_DRM_NOUVEAU_LED) += nouveau_led.o
>>  nouveau-y += nouveau_nvif.o
>>  nouveau-$(CONFIG_NOUVEAU_PLATFORM_DRIVER) += nouveau_platform.o
>>  nouveau-y += nouveau_usif.o # userspace <-> nvif
>> diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
>> index 78631fb61adf..715cd6f4dc31 100644
>> --- a/drivers/gpu/drm/nouveau/Kconfig
>> +++ b/drivers/gpu/drm/nouveau/Kconfig
>> @@ -46,6 +46,14 @@ config NOUVEAU_DEBUG
>>           The paranoia and spam levels will add a lot of extra checks which
>>           may potentially slow down driver operation.
>>
>> +config DRM_NOUVEAU_LED
>> +       bool "Support for logo LED"
>> +       depends on DRM_NOUVEAU && LEDS_CLASS
>> +       depends on !(DRM_NOUVEAU=y && LEDS_CLASS=m)
>> +       help
>> +         Say Y here to enabling controlling the brightness of the
>> +         LED behind NVIDIA logo on certain Titan cards.
>
> This is a very odd restriction... could this be written as
>
> depends on DRM_NOUVEAU
> select LEDS_CLASS
>
> Or will that not flip the LEDS_CLASS from m to y in case
> DRM_NOUVEAU=y? If not, is there a way to cause that to happen?
>
> Separately, perhaps we should just drop this LEDS_CLASS select into
> DRM_NOUVEAU? We've tended to avoid adding tons of options.
>

well, that would mean that you always need the LEDS_CLASS and maybe on
a tegra system you don't want to, so the led stuff should stay
completely optional. Don't know though.

Alex, maybe you want to clarify which dependencies should stay
optional? If nobody on your side care, then we won't care as well and
only add switches if users actually request it.

> Cheers,
>
>   -ilia
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau