[PATCH v6 0/2] drm/tinydrm: drm_of_find_backlight helper

[PATCH v6 0/2] drm/tinydrm: drm_of_find_backlight helper

[Meghana Madhyastha]

Move tinydrm_of_find_backlight to drm_of.c and rename it to
drm_of_find_backlight for better organizational structure. 

Changes in v6:
-Move function declarations to linux/backlight.h to prevent
 build errors for the dependencies issue.
-Remove docs for devm_drm_of_find_backlight_release as it is
 an internal function.

Meghana Madhyastha (2):
  drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c
  drm/tinydrm: Add devres versions of drm_of_find_backlight

 drivers/gpu/drm/drm_of.c                       | 84 ++++++++++++++++++++++++++
 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                      | 18 ++++++
 5 files changed, 103 insertions(+), 42 deletions(-)

-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v6 0/2] drm/tinydrm: drm_of_find_backlight helper

[Meghana Madhyastha]

Move tinydrm_of_find_backlight to drm_of.c and rename it to
drm_of_find_backlight for better organizational structure. 

Changes in v6:
-Move function declarations to linux/backlight.h to prevent
 build errors for the dependencies issue.
-Remove docs for devm_drm_of_find_backlight_release as it is
 an internal function.

Meghana Madhyastha (2):
  drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c
  drm/tinydrm: Add devres versions of drm_of_find_backlight

 drivers/gpu/drm/drm_of.c                       | 84 ++++++++++++++++++++++++++
 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                      | 18 ++++++
 5 files changed, 103 insertions(+), 42 deletions(-)

-- 
2.7.4

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

