Re: [PATCH v6 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Sep 30, 2017 at 09:04:57PM +0200, Noralf Trønnes wrote:
> 
> Den 30.09.2017 19.12, skrev Meghana Madhyastha:
> >Rename tinydrm_of_find_backlight to drm_of_find_backlight
> >and move it into drm_of.c from tinydrm-helpers.c. This is
> >because other drivers in the drm subsystem might need to call
> >this function. In that case and otherwise, it is better from
> >an organizational point of view to move it into drm_of.c along
> >with the other _of.c functions.
> >
> >Signed-off-by: Meghana Madhyastha <meghana.madhyastha@xxxxxxxxx>
> >---
> >Changes in v6:
> >-Move function declarations to linux/backlight.h to resolve
> >  module dependency issues.
> >
> >  drivers/gpu/drm/drm_of.c                       | 44 ++++++++++++++++++++++++++
> >  drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 -----------------------
> >  drivers/gpu/drm/tinydrm/mi0283qt.c             |  2 +-
> >  include/drm/tinydrm/tinydrm-helpers.h          |  1 -
> >  include/linux/backlight.h                      | 11 +++++++
> >  5 files changed, 56 insertions(+), 42 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
> >index 8dafbdf..d878d3a 100644
> >--- a/drivers/gpu/drm/drm_of.c
> >+++ b/drivers/gpu/drm/drm_of.c
> >@@ -1,6 +1,7 @@
> >  #include <linux/component.h>
> >  #include <linux/export.h>
> >  #include <linux/list.h>
> >+#include <linux/backlight.h>
> >  #include <linux/of_graph.h>
> >  #include <drm/drmP.h>
> >  #include <drm/drm_bridge.h>
> >@@ -260,3 +261,46 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
> >  	return ret;
> >  }
> >  EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge);
> >+
> >+/**
> >+ * drm_of_find_backlight - Find backlight device in device-tree
> >+ * @dev: Device
> >+ *
> >+ * This function looks for a DT node pointed to by a property named 'backlight'
> >+ * and uses of_find_backlight_by_node() to get the backlight device.
> >+ * Additionally if the brightness property is zero, it is set to
> >+ * max_brightness.
> >+ *
> >+ * Note: It is the responsibility of the caller to call put_device() when
> >+ * releasing the resource.
> >+ *
> >+ * 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 *drm_of_find_backlight(struct device *dev)
> >+{
> >+	struct backlight_device *backlight;
> >+	struct device_node *np;
> >+
> >+	np = of_parse_phandle(dev->of_node, "backlight", 0);
> >+	if (!np)
> >+		return NULL;
> >+
> >+	backlight = of_find_backlight_by_node(np);
> >+	of_node_put(np);
> >+
> >+	if (!backlight)
> >+		return ERR_PTR(-EPROBE_DEFER);
> >+
> >+	if (!backlight->props.brightness) {
> >+		backlight->props.brightness = backlight->props.max_brightness;
> >+		DRM_DEBUG_KMS("Backlight brightness set to %d\n",
> >+			      backlight->props.brightness);
> >+	}
> >+
> >+	return backlight;
> >+}
> >+EXPORT_SYMBOL(drm_of_find_backlight);
> >diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> >index bd6cce0..cd4c6a5 100644
> >--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> >+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
> >@@ -237,46 +237,6 @@ void tinydrm_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
> >  EXPORT_SYMBOL(tinydrm_xrgb8888_to_gray8);
> >  /**
> >- * tinydrm_of_find_backlight - Find backlight device in device-tree
> >- * @dev: Device
> >- *
> >- * This function looks for a DT node pointed to by a property named 'backlight'
> >- * and uses of_find_backlight_by_node() to get the backlight device.
> >- * Additionally if the brightness property is zero, it is set to
> >- * max_brightness.
> >- *
> >- * 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 *tinydrm_of_find_backlight(struct device *dev)
> >-{
> >-	struct backlight_device *backlight;
> >-	struct device_node *np;
> >-
> >-	np = of_parse_phandle(dev->of_node, "backlight", 0);
> >-	if (!np)
> >-		return NULL;
> >-
> >-	backlight = of_find_backlight_by_node(np);
> >-	of_node_put(np);
> >-
> >-	if (!backlight)
> >-		return ERR_PTR(-EPROBE_DEFER);
> >-
> >-	if (!backlight->props.brightness) {
> >-		backlight->props.brightness = backlight->props.max_brightness;
> >-		DRM_DEBUG_KMS("Backlight brightness set to %d\n",
> >-			      backlight->props.brightness);
> >-	}
> >-
> >-	return backlight;
> >-}
> >-EXPORT_SYMBOL(tinydrm_of_find_backlight);
> >-
> >-/**
> >   * tinydrm_enable_backlight - Enable backlight helper
> >   * @backlight: Backlight device
> >   *
> >diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
> >index 7e5bb7d..682b4db 100644
> >--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
> >+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
> >@@ -189,7 +189,7 @@ static int mi0283qt_probe(struct spi_device *spi)
> >  	if (IS_ERR(mipi->regulator))
> >  		return PTR_ERR(mipi->regulator);
> >-	mipi->backlight = tinydrm_of_find_backlight(dev);
> >+	mipi->backlight = drm_of_find_backlight(dev);
> >  	if (IS_ERR(mipi->backlight))
> >  		return PTR_ERR(mipi->backlight);
> >diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h
> >index d554ded..e40ef2d 100644
> >--- a/include/drm/tinydrm/tinydrm-helpers.h
> >+++ b/include/drm/tinydrm/tinydrm-helpers.h
> >@@ -46,7 +46,6 @@ void tinydrm_xrgb8888_to_rgb565(u16 *dst, void *vaddr,
> >  void tinydrm_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
> >  			       struct drm_clip_rect *clip);
> >-struct backlight_device *tinydrm_of_find_backlight(struct device *dev);
> >  int tinydrm_enable_backlight(struct backlight_device *backlight);
> >  int tinydrm_disable_backlight(struct backlight_device *backlight);
> >diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> >index 5f2fd61..47a095e 100644
> >--- a/include/linux/backlight.h
> >+++ b/include/linux/backlight.h
> >@@ -172,4 +172,15 @@ of_find_backlight_by_node(struct device_node *node)
> >  }
> >  #endif
> >+#if defined(CONFIG_OF) && IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> >+struct backlight_device *
> >+drm_of_find_backlight(struct device *dev);
> >+#else
> >+static inline struct backlight_device *
> >+drm_of_find_backlight(struct device *dev)
> >+{
> >+	return NULL;
> >+}
> >+#endif
> >+
> >  #endif
> 
> This isn't what I meant, you only change the of_find_backlight_by_node()
> declaration/definition. It should be like this:
> 
> #if defined(CONFIG_OF) && IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
> struct backlight_device *of_find_backlight_by_node(struct device_node
> *node);
> #else
> static inline struct backlight_device *
> of_find_backlight_by_node(struct device_node *node)
> {
>     return NULL;
> }
> #endif
I see. So basically add the function declaration of drm_of_find_backlight
and devm_drm_of_find_backlight in drm_of.h and this function declaration 
need not be inside the #ifdef CONFIG / #else part? 
So there need not be a dummy version of drm_of_find_backlight here
because we are anyway taking care of that in of_find_backliht by node?
Sorry for the many versions and thank you for the clarifications.

Regards,
Meghana
> And this change needs to be a patch on it's own. The backlight subsystem
> has it's own maintainer that will review this change.
> It's best to send him/them the whole patchset so they see the context.
> 
> Noralf.
> 
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux