Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1517342

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

Show all headers | View raw

On Tuesday, November 8, 2016 5:07:01 PM CET Lukas Wunner wrote:
> On Tue, Nov 08, 2016 at 04:52:49PM +0100, Arnd Bergmann wrote:
> > 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.
> 
> Why don't you just add empty inline stubs for nouveau_led_init / _fini /
> _suspend / _resume?
> 

That's what I was suggesting:

diff --git a/drivers/gpu/drm/nouveau/nouveau_led.h b/drivers/gpu/drm/nouveau/nouveau_led.h
index 9c9bb6ac938e..bc5f47cb516b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_led.h
+++ b/drivers/gpu/drm/nouveau/nouveau_led.h
@@ -35,21 +35,21 @@ struct nouveau_led {
 	struct led_classdev led;
 };
 
 static inline struct nouveau_led *
 nouveau_led(struct drm_device *dev)
 {
 	return nouveau_drm(dev)->led;
 }
 
 /* nouveau_led.c */
-#if IS_ENABLED(CONFIG_LEDS_CLASS)
+#if IS_REACHABLE(CONFIG_LEDS_CLASS)
 int  nouveau_led_init(struct drm_device *dev);
 void nouveau_led_suspend(struct drm_device *dev);
 void nouveau_led_resume(struct drm_device *dev);
 void nouveau_led_fini(struct drm_device *dev);
 #else
 static inline int  nouveau_led_init(struct drm_device *dev) { return 0; };
 static inline void nouveau_led_suspend(struct drm_device *dev) { };
 static inline void nouveau_led_resume(struct drm_device *dev) { };
 static inline void nouveau_led_fini(struct drm_device *dev) { };
 #endif

The downside is that now the nouveau_led_init() just won't be called
if CONFIG_LEDS_CLASS=m and CONFIG_DRM_NOUVEAU=y, which can be
surprising to users.

	Arnd

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] drm/nouveau: fix LEDS_CLASS=m configuration Arnd Bergmann <arnd@arndb.de> - 2016-11-08 15:00 +0100
  Re: [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration Ilia Mirkin <imirkin@alum.mit.edu> - 2016-11-08 16:50 +0100
    Re: [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration Arnd Bergmann <arnd@arndb.de> - 2016-11-08 17:00 +0100
      Re: [Nouveau] [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration Lukas Wunner <lukas@wunner.de> - 2016-11-08 17:10 +0100
        Re: [Nouveau] [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration Arnd Bergmann <arnd@arndb.de> - 2016-11-08 17:20 +0100
          Re: [Nouveau] [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration Karol Herbst <karolherbst@gmail.com> - 2016-11-08 17:30 +0100
            Re: [Nouveau] [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration Emil Velikov <emil.l.velikov@gmail.com> - 2016-11-08 17:40 +0100
    Re: [Nouveau] [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration Karol Herbst <karolherbst@gmail.com> - 2016-11-08 17:20 +0100
  Re: [PATCH] drm/nouveau: fix LEDS_CLASS=m configuration Martin Peres <martin.peres@free.fr> - 2016-11-08 17:20 +0100

csiph-web