From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932361AbcKHQQS (ORCPT ); Tue, 8 Nov 2016 11:16:18 -0500 Received: from mail-it0-f68.google.com ([209.85.214.68]:34535 "EHLO mail-it0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752039AbcKHQQN (ORCPT ); Tue, 8 Nov 2016 11:16:13 -0500 MIME-Version: 1.0 In-Reply-To: References: <20161108135721.2142330-1-arnd@arndb.de> From: Karol Herbst Date: Tue, 8 Nov 2016 17:16:12 +0100 Message-ID: Subject: Re: [Nouveau] [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration To: Ilia Mirkin Cc: Arnd Bergmann , "nouveau@lists.freedesktop.org" , Ben Skeggs , "dri-devel@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" , Alexandre Courbot Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2016-11-08 16:46 GMT+01:00 Ilia Mirkin : > 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. > 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