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@xxxxxxxxx>
---
Changes in v6:
-Move function declarations to linux/backlight.h to resolve
module dependency issues.
drivers/gpu/drm/drm_of.c | 44 ++++++++++++++++++++++++++
drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 -----------------------
drivers/gpu/drm/tinydrm/mi0283qt.c | 2 +-
include/drm/tinydrm/tinydrm-helpers.h | 1 -
include/linux/backlight.h | 11 +++++++
5 files changed, 56 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/drm_of.c b/drivers/gpu/drm/drm_of.c
index 8dafbdf..d878d3a 100644
--- a/drivers/gpu/drm/drm_of.c
+++ b/drivers/gpu/drm/drm_of.c
@@ -1,6 +1,7 @@
#include <linux/component.h>
#include <linux/export.h>
#include <linux/list.h>
+#include <linux/backlight.h>
#include <linux/of_graph.h>
#include <drm/drmP.h>
#include <drm/drm_bridge.h>
@@ -260,3 +261,46 @@ int drm_of_find_panel_or_bridge(const struct device_node *np,
return ret;
}
EXPORT_SYMBOL_GPL(drm_of_find_panel_or_bridge);
+
+/**
+ * drm_of_find_backlight - Find backlight device in device-tree
+ * @dev: Device
+ *
+ * This function looks for a DT node pointed to by a property named 'backlight'
+ * and uses of_find_backlight_by_node() to get the backlight device.
+ * Additionally if the brightness property is zero, it is set to
+ * max_brightness.
+ *
+ * Note: It is the responsibility of the caller to call put_device() when
+ * releasing the resource.
+ *
+ * Returns:
+ * NULL if there's no backlight property.
+ * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device
+ * is found.
+ * If the backlight device is found, a pointer to the structure is returned.
+ */
+struct backlight_device *drm_of_find_backlight(struct device *dev)
+{
+ struct backlight_device *backlight;
+ struct device_node *np;
+
+ np = of_parse_phandle(dev->of_node, "backlight", 0);
+ if (!np)
+ return NULL;
+
+ backlight = of_find_backlight_by_node(np);
+ of_node_put(np);
+
+ if (!backlight)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ if (!backlight->props.brightness) {
+ backlight->props.brightness = backlight->props.max_brightness;
+ DRM_DEBUG_KMS("Backlight brightness set to %d\n",
+ backlight->props.brightness);
+ }
+
+ return backlight;
+}
+EXPORT_SYMBOL(drm_of_find_backlight);
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index bd6cce0..cd4c6a5 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -237,46 +237,6 @@ void tinydrm_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
EXPORT_SYMBOL(tinydrm_xrgb8888_to_gray8);
/**
- * tinydrm_of_find_backlight - Find backlight device in device-tree
- * @dev: Device
- *
- * This function looks for a DT node pointed to by a property named 'backlight'
- * and uses of_find_backlight_by_node() to get the backlight device.
- * Additionally if the brightness property is zero, it is set to
- * max_brightness.
- *
- * Returns:
- * NULL if there's no backlight property.
- * Error pointer -EPROBE_DEFER if the DT node is found, but no backlight device
- * is found.
- * If the backlight device is found, a pointer to the structure is returned.
- */
-struct backlight_device *tinydrm_of_find_backlight(struct device *dev)
-{
- struct backlight_device *backlight;
- struct device_node *np;
-
- np = of_parse_phandle(dev->of_node, "backlight", 0);
- if (!np)
- return NULL;
-
- backlight = of_find_backlight_by_node(np);
- of_node_put(np);
-
- if (!backlight)
- return ERR_PTR(-EPROBE_DEFER);
-
- if (!backlight->props.brightness) {
- backlight->props.brightness = backlight->props.max_brightness;
- DRM_DEBUG_KMS("Backlight brightness set to %d\n",
- backlight->props.brightness);
- }
-
- return backlight;
-}
-EXPORT_SYMBOL(tinydrm_of_find_backlight);
-
-/**
* tinydrm_enable_backlight - Enable backlight helper
* @backlight: Backlight device
*
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 7e5bb7d..682b4db 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -189,7 +189,7 @@ static int mi0283qt_probe(struct spi_device *spi)
if (IS_ERR(mipi->regulator))
return PTR_ERR(mipi->regulator);
- mipi->backlight = tinydrm_of_find_backlight(dev);
+ mipi->backlight = drm_of_find_backlight(dev);
if (IS_ERR(mipi->backlight))
return PTR_ERR(mipi->backlight);
diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h
index d554ded..e40ef2d 100644
--- a/include/drm/tinydrm/tinydrm-helpers.h
+++ b/include/drm/tinydrm/tinydrm-helpers.h
@@ -46,7 +46,6 @@ void tinydrm_xrgb8888_to_rgb565(u16 *dst, void *vaddr,
void tinydrm_xrgb8888_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
struct drm_clip_rect *clip);
-struct backlight_device *tinydrm_of_find_backlight(struct device *dev);
int tinydrm_enable_backlight(struct backlight_device *backlight);
int tinydrm_disable_backlight(struct backlight_device *backlight);
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index 5f2fd61..47a095e 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -172,4 +172,15 @@ of_find_backlight_by_node(struct device_node *node)
}
#endif
+#if defined(CONFIG_OF) && IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
+struct backlight_device *
+drm_of_find_backlight(struct device *dev);
+#else
+static inline struct backlight_device *
+drm_of_find_backlight(struct device *dev)
+{
+ return NULL;
+}
+#endif
+
#endif
This isn't what I meant, you only change the of_find_backlight_by_node()
declaration/definition. It should be like this:
#if defined(CONFIG_OF) && IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
struct backlight_device *of_find_backlight_by_node(struct device_node
*node);
#else
static inline struct backlight_device *
of_find_backlight_by_node(struct device_node *node)
{
return NULL;
}
#endif
I see. So basically add the function declaration of drm_of_find_backlight
and devm_drm_of_find_backlight in drm_of.h and this function declaration
need not be inside the #ifdef CONFIG / #else part?
So there need not be a dummy version of drm_of_find_backlight here
because we are anyway taking care of that in of_find_backliht by node?
Sorry for the many versions and thank you for the clarifications.
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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
