diff mbox series

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

Message ID 20241212100636.45875-2-tzimmermann@suse.de
State New
Headers show
Series drm,fbdev: Fix module dependencies | expand

Commit Message

Thomas Zimmermann Dec. 12, 2024, 10:04 a.m. UTC
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(-)

Comments

Helge Deller Dec. 12, 2024, 6:44 p.m. UTC | #1
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"
Arnd Bergmann Dec. 12, 2024, 9:04 p.m. UTC | #2
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
Jani Nikula Dec. 12, 2024, 11:24 p.m. UTC | #3
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.
Helge Deller Dec. 12, 2024, 11:56 p.m. UTC | #4
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
Thomas Zimmermann Dec. 13, 2024, 7:26 a.m. UTC | #5
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
Thomas Zimmermann Dec. 13, 2024, 7:28 a.m. UTC | #6
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
Christophe Leroy Dec. 13, 2024, 7:44 a.m. UTC | #7
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"
Thomas Zimmermann Dec. 13, 2024, 8:05 a.m. UTC | #8
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"
>
Christophe Leroy Dec. 13, 2024, 8:33 a.m. UTC | #9
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.
Thomas Zimmermann Dec. 13, 2024, 8:35 a.m. UTC | #10
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
Thomas Zimmermann Dec. 13, 2024, 8:41 a.m. UTC | #11
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
Christophe Leroy Dec. 13, 2024, 8:41 a.m. UTC | #12
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
Christophe Leroy Dec. 13, 2024, 10:15 a.m. UTC | #13
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
Thomas Zimmermann Dec. 13, 2024, 10:24 a.m. UTC | #14
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
diff mbox series

Patch

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"