| Message ID | 20241212100636.45875-2-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm,fbdev: Fix module dependencies | expand |
On 12/12/24 11:04, 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 [...] I think in the fbdev drivers themselves you should do: select BACKLIGHT_CLASS_DEVICE instead of "depending" on it. This is the way as it's done in the DRM tiny and the i915/gma500 DRM drivers. So, 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 - select BACKLIGHT_CLASS_DEVICE + depends on BACKLIGHT_CLASS_DEVICE Would that fix the dependency warning? Helge > > 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. > > v2: > - s/BACKLIGHT_DEVICE_CLASS/BACKLIGHT_CLASS_DEVICE (Helge) > - Fix fbdev driver-dependency corner case (Arnd) > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/auxdisplay/Kconfig | 2 +- > drivers/macintosh/Kconfig | 1 + > drivers/staging/fbtft/Kconfig | 1 + > drivers/video/fbdev/Kconfig | 18 +++++++++++++----- > drivers/video/fbdev/core/Kconfig | 3 +-- > 5 files changed, 17 insertions(+), 8 deletions(-) > > diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig > index 21545ffba065..8934e6ad5772 100644 > --- a/drivers/auxdisplay/Kconfig > +++ b/drivers/auxdisplay/Kconfig > @@ -489,7 +489,7 @@ config IMG_ASCII_LCD > > config HT16K33 > tristate "Holtek Ht16K33 LED controller with keyscan" > - depends on FB && I2C && INPUT > + depends on FB && I2C && INPUT && BACKLIGHT_CLASS_DEVICE > select FB_SYSMEM_HELPERS > select INPUT_MATRIXKMAP > select FB_BACKLIGHT > diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig > index fb38f684444f..bf3824032d61 100644 > --- a/drivers/macintosh/Kconfig > +++ b/drivers/macintosh/Kconfig > @@ -120,6 +120,7 @@ config PMAC_MEDIABAY > config PMAC_BACKLIGHT > bool "Backlight control for LCD screens" > depends on PPC_PMAC && ADB_PMU && FB = y && (BROKEN || !PPC64) > + depends on BACKLIGHT_CLASS_DEVICE > select FB_BACKLIGHT > help > Say Y here to enable Macintosh specific extensions of the generic > diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig > index 77ab44362f16..dcf6a70455cc 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_CLASS_DEVICE > depends on GPIOLIB || COMPILE_TEST > select FB_BACKLIGHT > select FB_SYSMEM_HELPERS_DEFERRED > 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 > + select FB_BACKLIGHT > default y > help > Say Y here if you want to control the backlight of your display. > @@ -707,7 +709,6 @@ config FB_NVIDIA_BACKLIGHT > config FB_RIVA > tristate "nVidia Riva support" > depends on FB && PCI > - select FB_BACKLIGHT if FB_RIVA_BACKLIGHT > select FB_CFB_FILLRECT > select FB_CFB_COPYAREA > select FB_CFB_IMAGEBLIT > @@ -747,6 +748,8 @@ config FB_RIVA_DEBUG > config FB_RIVA_BACKLIGHT > bool "Support for backlight control" > depends on FB_RIVA > + depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_RIVA > + select FB_BACKLIGHT > default y > help > Say Y here if you want to control the backlight of your display. > @@ -934,7 +937,6 @@ config FB_MATROX_MAVEN > config FB_RADEON > tristate "ATI Radeon display support" > depends on FB && PCI > - select FB_BACKLIGHT if FB_RADEON_BACKLIGHT > select FB_CFB_FILLRECT > select FB_CFB_COPYAREA > select FB_CFB_IMAGEBLIT > @@ -960,6 +962,8 @@ config FB_RADEON_I2C > config FB_RADEON_BACKLIGHT > bool "Support for backlight control" > depends on FB_RADEON > + depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_RADEON > + select FB_BACKLIGHT > default y > help > Say Y here if you want to control the backlight of your display. > @@ -975,7 +979,6 @@ config FB_RADEON_DEBUG > config FB_ATY128 > tristate "ATI Rage128 display support" > depends on FB && PCI > - select FB_BACKLIGHT if FB_ATY128_BACKLIGHT > select FB_IOMEM_HELPERS > select FB_MACMODES if PPC_PMAC > help > @@ -989,6 +992,8 @@ config FB_ATY128 > config FB_ATY128_BACKLIGHT > bool "Support for backlight control" > depends on FB_ATY128 > + depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_ATY128 > + select FB_BACKLIGHT > default y > help > Say Y here if you want to control the backlight of your display. > @@ -999,7 +1004,6 @@ config FB_ATY > select FB_CFB_FILLRECT > select FB_CFB_COPYAREA > select FB_CFB_IMAGEBLIT > - select FB_BACKLIGHT if FB_ATY_BACKLIGHT > select FB_IOMEM_FOPS > select FB_MACMODES if PPC > select FB_ATY_CT if SPARC64 && PCI > @@ -1040,6 +1044,8 @@ config FB_ATY_GX > config FB_ATY_BACKLIGHT > bool "Support for backlight control" > depends on FB_ATY > + depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_ATY > + select FB_BACKLIGHT > default y > help > Say Y here if you want to control the backlight of your display. > @@ -1528,6 +1534,7 @@ config FB_SH_MOBILE_LCDC > depends on FB && HAVE_CLK && HAS_IOMEM > depends on SUPERH || COMPILE_TEST > depends on FB_DEVICE > + depends on BACKLIGHT_CLASS_DEVICE > select FB_BACKLIGHT > select FB_DEFERRED_IO > select FB_DMAMEM_HELPERS > @@ -1793,6 +1800,7 @@ config FB_SSD1307 > tristate "Solomon SSD1307 framebuffer support" > depends on FB && I2C > depends on GPIOLIB || COMPILE_TEST > + depends on BACKLIGHT_CLASS_DEVICE > select FB_BACKLIGHT > select FB_SYSMEM_HELPERS_DEFERRED > help > diff --git a/drivers/video/fbdev/core/Kconfig b/drivers/video/fbdev/core/Kconfig > index 0ab8848ba2f1..d554d8c543d4 100644 > --- a/drivers/video/fbdev/core/Kconfig > +++ b/drivers/video/fbdev/core/Kconfig > @@ -183,9 +183,8 @@ config FB_SYSMEM_HELPERS_DEFERRED > select FB_SYSMEM_HELPERS > > config FB_BACKLIGHT > - tristate > + bool > depends on FB > - select BACKLIGHT_CLASS_DEVICE > > config FB_MODE_HELPERS > bool "Enable Video Mode Handling Helpers"
On Thu, Dec 12, 2024, at 19:44, Helge Deller wrote: > On 12/12/24 11:04, 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 [...] > > I think in the fbdev drivers themselves you should do: > select BACKLIGHT_CLASS_DEVICE > instead of "depending" on it. > This is the way as it's done in the DRM tiny and the i915/gma500 DRM drivers. > > So, 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 > - select BACKLIGHT_CLASS_DEVICE > + depends on BACKLIGHT_CLASS_DEVICE > > > Would that fix the dependency warning? The above is generally a mistake and the root cause of the dependency loops. With very few exceptions, the solution in these cases is to find the inconsistent 'select' and change it into 'depends on'. I actually have a few more patches like this that I've been carrying for years now, e.g. one that changes all the 'select I2C' into appropriate dependencies. Arnd
On Thu, 12 Dec 2024, "Arnd Bergmann" <arnd@arndb.de> wrote: > On Thu, Dec 12, 2024, at 19:44, Helge Deller wrote: >> On 12/12/24 11:04, 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 [...] >> >> I think in the fbdev drivers themselves you should do: >> select BACKLIGHT_CLASS_DEVICE >> instead of "depending" on it. >> This is the way as it's done in the DRM tiny and the i915/gma500 DRM drivers. >> >> So, 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 >> - select BACKLIGHT_CLASS_DEVICE >> + depends on BACKLIGHT_CLASS_DEVICE >> >> >> Would that fix the dependency warning? > > The above is generally a mistake and the root cause of the > dependency loops. With very few exceptions, the solution in > these cases is to find the inconsistent 'select' and change > it into 'depends on'. Agreed. > I actually have a few more patches like this that I've > been carrying for years now, e.g. one that changes all the > 'select I2C' into appropriate dependencies. I've done that for BACKLIGHT_CLASS_DEVICE and some related configs years ago, but there was pushback, and I gave up. Nowadays when I see this I just chuckle briefly and move on. Occasionally I quote Documentation/kbuild/kconfig-language.rst: select should be used with care. select will force a symbol to a value without visiting the dependencies. By abusing select you are able to select a symbol FOO even if FOO depends on BAR that is not set. In general use select only for non-visible symbols (no prompts anywhere) and for symbols with no dependencies. That will limit the usefulness but on the other hand avoid the illegal configurations all over. If kconfig warned about selecting symbols with dependencies it would be painful for a while but eventually I think life would be easier. BR, Jani.
On 12/13/24 00:24, Jani Nikula wrote: > On Thu, 12 Dec 2024, "Arnd Bergmann" <arnd@arndb.de> wrote: >> On Thu, Dec 12, 2024, at 19:44, Helge Deller wrote: >>> On 12/12/24 11:04, 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 [...] >>> >>> I think in the fbdev drivers themselves you should do: >>> select BACKLIGHT_CLASS_DEVICE >>> instead of "depending" on it. >>> This is the way as it's done in the DRM tiny and the i915/gma500 DRM drivers. >>> >>> So, 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 >>> - select BACKLIGHT_CLASS_DEVICE >>> + depends on BACKLIGHT_CLASS_DEVICE >>> >>> >>> Would that fix the dependency warning? >> >> The above is generally a mistake and the root cause of the >> dependency loops. With very few exceptions, the solution in >> these cases is to find the inconsistent 'select' and change >> it into 'depends on'. > > Agreed. That's fine, but my point is that it should be consistent. For example: ~:/git-kernel/linux$ grep -r "select.*BACKLIGHT_CLASS_DEVICE" drivers/gpu/ drivers/gpu/drm/tilcdc/Kconfig: select BACKLIGHT_CLASS_DEVICE drivers/gpu/drm/nouveau/Kconfig: select BACKLIGHT_CLASS_DEVICE if DRM_NOUVEAU_BACKLIGHT drivers/gpu/drm/nouveau/Kconfig: select BACKLIGHT_CLASS_DEVICE if ACPI && X86 drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE drivers/gpu/drm/fsl-dcu/Kconfig: select BACKLIGHT_CLASS_DEVICE drivers/gpu/drm/i915/Kconfig: select BACKLIGHT_CLASS_DEVICE if ACPI drivers/gpu/drm/gma500/Kconfig: select BACKLIGHT_CLASS_DEVICE if ACPI drivers/gpu/drm/amd/amdgpu/Kconfig: select BACKLIGHT_CLASS_DEVICE drivers/gpu/drm/xe/Kconfig: select BACKLIGHT_CLASS_DEVICE if ACPI drivers/gpu/drm/solomon/Kconfig: select BACKLIGHT_CLASS_DEVICE drivers/gpu/drm/radeon/Kconfig: select BACKLIGHT_CLASS_DEVICE drivers/gpu/drm/renesas/shmobile/Kconfig: select BACKLIGHT_CLASS_DEVICE drivers/gpu/drm/gud/Kconfig: select BACKLIGHT_CLASS_DEVICE drivers/gpu/drm/bridge/Kconfig: select BACKLIGHT_CLASS_DEVICE All major drm graphics drivers *select* BACKLIGHT_CLASS_DEVICE. Are you changing them to "depend on" as well? Helge
Hi Am 13.12.24 um 00:56 schrieb Helge Deller: > On 12/13/24 00:24, Jani Nikula wrote: >> On Thu, 12 Dec 2024, "Arnd Bergmann" <arnd@arndb.de> wrote: >>> On Thu, Dec 12, 2024, at 19:44, Helge Deller wrote: >>>> On 12/12/24 11:04, 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 [...] >>>> >>>> I think in the fbdev drivers themselves you should do: >>>> select BACKLIGHT_CLASS_DEVICE >>>> instead of "depending" on it. >>>> This is the way as it's done in the DRM tiny and the i915/gma500 >>>> DRM drivers. >>>> >>>> So, 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 >>>> - select BACKLIGHT_CLASS_DEVICE >>>> + depends on BACKLIGHT_CLASS_DEVICE >>>> >>>> >>>> Would that fix the dependency warning? >>> >>> The above is generally a mistake and the root cause of the >>> dependency loops. With very few exceptions, the solution in >>> these cases is to find the inconsistent 'select' and change >>> it into 'depends on'. >> >> Agreed. > > That's fine, but my point is that it should be consistent. > For example: > > ~:/git-kernel/linux$ grep -r "select.*BACKLIGHT_CLASS_DEVICE" > drivers/gpu/ > drivers/gpu/drm/tilcdc/Kconfig: select BACKLIGHT_CLASS_DEVICE > drivers/gpu/drm/nouveau/Kconfig: select BACKLIGHT_CLASS_DEVICE > if DRM_NOUVEAU_BACKLIGHT > drivers/gpu/drm/nouveau/Kconfig: select BACKLIGHT_CLASS_DEVICE > if ACPI && X86 > drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE > drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE > drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE > drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE > drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE > drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE > drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE > drivers/gpu/drm/fsl-dcu/Kconfig: select BACKLIGHT_CLASS_DEVICE > drivers/gpu/drm/i915/Kconfig: select BACKLIGHT_CLASS_DEVICE if ACPI > drivers/gpu/drm/gma500/Kconfig: select BACKLIGHT_CLASS_DEVICE if ACPI > drivers/gpu/drm/amd/amdgpu/Kconfig: select BACKLIGHT_CLASS_DEVICE > drivers/gpu/drm/xe/Kconfig: select BACKLIGHT_CLASS_DEVICE if ACPI > drivers/gpu/drm/solomon/Kconfig: select BACKLIGHT_CLASS_DEVICE > drivers/gpu/drm/radeon/Kconfig: select BACKLIGHT_CLASS_DEVICE > drivers/gpu/drm/renesas/shmobile/Kconfig: select > BACKLIGHT_CLASS_DEVICE > drivers/gpu/drm/gud/Kconfig: select BACKLIGHT_CLASS_DEVICE > drivers/gpu/drm/bridge/Kconfig: select BACKLIGHT_CLASS_DEVICE > > All major drm graphics drivers *select* BACKLIGHT_CLASS_DEVICE. > Are you changing them to "depend on" as well? All these drivers should be changed to either 'depends on' or maybe 'imply'. Best regards Thomas > > Helge
Hi Am 12.12.24 um 22:04 schrieb Arnd Bergmann: > On Thu, Dec 12, 2024, at 19:44, Helge Deller wrote: >> On 12/12/24 11:04, 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 [...] >> I think in the fbdev drivers themselves you should do: >> select BACKLIGHT_CLASS_DEVICE >> instead of "depending" on it. >> This is the way as it's done in the DRM tiny and the i915/gma500 DRM drivers. >> >> So, 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 >> - select BACKLIGHT_CLASS_DEVICE >> + depends on BACKLIGHT_CLASS_DEVICE >> >> >> Would that fix the dependency warning? > The above is generally a mistake and the root cause of the > dependency loops. With very few exceptions, the solution in > these cases is to find the inconsistent 'select' and change > it into 'depends on'. > > I actually have a few more patches like this that I've > been carrying for years now, e.g. one that changes all the > 'select I2C' into appropriate dependencies. Thanks! If you have something for DRM, please submit. In the case of i2c, it might happen that the driver is useful without i2c support. But that's a discussion for the individual reviews. Best regards Thomas > > Arnd
Le 12/12/2024 à 11:04, Thomas Zimmermann a écrit : > 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. > > v2: > - s/BACKLIGHT_DEVICE_CLASS/BACKLIGHT_CLASS_DEVICE (Helge) > - Fix fbdev driver-dependency corner case (Arnd) > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/auxdisplay/Kconfig | 2 +- > drivers/macintosh/Kconfig | 1 + > drivers/staging/fbtft/Kconfig | 1 + > drivers/video/fbdev/Kconfig | 18 +++++++++++++----- > drivers/video/fbdev/core/Kconfig | 3 +-- > 5 files changed, 17 insertions(+), 8 deletions(-) Build fails which pmac32_defconfig : LD .tmp_vmlinux1 powerpc64-linux-ld: drivers/macintosh/via-pmu-backlight.o: in function `pmu_backlight_init': via-pmu-backlight.c:(.init.text+0xc0): undefined reference to `backlight_device_register' make[2]: *** [scripts/Makefile.vmlinux:77: vmlinux] Error 1 make[1]: *** [/home/chleroy/linux-powerpc/Makefile:1225: vmlinux] Error 2 > > diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig > index 21545ffba065..8934e6ad5772 100644 > --- a/drivers/auxdisplay/Kconfig > +++ b/drivers/auxdisplay/Kconfig > @@ -489,7 +489,7 @@ config IMG_ASCII_LCD > > config HT16K33 > tristate "Holtek Ht16K33 LED controller with keyscan" > - depends on FB && I2C && INPUT > + depends on FB && I2C && INPUT && BACKLIGHT_CLASS_DEVICE > select FB_SYSMEM_HELPERS > select INPUT_MATRIXKMAP > select FB_BACKLIGHT > diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig > index fb38f684444f..bf3824032d61 100644 > --- a/drivers/macintosh/Kconfig > +++ b/drivers/macintosh/Kconfig > @@ -120,6 +120,7 @@ config PMAC_MEDIABAY > config PMAC_BACKLIGHT > bool "Backlight control for LCD screens" > depends on PPC_PMAC && ADB_PMU && FB = y && (BROKEN || !PPC64) > + depends on BACKLIGHT_CLASS_DEVICE > select FB_BACKLIGHT > help > Say Y here to enable Macintosh specific extensions of the generic > diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig > index 77ab44362f16..dcf6a70455cc 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_CLASS_DEVICE > depends on GPIOLIB || COMPILE_TEST > select FB_BACKLIGHT > select FB_SYSMEM_HELPERS_DEFERRED > 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 > + select FB_BACKLIGHT > default y > help > Say Y here if you want to control the backlight of your display. > @@ -707,7 +709,6 @@ config FB_NVIDIA_BACKLIGHT > config FB_RIVA > tristate "nVidia Riva support" > depends on FB && PCI > - select FB_BACKLIGHT if FB_RIVA_BACKLIGHT > select FB_CFB_FILLRECT > select FB_CFB_COPYAREA > select FB_CFB_IMAGEBLIT > @@ -747,6 +748,8 @@ config FB_RIVA_DEBUG > config FB_RIVA_BACKLIGHT > bool "Support for backlight control" > depends on FB_RIVA > + depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_RIVA > + select FB_BACKLIGHT > default y > help > Say Y here if you want to control the backlight of your display. > @@ -934,7 +937,6 @@ config FB_MATROX_MAVEN > config FB_RADEON > tristate "ATI Radeon display support" > depends on FB && PCI > - select FB_BACKLIGHT if FB_RADEON_BACKLIGHT > select FB_CFB_FILLRECT > select FB_CFB_COPYAREA > select FB_CFB_IMAGEBLIT > @@ -960,6 +962,8 @@ config FB_RADEON_I2C > config FB_RADEON_BACKLIGHT > bool "Support for backlight control" > depends on FB_RADEON > + depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_RADEON > + select FB_BACKLIGHT > default y > help > Say Y here if you want to control the backlight of your display. > @@ -975,7 +979,6 @@ config FB_RADEON_DEBUG > config FB_ATY128 > tristate "ATI Rage128 display support" > depends on FB && PCI > - select FB_BACKLIGHT if FB_ATY128_BACKLIGHT > select FB_IOMEM_HELPERS > select FB_MACMODES if PPC_PMAC > help > @@ -989,6 +992,8 @@ config FB_ATY128 > config FB_ATY128_BACKLIGHT > bool "Support for backlight control" > depends on FB_ATY128 > + depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_ATY128 > + select FB_BACKLIGHT > default y > help > Say Y here if you want to control the backlight of your display. > @@ -999,7 +1004,6 @@ config FB_ATY > select FB_CFB_FILLRECT > select FB_CFB_COPYAREA > select FB_CFB_IMAGEBLIT > - select FB_BACKLIGHT if FB_ATY_BACKLIGHT > select FB_IOMEM_FOPS > select FB_MACMODES if PPC > select FB_ATY_CT if SPARC64 && PCI > @@ -1040,6 +1044,8 @@ config FB_ATY_GX > config FB_ATY_BACKLIGHT > bool "Support for backlight control" > depends on FB_ATY > + depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_ATY > + select FB_BACKLIGHT > default y > help > Say Y here if you want to control the backlight of your display. > @@ -1528,6 +1534,7 @@ config FB_SH_MOBILE_LCDC > depends on FB && HAVE_CLK && HAS_IOMEM > depends on SUPERH || COMPILE_TEST > depends on FB_DEVICE > + depends on BACKLIGHT_CLASS_DEVICE > select FB_BACKLIGHT > select FB_DEFERRED_IO > select FB_DMAMEM_HELPERS > @@ -1793,6 +1800,7 @@ config FB_SSD1307 > tristate "Solomon SSD1307 framebuffer support" > depends on FB && I2C > depends on GPIOLIB || COMPILE_TEST > + depends on BACKLIGHT_CLASS_DEVICE > select FB_BACKLIGHT > select FB_SYSMEM_HELPERS_DEFERRED > help > diff --git a/drivers/video/fbdev/core/Kconfig b/drivers/video/fbdev/core/Kconfig > index 0ab8848ba2f1..d554d8c543d4 100644 > --- a/drivers/video/fbdev/core/Kconfig > +++ b/drivers/video/fbdev/core/Kconfig > @@ -183,9 +183,8 @@ config FB_SYSMEM_HELPERS_DEFERRED > select FB_SYSMEM_HELPERS > > config FB_BACKLIGHT > - tristate > + bool > depends on FB > - select BACKLIGHT_CLASS_DEVICE > > config FB_MODE_HELPERS > bool "Enable Video Mode Handling Helpers"
Hi Am 13.12.24 um 08:44 schrieb Christophe Leroy: > > > Le 12/12/2024 à 11:04, Thomas Zimmermann a écrit : >> 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. >> >> v2: >> - s/BACKLIGHT_DEVICE_CLASS/BACKLIGHT_CLASS_DEVICE (Helge) >> - Fix fbdev driver-dependency corner case (Arnd) >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- >> drivers/auxdisplay/Kconfig | 2 +- >> drivers/macintosh/Kconfig | 1 + >> drivers/staging/fbtft/Kconfig | 1 + >> drivers/video/fbdev/Kconfig | 18 +++++++++++++----- >> drivers/video/fbdev/core/Kconfig | 3 +-- >> 5 files changed, 17 insertions(+), 8 deletions(-) > > Build fails which pmac32_defconfig : > > LD .tmp_vmlinux1 > powerpc64-linux-ld: drivers/macintosh/via-pmu-backlight.o: in function > `pmu_backlight_init': > via-pmu-backlight.c:(.init.text+0xc0): undefined reference to > `backlight_device_register' > make[2]: *** [scripts/Makefile.vmlinux:77: vmlinux] Error 1 > make[1]: *** [/home/chleroy/linux-powerpc/Makefile:1225: vmlinux] Error 2 The attached patch selects backlight support in the defconfigs that also have PMAC_BACKLIGHT=y. Can you please apply it on top of the patchset and report on the results? Best regards Thomas > > >> >> diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig >> index 21545ffba065..8934e6ad5772 100644 >> --- a/drivers/auxdisplay/Kconfig >> +++ b/drivers/auxdisplay/Kconfig >> @@ -489,7 +489,7 @@ config IMG_ASCII_LCD >> config HT16K33 >> tristate "Holtek Ht16K33 LED controller with keyscan" >> - depends on FB && I2C && INPUT >> + depends on FB && I2C && INPUT && BACKLIGHT_CLASS_DEVICE >> select FB_SYSMEM_HELPERS >> select INPUT_MATRIXKMAP >> select FB_BACKLIGHT >> diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig >> index fb38f684444f..bf3824032d61 100644 >> --- a/drivers/macintosh/Kconfig >> +++ b/drivers/macintosh/Kconfig >> @@ -120,6 +120,7 @@ config PMAC_MEDIABAY >> config PMAC_BACKLIGHT >> bool "Backlight control for LCD screens" >> depends on PPC_PMAC && ADB_PMU && FB = y && (BROKEN || !PPC64) >> + depends on BACKLIGHT_CLASS_DEVICE >> select FB_BACKLIGHT >> help >> Say Y here to enable Macintosh specific extensions of the >> generic >> diff --git a/drivers/staging/fbtft/Kconfig >> b/drivers/staging/fbtft/Kconfig >> index 77ab44362f16..dcf6a70455cc 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_CLASS_DEVICE >> depends on GPIOLIB || COMPILE_TEST >> select FB_BACKLIGHT >> select FB_SYSMEM_HELPERS_DEFERRED >> 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 >> + select FB_BACKLIGHT >> default y >> help >> Say Y here if you want to control the backlight of your display. >> @@ -707,7 +709,6 @@ config FB_NVIDIA_BACKLIGHT >> config FB_RIVA >> tristate "nVidia Riva support" >> depends on FB && PCI >> - select FB_BACKLIGHT if FB_RIVA_BACKLIGHT >> select FB_CFB_FILLRECT >> select FB_CFB_COPYAREA >> select FB_CFB_IMAGEBLIT >> @@ -747,6 +748,8 @@ config FB_RIVA_DEBUG >> config FB_RIVA_BACKLIGHT >> bool "Support for backlight control" >> depends on FB_RIVA >> + depends on BACKLIGHT_CLASS_DEVICE=y || >> BACKLIGHT_CLASS_DEVICE=FB_RIVA >> + select FB_BACKLIGHT >> default y >> help >> Say Y here if you want to control the backlight of your display. >> @@ -934,7 +937,6 @@ config FB_MATROX_MAVEN >> config FB_RADEON >> tristate "ATI Radeon display support" >> depends on FB && PCI >> - select FB_BACKLIGHT if FB_RADEON_BACKLIGHT >> select FB_CFB_FILLRECT >> select FB_CFB_COPYAREA >> select FB_CFB_IMAGEBLIT >> @@ -960,6 +962,8 @@ config FB_RADEON_I2C >> config FB_RADEON_BACKLIGHT >> bool "Support for backlight control" >> depends on FB_RADEON >> + depends on BACKLIGHT_CLASS_DEVICE=y || >> BACKLIGHT_CLASS_DEVICE=FB_RADEON >> + select FB_BACKLIGHT >> default y >> help >> Say Y here if you want to control the backlight of your display. >> @@ -975,7 +979,6 @@ config FB_RADEON_DEBUG >> config FB_ATY128 >> tristate "ATI Rage128 display support" >> depends on FB && PCI >> - select FB_BACKLIGHT if FB_ATY128_BACKLIGHT >> select FB_IOMEM_HELPERS >> select FB_MACMODES if PPC_PMAC >> help >> @@ -989,6 +992,8 @@ config FB_ATY128 >> config FB_ATY128_BACKLIGHT >> bool "Support for backlight control" >> depends on FB_ATY128 >> + depends on BACKLIGHT_CLASS_DEVICE=y || >> BACKLIGHT_CLASS_DEVICE=FB_ATY128 >> + select FB_BACKLIGHT >> default y >> help >> Say Y here if you want to control the backlight of your display. >> @@ -999,7 +1004,6 @@ config FB_ATY >> select FB_CFB_FILLRECT >> select FB_CFB_COPYAREA >> select FB_CFB_IMAGEBLIT >> - select FB_BACKLIGHT if FB_ATY_BACKLIGHT >> select FB_IOMEM_FOPS >> select FB_MACMODES if PPC >> select FB_ATY_CT if SPARC64 && PCI >> @@ -1040,6 +1044,8 @@ config FB_ATY_GX >> config FB_ATY_BACKLIGHT >> bool "Support for backlight control" >> depends on FB_ATY >> + depends on BACKLIGHT_CLASS_DEVICE=y || >> BACKLIGHT_CLASS_DEVICE=FB_ATY >> + select FB_BACKLIGHT >> default y >> help >> Say Y here if you want to control the backlight of your display. >> @@ -1528,6 +1534,7 @@ config FB_SH_MOBILE_LCDC >> depends on FB && HAVE_CLK && HAS_IOMEM >> depends on SUPERH || COMPILE_TEST >> depends on FB_DEVICE >> + depends on BACKLIGHT_CLASS_DEVICE >> select FB_BACKLIGHT >> select FB_DEFERRED_IO >> select FB_DMAMEM_HELPERS >> @@ -1793,6 +1800,7 @@ config FB_SSD1307 >> tristate "Solomon SSD1307 framebuffer support" >> depends on FB && I2C >> depends on GPIOLIB || COMPILE_TEST >> + depends on BACKLIGHT_CLASS_DEVICE >> select FB_BACKLIGHT >> select FB_SYSMEM_HELPERS_DEFERRED >> help >> diff --git a/drivers/video/fbdev/core/Kconfig >> b/drivers/video/fbdev/core/Kconfig >> index 0ab8848ba2f1..d554d8c543d4 100644 >> --- a/drivers/video/fbdev/core/Kconfig >> +++ b/drivers/video/fbdev/core/Kconfig >> @@ -183,9 +183,8 @@ config FB_SYSMEM_HELPERS_DEFERRED >> select FB_SYSMEM_HELPERS >> config FB_BACKLIGHT >> - tristate >> + bool >> depends on FB >> - select BACKLIGHT_CLASS_DEVICE >> config FB_MODE_HELPERS >> bool "Enable Video Mode Handling Helpers" >
Le 13/12/2024 à 09:05, Thomas Zimmermann a écrit : > Hi > > > Am 13.12.24 um 08:44 schrieb Christophe Leroy: >> >> >> Le 12/12/2024 à 11:04, Thomas Zimmermann a écrit : >>> 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. >>> >>> v2: >>> - s/BACKLIGHT_DEVICE_CLASS/BACKLIGHT_CLASS_DEVICE (Helge) >>> - Fix fbdev driver-dependency corner case (Arnd) >>> >>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >>> --- >>> drivers/auxdisplay/Kconfig | 2 +- >>> drivers/macintosh/Kconfig | 1 + >>> drivers/staging/fbtft/Kconfig | 1 + >>> drivers/video/fbdev/Kconfig | 18 +++++++++++++----- >>> drivers/video/fbdev/core/Kconfig | 3 +-- >>> 5 files changed, 17 insertions(+), 8 deletions(-) >> >> Build fails which pmac32_defconfig : >> >> LD .tmp_vmlinux1 >> powerpc64-linux-ld: drivers/macintosh/via-pmu-backlight.o: in function >> `pmu_backlight_init': >> via-pmu-backlight.c:(.init.text+0xc0): undefined reference to >> `backlight_device_register' >> make[2]: *** [scripts/Makefile.vmlinux:77: vmlinux] Error 1 >> make[1]: *** [/home/chleroy/linux-powerpc/Makefile:1225: vmlinux] Error 2 > > The attached patch selects backlight support in the defconfigs that also > have PMAC_BACKLIGHT=y. Can you please apply it on top of the patchset > and report on the results? > That works for the defconfig but it is still possible to change CONFIG_BACKLIGHT_CLASS_DEVICE manually. If it is necessary for PMAC_BACKLIGHT then it shouldn't be possible to deselect it.
Hi Am 13.12.24 um 09:33 schrieb Christophe Leroy: > > > Le 13/12/2024 à 09:05, Thomas Zimmermann a écrit : >> Hi >> >> >> Am 13.12.24 um 08:44 schrieb Christophe Leroy: >>> >>> >>> Le 12/12/2024 à 11:04, Thomas Zimmermann a écrit : >>>> 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. >>>> >>>> v2: >>>> - s/BACKLIGHT_DEVICE_CLASS/BACKLIGHT_CLASS_DEVICE (Helge) >>>> - Fix fbdev driver-dependency corner case (Arnd) >>>> >>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >>>> --- >>>> drivers/auxdisplay/Kconfig | 2 +- >>>> drivers/macintosh/Kconfig | 1 + >>>> drivers/staging/fbtft/Kconfig | 1 + >>>> drivers/video/fbdev/Kconfig | 18 +++++++++++++----- >>>> drivers/video/fbdev/core/Kconfig | 3 +-- >>>> 5 files changed, 17 insertions(+), 8 deletions(-) >>> >>> Build fails which pmac32_defconfig : >>> >>> LD .tmp_vmlinux1 >>> powerpc64-linux-ld: drivers/macintosh/via-pmu-backlight.o: in >>> function `pmu_backlight_init': >>> via-pmu-backlight.c:(.init.text+0xc0): undefined reference to >>> `backlight_device_register' >>> make[2]: *** [scripts/Makefile.vmlinux:77: vmlinux] Error 1 >>> make[1]: *** [/home/chleroy/linux-powerpc/Makefile:1225: vmlinux] >>> Error 2 >> >> The attached patch selects backlight support in the defconfigs that >> also have PMAC_BACKLIGHT=y. Can you please apply it on top of the >> patchset and report on the results? >> > > That works for the defconfig but it is still possible to change > CONFIG_BACKLIGHT_CLASS_DEVICE manually. > > If it is necessary for PMAC_BACKLIGHT then it shouldn't be possible to > deselect it. If you disable CONFIG_BACKLIGHT_CLASS_DEVICE, shouldn't that auto-disable PMAC_BACKLIGHT as well? Best regards Thomas
Hi Am 13.12.24 um 09:33 schrieb Christophe Leroy: > >> >> The attached patch selects backlight support in the defconfigs that >> also have PMAC_BACKLIGHT=y. Can you please apply it on top of the >> patchset and report on the results? >> > > That works for the defconfig but it is still possible to change > CONFIG_BACKLIGHT_CLASS_DEVICE manually. > > If it is necessary for PMAC_BACKLIGHT then it shouldn't be possible to > deselect it. Here's another patch that make it depend on BACKLIGHT_CLASS_DEVICE=y. Can you please try this as well? Best regards Thomas
Le 13/12/2024 à 09:35, Thomas Zimmermann a écrit : > Hi > > > Am 13.12.24 um 09:33 schrieb Christophe Leroy: >> >> >> Le 13/12/2024 à 09:05, Thomas Zimmermann a écrit : >>> Hi >>> >>> >>> Am 13.12.24 um 08:44 schrieb Christophe Leroy: >>>> >>>> >>>> Le 12/12/2024 à 11:04, Thomas Zimmermann a écrit : >>>>> 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. >>>>> >>>>> v2: >>>>> - s/BACKLIGHT_DEVICE_CLASS/BACKLIGHT_CLASS_DEVICE (Helge) >>>>> - Fix fbdev driver-dependency corner case (Arnd) >>>>> >>>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >>>>> --- >>>>> drivers/auxdisplay/Kconfig | 2 +- >>>>> drivers/macintosh/Kconfig | 1 + >>>>> drivers/staging/fbtft/Kconfig | 1 + >>>>> drivers/video/fbdev/Kconfig | 18 +++++++++++++----- >>>>> drivers/video/fbdev/core/Kconfig | 3 +-- >>>>> 5 files changed, 17 insertions(+), 8 deletions(-) >>>> >>>> Build fails which pmac32_defconfig : >>>> >>>> LD .tmp_vmlinux1 >>>> powerpc64-linux-ld: drivers/macintosh/via-pmu-backlight.o: in >>>> function `pmu_backlight_init': >>>> via-pmu-backlight.c:(.init.text+0xc0): undefined reference to >>>> `backlight_device_register' >>>> make[2]: *** [scripts/Makefile.vmlinux:77: vmlinux] Error 1 >>>> make[1]: *** [/home/chleroy/linux-powerpc/Makefile:1225: vmlinux] >>>> Error 2 >>> >>> The attached patch selects backlight support in the defconfigs that >>> also have PMAC_BACKLIGHT=y. Can you please apply it on top of the >>> patchset and report on the results? >>> >> >> That works for the defconfig but it is still possible to change >> CONFIG_BACKLIGHT_CLASS_DEVICE manually. >> >> If it is necessary for PMAC_BACKLIGHT then it shouldn't be possible to >> deselect it. > > If you disable CONFIG_BACKLIGHT_CLASS_DEVICE, shouldn't that auto- > disable PMAC_BACKLIGHT as well? For the time being it doesn't, hence the build failure. You can do it that way if you want, you need to add a dependency for that. Other solution is that PMAC_BACKLIGHT selects CONFIG_BACKLIGHT_CLASS_DEVICE. Christophe
Le 13/12/2024 à 09:41, Thomas Zimmermann a écrit : > Hi > > > Am 13.12.24 um 09:33 schrieb Christophe Leroy: >> >>> >>> The attached patch selects backlight support in the defconfigs that >>> also have PMAC_BACKLIGHT=y. Can you please apply it on top of the >>> patchset and report on the results? >>> >> >> That works for the defconfig but it is still possible to change >> CONFIG_BACKLIGHT_CLASS_DEVICE manually. >> >> If it is necessary for PMAC_BACKLIGHT then it shouldn't be possible to >> deselect it. > > Here's another patch that make it depend on BACKLIGHT_CLASS_DEVICE=y. > Can you please try this as well? That looks good, no build failure anymore with BACKLIGHT_CLASS_DEVICE=m
Hi Am 13.12.24 um 11:15 schrieb Christophe Leroy: > > > Le 13/12/2024 à 09:41, Thomas Zimmermann a écrit : >> Hi >> >> >> Am 13.12.24 um 09:33 schrieb Christophe Leroy: >>> >>>> >>>> The attached patch selects backlight support in the defconfigs that >>>> also have PMAC_BACKLIGHT=y. Can you please apply it on top of the >>>> patchset and report on the results? >>>> >>> >>> That works for the defconfig but it is still possible to change >>> CONFIG_BACKLIGHT_CLASS_DEVICE manually. >>> >>> If it is necessary for PMAC_BACKLIGHT then it shouldn't be possible >>> to deselect it. >> >> Here's another patch that make it depend on BACKLIGHT_CLASS_DEVICE=y. >> Can you please try this as well? > > That looks good, no build failure anymore with BACKLIGHT_CLASS_DEVICE=m Great, I'll add this change to the next iteration. Best regards Thomas
On Fri, Dec 13, 2024 at 08:26:19AM +0100, Thomas Zimmermann wrote: > Hi > > > Am 13.12.24 um 00:56 schrieb Helge Deller: > > On 12/13/24 00:24, Jani Nikula wrote: > > > On Thu, 12 Dec 2024, "Arnd Bergmann" <arnd@arndb.de> wrote: > > > > On Thu, Dec 12, 2024, at 19:44, Helge Deller wrote: > > > > > On 12/12/24 11:04, 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 [...] > > > > > > > > > > I think in the fbdev drivers themselves you should do: > > > > > select BACKLIGHT_CLASS_DEVICE > > > > > instead of "depending" on it. > > > > > This is the way as it's done in the DRM tiny and the > > > > > i915/gma500 DRM drivers. > > > > > > > > > > So, 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 > > > > > - select BACKLIGHT_CLASS_DEVICE > > > > > + depends on BACKLIGHT_CLASS_DEVICE > > > > > > > > > > > > > > > Would that fix the dependency warning? > > > > > > > > The above is generally a mistake and the root cause of the > > > > dependency loops. With very few exceptions, the solution in > > > > these cases is to find the inconsistent 'select' and change > > > > it into 'depends on'. > > > > > > Agreed. > > > > That's fine, but my point is that it should be consistent. > > For example: > > > > ~:/git-kernel/linux$ grep -r "select.*BACKLIGHT_CLASS_DEVICE" > > drivers/gpu/ > > drivers/gpu/drm/tilcdc/Kconfig: select BACKLIGHT_CLASS_DEVICE > > drivers/gpu/drm/nouveau/Kconfig: select BACKLIGHT_CLASS_DEVICE if > > DRM_NOUVEAU_BACKLIGHT > > drivers/gpu/drm/nouveau/Kconfig: select BACKLIGHT_CLASS_DEVICE if > > ACPI && X86 > > drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE > > drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE > > drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE > > drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE > > drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE > > drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE > > drivers/gpu/drm/tiny/Kconfig: select BACKLIGHT_CLASS_DEVICE > > drivers/gpu/drm/fsl-dcu/Kconfig: select BACKLIGHT_CLASS_DEVICE > > drivers/gpu/drm/i915/Kconfig: select BACKLIGHT_CLASS_DEVICE if ACPI > > drivers/gpu/drm/gma500/Kconfig: select BACKLIGHT_CLASS_DEVICE if ACPI > > drivers/gpu/drm/amd/amdgpu/Kconfig: select BACKLIGHT_CLASS_DEVICE > > drivers/gpu/drm/xe/Kconfig: select BACKLIGHT_CLASS_DEVICE if ACPI > > drivers/gpu/drm/solomon/Kconfig: select BACKLIGHT_CLASS_DEVICE > > drivers/gpu/drm/radeon/Kconfig: select BACKLIGHT_CLASS_DEVICE > > drivers/gpu/drm/renesas/shmobile/Kconfig: select > > BACKLIGHT_CLASS_DEVICE > > drivers/gpu/drm/gud/Kconfig: select BACKLIGHT_CLASS_DEVICE > > drivers/gpu/drm/bridge/Kconfig: select BACKLIGHT_CLASS_DEVICE > > > > All major drm graphics drivers *select* BACKLIGHT_CLASS_DEVICE. > > Are you changing them to "depend on" as well? > > All these drivers should be changed to either 'depends on' or maybe 'imply'. Yeah, select on non-leaf/library function Kconfig symbols tends to be a complete pain. There's some push to use select so it's easier for people to enable complex drivers, but I honestly don't think it's worth it. menuconfig can give you a list of things you need to enable first, so it's all discoverable enough (but a bit painful to get them all if it's a really complex driver with lots of dependencies). tldr; I concur fully, please no more select but instead less. -Sima > > Best regards > Thomas > > > > > 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) >
diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig index 21545ffba065..8934e6ad5772 100644 --- a/drivers/auxdisplay/Kconfig +++ b/drivers/auxdisplay/Kconfig @@ -489,7 +489,7 @@ config IMG_ASCII_LCD config HT16K33 tristate "Holtek Ht16K33 LED controller with keyscan" - depends on FB && I2C && INPUT + depends on FB && I2C && INPUT && BACKLIGHT_CLASS_DEVICE select FB_SYSMEM_HELPERS select INPUT_MATRIXKMAP select FB_BACKLIGHT diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig index fb38f684444f..bf3824032d61 100644 --- a/drivers/macintosh/Kconfig +++ b/drivers/macintosh/Kconfig @@ -120,6 +120,7 @@ config PMAC_MEDIABAY config PMAC_BACKLIGHT bool "Backlight control for LCD screens" depends on PPC_PMAC && ADB_PMU && FB = y && (BROKEN || !PPC64) + depends on BACKLIGHT_CLASS_DEVICE select FB_BACKLIGHT help Say Y here to enable Macintosh specific extensions of the generic diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig index 77ab44362f16..dcf6a70455cc 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_CLASS_DEVICE depends on GPIOLIB || COMPILE_TEST select FB_BACKLIGHT select FB_SYSMEM_HELPERS_DEFERRED 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 + select FB_BACKLIGHT default y help Say Y here if you want to control the backlight of your display. @@ -707,7 +709,6 @@ config FB_NVIDIA_BACKLIGHT config FB_RIVA tristate "nVidia Riva support" depends on FB && PCI - select FB_BACKLIGHT if FB_RIVA_BACKLIGHT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -747,6 +748,8 @@ config FB_RIVA_DEBUG config FB_RIVA_BACKLIGHT bool "Support for backlight control" depends on FB_RIVA + depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_RIVA + select FB_BACKLIGHT default y help Say Y here if you want to control the backlight of your display. @@ -934,7 +937,6 @@ config FB_MATROX_MAVEN config FB_RADEON tristate "ATI Radeon display support" depends on FB && PCI - select FB_BACKLIGHT if FB_RADEON_BACKLIGHT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -960,6 +962,8 @@ config FB_RADEON_I2C config FB_RADEON_BACKLIGHT bool "Support for backlight control" depends on FB_RADEON + depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_RADEON + select FB_BACKLIGHT default y help Say Y here if you want to control the backlight of your display. @@ -975,7 +979,6 @@ config FB_RADEON_DEBUG config FB_ATY128 tristate "ATI Rage128 display support" depends on FB && PCI - select FB_BACKLIGHT if FB_ATY128_BACKLIGHT select FB_IOMEM_HELPERS select FB_MACMODES if PPC_PMAC help @@ -989,6 +992,8 @@ config FB_ATY128 config FB_ATY128_BACKLIGHT bool "Support for backlight control" depends on FB_ATY128 + depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_ATY128 + select FB_BACKLIGHT default y help Say Y here if you want to control the backlight of your display. @@ -999,7 +1004,6 @@ config FB_ATY select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT - select FB_BACKLIGHT if FB_ATY_BACKLIGHT select FB_IOMEM_FOPS select FB_MACMODES if PPC select FB_ATY_CT if SPARC64 && PCI @@ -1040,6 +1044,8 @@ config FB_ATY_GX config FB_ATY_BACKLIGHT bool "Support for backlight control" depends on FB_ATY + depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_ATY + select FB_BACKLIGHT default y help Say Y here if you want to control the backlight of your display. @@ -1528,6 +1534,7 @@ config FB_SH_MOBILE_LCDC depends on FB && HAVE_CLK && HAS_IOMEM depends on SUPERH || COMPILE_TEST depends on FB_DEVICE + depends on BACKLIGHT_CLASS_DEVICE select FB_BACKLIGHT select FB_DEFERRED_IO select FB_DMAMEM_HELPERS @@ -1793,6 +1800,7 @@ config FB_SSD1307 tristate "Solomon SSD1307 framebuffer support" depends on FB && I2C depends on GPIOLIB || COMPILE_TEST + depends on BACKLIGHT_CLASS_DEVICE select FB_BACKLIGHT select FB_SYSMEM_HELPERS_DEFERRED help diff --git a/drivers/video/fbdev/core/Kconfig b/drivers/video/fbdev/core/Kconfig index 0ab8848ba2f1..d554d8c543d4 100644 --- a/drivers/video/fbdev/core/Kconfig +++ b/drivers/video/fbdev/core/Kconfig @@ -183,9 +183,8 @@ config FB_SYSMEM_HELPERS_DEFERRED select FB_SYSMEM_HELPERS config FB_BACKLIGHT - tristate + bool depends on FB - select BACKLIGHT_CLASS_DEVICE config FB_MODE_HELPERS bool "Enable Video Mode Handling Helpers"
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. v2: - s/BACKLIGHT_DEVICE_CLASS/BACKLIGHT_CLASS_DEVICE (Helge) - Fix fbdev driver-dependency corner case (Arnd) Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/auxdisplay/Kconfig | 2 +- drivers/macintosh/Kconfig | 1 + drivers/staging/fbtft/Kconfig | 1 + drivers/video/fbdev/Kconfig | 18 +++++++++++++----- drivers/video/fbdev/core/Kconfig | 3 +-- 5 files changed, 17 insertions(+), 8 deletions(-)