On Fri, Nov 16, 2012 at 01:16:45PM -0800, Andrew Morton wrote: > On Fri, 9 Nov 2012 15:04:38 +0100 > Thierry Reding <[email protected]> wrote: > > > This function finds the struct backlight_device for a given device tree > > node. A dummy function is provided so that it safely compiles out if OF > > support is disabled. > > > > Signed-off-by: Thierry Reding <[email protected]> > > --- > > drivers/video/backlight/backlight.c | 17 +++++++++++++++++ > > include/linux/backlight.h | 10 ++++++++++ > > 2 files changed, 27 insertions(+) > > > > diff --git a/drivers/video/backlight/backlight.c > > b/drivers/video/backlight/backlight.c > > index 297db2f..0d1ed4f 100644 > > --- a/drivers/video/backlight/backlight.c > > +++ b/drivers/video/backlight/backlight.c > > @@ -370,6 +370,23 @@ void backlight_device_unregister(struct > > backlight_device *bd) > > } > > EXPORT_SYMBOL(backlight_device_unregister); > > > > +#if IS_ENABLED(CONFIG_OF) > > Using IS_ENABLED() was odd. We'll never support CONFIG_OF=m, so can't > we use plain old "#ifdef CONFIG_OF" here? > > --- > a/drivers/video/backlight/backlight.c~backlight-add-of_find_backlight_by_node-function-fix > +++ a/drivers/video/backlight/backlight.c > @@ -370,7 +370,7 @@ void backlight_device_unregister(struct > } > EXPORT_SYMBOL(backlight_device_unregister); > > -#if IS_ENABLED(CONFIG_OF) > +#ifdef CONFIG_OF > static int of_parent_match(struct device *dev, void *data) > { > return dev->parent && dev->parent->of_node == data; > --- > a/include/linux/backlight.h~backlight-add-of_find_backlight_by_node-function-fix > +++ a/include/linux/backlight.h > @@ -134,7 +134,7 @@ struct generic_bl_info { > void (*kick_battery)(void); > }; > > -#if IS_ENABLED(CONFIG_OF) > +#ifdef CONFIG_OF > struct backlight_device *of_find_backlight_by_node(struct device_node *node); > #else > static inline struct backlight_device * > _
Yes, that should work.
> > +static int of_parent_match(struct device *dev, void *data)
> > +{
> > + return dev->parent && dev->parent->of_node == data;
> > +}
> > +
> > +struct backlight_device *of_find_backlight_by_node(struct device_node
> > *node)
> > +{
> > + struct device *dev;
> > +
> > + dev = class_find_device(backlight_class, NULL, node, of_parent_match);
> > +
> > + return dev ? to_backlight_device(dev) : NULL;
> > +}
> > +EXPORT_SYMBOL(of_find_backlight_by_node);
>
> It's a global, exported-to-modules function. We should document such
> major interfaces. Unless they are dead trivial, but I don't think this
> one is that simple. The semantics of the return value could be
> explained, and callers should be told that of_find_backlight_by_node()
> took a ref on the returned device, and that they need to run
> put_device(retval->dev), if retval was not NULL.
>
> And anything else which might be useful.
Yes, I should have thought about that. I'll add some proper kerneldoc
and repost.
Thanks for reviewing,
Thierry
pgpBVblqXYwNf.pgp
Description: PGP signature
