[PATCH v7 1/3] backlight: Add IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)

[Daniel Thompson]

On 01/10/17 18:26, Meghana Madhyastha wrote:
> Add IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE) as part of the
> if directive for the function declaration of
> of_find_backlight_by_node in order to avoid module dependency
> errors.

Module dependency errors? Does you mean mean use of undefined symbols?


> Signed-off-by: Meghana Madhyastha <meghana.madhyastha at gmail.com>
> ---
> Changes in v7:
> -This patch did not exist in v6.

So I'm coming to this patchset cold but can you explain *why* something 
wants to call of_find_backlight_by_node() when there is no backlight 
support enabled. Why isn't the code that called is conditional on 
BACKLIGHT_CLASS_DEVICE?

The undefined symbol issue is a pain but to be honest I'd rather solve 
the use of undefined symbols by avoiding declaring them; this making 
them into compile errors rather than link errors.


>   include/linux/backlight.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index 5f2fd61..a52ce82 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -162,7 +162,7 @@ struct generic_bl_info {
>   	void (*kick_battery)(void);
>   };
>   
> -#ifdef CONFIG_OF
> +#if defined CONFIG_OF && IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)

The above comments are more important but why does this mix defined and 
IS_ENABLED? Couldn't they both use defined (and preferably with the 
optional brackets around the CONFIG_ symbol).


Daniel.