Re: [PATCH 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
From: Helge Deller <deller@gmx.de>
Date: 2024-12-11 00:04:50
Also in:
dri-devel, linux-fbdev, linux-staging
On 12/11/24 00:53, Helge Deller wrote:
quoted hunk ↗ jump to hunk
On 12/11/24 00:37, Helge Deller wrote:quoted
On 12/10/24 16:41, Thomas Zimmermann wrote:quoted
Hi Am 10.12.24 um 15:34 schrieb Helge Deller:quoted
On 12/10/24 15:29, Helge Deller wrote:quoted
On 12/10/24 15:09, Thomas Zimmermann wrote:quoted
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_TFTtristate "Support for small TFT LCD display modules" depends on FB && SPI depends on FB_DEVICE + depends on BACKLIGHT_DEVICE_CLASSTypo. Should be BACKLIGHT_CLASS_DEVICE...Ah, thanks. I'll better check the rest of the series for similar mistakes.quoted
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.quoted
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.quoted
The DRM panel drivers already depend on the backlight instead of selecting it. It's the correct approach.Sounds wrong IMHO.quoted
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_BACKLIGHTSo, 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
?