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

[Helge Deller]

On 12/11/24 00:53, Helge Deller wrote:
> On 12/11/24 00:37, Helge Deller wrote:
>> On 12/10/24 16:41, Thomas Zimmermann wrote:
>>> Hi
>>>
>>>
>>> Am 10.12.24 um 15:34 schrieb Helge Deller:
>>>> On 12/10/24 15:29, Helge Deller wrote:
>>>>> On 12/10/24 15:09, Thomas Zimmermann wrote:
>>>>>> diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
>>>>>> index 77ab44362f16..577e91ff7bf6 100644
>>>>>> --- a/drivers/staging/fbtft/Kconfig
>>>>>> +++ b/drivers/staging/fbtft/Kconfig
>>>>>> @@ -3,6 +3,7 @@ menuconfig FB_TFT
>>>>>>       tristate "Support for small TFT LCD display modules"
>>>>>>       depends on FB && SPI
>>>>>>       depends on FB_DEVICE
>>>>>> +    depends on BACKLIGHT_DEVICE_CLASS
>>>>>
>>>>> Typo. Should be BACKLIGHT_CLASS_DEVICE...
>>>
>>> Ah, thanks. I'll better check the rest of the series for similar mistakes.
>>>
>>>>
>>>> Beside the typo:
>>>> In this case, doesn't it make sense to "select BACKLIGHT_DEVICE_CLASS" instead?
>>>
>>> That causes the dependency error mentioned in the commit message. This time it's just for fbtft instead of shmobilefb.
>>>
>>>> If people want the fbtft, backlight support should be enabled too.
>>>
>>> As a user-visible option, it should not be auto-selected
>>> unnecessarily.
>>
>> Right, it should not be auto-selected.
>> Unless if fbtft really needs it enabled to function.
>> IMHO all fb/drm drivers have higher priority than some low-level
>> background backlight controller code.
>>
>>> The DRM panel drivers already depend on the backlight
>>> instead of selecting it. It's the correct approach.
>>
>> Sounds wrong IMHO.
>>
>>> As I mentioned
>>> in the cover letter, the few remaining driver that select it should
>>> probably be updated.
>>
>> That dependency sounds weird, but maybe I simply misunderstand your logic...?
>>
>> As a Linux end user I usually know which graphic cards are in my machine
>> and which ones I want to enable.
>> But as a normal user I think I shouldn't be expected to know
>> that I first need to enable the "backlight class device"
>> so that I'm then able to afterwards enable the fbtft (or any other drm/fb driver).
>>
>> Am I wrong?
>
> Looking closer on this...
> You propose:
>
> --- a/drivers/staging/fbtft/Kconfig
> +++ b/drivers/staging/fbtft/Kconfig
> @@ -3,6 +3,7 @@ menuconfig FB_TFT
>       tristate "Support for small TFT LCD display modules"
>       depends on FB && SPI
>       depends on FB_DEVICE
> +    depends on BACKLIGHT_DEVICE_CLASS
>       depends on GPIOLIB || COMPILE_TEST
>       select FB_BACKLIGHT
>
> So, it will depend on BACKLIGHT_DEVICE_CLASS.
> But there is "select FB_BACKLIGHT" as well, which is:
> config FB_BACKLIGHT
>          tristate
>          depends on FB
>          select BACKLIGHT_CLASS_DEVICE
>
> So, you end up with selecting and depending on BACKLIGHT_CLASS_DEVICE ?

Ok. Ignore this ^^^ . I now understand your cover letter.

Looking at your DRM tiny drivers and the i915/gma500 DRM drivers,
there is a "select BACKLIGHT_CLASS_DEVICE" in those.

So, isn't the right approach then 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
   +       depends on BACKLIGHT_CLASS_DEVICE

?