[PATCH v3 2/2] drm/tinydrm: Add devres versions of drm_of_find_backlight

[Meghana Madhyastha]

On Thu, Sep 28, 2017 at 06:19:35PM +0200, Noralf Trønnes wrote:
> 
> Den 28.09.2017 11.15, skrev Meghana Madhyastha:
> >Add devm_drm_of_find_backlight and the corresponding release
> >function because some drivers such as tinydrm use devres versions
> >of functions for requiring device resources.
> >
> >Signed-off-by: Meghana Madhyastha <meghana.madhyastha at gmail.com>
> >---
> >Changes in v3:
> >-None
> >
> >  drivers/gpu/drm/drm_of.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/drm/drm_of.h     |  2 ++
> >  2 files changed, 50 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> >index d878d3a..238e8e5 100644
> >--- a/drivers/gpu/drm/drm_of.c
> >+++ b/drivers/gpu/drm/drm_of.c
> >@@ -304,3 +304,51 @@ struct backlight_device *drm_of_find_backlight(struct device *dev)
> >  	return backlight;
> >  }
> >  EXPORT_SYMBOL(drm_of_find_backlight);
> >+
> >+/**
> >+ * devm_drm_of_find_backlight_release - Release backlight device
> >+ * @dev: Device
> >+ *
> >+ * This is the release function corresponding to the devm_drm_of_find_backlight.
> >+ * Each devres entry is associated with a release function.
> >+ */
> 
> This is an internal function so no need for docs or exporting. I know
> that some devm_ functions have explicit release functions, but I don't
> think this is necessary since those users can just use
> drm_of_find_backlight() directly instead.

I have a question here. devm_drm_of_find_backlight_release is a
wrapper around put_device which is passed as a parameter to
devm_add_action in devm_drm_of_find_backlight. So isn't the function
useful here ?

> >+static void devm_drm_of_find_backlight_release(void *data)
> >+{
> >+	put_device(data);
> >+}
> >+EXPORT_SYMBOL(devm_drm_of_find_backlight_release);
> >+
> >+/**
> >+ * devm_drm_of_find_backlight - Find backlight device in device-tree
> >+ * devres version of the function
> >+ * @dev: Device
> >+ *
> >+ * This is the devres version of the function drm_of_find_backlight.
> >+ * Some drivers such as tinydrm use devres versions of functions for
> 
> No need to mention tinydrm here.
> 
> >+ * requiring device resources.
> >+ *
> >+ * Returns:
> >+ * NULL if there's no backlight property.
> >+ * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device
> >+ * is found.
> >+ * If the backlight device is found, a pointer to the structure is returned.
> >+ */
> >+struct backlight_device *devm_drm_of_find_backlight(struct device *dev)
> >+{
> >+	struct backlight_device *backlight;
> >+	int ret;
> >+
> >+	backlight = drm_of_find_backlight(dev);
> >+	if (IS_ERR_OR_NULL(backlight))
> >+		return backlight;
> >+
> >+	ret = devm_add_action(dev, devm_drm_of_find_backlight_release,
> >+			      &backlight->dev);
> >+	if (ret) {
> >+		put_device(&backlight->dev);
> >+		return ERR_PTR(ret);
> >+	}
> >+
> >+	return backlight;
> >+}
> >+EXPORT_SYMBOL(devm_drm_of_find_backlight);
> >diff --git a/include/drm/drm_of.h b/include/drm/drm_of.h
> >index e8fba5b..071fb3b 100644
> >--- a/include/drm/drm_of.h
> >+++ b/include/drm/drm_of.h
> >@@ -30,7 +30,9 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
> >  				struct drm_panel **panel,
> >  				struct drm_bridge **bridge);
> >  struct backlight_device *drm_of_find_backlight(struct device *dev);
> >+struct backlight_device *devm_drm_of_find_backlight(struct device *dev);
> >  #else
> 
> We need a dummy version of devm_drm_of_find_backlight() here that
> returns NULL as in the previous patch.
> 
> >+static void devm_drm_of_find_backlight_release(void *data);
> 
> And this isn't needed as explained above.
> >  static inline uint32_t drm_of_find_possible_crtcs(struct drm_device *dev,
> >  						  struct device_node *port)
> >  {
> 
> I'd appreciate if you could also switch mi0283qt over to this helper :-)
> 
> Noralf.
> 
>