On Fri, Dec 13, 2024 at 08:26:19AM +0100, Thomas Zimmermann wrote: > 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@xxxxxxxx> 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'. Yeah, select on non-leaf/library function Kconfig symbols tends to be a complete pain. There's some push to use select so it's easier for people to enable complex drivers, but I honestly don't think it's worth it. menuconfig can give you a list of things you need to enable first, so it's all discoverable enough (but a bit painful to get them all if it's a really complex driver with lots of dependencies). tldr; I concur fully, please no more select but instead less. -Sima > > 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) > -- Simona Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