[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@gmail.com>
---
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
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[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@gmail.com>
---
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
-- 
2.7.4

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

[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@gmail.com>
---
Changes in v6:
- Remove docstring for devm_drm_of_backlight_release because it is
 and internal function.

 drivers/gpu/drm/drm_of.c           | 40 ++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/tinydrm/mi0283qt.c |  2 +-
 include/linux/backlight.h          |  7 +++++++
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index d878d3a..6fb3fe5 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -304,3 +304,43 @@ struct backlight_device *drm_of_find_backlight(struct device *dev)
 	return backlight;
 }
 EXPORT_SYMBOL(drm_of_find_backlight);
+
+static void devm_drm_of_find_backlight_release(void *data)
+{
+	put_device(data);
+}
+
+/**
+ * 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 use devres versions of functions for
+ * 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/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 682b4db..6724e5e 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 = drm_of_find_backlight(dev);
+	mipi->backlight = devm_drm_of_find_backlight(dev);
 	if (IS_ERR(mipi->backlight))
 		return PTR_ERR(mipi->backlight);
 
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 47a095e..f601f15 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -175,12 +175,19 @@ of_find_backlight_by_node(struct device_node *node)
 #if defined(CONFIG_OF) && IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
 struct backlight_device *
 drm_of_find_backlight(struct device *dev);
+devm_drm_of_find_backlight(struct device *dev);
 #else
 static inline struct backlight_device *
 drm_of_find_backlight(struct device *dev)
 {
 	return NULL;
 }
+
+static inline struct backlight_device *
+devm_drm_of_find_backlight(struct device *dev)
+{
+	return NULL;
+}
 #endif
 
 #endif
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[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@gmail.com>
---
Changes in v6:
- Remove docstring for devm_drm_of_backlight_release because it is
 and internal function.

 drivers/gpu/drm/drm_of.c           | 40 ++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/tinydrm/mi0283qt.c |  2 +-
 include/linux/backlight.h          |  7 +++++++
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index d878d3a..6fb3fe5 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -304,3 +304,43 @@ struct backlight_device *drm_of_find_backlight(struct device *dev)
 	return backlight;
 }
 EXPORT_SYMBOL(drm_of_find_backlight);
+
+static void devm_drm_of_find_backlight_release(void *data)
+{
+	put_device(data);
+}
+
+/**
+ * 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 use devres versions of functions for
+ * 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/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 682b4db..6724e5e 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 = drm_of_find_backlight(dev);
+	mipi->backlight = devm_drm_of_find_backlight(dev);
 	if (IS_ERR(mipi->backlight))
 		return PTR_ERR(mipi->backlight);
 
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 47a095e..f601f15 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -175,12 +175,19 @@ of_find_backlight_by_node(struct device_node *node)
 #if defined(CONFIG_OF) && IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
 struct backlight_device *
 drm_of_find_backlight(struct device *dev);
+devm_drm_of_find_backlight(struct device *dev);
 #else
 static inline struct backlight_device *
 drm_of_find_backlight(struct device *dev)
 {
 	return NULL;
 }
+
+static inline struct backlight_device *
+devm_drm_of_find_backlight(struct device *dev)
+{
+	return NULL;
+}
 #endif
 
 #endif
-- 
2.7.4

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

[Noralf Trønnes]

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@gmail.com>
> ---
> 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

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

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

[Noralf Trønnes]

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@gmail.com>
> ---
> 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

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.

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

[Meghana Madhyastha]

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

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

[Meghana Madhyastha]

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@gmail.com>
> >---
> >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.
> 

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

[Noralf Trønnes]

Den 01.10.2017 06.14, skrev Meghana Madhyastha:
> 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@gmail.com>
>>> ---
>>> 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.

You need the dummy part in drm_of.h because you need to handle the
CONFIG_OF option here as well.

If drm_of_find_backlight() was only 2-3 lines of code, you could have
made a static inline function in drm_of.h outside the #if/#else/#endif,
but since it's longer than that it should be in drm_of.c.

CONFIG_OF decides if drm_of.c is built (as part of drm.o) or not:

drivers/gpu/drm/Makefile:
drm-$(CONFIG_OF) += drm_of.o

This means we need a function prototype declaration for the CONFIG_OF=y
case, and an inline function definition for the CONFIG_OF=n case.

Noralf.

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

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

[Noralf Trønnes]

Den 01.10.2017 06.14, skrev Meghana Madhyastha:
> 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@gmail.com>
>>> ---
>>> 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.

You need the dummy part in drm_of.h because you need to handle the
CONFIG_OF option here as well.

If drm_of_find_backlight() was only 2-3 lines of code, you could have
made a static inline function in drm_of.h outside the #if/#else/#endif,
but since it's longer than that it should be in drm_of.c.

CONFIG_OF decides if drm_of.c is built (as part of drm.o) or not:

drivers/gpu/drm/Makefile:
drm-$(CONFIG_OF) += drm_of.o

This means we need a function prototype declaration for the CONFIG_OF=y
case, and an inline function definition for the CONFIG_OF=n case.

Noralf.

> 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.
>>

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

[Meghana Madhyastha]

On Sun, Oct 01, 2017 at 03:26:36PM +0200, Noralf Trønnes wrote:
> 
> Den 01.10.2017 06.14, skrev Meghana Madhyastha:
> >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@gmail.com>
> >>>---
> >>>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.
> 
> You need the dummy part in drm_of.h because you need to handle the
> CONFIG_OF option here as well.
> 
> If drm_of_find_backlight() was only 2-3 lines of code, you could have
> made a static inline function in drm_of.h outside the #if/#else/#endif,
> but since it's longer than that it should be in drm_of.c.
> 
> CONFIG_OF decides if drm_of.c is built (as part of drm.o) or not:
> 
> drivers/gpu/drm/Makefile:
> drm-$(CONFIG_OF) += drm_of.o
> 
> This means we need a function prototype declaration for the CONFIG_OF=y
> case, and an inline function definition for the CONFIG_OF=n case.
> 
> Noralf.

I have one more question. In that case, why wouldn't my previous patch
work ? (i.e  definition and declaration of drm_of_find_backlight
in linux/backlight.h with if/else directives)

Regards,
Meghana

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

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

[Meghana Madhyastha]

On Sun, Oct 01, 2017 at 03:26:36PM +0200, Noralf Tr�nnes wrote:
> 
> Den 01.10.2017 06.14, skrev Meghana Madhyastha:
> >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@gmail.com>
> >>>---
> >>>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.
> 
> You need the dummy part in drm_of.h because you need to handle the
> CONFIG_OF option here as well.
> 
> If drm_of_find_backlight() was only 2-3 lines of code, you could have
> made a static inline function in drm_of.h outside the #if/#else/#endif,
> but since it's longer than that it should be in drm_of.c.
> 
> CONFIG_OF decides if drm_of.c is built (as part of drm.o) or not:
> 
> drivers/gpu/drm/Makefile:
> drm-$(CONFIG_OF) += drm_of.o
> 
> This means we need a function prototype declaration for the CONFIG_OF=y
> case, and an inline function definition for the CONFIG_OF=n case.
> 
> Noralf.

I have one more question. In that case, why wouldn't my previous patch
work ? (i.e  definition and declaration of drm_of_find_backlight
in linux/backlight.h with if/else directives)

Regards,
Meghana

> >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.
> >>
> 

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

[Noralf Trønnes]

Den 01.10.2017 15.34, skrev Meghana Madhyastha:
> On Sun, Oct 01, 2017 at 03:26:36PM +0200, Noralf Trønnes wrote:
>> Den 01.10.2017 06.14, skrev Meghana Madhyastha:
>>> 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@gmail.com>
>>>>> ---
>>>>> 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.
>> You need the dummy part in drm_of.h because you need to handle the
>> CONFIG_OF option here as well.
>>
>> If drm_of_find_backlight() was only 2-3 lines of code, you could have
>> made a static inline function in drm_of.h outside the #if/#else/#endif,
>> but since it's longer than that it should be in drm_of.c.
>>
>> CONFIG_OF decides if drm_of.c is built (as part of drm.o) or not:
>>
>> drivers/gpu/drm/Makefile:
>> drm-$(CONFIG_OF) += drm_of.o
>>
>> This means we need a function prototype declaration for the CONFIG_OF=y
>> case, and an inline function definition for the CONFIG_OF=n case.
>>
>> Noralf.
> I have one more question. In that case, why wouldn't my previous patch
> work ? (i.e  definition and declaration of drm_of_find_backlight
> in linux/backlight.h with if/else directives)

It will work, the compiler doesn't complain as long as you include
backlight.h and make sure drm_of.c is built. But that would result
in a complexity that humans would struggle to get right. And in this
case it's even 2 different subsystems with different maintainers...

So in order to succeed with complex projects, humans need some more
rules than what the compiler requires. And one of those are:
   A header file is usually connected to a source file.

backlight.h contains prototypes and definitions for backlight.c
drm_of.h contains prototypes and definitions for drm_of.c


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

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

[Noralf Trønnes]

Den 01.10.2017 15.34, skrev Meghana Madhyastha:
> On Sun, Oct 01, 2017 at 03:26:36PM +0200, Noralf Trønnes wrote:
>> Den 01.10.2017 06.14, skrev Meghana Madhyastha:
>>> 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@gmail.com>
>>>>> ---
>>>>> 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.
>> You need the dummy part in drm_of.h because you need to handle the
>> CONFIG_OF option here as well.
>>
>> If drm_of_find_backlight() was only 2-3 lines of code, you could have
>> made a static inline function in drm_of.h outside the #if/#else/#endif,
>> but since it's longer than that it should be in drm_of.c.
>>
>> CONFIG_OF decides if drm_of.c is built (as part of drm.o) or not:
>>
>> drivers/gpu/drm/Makefile:
>> drm-$(CONFIG_OF) += drm_of.o
>>
>> This means we need a function prototype declaration for the CONFIG_OF=y
>> case, and an inline function definition for the CONFIG_OF=n case.
>>
>> Noralf.
> I have one more question. In that case, why wouldn't my previous patch
> work ? (i.e  definition and declaration of drm_of_find_backlight
> in linux/backlight.h with if/else directives)

It will work, the compiler doesn't complain as long as you include
backlight.h and make sure drm_of.c is built. But that would result
in a complexity that humans would struggle to get right. And in this
case it's even 2 different subsystems with different maintainers...

So in order to succeed with complex projects, humans need some more
rules than what the compiler requires. And one of those are:
   A header file is usually connected to a source file.

backlight.h contains prototypes and definitions for backlight.c
drm_of.h contains prototypes and definitions for drm_of.c


> Regards,
> Meghana
>
>>> 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.
>>>>

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

[Meghana Madhyastha]

On Sun, Oct 01, 2017 at 04:47:26PM +0200, Noralf Trønnes wrote:
> 
> Den 01.10.2017 15.34, skrev Meghana Madhyastha:
> >On Sun, Oct 01, 2017 at 03:26:36PM +0200, Noralf Trønnes wrote:
> >>Den 01.10.2017 06.14, skrev Meghana Madhyastha:
> >>>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@gmail.com>
> >>>>>---
> >>>>>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.
> >>You need the dummy part in drm_of.h because you need to handle the
> >>CONFIG_OF option here as well.
> >>
> >>If drm_of_find_backlight() was only 2-3 lines of code, you could have
> >>made a static inline function in drm_of.h outside the #if/#else/#endif,
> >>but since it's longer than that it should be in drm_of.c.
> >>
> >>CONFIG_OF decides if drm_of.c is built (as part of drm.o) or not:
> >>
> >>drivers/gpu/drm/Makefile:
> >>drm-$(CONFIG_OF) += drm_of.o
> >>
> >>This means we need a function prototype declaration for the CONFIG_OF=y
> >>case, and an inline function definition for the CONFIG_OF=n case.
> >>
> >>Noralf.
> >I have one more question. In that case, why wouldn't my previous patch
> >work ? (i.e  definition and declaration of drm_of_find_backlight
> >in linux/backlight.h with if/else directives)
> 
> It will work, the compiler doesn't complain as long as you include
> backlight.h and make sure drm_of.c is built. But that would result
> in a complexity that humans would struggle to get right. And in this
> case it's even 2 different subsystems with different maintainers...
> 
> So in order to succeed with complex projects, humans need some more
> rules than what the compiler requires. And one of those are:
>   A header file is usually connected to a source file.
> 
> backlight.h contains prototypes and definitions for backlight.c
> drm_of.h contains prototypes and definitions for drm_of.c

Okay that makes sense. Initially, I thought that we could add the
declarations/definitions along with of_find_backlight_by_node in
linux/backlight.h as I hadn't realized that each .c file should strictly
have a one to one correspondence with the .h file of the same name but
as you mentioned, it is necessary in a large/complex project like this.

Thank you for the clarifications !

Regards,
Meghana

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

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

[Meghana Madhyastha]

On Sun, Oct 01, 2017 at 04:47:26PM +0200, Noralf Tr�nnes wrote:
> 
> Den 01.10.2017 15.34, skrev Meghana Madhyastha:
> >On Sun, Oct 01, 2017 at 03:26:36PM +0200, Noralf Tr�nnes wrote:
> >>Den 01.10.2017 06.14, skrev Meghana Madhyastha:
> >>>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@gmail.com>
> >>>>>---
> >>>>>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.
> >>You need the dummy part in drm_of.h because you need to handle the
> >>CONFIG_OF option here as well.
> >>
> >>If drm_of_find_backlight() was only 2-3 lines of code, you could have
> >>made a static inline function in drm_of.h outside the #if/#else/#endif,
> >>but since it's longer than that it should be in drm_of.c.
> >>
> >>CONFIG_OF decides if drm_of.c is built (as part of drm.o) or not:
> >>
> >>drivers/gpu/drm/Makefile:
> >>drm-$(CONFIG_OF) += drm_of.o
> >>
> >>This means we need a function prototype declaration for the CONFIG_OF=y
> >>case, and an inline function definition for the CONFIG_OF=n case.
> >>
> >>Noralf.
> >I have one more question. In that case, why wouldn't my previous patch
> >work ? (i.e  definition and declaration of drm_of_find_backlight
> >in linux/backlight.h with if/else directives)
> 
> It will work, the compiler doesn't complain as long as you include
> backlight.h and make sure drm_of.c is built. But that would result
> in a complexity that humans would struggle to get right. And in this
> case it's even 2 different subsystems with different maintainers...
> 
> So in order to succeed with complex projects, humans need some more
> rules than what the compiler requires. And one of those are:
> � A header file is usually connected to a source file.
> 
> backlight.h contains prototypes and definitions for backlight.c
> drm_of.h contains prototypes and definitions for drm_of.c

Okay that makes sense. Initially, I thought that we could add the
declarations/definitions along with of_find_backlight_by_node in
linux/backlight.h as I hadn't realized that each .c file should strictly
have a one to one correspondence with the .h file of the same name but
as you mentioned, it is necessary in a large/complex project like this.

Thank you for the clarifications !

Regards,
Meghana

> 
> >Regards,
> >Meghana
> >
> >>>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.
> >>>>
> 

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2298 bytes --]

Hi Meghana,

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.14-rc3 next-20170929]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Meghana-Madhyastha/drm-tinydrm-drm_of_find_backlight-helper/20171003-011920
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: arm-sunxi_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/drm_of.c:283:26: error: redefinition of 'drm_of_find_backlight'
    struct backlight_device *drm_of_find_backlight(struct device *dev)
                             ^~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/drm_of.c:4:0:
   include/linux/backlight.h:180:1: note: previous definition of 'drm_of_find_backlight' was here
    drm_of_find_backlight(struct device *dev)
    ^~~~~~~~~~~~~~~~~~~~~

vim +/drm_of_find_backlight +283 drivers/gpu/drm/drm_of.c

   264	
   265	/**
   266	 * drm_of_find_backlight - Find backlight device in device-tree
   267	 * @dev: Device
   268	 *
   269	 * This function looks for a DT node pointed to by a property named 'backlight'
   270	 * and uses of_find_backlight_by_node() to get the backlight device.
   271	 * Additionally if the brightness property is zero, it is set to
   272	 * max_brightness.
   273	 *
   274	 * Note: It is the responsibility of the caller to call put_device() when
   275	 * releasing the resource.
   276	 *
   277	 * Returns:
   278	 * NULL if there's no backlight property.
   279	 * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device
   280	 * is found.
   281	 * If the backlight device is found, a pointer to the structure is returned.
   282	 */
 > 283	struct backlight_device *drm_of_find_backlight(struct device *dev)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23035 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2298 bytes --]

Hi Meghana,

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.14-rc3 next-20170929]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Meghana-Madhyastha/drm-tinydrm-drm_of_find_backlight-helper/20171003-011920
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: arm-sunxi_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/drm_of.c:283:26: error: redefinition of 'drm_of_find_backlight'
    struct backlight_device *drm_of_find_backlight(struct device *dev)
                             ^~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/drm_of.c:4:0:
   include/linux/backlight.h:180:1: note: previous definition of 'drm_of_find_backlight' was here
    drm_of_find_backlight(struct device *dev)
    ^~~~~~~~~~~~~~~~~~~~~

