Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752006AbcKHPqK (ORCPT ); Tue, 8 Nov 2016 10:46:10 -0500 Received: from mail-it0-f65.google.com ([209.85.214.65]:33253 "EHLO mail-it0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750980AbcKHPqJ (ORCPT ); Tue, 8 Nov 2016 10:46:09 -0500 MIME-Version: 1.0 In-Reply-To: <20161108135721.2142330-1-arnd@arndb.de> References: <20161108135721.2142330-1-arnd@arndb.de> From: Ilia Mirkin Date: Tue, 8 Nov 2016 10:46:07 -0500 X-Google-Sender-Auth: 3Og4dWcsNBs9A-W2Z1_DIP3v4Bg Message-ID: Subject: Re: [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration To: Arnd Bergmann Cc: Ben Skeggs , "nouveau@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2972 Lines: 67 On Tue, Nov 8, 2016 at 8:56 AM, Arnd Bergmann 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 > --- > 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. Cheers, -ilia