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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi


Am 11.12.24 um 00:37 schrieb Helge Deller:
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.

By that logic, we'd list always list all drivers and each driver would auso-select the subsystems it requires. So each fbdev driver would select CONFIG_FB.

That's not how it works, of course. Instead, each subsystem is user-selected and Kconfig offers the drivers that have their dependencies met. The documentation for Kconfig clearly states that select should be used carefully. [1]

[1] https://elixir.bootlin.com/linux/v6.12.4/source/Documentation/kbuild/kconfig-language.rst#L137


The DRM panel drivers already depend on the backlight
instead of selecting it. It's the correct approach.

Sounds wrong IMHO.

Generally, it's the right approach. I guess what could be done is to make backlight support optional in the driver code, and use the imply attribute [2] instead of depends. So the driver would indicate a preference for backlight support, but still work without. That could also be done for the fbdev drivers, of course.

[2] https://elixir.bootlin.com/linux/v6.12.4/source/Documentation/kbuild/kconfig-language.rst#L163

Best regards
Thomas


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?

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)





[Index of Archives]     [Linux Driver Development]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux