Den 05.10.2017 05.53, skrev Meghana Madhyastha:
On Tue, Oct 03, 2017 at 07:38:29PM +0200, Noralf Trønnes wrote:
Den 03.10.2017 18.02, skrev Noralf Trønnes:
Den 03.10.2017 16.58, skrev Noralf Trønnes:
Den 02.10.2017 10.13, skrev Jani Nikula:
On Mon, 02 Oct 2017, Daniel Vetter <daniel@xxxxxxxx> wrote:
Also adding Jani, who looked at the backlight Kconfig mess in the
past.
I've even sent patches to fix some of the dependency mess, but the
problem is social not technical. The problem is that people think
"select" is more convenient than "depends" because they can just enable
a config that way, while "depends" would require finding and enabling
all the dependencies before the menu option even shows up.
I don't deny, that's annoying. But it's also abuse of select, see
Documentation/kbuild/kconfig-language.txt:
Note:
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.
The real fix would be making finding and enabling dependencies
recursively more convenient, but I don't see that happening anytime
soon.
If we don't select BACKLIGHT in drivers, we can end up with CONFIG_DRM=y
and CONFIG_BACKLIGHT_CLASS_DEVICE=m.
So we can either do:
menuconfig DRM
depends on (BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=n)
No, this was the thing that didn't work:
drivers/gpu/drm/Kconfig:7: symbol DRM depends on
BACKLIGHT_CLASS_DEVICE
drivers/video/backlight/Kconfig:158: symbol BACKLIGHT_CLASS_DEVICE is
selected by DRM_RADEON
drivers/gpu/drm/Kconfig:164: symbol DRM_RADEON depends on DRM
How about putting the dependency in the driver combined with IS_REACHABLE,
but not in backlight.h as I first proposed, that won't work:
menuconfig DRM_TINYDRM
- select BACKLIGHT_LCD_SUPPORT
- select BACKLIGHT_CLASS_DEVICE
+ depends on (BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=n)
struct backlight_device *drm_of_find_backlight(struct device *dev)
{
struct backlight_device *backlight;
struct device_node *np;
if (!IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE))
return NULL;
[...]
}
But the IS_REACHABLE doesn't feel right.
Maybe the problem is putting a function in DRM that really doesn't
belong there. How about calling it of_find_backlight() instead and
put it in backlight.[hc]?
Wouldn't it make more sense to put the function in DRM ? Because all of
the callers are located in DRM and so I'd think it makes more sense to
put it in DRM along with the other _of functions.
I don't think it matters where the callers are, but the fact that the
functions
are all about backlight and nothing about DRM, except the error messages.
I've made a suggestion in the last version where we have backlight
maintainer attention.
Noralf.
Noralf.
Or we can:
-#ifdef CONFIG_OF
+#if IS_ENABLED(CONFIG_OF) &&
IS_REACHABLE(CONFIG_BACKLIGHT_CLASS_DEVICE)
struct backlight_device *of_find_backlight_by_node(struct device_node
*node);
#else
static inline struct backlight_device *
I have one question. Why isn't your previous solution a good solution?
i.e IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE) which was part of v7
Using the second one it won't be obvious to users why backlight doesn't
work,
they have after all enabled backlight (but as module and DRM builtin).
So I suppose the first one is preferred.
What effect will such a change have on an existing configuration where
DRM=y and BACKLIGHT_CLASS_DEVICE=m? Will DRM become a module or will it
be disabled?
Why would it be disabled in this case ?
Thanks and regards,
Meghana
Noralf.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
