[PATCH] backlight: use of_find_backlight_by_node stub when backlight class disabled

[PATCH] backlight: use of_find_backlight_by_node stub when backlight class disabled

[Heiko Stübner]

Drivers may want to search for an optional backlight even when the backlight
class is disabled. In this case the linker would miss the function referenced
in the backlight header.

Therefore use the stub function also when the backlight class is disabled.

Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 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 adb14a8..d9cb644 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -157,7 +157,7 @@ struct generic_bl_info {
 	void (*kick_battery)(void);
 };
 
-#ifdef CONFIG_OF
+#if defined(CONFIG_OF) && defined(CONFIG_BACKLIGHT_CLASS_DEVICE)
 struct backlight_device *of_find_backlight_by_node(struct device_node *node);
 #else
 static inline struct backlight_device *
-- 
2.0.1

Re: [PATCH] backlight: use of_find_backlight_by_node stub when backlight class disabled

[Jingoo Han]

On Wednesday, October 29, 2014 9:20 AM, Heiko Stübner wrote:
> 
> Drivers may want to search for an optional backlight even when the backlight
> class is disabled. In this case the linker would miss the function referenced
> in the backlight header.
> 
> Therefore use the stub function also when the backlight class is disabled.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>

of_find_backlight_by_node()is defined at ./drivers/video/backlight/backlight.c
file. Also, this file can be built when CONFIG_BACKLIGHT_CLASS_DEVICE=y.
So, in order to prevent the possible build error, this patch looks good.

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  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 adb14a8..d9cb644 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -157,7 +157,7 @@ struct generic_bl_info {
>  	void (*kick_battery)(void);
>  };
> 
> -#ifdef CONFIG_OF
> +#if defined(CONFIG_OF) && defined(CONFIG_BACKLIGHT_CLASS_DEVICE)
>  struct backlight_device *of_find_backlight_by_node(struct device_node *node);
>  #else
>  static inline struct backlight_device *
> --
> 2.0.1

Re: [PATCH] backlight: use of_find_backlight_by_node stub when backlight class disabled

[Lee Jones]

On Wed, 29 Oct 2014, Heiko Stübner wrote:

> Drivers may want to search for an optional backlight even when the backlight
> class is disabled. In this case the linker would miss the function referenced
> in the backlight header.
> 
> Therefore use the stub function also when the backlight class is disabled.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  include/linux/backlight.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied to Backlight -next with Jingoo's Ack.

> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index adb14a8..d9cb644 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -157,7 +157,7 @@ struct generic_bl_info {
>  	void (*kick_battery)(void);
>  };
>  
> -#ifdef CONFIG_OF
> +#if defined(CONFIG_OF) && defined(CONFIG_BACKLIGHT_CLASS_DEVICE)
>  struct backlight_device *of_find_backlight_by_node(struct device_node *node);
>  #else
>  static inline struct backlight_device *

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] backlight: use of_find_backlight_by_node stub when backlight class disabled

[Lee Jones]

On Mon, 03 Nov 2014, Lee Jones wrote:

> On Wed, 29 Oct 2014, Heiko Stübner wrote:
> 
> > Drivers may want to search for an optional backlight even when the backlight
> > class is disabled. In this case the linker would miss the function referenced
> > in the backlight header.
> > 
> > Therefore use the stub function also when the backlight class is disabled.
> > 
> > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > ---
> >  include/linux/backlight.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Applied to Backlight -next with Jingoo's Ack.

I've removed this patch, as it causes unexpected:

  Redefinition of of_find_backlight_by_node()

... error.

Bear in mind that defined(CONFIG_BACKLIGHT_CLASS_DEVICE) is false if
it's built in as a module.  Change to nested #ifdefs instead.

> > diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> > index adb14a8..d9cb644 100644
> > --- a/include/linux/backlight.h
> > +++ b/include/linux/backlight.h
> > @@ -157,7 +157,7 @@ struct generic_bl_info {
> >  	void (*kick_battery)(void);
> >  };
> >  
> > -#ifdef CONFIG_OF
> > +#if defined(CONFIG_OF) && defined(CONFIG_BACKLIGHT_CLASS_DEVICE)
> >  struct backlight_device *of_find_backlight_by_node(struct device_node *node);
> >  #else
> >  static inline struct backlight_device *
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] backlight: use of_find_backlight_by_node stub when backlight class disabled

[Jingoo Han]

On Tuesday, November 04, 2014 6:08 PM, Heiko Stübner wrote:
> On Mon, 03 Nov 2014, Lee Jones wrote:
> 
> > On Wed, 29 Oct 2014, Heiko Stübner wrote:
> >
> > > Drivers may want to search for an optional backlight even when the backlight
> > > class is disabled. In this case the linker would miss the function referenced
> > > in the backlight header.
> > >
> > > Therefore use the stub function also when the backlight class is disabled.
> > >
> > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > ---
> > >  include/linux/backlight.h | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Applied to Backlight -next with Jingoo's Ack.
> 
> I've removed this patch, as it causes unexpected:
> 
>   Redefinition of of_find_backlight_by_node()

