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

From: Arnd Bergmann
Date: Tue Nov 08 2016 - 11:12:53 EST


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