* Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
2024-12-12 10:04 ` [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE Thomas Zimmermann
@ 2024-12-12 18:44 ` Helge Deller
2024-12-12 21:04 ` Arnd Bergmann
2024-12-13 7:44 ` Christophe Leroy
2024-12-22 6:25 ` Helge Deller
2 siblings, 1 reply; 22+ messages in thread
From: Helge Deller @ 2024-12-12 18:44 UTC (permalink / raw)
To: Thomas Zimmermann, javierm, arnd, simona, airlied
Cc: dri-devel, linux-fbdev, linux-staging, linuxppc-dev
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"
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
2024-12-12 18:44 ` Helge Deller
@ 2024-12-12 21:04 ` Arnd Bergmann
2024-12-12 23:24 ` Jani Nikula
2024-12-13 7:28 ` Thomas Zimmermann
0 siblings, 2 replies; 22+ messages in thread
From: Arnd Bergmann @ 2024-12-12 21:04 UTC (permalink / raw)
To: Helge Deller, Thomas Zimmermann, Javier Martinez Canillas,
Simona Vetter, Dave Airlie
Cc: dri-devel, linux-fbdev, linux-staging, linuxppc-dev
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
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
2024-12-12 21:04 ` Arnd Bergmann
@ 2024-12-12 23:24 ` Jani Nikula
2024-12-12 23:56 ` Helge Deller
2024-12-13 7:28 ` Thomas Zimmermann
1 sibling, 1 reply; 22+ messages in thread
From: Jani Nikula @ 2024-12-12 23:24 UTC (permalink / raw)
To: Arnd Bergmann, Helge Deller, Thomas Zimmermann,
Javier Martinez Canillas, Simona Vetter, Dave Airlie
Cc: dri-devel, linux-fbdev, linux-staging, linuxppc-dev
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.
--
Jani Nikula, Intel
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
2024-12-12 23:24 ` Jani Nikula
@ 2024-12-12 23:56 ` Helge Deller
2024-12-13 7:26 ` Thomas Zimmermann
0 siblings, 1 reply; 22+ messages in thread
From: Helge Deller @ 2024-12-12 23:56 UTC (permalink / raw)
To: Jani Nikula, Arnd Bergmann, Thomas Zimmermann,
Javier Martinez Canillas, Simona Vetter, Dave Airlie
Cc: dri-devel, linux-fbdev, linux-staging, linuxppc-dev
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
2024-12-12 23:56 ` Helge Deller
@ 2024-12-13 7:26 ` Thomas Zimmermann
2024-12-16 14:41 ` Simona Vetter
0 siblings, 1 reply; 22+ messages in thread
From: Thomas Zimmermann @ 2024-12-13 7:26 UTC (permalink / raw)
To: Helge Deller, Jani Nikula, Arnd Bergmann,
Javier Martinez Canillas, Simona Vetter, Dave Airlie
Cc: dri-devel, linux-fbdev, linux-staging, linuxppc-dev
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
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)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
2024-12-13 7:26 ` Thomas Zimmermann
@ 2024-12-16 14:41 ` Simona Vetter
0 siblings, 0 replies; 22+ messages in thread
From: Simona Vetter @ 2024-12-16 14:41 UTC (permalink / raw)
To: Thomas Zimmermann
Cc: Helge Deller, Jani Nikula, Arnd Bergmann,
Javier Martinez Canillas, Simona Vetter, Dave Airlie, dri-devel,
linux-fbdev, linux-staging, linuxppc-dev
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)
>
--
Simona Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
2024-12-12 21:04 ` Arnd Bergmann
2024-12-12 23:24 ` Jani Nikula
@ 2024-12-13 7:28 ` Thomas Zimmermann
1 sibling, 0 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2024-12-13 7:28 UTC (permalink / raw)
To: Arnd Bergmann, Helge Deller, Javier Martinez Canillas,
Simona Vetter, Dave Airlie
Cc: dri-devel, linux-fbdev, linux-staging, linuxppc-dev
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
--
--
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)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
2024-12-12 10:04 ` [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE Thomas Zimmermann
2024-12-12 18:44 ` Helge Deller
@ 2024-12-13 7:44 ` Christophe Leroy
2024-12-13 8:05 ` Thomas Zimmermann
2024-12-22 6:25 ` Helge Deller
2 siblings, 1 reply; 22+ messages in thread
From: Christophe Leroy @ 2024-12-13 7:44 UTC (permalink / raw)
To: Thomas Zimmermann, javierm, arnd, deller, simona, airlied
Cc: dri-devel, linux-fbdev, linux-staging, linuxppc-dev
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"
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
2024-12-13 7:44 ` Christophe Leroy
@ 2024-12-13 8:05 ` Thomas Zimmermann
2024-12-13 8:33 ` Christophe Leroy
0 siblings, 1 reply; 22+ messages in thread
From: Thomas Zimmermann @ 2024-12-13 8:05 UTC (permalink / raw)
To: Christophe Leroy, javierm, arnd, deller, simona, airlied
Cc: dri-devel, linux-fbdev, linux-staging, linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 9476 bytes --]
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"
>
--
--
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)
[-- Attachment #2: 0001-add-BACKLIGHT_CLASS_DEVICE-on-PPC-defconfigs.patch --]
[-- Type: text/x-patch, Size: 1226 bytes --]
From b0283396aeed19cef8e1340c05103929cb41faf6 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: Fri, 13 Dec 2024 09:02:41 +0100
Subject: [PATCH] add BACKLIGHT_CLASS_DEVICE on PPC defconfigs
---
arch/powerpc/configs/pmac32_defconfig | 1 +
arch/powerpc/configs/ppc6xx_defconfig | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/powerpc/configs/pmac32_defconfig b/arch/powerpc/configs/pmac32_defconfig
index 57ded82c2840..e8b3f67bf3f5 100644
--- a/arch/powerpc/configs/pmac32_defconfig
+++ b/arch/powerpc/configs/pmac32_defconfig
@@ -208,6 +208,7 @@ CONFIG_FB_ATY=y
CONFIG_FB_ATY_CT=y
CONFIG_FB_ATY_GX=y
CONFIG_FB_3DFX=y
+CONFIG_BACKLIGHT_CLASS_DEVICE=y
# CONFIG_VGA_CONSOLE is not set
CONFIG_FRAMEBUFFER_CONSOLE=y
CONFIG_LOGO=y
diff --git a/arch/powerpc/configs/ppc6xx_defconfig b/arch/powerpc/configs/ppc6xx_defconfig
index 4d77e17541e9..ca0c90e95837 100644
--- a/arch/powerpc/configs/ppc6xx_defconfig
+++ b/arch/powerpc/configs/ppc6xx_defconfig
@@ -716,6 +716,7 @@ CONFIG_FB_TRIDENT=m
CONFIG_FB_SM501=m
CONFIG_FB_IBM_GXT4500=y
CONFIG_LCD_PLATFORM=m
+CONFIG_BACKLIGHT_CLASS_DEVICE=y
CONFIG_FRAMEBUFFER_CONSOLE=y
CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y
CONFIG_LOGO=y
--
2.47.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
2024-12-13 8:05 ` Thomas Zimmermann
@ 2024-12-13 8:33 ` Christophe Leroy
2024-12-13 8:35 ` Thomas Zimmermann
2024-12-13 8:41 ` Thomas Zimmermann
0 siblings, 2 replies; 22+ messages in thread
From: Christophe Leroy @ 2024-12-13 8:33 UTC (permalink / raw)
To: Thomas Zimmermann, javierm, arnd, deller, simona, airlied
Cc: dri-devel, linux-fbdev, linux-staging, linuxppc-dev
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.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
2024-12-13 8:33 ` Christophe Leroy
@ 2024-12-13 8:35 ` Thomas Zimmermann
2024-12-13 8:41 ` Christophe Leroy
2024-12-13 8:41 ` Thomas Zimmermann
1 sibling, 1 reply; 22+ messages in thread
From: Thomas Zimmermann @ 2024-12-13 8:35 UTC (permalink / raw)
To: Christophe Leroy, javierm, arnd, deller, simona, airlied
Cc: dri-devel, linux-fbdev, linux-staging, linuxppc-dev
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
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)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
2024-12-13 8:35 ` Thomas Zimmermann
@ 2024-12-13 8:41 ` Christophe Leroy
0 siblings, 0 replies; 22+ messages in thread
From: Christophe Leroy @ 2024-12-13 8:41 UTC (permalink / raw)
To: Thomas Zimmermann, javierm, arnd, deller, simona, airlied
Cc: dri-devel, linux-fbdev, linux-staging, linuxppc-dev
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
2024-12-13 8:33 ` Christophe Leroy
2024-12-13 8:35 ` Thomas Zimmermann
@ 2024-12-13 8:41 ` Thomas Zimmermann
2024-12-13 10:15 ` Christophe Leroy
1 sibling, 1 reply; 22+ messages in thread
From: Thomas Zimmermann @ 2024-12-13 8:41 UTC (permalink / raw)
To: Christophe Leroy, javierm, arnd, deller, simona, airlied
Cc: dri-devel, linux-fbdev, linux-staging, linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 791 bytes --]
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
--
--
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)
[-- Attachment #2: 0001-add-BACKLIGHT_CLASS_DEVICE-on-PPC-defconfigs.patch --]
[-- Type: text/x-patch, Size: 1803 bytes --]
From ac9c7c3d9413021e7fae06966160d58eb3c5c5d7 Mon Sep 17 00:00:00 2001
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: Fri, 13 Dec 2024 09:02:41 +0100
Subject: [PATCH] add BACKLIGHT_CLASS_DEVICE on PPC defconfigs
---
arch/powerpc/configs/pmac32_defconfig | 1 +
arch/powerpc/configs/ppc6xx_defconfig | 1 +
drivers/macintosh/Kconfig | 2 +-
3 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/configs/pmac32_defconfig b/arch/powerpc/configs/pmac32_defconfig
index 57ded82c2840..e8b3f67bf3f5 100644
--- a/arch/powerpc/configs/pmac32_defconfig
+++ b/arch/powerpc/configs/pmac32_defconfig
@@ -208,6 +208,7 @@ CONFIG_FB_ATY=y
CONFIG_FB_ATY_CT=y
CONFIG_FB_ATY_GX=y
CONFIG_FB_3DFX=y
+CONFIG_BACKLIGHT_CLASS_DEVICE=y
# CONFIG_VGA_CONSOLE is not set
CONFIG_FRAMEBUFFER_CONSOLE=y
CONFIG_LOGO=y
diff --git a/arch/powerpc/configs/ppc6xx_defconfig b/arch/powerpc/configs/ppc6xx_defconfig
index 4d77e17541e9..ca0c90e95837 100644
--- a/arch/powerpc/configs/ppc6xx_defconfig
+++ b/arch/powerpc/configs/ppc6xx_defconfig
@@ -716,6 +716,7 @@ CONFIG_FB_TRIDENT=m
CONFIG_FB_SM501=m
CONFIG_FB_IBM_GXT4500=y
CONFIG_LCD_PLATFORM=m
+CONFIG_BACKLIGHT_CLASS_DEVICE=y
CONFIG_FRAMEBUFFER_CONSOLE=y
CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y
CONFIG_LOGO=y
diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig
index bf3824032d61..d00e713c1092 100644
--- a/drivers/macintosh/Kconfig
+++ b/drivers/macintosh/Kconfig
@@ -120,7 +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
+ depends on BACKLIGHT_CLASS_DEVICE=y
select FB_BACKLIGHT
help
Say Y here to enable Macintosh specific extensions of the generic
--
2.47.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
2024-12-13 8:41 ` Thomas Zimmermann
@ 2024-12-13 10:15 ` Christophe Leroy
2024-12-13 10:24 ` Thomas Zimmermann
0 siblings, 1 reply; 22+ messages in thread
From: Christophe Leroy @ 2024-12-13 10:15 UTC (permalink / raw)
To: Thomas Zimmermann, javierm, arnd, deller, simona, airlied
Cc: dri-devel, linux-fbdev, linux-staging, linuxppc-dev
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
2024-12-13 10:15 ` Christophe Leroy
@ 2024-12-13 10:24 ` Thomas Zimmermann
0 siblings, 0 replies; 22+ messages in thread
From: Thomas Zimmermann @ 2024-12-13 10:24 UTC (permalink / raw)
To: Christophe Leroy, javierm, arnd, deller, simona, airlied
Cc: dri-devel, linux-fbdev, linux-staging, linuxppc-dev
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
--
--
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)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE
2024-12-12 10:04 ` [PATCH v2 1/3] fbdev: Fix recursive dependencies wrt BACKLIGHT_CLASS_DEVICE Thomas Zimmermann
2024-12-12 18:44 ` Helge Deller
2024-12-13 7:44 ` Christophe Leroy
@ 2024-12-22 6:25 ` Helge Deller
2 siblings, 0 replies; 22+ messages in thread
From: Helge Deller @ 2024-12-22 6:25 UTC (permalink / raw)
To: Thomas Zimmermann, javierm, arnd, simona, airlied
Cc: dri-devel, linux-fbdev, linux-staging, linuxppc-dev
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
>
> 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
BACKLIGHT_CLASS_DEVICE is of type tristate.
> + 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
here too. BACKLIGHT_CLASS_DEVICE is of type tristate.
> + 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
and here.
> + 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
and here.
> + 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
and here
> + 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"
^ permalink raw reply [flat|nested] 22+ messages in thread