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

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

 



On 12/22/24 21:50, Arnd Bergmann wrote:
On Sun, Dec 22, 2024, at 21:15, Helge Deller wrote:
On 12/22/24 17:09, Thomas Zimmermann wrote:

+    depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_NVIDIA

Seems wrong. BACKLIGHT_CLASS_DEVICE is of type tristate.
There are more of those, please check.

It's exactly correct. Linking would fail with FB_NVIDIA=y and BACKLIGHT=m. The above construct avoids this case. Please see Arnd's review comment at [1].

I'm not arguing against "depends on BACKLIGHT_CLASS_DEVICE=y".
It's the "BACKLIGHT_CLASS_DEVICE=FB_NVIDIA" which is wrong.
BACKLIGHT_CLASS_DEVICE is tristate, so either "y", "n" or "m", but
never "FB_NVIDIA".

There are multiple ways to express this, but that line is a correct
way to allow all of

   BACKLIGHT_CLASS_DEVICE=y, FB_NVIDIA=m, FB_NVIDIA_BACKLIGHT=y
   BACKLIGHT_CLASS_DEVICE=y, FB_NVIDIA=y, FB_NVIDIA_BACKLIGHT=y
   BACKLIGHT_CLASS_DEVICE=m, FB_NVIDIA=m, FB_NVIDIA_BACKLIGHT=y

but disallow the broken

   BACKLIGHT_CLASS_DEVICE=m, FB_NVIDIA=y, FB_NVIDIA_BACKLIGHT=y

Ouch.
So, in BACKLIGHT_CLASS_DEVICE=FB_NVIDIA,  "FB_NVIDIA" is simply
being replaced by the current value of the FB_NVIDIA config option
(which is then one of: y/n/m).
I didn't thought of that!

If you find this line too confusing, can you suggest a different
expression that has the same behavior?

It's confusing, but probably the shortest one.

Arnd, thanks for explaining!

Thomas, you may add
Acked-by: Helge Deller <deller@xxxxxx>
to the series.
In case you want me to take the patch, please let me know.

Helge




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux