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.
> 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
