Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S969116AbdDSUjW (ORCPT ); Wed, 19 Apr 2017 16:39:22 -0400 Received: from mail-yb0-f194.google.com ([209.85.213.194]:33510 "EHLO mail-yb0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S937521AbdDSUjR (ORCPT ); Wed, 19 Apr 2017 16:39:17 -0400 MIME-Version: 1.0 In-Reply-To: <2979804.i6EKSW6ohY@avalon> References: <20170419175939.189098-1-arnd@arndb.de> <2979804.i6EKSW6ohY@avalon> From: Arnd Bergmann Date: Wed, 19 Apr 2017 22:39:16 +0200 X-Google-Sender-Auth: LCL2p8Z_5E1BXyRv2zO83ee3WuE Message-ID: Subject: Re: [PATCH 1/3] drm/omap: displays: panel-dpi: add backlight dependency To: Laurent Pinchart Cc: Tomi Valkeinen , David Airlie , dri-devel , Linux Kernel Mailing List 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: 1908 Lines: 51 On Wed, Apr 19, 2017 at 10:21 PM, Laurent Pinchart wrote: >> >> This adds a dependency like we have for the other panel drivers. > > I believe the dependency should be made optional. DPI panels that don't need > backlight control should be supported by a kernel that has backlight support > compiled out. That would be nice in principle, but I fear this would cause additional problems. > --- a/include/linux/backlight.h > +++ b/include/linux/backlight.h > @@ -162,7 +162,7 @@ struct generic_bl_info { > void (*kick_battery)(void); > }; > > -#ifdef CONFIG_OF > +#if IS_ENABLED(CONFIG_OF) && IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE) > struct backlight_device *of_find_backlight_by_node(struct device_node *node); > #else > static inline struct backlight_device * > > > We might need to create stubs for backlight_force_update() and > backlight_device_set_brightness() too. > With BACKLIGHT_CLASS_DEVICE=m, you still get a link error when the user is in a built-in driver. Using 'depends on' usually solves this (except for drivers that cannot be modules). There are three possible workarounds for this that I can think of: - Use 'depends on BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=n' in each driver that implements optional backlight support. We do this elsewhere, but it's confusing and easy to get wrong. - use IS_REACHABLE() instead of IS_ENABLED() when testing for backlight support. This will always result in a kernel that builds cleanly, but can be surprising for users when backlight support is a module that gets loaded at boot, but it is still not used. - Make BACKLIGHT_CLASS_DEVICE a 'bool' symbol instead, and force the core API code to always be built-in or completely disabled. This makes it really easy to use, at the expense of a larger kernel image for those that currently use a loadable module. Arnd