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

[Helge Deller <deller@gmx.de>]

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 ?

Helge