Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE

[Helge Deller <deller@gmx.de>]

On 12/13/24 00:24, Jani Nikula wrote:
> On Thu, 12 Dec 2024, "Arnd Bergmann" <arnd@arndb.de> wrote:
>> On Thu, Dec 12, 2024, at 19:44, Helge Deller wrote:
>>> On 12/12/24 11:04, Thomas Zimmermann wrote:
>>>> Do not select BACKLIGHT_CLASS_DEVICE from FB_BACKLIGHT. The latter
>>>> only controls backlight support within fbdev core code and data
>>>> structures.
>>>>
>>>> Make fbdev drivers depend on BACKLIGHT_CLASS_DEVICE and let users
>>>> select it explicitly. Fixes warnings about recursive dependencies,
>>>> such as [...]
>>>
>>> I think in the fbdev drivers themselves you should do:
>>> 	select BACKLIGHT_CLASS_DEVICE
>>> instead of "depending" on it.
>>> This is the way as it's done in the DRM tiny and the i915/gma500 DRM drivers.
>>>
>>> So, something like:
>>>
>>> --- a/drivers/staging/fbtft/Kconfig
>>>          tristate "Support for small TFT LCD display modules"
>>>          depends on FB && SPI
>>>          depends on FB_DEVICE
>>>     +    select BACKLIGHT_DEVICE_CLASS
>>>          depends on GPIOLIB || COMPILE_TEST
>>>          select FB_BACKLIGHT
>>>
>>> config FB_BACKLIGHT
>>>             tristate
>>>             depends on FB
>>>     -	  select BACKLIGHT_CLASS_DEVICE
>>>     +       depends on BACKLIGHT_CLASS_DEVICE
>>>
>>>
>>> Would that fix the dependency warning?
>>
>> The above is generally a mistake and the root cause of the
>> dependency loops. With very few exceptions, the solution in
>> these cases is to find the inconsistent 'select' and change
>> it into 'depends on'.
>
> Agreed.

That's fine, but my point is that it should be consistent.
For example:

~:/git-kernel/linux$ grep -r "select.*BACKLIGHT_CLASS_DEVICE" drivers/gpu/
drivers/gpu/drm/tilcdc/Kconfig: select BACKLIGHT_CLASS_DEVICE
drivers/gpu/drm/nouveau/Kconfig:        select BACKLIGHT_CLASS_DEVICE if DRM_NOUVEAU_BACKLIGHT
drivers/gpu/drm/nouveau/Kconfig:        select BACKLIGHT_CLASS_DEVICE if ACPI && X86
drivers/gpu/drm/tiny/Kconfig:   select BACKLIGHT_CLASS_DEVICE
drivers/gpu/drm/tiny/Kconfig:   select BACKLIGHT_CLASS_DEVICE
drivers/gpu/drm/tiny/Kconfig:   select BACKLIGHT_CLASS_DEVICE
drivers/gpu/drm/tiny/Kconfig:   select BACKLIGHT_CLASS_DEVICE
drivers/gpu/drm/tiny/Kconfig:   select BACKLIGHT_CLASS_DEVICE
drivers/gpu/drm/tiny/Kconfig:   select BACKLIGHT_CLASS_DEVICE
drivers/gpu/drm/tiny/Kconfig:   select BACKLIGHT_CLASS_DEVICE
drivers/gpu/drm/fsl-dcu/Kconfig:        select BACKLIGHT_CLASS_DEVICE
drivers/gpu/drm/i915/Kconfig:   select BACKLIGHT_CLASS_DEVICE if ACPI
drivers/gpu/drm/gma500/Kconfig: select BACKLIGHT_CLASS_DEVICE if ACPI
drivers/gpu/drm/amd/amdgpu/Kconfig:     select BACKLIGHT_CLASS_DEVICE
drivers/gpu/drm/xe/Kconfig:     select BACKLIGHT_CLASS_DEVICE if ACPI
drivers/gpu/drm/solomon/Kconfig:        select BACKLIGHT_CLASS_DEVICE
drivers/gpu/drm/radeon/Kconfig: select BACKLIGHT_CLASS_DEVICE
drivers/gpu/drm/renesas/shmobile/Kconfig:       select BACKLIGHT_CLASS_DEVICE
drivers/gpu/drm/gud/Kconfig:    select BACKLIGHT_CLASS_DEVICE
drivers/gpu/drm/bridge/Kconfig: select BACKLIGHT_CLASS_DEVICE

All major drm graphics drivers *select* BACKLIGHT_CLASS_DEVICE.
Are you changing them to "depend on" as well?

Helge