vim +/drm_of_find_backlight +283 drivers/gpu/drm/drm_of.c

   264	
   265	/**
   266	 * drm_of_find_backlight - Find backlight device in device-tree
   267	 * @dev: Device
   268	 *
   269	 * This function looks for a DT node pointed to by a property named 'backlight'
   270	 * and uses of_find_backlight_by_node() to get the backlight device.
   271	 * Additionally if the brightness property is zero, it is set to
   272	 * max_brightness.
   273	 *
   274	 * Note: It is the responsibility of the caller to call put_device() when
   275	 * releasing the resource.
   276	 *
   277	 * Returns:
   278	 * NULL if there's no backlight property.
   279	 * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device
   280	 * is found.
   281	 * If the backlight device is found, a pointer to the structure is returned.
   282	 */
 > 283	struct backlight_device *drm_of_find_backlight(struct device *dev)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23035 bytes --]

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4259 bytes --]

Hi Meghana,

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.14-rc3 next-20170929]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Meghana-Madhyastha/drm-tinydrm-drm_of_find_backlight-helper/20171003-011920
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: arm-sunxi_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/drm_of.c:283:26: error: redefinition of 'drm_of_find_backlight'
    struct backlight_device *drm_of_find_backlight(struct device *dev)
                             ^~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/drm_of.c:4:0:
   include/linux/backlight.h:181:1: note: previous definition of 'drm_of_find_backlight' was here
    drm_of_find_backlight(struct device *dev)
    ^~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/drm_of.c:328:26: error: redefinition of 'devm_drm_of_find_backlight'
    struct backlight_device *devm_drm_of_find_backlight(struct device *dev)
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/drm_of.c:4:0:
   include/linux/backlight.h:187:1: note: previous definition of 'devm_drm_of_find_backlight' was here
    devm_drm_of_find_backlight(struct device *dev)
    ^~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/devm_drm_of_find_backlight +328 drivers/gpu/drm/drm_of.c

   264	
   265	/**
   266	 * drm_of_find_backlight - Find backlight device in device-tree
   267	 * @dev: Device
   268	 *
   269	 * This function looks for a DT node pointed to by a property named 'backlight'
   270	 * and uses of_find_backlight_by_node() to get the backlight device.
   271	 * Additionally if the brightness property is zero, it is set to
   272	 * max_brightness.
   273	 *
   274	 * Note: It is the responsibility of the caller to call put_device() when
   275	 * releasing the resource.
   276	 *
   277	 * Returns:
   278	 * NULL if there's no backlight property.
   279	 * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device
   280	 * is found.
   281	 * If the backlight device is found, a pointer to the structure is returned.
   282	 */
 > 283	struct backlight_device *drm_of_find_backlight(struct device *dev)
   284	{
   285		struct backlight_device *backlight;
   286		struct device_node *np;
   287	
   288		np = of_parse_phandle(dev->of_node, "backlight", 0);
   289		if (!np)
   290			return NULL;
   291	
   292		backlight = of_find_backlight_by_node(np);
   293		of_node_put(np);
   294	
   295		if (!backlight)
   296			return ERR_PTR(-EPROBE_DEFER);
   297	
   298		if (!backlight->props.brightness) {
   299			backlight->props.brightness = backlight->props.max_brightness;
   300			DRM_DEBUG_KMS("Backlight brightness set to %d\n",
   301				      backlight->props.brightness);
   302		}
   303	
   304		return backlight;
   305	}
   306	EXPORT_SYMBOL(drm_of_find_backlight);
   307	
   308	static void devm_drm_of_find_backlight_release(void *data)
   309	{
   310		put_device(data);
   311	}
   312	
   313	/**
   314	 * devm_drm_of_find_backlight - Find backlight device in device-tree
   315	 * devres version of the function
   316	 * @dev: Device
   317	 *
   318	 * This is the devres version of the function drm_of_find_backlight.
   319	 * Some drivers use devres versions of functions for
   320	 * requiring device resources.
   321	 *
   322	 * Returns:
   323	 * NULL if there's no backlight property.
   324	 * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device
   325	 * is found.
   326	 * If the backlight device is found, a pointer to the structure is returned.
   327	 */
 > 328	struct backlight_device *devm_drm_of_find_backlight(struct device *dev)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23035 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4259 bytes --]

