Re: [PATCH] backlight: Add of_find_backlight_by_node() function
From: Thierry Reding <hidden>
Date: 2012-11-16 21:34:28
Also in:
linux-devicetree, lkml
On Fri, Nov 16, 2012 at 01:16:45PM -0800, Andrew Morton wrote:
quoted hunk ↗ jump to hunk
On Fri, 9 Nov 2012 15:04:38 +0100 Thierry Reding [off-list ref] wrote:quoted
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 <redacted> --- 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.
quoted
+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
Attachments
- (unnamed) [application/pgp-signature] 836 bytes