I reproduced the same build error.

Then, how about folding the following two patches into
one single patch? These two patches were already sent by Heiko Stübner.

  [PATCH] backlight: use of_find_backlight_by_node stub when backlight class disabled
  [PATCH] backlight: extend of_find_backlight_by_node stub-check to modules

Then, the one single patch will do as follows.

-#ifdef CONFIG_OF
+#if defined(CONFIG_OF) && (defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || \
+			   defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))

In this case, I cannot find any build errors.
Thank you.

Best regards,
Jingoo Han

> 
> ... error.
> 
> Bear in mind that defined(CONFIG_BACKLIGHT_CLASS_DEVICE) is false if
> it's built in as a module.  Change to nested #ifdefs instead.
> 
> > > diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> > > index adb14a8..d9cb644 100644
> > > --- a/include/linux/backlight.h
> > > +++ b/include/linux/backlight.h
> > > @@ -157,7 +157,7 @@ struct generic_bl_info {
> > >  	void (*kick_battery)(void);
> > >  };
> > >
> > > -#ifdef CONFIG_OF
> > > +#if defined(CONFIG_OF) && defined(CONFIG_BACKLIGHT_CLASS_DEVICE)
> > >  struct backlight_device *of_find_backlight_by_node(struct device_node *node);
> > >  #else
> > >  static inline struct backlight_device *
> >
> 
> --
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] backlight: use of_find_backlight_by_node stub when backlight class disabled

[Lee Jones]

On Tue, 04 Nov 2014, Jingoo Han wrote:

> On Tuesday, November 04, 2014 6:08 PM, Heiko Stübner wrote:
> > On Mon, 03 Nov 2014, Lee Jones wrote:
> > 
> > > On Wed, 29 Oct 2014, Heiko Stübner wrote:
> > >
> > > > Drivers may want to search for an optional backlight even when the backlight
> > > > class is disabled. In this case the linker would miss the function referenced
> > > > in the backlight header.
> > > >
> > > > Therefore use the stub function also when the backlight class is disabled.
> > > >
> > > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > > ---
> > > >  include/linux/backlight.h | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > Applied to Backlight -next with Jingoo's Ack.
> > 
> > I've removed this patch, as it causes unexpected:
> > 
> >   Redefinition of of_find_backlight_by_node()
> 
> I reproduced the same build error.
> 
> Then, how about folding the following two patches into
> one single patch? These two patches were already sent by Heiko Stübner.
> 
>   [PATCH] backlight: use of_find_backlight_by_node stub when backlight class disabled
>   [PATCH] backlight: extend of_find_backlight_by_node stub-check to modules
> 
> Then, the one single patch will do as follows.
> 
> -#ifdef CONFIG_OF
> +#if defined(CONFIG_OF) && (defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || \
> +			   defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
> 
> In this case, I cannot find any build errors.

That's a neat trick.  I didn't know you could do that.

However, it's bit messy consider different formatting, or a nested
#ifdef instead please.

> > ... error.
> > 
> > Bear in mind that defined(CONFIG_BACKLIGHT_CLASS_DEVICE) is false if
> > it's built in as a module.  Change to nested #ifdefs instead.
> > 
> > > > diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> > > > index adb14a8..d9cb644 100644
> > > > --- a/include/linux/backlight.h
> > > > +++ b/include/linux/backlight.h
> > > > @@ -157,7 +157,7 @@ struct generic_bl_info {
> > > >  	void (*kick_battery)(void);
> > > >  };
> > > >
> > > > -#ifdef CONFIG_OF
> > > > +#if defined(CONFIG_OF) && defined(CONFIG_BACKLIGHT_CLASS_DEVICE)
> > > >  struct backlight_device *of_find_backlight_by_node(struct device_node *node);
> > > >  #else
> > > >  static inline struct backlight_device *
> > >
> > 
> 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] backlight: use of_find_backlight_by_node stub when backlight class disabled

[Heiko Stübner]

Am Dienstag, 4. November 2014, 14:42:20 schrieb Lee Jones:
> On Tue, 04 Nov 2014, Jingoo Han wrote:
> > On Tuesday, November 04, 2014 6:08 PM, Heiko Stübner wrote:
> > > On Mon, 03 Nov 2014, Lee Jones wrote:
> > > > On Wed, 29 Oct 2014, Heiko Stübner wrote:
> > > > > Drivers may want to search for an optional backlight even when the
> > > > > backlight class is disabled. In this case the linker would miss the
> > > > > function referenced in the backlight header.
> > > > > 
> > > > > Therefore use the stub function also when the backlight class is
> > > > > disabled.
> > > > > 
> > > > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > > > ---
> > > > > 
> > > > >  include/linux/backlight.h | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > Applied to Backlight -next with Jingoo's Ack.
> > > 
> > > I've removed this patch, as it causes unexpected:
> > >   Redefinition of of_find_backlight_by_node()
> > 
> > I reproduced the same build error.
> > 
> > Then, how about folding the following two patches into
> > one single patch? These two patches were already sent by Heiko Stübner.
> > 
> >   [PATCH] backlight: use of_find_backlight_by_node stub when backlight
> >   class disabled [PATCH] backlight: extend of_find_backlight_by_node
> >   stub-check to modules> 
> > Then, the one single patch will do as follows.
> > 
> > -#ifdef CONFIG_OF
> > +#if defined(CONFIG_OF) && (defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || \
> > +			   defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
> > 
> > In this case, I cannot find any build errors.
> 
> That's a neat trick.  I didn't know you could do that.
> 
> However, it's bit messy consider different formatting, or a nested
> #ifdef instead please.

