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

[Thomas Zimmermann <tzimmermann@suse.de>]

Hi


Am 13.12.24 um 00:56 schrieb Helge Deller:
> 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?

All these drivers should be changed to either 'depends on' or maybe 'imply'.

Best regards
Thomas

>
> Helge

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)