Re: [PATCH 7/8] fbdev: rework backlight dependencies

On Fri, Apr 17, 2020 at 7:04 PM Sam Ravnborg [off-list ref] wrote:

Hi Arnd.

On Fri, Apr 17, 2020 at 05:55:52PM +0200, Arnd Bergmann wrote:

quoted

Rather than having CONFIG_FB_BACKLIGHT select CONFIG_BACKLIGHT_CLASS_DEVICE,
make any driver that needs it have a dependency on the class device
being available, to prevent circular dependencies.

This is the same way that the backlight is already treated for the DRM
subsystem.

I am not happy with the direction of this patch.
It is not easy to understand that one has to enable backlight to
be allowed to select a display or an fbdev driver.


How about somthing like this:

config BACKLIGHT_CLASS_DEVICE
        tristate
        # Will enable backlight module
        # Has no dependencies

config  BACKLIGHT
        bool "Backlight drivers"
        # Will make the backlight drivers visible - a visibility option
        # only

All drivers outside video/backlight/ can then select
BACKLIGHT_CLASS_DEVICE to get the backlight core
module.
Or in other words BACKLIGHT_CLASS_DEVICE is used as a sort
of library symbol that gices us access to backlight functionality.

Right, this could work as long as nobody puts back any "depends on
BACKLIGHT_CLASS_DEVICE, but it does cause another problem: There
are a couple of drivers that can optionally use backlight support
or just leave it out depending on CONFIG_BACKLIGHT_CLASS_DEVICE.

Changing them to select the class device support unconditionally
would make it impossible to build those drivers without it.
Instead we could all need an individual Kconfig symbol, or use
have "select BACKLIGHT_CLASS_DEVICE if BACKLIGHT"
in each case, but I'm not sure if that's still a win over having a
simple 'depends on BACKLIGHT_CLASS_DEVICE'.

On a related note, the NEW_LEDS/LEDS_CLASS option probably
falls into the same category, and could also be done as you suggest.
At the moment, this has the problem that both symbols are user
visible and also frequently selected by device drivers (but not others),
which is causing another set of dependency issues.

      Arnd