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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to