I guess it is a matter of me "not seeing the forrest for the trees", but how
would a nested ifdef look like, as this would result in 3 possible results
when for CONFIG_OF first and then for one of the BACKLIGHT_CLASS defines?


Formatting wise, when applied both defined(CONFIG_BACKLIGHT_foo) parts
are exactly below each other, making it (hopefully) clear where the "or" is
part of. What would look better?


Thanks
Heiko

Re: [PATCH] backlight: use of_find_backlight_by_node stub when backlight class disabled

[Lee Jones]

On Tue, 04 Nov 2014, Heiko Stübner wrote:

> Am Dienstag, 4. November 2014, 14:42:20 schrieb Lee Jones:
> > On Tue, 04 Nov 2014, Jingoo Han wrote:
> > > On Tuesday, November 04, 2014 6:08 PM, Heiko Stübner wrote:
> > > > On Mon, 03 Nov 2014, Lee Jones wrote:
> > > > > On Wed, 29 Oct 2014, Heiko Stübner wrote:
> > > > > > Drivers may want to search for an optional backlight even when the
> > > > > > backlight class is disabled. In this case the linker would miss the
> > > > > > function referenced in the backlight header.
> > > > > > 
> > > > > > Therefore use the stub function also when the backlight class is
> > > > > > disabled.
> > > > > > 
> > > > > > Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> > > > > > ---
> > > > > > 
> > > > > >  include/linux/backlight.h | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > 
> > > > > Applied to Backlight -next with Jingoo's Ack.
> > > > 
> > > > I've removed this patch, as it causes unexpected:
> > > >   Redefinition of of_find_backlight_by_node()
> > > 
> > > I reproduced the same build error.
> > > 
> > > Then, how about folding the following two patches into
> > > one single patch? These two patches were already sent by Heiko Stübner.
> > > 
> > >   [PATCH] backlight: use of_find_backlight_by_node stub when backlight
> > >   class disabled [PATCH] backlight: extend of_find_backlight_by_node
> > >   stub-check to modules> 
> > > Then, the one single patch will do as follows.
> > > 
> > > -#ifdef CONFIG_OF
> > > +#if defined(CONFIG_OF) && (defined(CONFIG_BACKLIGHT_CLASS_DEVICE) || \
> > > +			   defined(CONFIG_BACKLIGHT_CLASS_DEVICE_MODULE))
> > > 
> > > In this case, I cannot find any build errors.
> > 
> > That's a neat trick.  I didn't know you could do that.
> > 
> > However, it's bit messy consider different formatting, or a nested
> > #ifdef instead please.
> 
> I guess it is a matter of me "not seeing the forrest for the trees", but how
> would a nested ifdef look like, as this would result in 3 possible results
> when for CONFIG_OF first and then for one of the BACKLIGHT_CLASS defines?
> 
> Formatting wise, when applied both defined(CONFIG_BACKLIGHT_foo) parts
> are exactly below each other, making it (hopefully) clear where the "or" is
> part of. What would look better?

Actually there is a better way still:

#ifdef CONFIG_OF && IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
struct backlight_device *of_find_backlight_by_node(struct device_node *node);
#else
static inline struct backlight_device *
of_find_backlight_by_node(struct device_node *node)
{
      return NULL;
}
#endif


-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] backlight: use of_find_backlight_by_node stub when backlight class disabled

[Heiko Stübner]

Hi,

Am Dienstag, 4. November 2014, 17:15:32 schrieb Lee Jones:
> Actually there is a better way still:
> 
> #ifdef CONFIG_OF && IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> struct backlight_device *of_find_backlight_by_node(struct device_node
> *node); #else
> static inline struct backlight_device *
> of_find_backlight_by_node(struct device_node *node)
> {
>       return NULL;
> }
> #endif

After further looking at the problem, I'm actually not even sure, if my 
approach is the best one at all.

The problem I was trying to fix was panel-simple (drivers/gpu/drm/panel/panel-
simple.c) checking for an optional backlight, while not depending on the 
backlight-class itself.

As both components do not have a relationship, there exist the possibility of 
panel-simply being build into the kernel while the backlight_class is build as 
module. So while the IS_ENABLED check would define the prototype, panel-generic 
would still miss the function when linking.

Should panel-generic simply depend in the backlight_class instead?


Thanks
Heiko