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

Open Source and information security mailing list archives

Message-ID: <CAEXux-b=DFByxBG2ozcrDeK--PGMyQcXVLaWGOsfic5ZcUS0Jw@mail.gmail.com>
Date:   Tue, 8 Nov 2016 17:16:12 +0100
From:   Karol Herbst <karolherbst@...il.com>
To:     Ilia Mirkin <imirkin@...m.mit.edu>
Cc:     Arnd Bergmann <arnd@...db.de>,
        "nouveau@...ts.freedesktop.org" <nouveau@...ts.freedesktop.org>,
        Ben Skeggs <bskeggs@...hat.com>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Alexandre Courbot <acourbot@...dia.com>
Subject: Re: [Nouveau] [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration

2016-11-08 16:46 GMT+01:00 Ilia Mirkin <imirkin@...m.mit.edu>:
> On Tue, Nov 8, 2016 at 8:56 AM, Arnd Bergmann <arnd@...db.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@...db.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@...ts.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives