linux-kernel - Re: [PATCH] drm/nouveau: fix LEDS

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
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <12373137.2mgsQIhOfV@wuerfel>
Date:   Tue, 08 Nov 2016 16:52:49 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Ilia Mirkin <imirkin@...m.mit.edu>
Cc:     Ben Skeggs <bskeggs@...hat.com>,
        "nouveau@...ts.freedesktop.org" <nouveau@...ts.freedesktop.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration

On Tuesday, November 8, 2016 10:46:07 AM CET Ilia Mirkin wrote:
> > 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?

Using 'select' on user-selectable symbols risks introducing
circular dependencies.

> Separately, perhaps we should just drop this LEDS_CLASS select into
> DRM_NOUVEAU? We've tended to avoid adding tons of options.

My first attempt was to add "depends on LEDS_CLASS || !LEDS_CLASS"
to DRM_NOUVEAU, which led to one long circle:

drivers/usb/Kconfig:39:error: recursive dependency detected!
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/usb/Kconfig:39:	symbol USB is selected by MOUSE_APPLETOUCH
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/input/mouse/Kconfig:187:	symbol MOUSE_APPLETOUCH depends on INPUT
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/input/Kconfig:8:	symbol INPUT is selected by VT
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/tty/Kconfig:12:	symbol VT is selected by FB_STI
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/video/fbdev/Kconfig:669:	symbol FB_STI depends on FB
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/video/fbdev/Kconfig:5:	symbol FB is selected by DRM_KMS_FB_HELPER
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/gpu/drm/Kconfig:42:	symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/gpu/drm/Kconfig:36:	symbol DRM_KMS_HELPER is selected by DRM_NOUVEAU
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/gpu/drm/nouveau/Kconfig:1:	symbol DRM_NOUVEAU depends on LEDS_CLASS
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/leds/Kconfig:16:	symbol LEDS_CLASS is selected by OMAP_DEBUG_LEDS
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
arch/arm/plat-omap/Kconfig:19:	symbol OMAP_DEBUG_LEDS depends on NEW_LEDS
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/leds/Kconfig:8:	symbol NEW_LEDS is selected by ATH9K_HTC
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/net/wireless/ath/ath9k/Kconfig:158:	symbol ATH9K_HTC depends on USB
Tue, 08 Nov 2016 11:49:44 +0100 build/0x3053A542_defconfig warnings
drivers/usb/Kconfig:39:error: recursive dependency detected!
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/usb/Kconfig:39:	symbol USB is selected by MOUSE_APPLETOUCH
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/input/mouse/Kconfig:187:	symbol MOUSE_APPLETOUCH depends on INPUT
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/input/Kconfig:8:	symbol INPUT is selected by VT
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/tty/Kconfig:12:	symbol VT is selected by FB_STI
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/video/fbdev/Kconfig:669:	symbol FB_STI depends on FB
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/video/fbdev/Kconfig:5:	symbol FB is selected by DRM_KMS_FB_HELPER
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/gpu/drm/Kconfig:42:	symbol DRM_KMS_FB_HELPER depends on DRM_KMS_HELPER
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/gpu/drm/Kconfig:36:	symbol DRM_KMS_HELPER is selected by DRM_NOUVEAU
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/gpu/drm/nouveau/Kconfig:1:	symbol DRM_NOUVEAU depends on LEDS_CLASS
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/leds/Kconfig:16:	symbol LEDS_CLASS is selected by OMAP_DEBUG_LEDS
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
arch/arm/plat-omap/Kconfig:19:	symbol OMAP_DEBUG_LEDS depends on NEW_LEDS
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/leds/Kconfig:8:	symbol NEW_LEDS is selected by ATH9K_HTC
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"
drivers/net/wireless/ath/ath9k/Kconfig:158:	symbol ATH9K_HTC depends on USB

The underlying problem is that we already have a number of other
symbols that either have "depends on LEDS_CLASS" or
"select LEDS_CLASS". To clean that up properly, we should either
make the symbol itself hidden and only select it from other drivers,
or use "depends on LEDS_CLASS" everywhere.

Another option is to use the IS_REACHABLE() macro instead of IS_ENABLED()
in the header file, to stub out the calls into the new file, but
that can be a bit confusing.

	Arnd