On 25/10/17 08:39, Sean Paul wrote:
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index b88fabb..f98b684 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -159,6 +159,16 @@ static inline int backlight_disable(struct backlight_device *bd)
return backlight_update_status(bd);
}
+/**
+ * backlight_put - Drop backlight reference
+ * @bd: the backlight device to put
+ */
+static inline void backlight_put(struct backlight_device *bd)
+{
+ if (bd)
+ put_device(&bd->dev);
+}
As I mentioned in my last review, I don't think we need this function.
Just to check... some DRM drivers do allow themselves to work if
CONFIG_BACKLIGHT_CLASS_DEVICE isn't enabled (as discussed earlier in the
thread ;-). Did you consider what happens when this happens.
Wrapped up with backlight_put() we are sure never to accidentally
dereference bd in DRM de-init paths when CONFIG_BACKLIGHT_CLASS_DEVICVE if
it is not set. With a raw put_device() all that conditional logic ends up on
the driver.
Thanks for pointing that out, definitely a fair point.
I don't really mind putting the NULL check in the driver code. The return
values of of_find_backlight are well documented, so it shouldn't be too much
of a problem.
That said, this is your rodeo, so if you prefer to supply the put helper, that's
A-Ok with me.
Well... I don't want to add something with no callers... but if
something else in the patch set (or near future) uses it then I'm
certainly happy to have it!
Daniel.
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