Hi Meghana,

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.14-rc3 next-20170929]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Meghana-Madhyastha/drm-tinydrm-drm_of_find_backlight-helper/20171003-011920
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: arm-sunxi_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

   drivers/gpu/drm/drm_of.c:283:26: error: redefinition of 'drm_of_find_backlight'
    struct backlight_device *drm_of_find_backlight(struct device *dev)
                             ^~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/drm_of.c:4:0:
   include/linux/backlight.h:181:1: note: previous definition of 'drm_of_find_backlight' was here
    drm_of_find_backlight(struct device *dev)
    ^~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/drm_of.c:328:26: error: redefinition of 'devm_drm_of_find_backlight'
    struct backlight_device *devm_drm_of_find_backlight(struct device *dev)
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/gpu/drm/drm_of.c:4:0:
   include/linux/backlight.h:187:1: note: previous definition of 'devm_drm_of_find_backlight' was here
    devm_drm_of_find_backlight(struct device *dev)
    ^~~~~~~~~~~~~~~~~~~~~~~~~~

vim +/devm_drm_of_find_backlight +328 drivers/gpu/drm/drm_of.c

   264	
   265	/**
   266	 * drm_of_find_backlight - Find backlight device in device-tree
   267	 * @dev: Device
   268	 *
   269	 * This function looks for a DT node pointed to by a property named 'backlight'
   270	 * and uses of_find_backlight_by_node() to get the backlight device.
   271	 * Additionally if the brightness property is zero, it is set to
   272	 * max_brightness.
   273	 *
   274	 * Note: It is the responsibility of the caller to call put_device() when
   275	 * releasing the resource.
   276	 *
   277	 * Returns:
   278	 * NULL if there's no backlight property.
   279	 * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device
   280	 * is found.
   281	 * If the backlight device is found, a pointer to the structure is returned.
   282	 */
 > 283	struct backlight_device *drm_of_find_backlight(struct device *dev)
   284	{
   285		struct backlight_device *backlight;
   286		struct device_node *np;
   287	
   288		np = of_parse_phandle(dev->of_node, "backlight", 0);
   289		if (!np)
   290			return NULL;
   291	
   292		backlight = of_find_backlight_by_node(np);
   293		of_node_put(np);
   294	
   295		if (!backlight)
   296			return ERR_PTR(-EPROBE_DEFER);
   297	
   298		if (!backlight->props.brightness) {
   299			backlight->props.brightness = backlight->props.max_brightness;
   300			DRM_DEBUG_KMS("Backlight brightness set to %d\n",
   301				      backlight->props.brightness);
   302		}
   303	
   304		return backlight;
   305	}
   306	EXPORT_SYMBOL(drm_of_find_backlight);
   307	
   308	static void devm_drm_of_find_backlight_release(void *data)
   309	{
   310		put_device(data);
   311	}
   312	
   313	/**
   314	 * devm_drm_of_find_backlight - Find backlight device in device-tree
   315	 * devres version of the function
   316	 * @dev: Device
   317	 *
   318	 * This is the devres version of the function drm_of_find_backlight.
   319	 * Some drivers use devres versions of functions for
   320	 * requiring device resources.
   321	 *
   322	 * Returns:
   323	 * NULL if there's no backlight property.
   324	 * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device
   325	 * is found.
   326	 * If the backlight device is found, a pointer to the structure is returned.
   327	 */
 > 328	struct backlight_device *devm_drm_of_find_backlight(struct device *dev)

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23035 bytes --]