[PATCH v3 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
Helge Deller
deller at gmx.de
Sun Dec 22 20:15:33 UTC 2024
On 12/22/24 17:09, Thomas Zimmermann wrote:
> Hi
>
>
> Am 22.12.24 um 07:31 schrieb Helge Deller:
>> On 12/16/24 08:42, 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
>>>
>>> error: recursive dependency detected!
>>> symbol BACKLIGHT_CLASS_DEVICE is selected by FB_BACKLIGHT
>>> symbol FB_BACKLIGHT is selected by FB_SH_MOBILE_LCDC
>>> symbol FB_SH_MOBILE_LCDC depends on FB_DEVICE
>>> symbol FB_DEVICE depends on FB_CORE
>>> symbol FB_CORE is selected by DRM_GEM_DMA_HELPER
>>> symbol DRM_GEM_DMA_HELPER is selected by DRM_PANEL_ILITEK_ILI9341
>>> symbol DRM_PANEL_ILITEK_ILI9341 depends on BACKLIGHT_CLASS_DEVICE
>>>
>>> BACKLIGHT_CLASS_DEVICE is user-selectable, so making drivers adapt to
>>> it is the correct approach in any case. For most drivers, backlight
>>> support is also configurable separately.
>>>
>>> v3:
>>> - Select BACKLIGHT_CLASS_DEVICE in PowerMac defconfigs (Christophe)
>>> - Fix PMAC_BACKLIGHT module dependency corner cases (Christophe)
>>> v2:
>>> - s/BACKLIGHT_DEVICE_CLASS/BACKLIGHT_CLASS_DEVICE (Helge)
>>> - Fix fbdev driver-dependency corner case (Arnd)
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>>> ---
>>> arch/powerpc/configs/pmac32_defconfig | 1 +
>>> arch/powerpc/configs/ppc6xx_defconfig | 1 +
>>> drivers/auxdisplay/Kconfig | 2 +-
>>> drivers/macintosh/Kconfig | 1 +
>>> drivers/staging/fbtft/Kconfig | 1 +
>>> drivers/video/fbdev/Kconfig | 18 +++++++++++++-----
>>> drivers/video/fbdev/core/Kconfig | 3 +--
>>> 7 files changed, 19 insertions(+), 8 deletions(-)
>>>
>>> ...
>>> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
>>> index de035071fedb..55c6686f091e 100644
>>> --- a/drivers/video/fbdev/Kconfig
>>> +++ b/drivers/video/fbdev/Kconfig
>>> @@ -649,6 +649,7 @@ config FB_S1D13XXX
>>> config FB_ATMEL
>>> tristate "AT91 LCD Controller support"
>>> depends on FB && OF && HAVE_CLK && HAS_IOMEM
>>> + depends on BACKLIGHT_CLASS_DEVICE
>>> depends on HAVE_FB_ATMEL || COMPILE_TEST
>>> select FB_BACKLIGHT
>>> select FB_IOMEM_HELPERS
>>> @@ -660,7 +661,6 @@ config FB_ATMEL
>>> config FB_NVIDIA
>>> tristate "nVidia Framebuffer Support"
>>> depends on FB && PCI
>>> - select FB_BACKLIGHT if FB_NVIDIA_BACKLIGHT
>>> select FB_CFB_FILLRECT
>>> select FB_CFB_COPYAREA
>>> select FB_CFB_IMAGEBLIT
>>> @@ -700,6 +700,8 @@ config FB_NVIDIA_DEBUG
>>> config FB_NVIDIA_BACKLIGHT
>>> bool "Support for backlight control"
>>> depends on FB_NVIDIA
>>> + depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_NVIDIA
>>
>> Seems wrong. BACKLIGHT_CLASS_DEVICE is of type tristate.
>> There are more of those, please check.
>
> It's exactly correct. Linking would fail with FB_NVIDIA=y and BACKLIGHT=m. The above construct avoids this case. Please see Arnd's review comment at [1].
I'm not arguing against "depends on BACKLIGHT_CLASS_DEVICE=y".
It's the "BACKLIGHT_CLASS_DEVICE=FB_NVIDIA" which is wrong.
BACKLIGHT_CLASS_DEVICE is tristate, so either "y", "n" or "m", but never "FB_NVIDIA".
> That's also why I mentioned that 'imply' could be used rather than 'depends on'. It would handle this situation automatically.
>
> Best regards
> Thomas
>
> [1] https://patchwork.freedesktop.org/patch/628099/?series=142356&rev=1
More information about the dri-devel
mailing list