Move drm helper functions from tinydrm-helpers to linux/backlight for
ease of use by callers in other drivers.
Changes in v18:
-Fixed warnings resulting from passing device_node* to of_find_backlight.
Fixed it by passing struct device* to of_find_backlight
Meghana Madhyastha (10):
video: backlight: Add helpers to enable and disable backlight
drm/tinydrm: Convert tinydrm_enable/disable_backlight to
backlight_enable/disable
video: backlight: Add of_find_backlight helper in backlight.c
drm/tinydrm: Replace tinydrm_of_find_backlight with of_find_backlight
video: backlight: Add devres versions of of_find_backlight
drm/tinydrm: Call devres version of of_find_backlight
drm/panel: Use backlight_enable/disable helpers
drm/omapdrm: Use backlight_enable/disable helpers
drm/panel: Use of_find_backlight helper
drm/omapdrm: Use of_find_backlight helper
drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 44 ++++--------
drivers/gpu/drm/panel/panel-innolux-p079zca.c | 30 ++------
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 6 +-
drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 38 ++--------
drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 37 ++--------
drivers/gpu/drm/tinydrm/Kconfig | 2 -
drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 95 -------------------------
drivers/gpu/drm/tinydrm/mi0283qt.c | 3 +-
drivers/gpu/drm/tinydrm/mipi-dbi.c | 4 +-
drivers/gpu/drm/tinydrm/st7735r.c | 3 +-
drivers/video/backlight/backlight.c | 73 +++++++++++++++++++
include/drm/tinydrm/tinydrm-helpers.h | 4 --
include/linux/backlight.h | 58 +++++++++++++++
13 files changed, 171 insertions(+), 226 deletions(-)
--
2.11.0
Add helper functions backlight_enable and backlight_disable to
enable/disable a backlight device. These helper functions can
then be used by different drm and tinydrm drivers to avoid
repetition of code and also to enforce a uniform and consistent
way to enable/disable a backlight device.
Signed-off-by: Meghana Madhyastha <[email protected]>
---
Acked-by: Daniel Thompson <[email protected]>
Reviewed-by: Noralf Tr?nnes <[email protected]>
Reviewed-by: Sean Paul<[email protected]>
include/linux/backlight.h | 32 ++++++++++++++++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index af7003548..ace825e2c 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -130,6 +130,38 @@ static inline int backlight_update_status(struct backlight_device *bd)
return ret;
}
+/**
+ * backlight_enable - Enable backlight
+ * @bd: the backlight device to enable
+ */
+static inline int backlight_enable(struct backlight_device *bd)
+{
+ if (!bd)
+ return 0;
+
+ bd->props.power = FB_BLANK_UNBLANK;
+ bd->props.fb_blank = FB_BLANK_UNBLANK;
+ bd->props.state &= ~BL_CORE_FBBLANK;
+
+ return backlight_update_status(bd);
+}
+
+/**
+ * backlight_disable - Disable backlight
+ * @bd: the backlight device to disable
+ */
+static inline int backlight_disable(struct backlight_device *bd)
+{
+ if (!bd)
+ return 0;
+
+ bd->props.power = FB_BLANK_POWERDOWN;
+ bd->props.fb_blank = FB_BLANK_POWERDOWN;
+ bd->props.state |= BL_CORE_FBBLANK;
+
+ return backlight_update_status(bd);
+}
+
extern struct backlight_device *backlight_device_register(const char *name,
struct device *dev, void *devdata, const struct backlight_ops *ops,
const struct backlight_properties *props);
--
2.11.0
Remove tinydrm_enable/disable_backlight and let the callers call the
more generic backlight_enable/disable helpers
Signed-off-by: Meghana Madhyastha <[email protected]>
---
Reviewed-by: Noralf Tr?nnes <[email protected]>
Reviewed-by: Sean Paul<[email protected]>
drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 55 --------------------------
drivers/gpu/drm/tinydrm/mipi-dbi.c | 4 +-
include/drm/tinydrm/tinydrm-helpers.h | 2 -
3 files changed, 2 insertions(+), 59 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index bf96072d1..7326e1758 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -276,61 +276,6 @@ struct backlight_device *tinydrm_of_find_backlight(struct device *dev)
}
EXPORT_SYMBOL(tinydrm_of_find_backlight);
-/**
- * tinydrm_enable_backlight - Enable backlight helper
- * @backlight: Backlight device
- *
- * Returns:
- * Zero on success, negative error code on failure.
- */
-int tinydrm_enable_backlight(struct backlight_device *backlight)
-{
- unsigned int old_state;
- int ret;
-
- if (!backlight)
- return 0;
-
- old_state = backlight->props.state;
- backlight->props.state &= ~BL_CORE_FBBLANK;
- DRM_DEBUG_KMS("Backlight state: 0x%x -> 0x%x\n", old_state,
- backlight->props.state);
-
- ret = backlight_update_status(backlight);
- if (ret)
- DRM_ERROR("Failed to enable backlight %d\n", ret);
-
- return ret;
-}
-EXPORT_SYMBOL(tinydrm_enable_backlight);
-
-/**
- * tinydrm_disable_backlight - Disable backlight helper
- * @backlight: Backlight device
- *
- * Returns:
- * Zero on success, negative error code on failure.
- */
-int tinydrm_disable_backlight(struct backlight_device *backlight)
-{
- unsigned int old_state;
- int ret;
-
- if (!backlight)
- return 0;
-
- old_state = backlight->props.state;
- backlight->props.state |= BL_CORE_FBBLANK;
- DRM_DEBUG_KMS("Backlight state: 0x%x -> 0x%x\n", old_state,
- backlight->props.state);
- ret = backlight_update_status(backlight);
- if (ret)
- DRM_ERROR("Failed to disable backlight %d\n", ret);
-
- return ret;
-}
-EXPORT_SYMBOL(tinydrm_disable_backlight);
-
#if IS_ENABLED(CONFIG_SPI)
/**
diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c
index 75dd65c57..9e903812b 100644
--- a/drivers/gpu/drm/tinydrm/mipi-dbi.c
+++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c
@@ -286,7 +286,7 @@ void mipi_dbi_enable_flush(struct mipi_dbi *mipi)
if (fb)
fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0);
- tinydrm_enable_backlight(mipi->backlight);
+ backlight_enable(mipi->backlight);
}
EXPORT_SYMBOL(mipi_dbi_enable_flush);
@@ -325,7 +325,7 @@ void mipi_dbi_pipe_disable(struct drm_simple_display_pipe *pipe)
mipi->enabled = false;
if (mipi->backlight)
- tinydrm_disable_backlight(mipi->backlight);
+ backlight_disable(mipi->backlight);
else
mipi_dbi_blank(mipi);
diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h
index d554ded60..f54fae03e 100644
--- a/include/drm/tinydrm/tinydrm-helpers.h
+++ b/include/drm/tinydrm/tinydrm-helpers.h
@@ -47,8 +47,6 @@ 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);
size_t tinydrm_spi_max_transfer_size(struct spi_device *spi, size_t max_len);
bool tinydrm_spi_bpw_supported(struct spi_device *spi, u8 bpw);
--
2.11.0
Remove tinydrm_of_find_backlight from tinydrm-helpers.c. We now have
a generic of_find_backlight defined in backlight.c. Let the callers
of tinydrm_of_find_backlight call of_find_backlight. Also, remove
select BACKLIGHT_LCD_SUPPORT and select BACKLIGHT_CLASS_DEVICE from
tinydrm/Kconfig as it is a hack that is no longer needed.
Signed-off-by: Meghana Madhyastha <[email protected]>
---
Reviewed-by: Noralf Tr?nnes <[email protected]>
Reviewed-by: Sean Paul<[email protected]>
drivers/gpu/drm/tinydrm/Kconfig | 2 --
drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 --------------------------
drivers/gpu/drm/tinydrm/mi0283qt.c | 3 +-
drivers/gpu/drm/tinydrm/st7735r.c | 3 +-
include/drm/tinydrm/tinydrm-helpers.h | 2 --
5 files changed, 4 insertions(+), 46 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/Kconfig b/drivers/gpu/drm/tinydrm/Kconfig
index b0e567d41..13339be84 100644
--- a/drivers/gpu/drm/tinydrm/Kconfig
+++ b/drivers/gpu/drm/tinydrm/Kconfig
@@ -3,8 +3,6 @@ menuconfig DRM_TINYDRM
depends on DRM
select DRM_KMS_HELPER
select DRM_KMS_CMA_HELPER
- select BACKLIGHT_LCD_SUPPORT
- select BACKLIGHT_CLASS_DEVICE
help
Choose this option if you have a tinydrm supported display.
If M is selected the module will be called tinydrm.
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
index 7326e1758..d1c3ce9ab 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c
@@ -236,46 +236,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);
-
#if IS_ENABLED(CONFIG_SPI)
/**
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index 79cb5af5a..a8aafce36 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -9,6 +9,7 @@
* (at your option) any later version.
*/
+#include <linux/backlight.h>
#include <linux/delay.h>
#include <linux/gpio/consumer.h>
#include <linux/module.h>
@@ -195,7 +196,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 = of_find_backlight(dev);
if (IS_ERR(mipi->backlight))
return PTR_ERR(mipi->backlight);
diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c
index 08b4fb18e..2e6b7b8ec 100644
--- a/drivers/gpu/drm/tinydrm/st7735r.c
+++ b/drivers/gpu/drm/tinydrm/st7735r.c
@@ -5,6 +5,7 @@
* Copyright 2017 David Lechner <[email protected]>
*/
+#include <linux/backlight.h>
#include <linux/delay.h>
#include <linux/dma-buf.h>
#include <linux/gpio/consumer.h>
@@ -163,7 +164,7 @@ static int st7735r_probe(struct spi_device *spi)
return PTR_ERR(dc);
}
- mipi->backlight = tinydrm_of_find_backlight(dev);
+ mipi->backlight = 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 f54fae03e..0a4ddbc04 100644
--- a/include/drm/tinydrm/tinydrm-helpers.h
+++ b/include/drm/tinydrm/tinydrm-helpers.h
@@ -46,8 +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);
-
size_t tinydrm_spi_max_transfer_size(struct spi_device *spi, size_t max_len);
bool tinydrm_spi_bpw_supported(struct spi_device *spi, u8 bpw);
int tinydrm_spi_transfer(struct spi_device *spi, u32 speed_hz,
--
2.11.0
Add of_find_backlight, a helper function which is a generic version
of tinydrm_of_find_backlight that can be used by other drivers to avoid
repetition of code and simplify things.
Signed-off-by: Meghana Madhyastha <[email protected]>
---
Acked-by: Daniel Thompson <[email protected]>
Reviewed-by: Noralf Tr?nnes <[email protected]>
Reviewed-by: Sean Paul<[email protected]>
drivers/video/backlight/backlight.c | 43 +++++++++++++++++++++++++++++++++++++
include/linux/backlight.h | 19 ++++++++++++++++
2 files changed, 62 insertions(+)
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 8049e7656..553bf5c48 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -580,6 +580,49 @@ struct backlight_device *of_find_backlight_by_node(struct device_node *node)
EXPORT_SYMBOL(of_find_backlight_by_node);
#endif
+/**
+ * of_find_backlight - Get backlight device
+ * @dev: Device
+ *
+ * This function looks for a property named 'backlight' on the DT node
+ * connected to @dev and looks up the backlight device.
+ *
+ * Call backlight_put() to drop the reference on the backlight device.
+ *
+ * Returns:
+ * A pointer to the backlight device if found.
+ * Error pointer -EPROBE_DEFER if the DT property is set, but no backlight
+ * device is found.
+ * NULL if there's no backlight property.
+ */
+struct backlight_device *of_find_backlight(struct device *dev)
+{
+ struct backlight_device *bd = NULL;
+ struct device_node *np;
+
+ if (!dev)
+ return NULL;
+
+ if (IS_ENABLED(CONFIG_OF) && dev->of_node) {
+ np = of_parse_phandle(dev->of_node, "backlight", 0);
+ if (np) {
+ bd = of_find_backlight_by_node(np);
+ of_node_put(np);
+ if (!bd)
+ return ERR_PTR(-EPROBE_DEFER);
+ /*
+ * Note: gpio_backlight uses brightness as
+ * power state during probe
+ */
+ if (!bd->props.brightness)
+ bd->props.brightness = bd->props.max_brightness;
+ }
+ }
+
+ return bd;
+}
+EXPORT_SYMBOL(of_find_backlight);
+
static void __exit backlight_class_exit(void)
{
class_destroy(backlight_class);
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index ace825e2c..ddc9bade4 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -162,6 +162,16 @@ static inline int backlight_disable(struct backlight_device *bd)
return backlight_update_status(bd);
}
+/**
+ * backlight_put - Drop backlight reference
+ * @bd: the backlight device to put
+ */
+static inline void backlight_put(struct backlight_device *bd)
+{
+ if (bd)
+ put_device(&bd->dev);
+}
+
extern struct backlight_device *backlight_device_register(const char *name,
struct device *dev, void *devdata, const struct backlight_ops *ops,
const struct backlight_properties *props);
@@ -205,4 +215,13 @@ of_find_backlight_by_node(struct device_node *node)
}
#endif
+#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
+struct backlight_device *of_find_backlight(struct device *dev);
+#else
+static inline struct backlight_device *of_find_backlight(struct device *dev)
+{
+ return NULL;
+}
+#endif
+
#endif
--
2.11.0
Add devm_of_find_backlight and the corresponding release
function because some drivers use devres versions of functions
for acquiring device resources.
Signed-off-by: Meghana Madhyastha <[email protected]>
---
Acked-by: Daniel Thompson <[email protected]>
Reviewed-by: Noralf Tr?nnes <[email protected]>
Reviewed-by: Sean Paul<[email protected]>
drivers/video/backlight/backlight.c | 30 ++++++++++++++++++++++++++++++
include/linux/backlight.h | 7 +++++++
2 files changed, 37 insertions(+)
diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 553bf5c48..deb824bef 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -623,6 +623,36 @@ struct backlight_device *of_find_backlight(struct device *dev)
}
EXPORT_SYMBOL(of_find_backlight);
+static void devm_backlight_release(void *data)
+{
+ backlight_put(data);
+}
+
+/**
+ * devm_of_find_backlight - Resource-managed of_find_backlight()
+ * @dev: Device
+ *
+ * Device managed version of of_find_backlight().
+ * The reference on the backlight device is automatically
+ * dropped on driver detach.
+ */
+struct backlight_device *devm_of_find_backlight(struct device *dev)
+{
+ struct backlight_device *bd;
+ int ret;
+
+ bd = of_find_backlight(dev);
+ if (IS_ERR_OR_NULL(bd))
+ return bd;
+ ret = devm_add_action(dev, devm_backlight_release, bd);
+ if (ret) {
+ backlight_put(bd);
+ return ERR_PTR(ret);
+ }
+ return bd;
+}
+EXPORT_SYMBOL(devm_of_find_backlight);
+
static void __exit backlight_class_exit(void)
{
class_destroy(backlight_class);
diff --git a/include/linux/backlight.h b/include/linux/backlight.h
index ddc9bade4..2baab6f38 100644
--- a/include/linux/backlight.h
+++ b/include/linux/backlight.h
@@ -217,11 +217,18 @@ of_find_backlight_by_node(struct device_node *node)
#if IS_ENABLED(CONFIG_BACKLIGHT_CLASS_DEVICE)
struct backlight_device *of_find_backlight(struct device *dev);
+struct backlight_device *devm_of_find_backlight(struct device *dev);
#else
static inline struct backlight_device *of_find_backlight(struct device *dev)
{
return NULL;
}
+
+static inline struct backlight_device *
+devm_of_find_backlight(struct device *dev)
+{
+ return NULL;
+}
#endif
#endif
--
2.11.0
Call devm_of_find_backlight (the devres version) instead of
of_find_backlight.
Signed-off-by: Meghana Madhyastha <[email protected]>
---
Reviewed-by: Noralf Tr?nnes <[email protected]>
Reviewed-by: Sean Paul<[email protected]>
drivers/gpu/drm/tinydrm/mi0283qt.c | 2 +-
drivers/gpu/drm/tinydrm/st7735r.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c
index a8aafce36..d8ed6e6f8 100644
--- a/drivers/gpu/drm/tinydrm/mi0283qt.c
+++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
@@ -196,7 +196,7 @@ static int mi0283qt_probe(struct spi_device *spi)
if (IS_ERR(mipi->regulator))
return PTR_ERR(mipi->regulator);
- mipi->backlight = of_find_backlight(dev);
+ mipi->backlight = devm_of_find_backlight(dev);
if (IS_ERR(mipi->backlight))
return PTR_ERR(mipi->backlight);
diff --git a/drivers/gpu/drm/tinydrm/st7735r.c b/drivers/gpu/drm/tinydrm/st7735r.c
index 2e6b7b8ec..67d197ecf 100644
--- a/drivers/gpu/drm/tinydrm/st7735r.c
+++ b/drivers/gpu/drm/tinydrm/st7735r.c
@@ -164,7 +164,7 @@ static int st7735r_probe(struct spi_device *spi)
return PTR_ERR(dc);
}
- mipi->backlight = of_find_backlight(dev);
+ mipi->backlight = devm_of_find_backlight(dev);
if (IS_ERR(mipi->backlight))
return PTR_ERR(mipi->backlight);
--
2.11.0
Use backlight_enable/disable helpers instead of changing
the property and calling backlight_update_status for cleaner
and simpler code and also to avoid repetitions.
Signed-off-by: Meghana Madhyastha <[email protected]>
---
drivers/gpu/drm/panel/panel-innolux-p079zca.c | 6 ++----
drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 6 ++----
drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 10 ++--------
drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 10 ++--------
4 files changed, 8 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
index 6ba93449f..4c1b29eec 100644
--- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
+++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
@@ -45,8 +45,7 @@ static int innolux_panel_disable(struct drm_panel *panel)
if (!innolux->enabled)
return 0;
- innolux->backlight->props.power = FB_BLANK_POWERDOWN;
- backlight_update_status(innolux->backlight);
+ backlight_disable(innolux->backlight);
err = mipi_dsi_dcs_set_display_off(innolux->link);
if (err < 0)
@@ -151,8 +150,7 @@ static int innolux_panel_enable(struct drm_panel *panel)
if (innolux->enabled)
return 0;
- innolux->backlight->props.power = FB_BLANK_UNBLANK;
- ret = backlight_update_status(innolux->backlight);
+ ret = backlight_enable(innolux->backlight);
if (ret) {
DRM_DEV_ERROR(panel->drm->dev,
"Failed to enable backlight %d\n", ret);
diff --git a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
index 5b2340ef7..0a94ab79a 100644
--- a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
+++ b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
@@ -192,8 +192,7 @@ static int jdi_panel_disable(struct drm_panel *panel)
if (!jdi->enabled)
return 0;
- jdi->backlight->props.power = FB_BLANK_POWERDOWN;
- backlight_update_status(jdi->backlight);
+ backlight_disable(jdi->backlight);
jdi->enabled = false;
@@ -289,8 +288,7 @@ static int jdi_panel_enable(struct drm_panel *panel)
if (jdi->enabled)
return 0;
- jdi->backlight->props.power = FB_BLANK_UNBLANK;
- backlight_update_status(jdi->backlight);
+ backlight_enable(jdi->backlight);
jdi->enabled = true;
diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
index 3cce3ca19..072c0fc79 100644
--- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
@@ -96,10 +96,7 @@ static int sharp_panel_disable(struct drm_panel *panel)
if (!sharp->enabled)
return 0;
- if (sharp->backlight) {
- sharp->backlight->props.power = FB_BLANK_POWERDOWN;
- backlight_update_status(sharp->backlight);
- }
+ backlight_disable(sharp->backlight);
sharp->enabled = false;
@@ -263,10 +260,7 @@ static int sharp_panel_enable(struct drm_panel *panel)
if (sharp->enabled)
return 0;
- if (sharp->backlight) {
- sharp->backlight->props.power = FB_BLANK_UNBLANK;
- backlight_update_status(sharp->backlight);
- }
+ backlight_enable(sharp->backlight);
sharp->enabled = true;
diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
index 3aeb0bda4..8a5137963 100644
--- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
@@ -117,10 +117,7 @@ static int sharp_nt_panel_disable(struct drm_panel *panel)
if (!sharp_nt->enabled)
return 0;
- if (sharp_nt->backlight) {
- sharp_nt->backlight->props.power = FB_BLANK_POWERDOWN;
- backlight_update_status(sharp_nt->backlight);
- }
+ backlight_disable(sharp_nt->backlight);
sharp_nt->enabled = false;
@@ -203,10 +200,7 @@ static int sharp_nt_panel_enable(struct drm_panel *panel)
if (sharp_nt->enabled)
return 0;
- if (sharp_nt->backlight) {
- sharp_nt->backlight->props.power = FB_BLANK_UNBLANK;
- backlight_update_status(sharp_nt->backlight);
- }
+ backlight_enable(sharp_nt->backlight);
sharp_nt->enabled = true;
--
2.11.0
Use backlight_enable/disable helpers instead of changing
the property and calling backlight_update_status for cleaner
and simpler code and also to avoid repetitions.
Signed-off-by: Meghana Madhyastha <[email protected]>
---
drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
index e065f7e10..ac9596251 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
@@ -87,11 +87,7 @@ static int panel_dpi_enable(struct omap_dss_device *dssdev)
}
gpiod_set_value_cansleep(ddata->enable_gpio, 1);
-
- if (ddata->backlight) {
- ddata->backlight->props.power = FB_BLANK_UNBLANK;
- backlight_update_status(ddata->backlight);
- }
+ backlight_enable(ddata->backlight);
dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
@@ -106,10 +102,7 @@ static void panel_dpi_disable(struct omap_dss_device *dssdev)
if (!omapdss_device_is_enabled(dssdev))
return;
- if (ddata->backlight) {
- ddata->backlight->props.power = FB_BLANK_POWERDOWN;
- backlight_update_status(ddata->backlight);
- }
+ backlight_disable(ddata->backlight);
gpiod_set_value_cansleep(ddata->enable_gpio, 0);
regulator_disable(ddata->vcc_supply);
--
2.11.0
Replace of_find_backlight_by_node and of the code around it
with of_find_backlight helper to avoid repetition of code.
Signed-off-by: Meghana Madhyastha <[email protected]>
---
Changes in v18:
-Fixed warnings resulting from passing device_node* to of_find_backlight.
Fixed it by passing struct device* to of_find_backlight
drivers/gpu/drm/panel/panel-innolux-p079zca.c | 24 ++++-----------------
drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 28 +++++--------------------
drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 27 ++++--------------------
3 files changed, 13 insertions(+), 66 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
index 4c1b29eec..57df39b5c 100644
--- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
+++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
@@ -215,7 +215,6 @@ MODULE_DEVICE_TABLE(of, innolux_of_match);
static int innolux_panel_add(struct innolux_panel *innolux)
{
struct device *dev = &innolux->link->dev;
- struct device_node *np;
int err;
innolux->supply = devm_regulator_get(dev, "power");
@@ -230,37 +229,22 @@ static int innolux_panel_add(struct innolux_panel *innolux)
innolux->enable_gpio = NULL;
}
- np = of_parse_phandle(dev->of_node, "backlight", 0);
- if (np) {
- innolux->backlight = of_find_backlight_by_node(np);
- of_node_put(np);
+ innolux->backlight = devm_of_find_backlight(dev);
- if (!innolux->backlight)
- return -EPROBE_DEFER;
- }
+ if (IS_ERR(innolux->backlight))
+ return PTR_ERR(innolux->backlight);
drm_panel_init(&innolux->base);
innolux->base.funcs = &innolux_panel_funcs;
innolux->base.dev = &innolux->link->dev;
- err = drm_panel_add(&innolux->base);
- if (err < 0)
- goto put_backlight;
-
- return 0;
-
-put_backlight:
- put_device(&innolux->backlight->dev);
-
- return err;
+ return drm_panel_add(&innolux->base);
}
static void innolux_panel_del(struct innolux_panel *innolux)
{
if (innolux->base.dev)
drm_panel_remove(&innolux->base);
-
- put_device(&innolux->backlight->dev);
}
static int innolux_panel_probe(struct mipi_dsi_device *dsi)
diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
index 072c0fc79..6bf8730f1 100644
--- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
@@ -318,8 +318,7 @@ MODULE_DEVICE_TABLE(of, sharp_of_match);
static int sharp_panel_add(struct sharp_panel *sharp)
{
- struct device_node *np;
- int err;
+ struct device *dev = &sharp->link1->dev;
sharp->mode = &default_mode;
@@ -327,30 +326,16 @@ static int sharp_panel_add(struct sharp_panel *sharp)
if (IS_ERR(sharp->supply))
return PTR_ERR(sharp->supply);
- np = of_parse_phandle(sharp->link1->dev.of_node, "backlight", 0);
- if (np) {
- sharp->backlight = of_find_backlight_by_node(np);
- of_node_put(np);
+ sharp->backlight = devm_of_find_backlight(dev);
- if (!sharp->backlight)
- return -EPROBE_DEFER;
- }
+ if (IS_ERR(sharp->backlight))
+ return PTR_ERR(sharp->backlight);
drm_panel_init(&sharp->base);
sharp->base.funcs = &sharp_panel_funcs;
sharp->base.dev = &sharp->link1->dev;
- err = drm_panel_add(&sharp->base);
- if (err < 0)
- goto put_backlight;
-
- return 0;
-
-put_backlight:
- if (sharp->backlight)
- put_device(&sharp->backlight->dev);
-
- return err;
+ return drm_panel_add(&sharp->base);
}
static void sharp_panel_del(struct sharp_panel *sharp)
@@ -358,9 +343,6 @@ static void sharp_panel_del(struct sharp_panel *sharp)
if (sharp->base.dev)
drm_panel_remove(&sharp->base);
- if (sharp->backlight)
- put_device(&sharp->backlight->dev);
-
if (sharp->link2)
put_device(&sharp->link2->dev);
}
diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
index 8a5137963..494aa9b16 100644
--- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
@@ -253,8 +253,6 @@ static const struct drm_panel_funcs sharp_nt_panel_funcs = {
static int sharp_nt_panel_add(struct sharp_nt_panel *sharp_nt)
{
struct device *dev = &sharp_nt->dsi->dev;
- struct device_node *np;
- int ret;
sharp_nt->mode = &default_mode;
@@ -271,39 +269,22 @@ static int sharp_nt_panel_add(struct sharp_nt_panel *sharp_nt)
gpiod_set_value(sharp_nt->reset_gpio, 0);
}
- np = of_parse_phandle(dev->of_node, "backlight", 0);
- if (np) {
- sharp_nt->backlight = of_find_backlight_by_node(np);
- of_node_put(np);
+ sharp_nt->backlight = devm_of_find_backlight(dev);
- if (!sharp_nt->backlight)
- return -EPROBE_DEFER;
- }
+ if (IS_ERR(sharp_nt->backlight))
+ return PTR_ERR(sharp_nt->backlight);
drm_panel_init(&sharp_nt->base);
sharp_nt->base.funcs = &sharp_nt_panel_funcs;
sharp_nt->base.dev = &sharp_nt->dsi->dev;
- ret = drm_panel_add(&sharp_nt->base);
- if (ret < 0)
- goto put_backlight;
-
- return 0;
-
-put_backlight:
- if (sharp_nt->backlight)
- put_device(&sharp_nt->backlight->dev);
-
- return ret;
+ return drm_panel_add(&sharp_nt->base);
}
static void sharp_nt_panel_del(struct sharp_nt_panel *sharp_nt)
{
if (sharp_nt->base.dev)
drm_panel_remove(&sharp_nt->base);
-
- if (sharp_nt->backlight)
- put_device(&sharp_nt->backlight->dev);
}
static int sharp_nt_panel_probe(struct mipi_dsi_device *dsi)
--
2.11.0
Replace of_find_backlight_by_node and of the code around it
with of_find_backlight helper to avoid repetition of code.
Signed-off-by: Meghana Madhyastha <[email protected]>
---
Changes in v18:
-Fixed warnings resulting from passing device_node* to of_find_backlight.
Fixed it by passing struct device* to of_find_backlight
drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 33 ++++++++++------------------
1 file changed, 11 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
index ac9596251..93b7a176d 100644
--- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
+++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
@@ -156,14 +156,14 @@ static struct omap_dss_driver panel_dpi_ops = {
static int panel_dpi_probe_of(struct platform_device *pdev)
{
struct panel_drv_data *ddata = platform_get_drvdata(pdev);
+ struct device *dev = &pdev->dev;
struct device_node *node = pdev->dev.of_node;
- struct device_node *bl_node;
struct omap_dss_device *in;
int r;
struct display_timing timing;
struct gpio_desc *gpio;
- gpio = devm_gpiod_get_optional(&pdev->dev, "enable", GPIOD_OUT_LOW);
+ gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
if (IS_ERR(gpio))
return PTR_ERR(gpio);
@@ -175,47 +175,36 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
* timing and order relative to the enable gpio. So for now it's just
* ensured that the reset line isn't active.
*/
- gpio = devm_gpiod_get_optional(&pdev->dev, "reset", GPIOD_OUT_LOW);
+ gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
if (IS_ERR(gpio))
return PTR_ERR(gpio);
- ddata->vcc_supply = devm_regulator_get(&pdev->dev, "vcc");
+ ddata->vcc_supply = devm_regulator_get(dev, "vcc");
if (IS_ERR(ddata->vcc_supply))
return PTR_ERR(ddata->vcc_supply);
- bl_node = of_parse_phandle(node, "backlight", 0);
- if (bl_node) {
- ddata->backlight = of_find_backlight_by_node(bl_node);
- of_node_put(bl_node);
+ ddata->backlight = of_find_backlight(dev);
- if (!ddata->backlight)
- return -EPROBE_DEFER;
- }
+ if (IS_ERR(ddata->backlight))
+ return PTR_ERR(ddata->backlight);
r = of_get_display_timing(node, "panel-timing", &timing);
if (r) {
- dev_err(&pdev->dev, "failed to get video timing\n");
- goto error_free_backlight;
+ dev_err(dev, "failed to get video timing\n");
+ return r;
}
videomode_from_timing(&timing, &ddata->vm);
in = omapdss_of_find_source_for_first_ep(node);
if (IS_ERR(in)) {
- dev_err(&pdev->dev, "failed to find video source\n");
- r = PTR_ERR(in);
- goto error_free_backlight;
+ dev_err(dev, "failed to find video source\n");
+ return PTR_ERR(in);
}
ddata->in = in;
return 0;
-
-error_free_backlight:
- if (ddata->backlight)
- put_device(&ddata->backlight->dev);
-
- return r;
}
static int panel_dpi_probe(struct platform_device *pdev)
--
2.11.0
On Mon, Jan 22, 2018 at 02:49:28PM +0000, Meghana Madhyastha wrote:
> Add helper functions backlight_enable and backlight_disable to
> enable/disable a backlight device. These helper functions can
> then be used by different drm and tinydrm drivers to avoid
> repetition of code and also to enforce a uniform and consistent
> way to enable/disable a backlight device.
>
> Signed-off-by: Meghana Madhyastha <[email protected]>
> ---
> Acked-by: Daniel Thompson <[email protected]>
> Reviewed-by: Noralf Tr?nnes <[email protected]>
> Reviewed-by: Sean Paul<[email protected]>
Just in case there's another spin... the additional tags are normally
presented above the --- (otherwise they are unlikely to be copied into
the git history when they are applied).
Daniel.
>
> include/linux/backlight.h | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
> index af7003548..ace825e2c 100644
> --- a/include/linux/backlight.h
> +++ b/include/linux/backlight.h
> @@ -130,6 +130,38 @@ static inline int backlight_update_status(struct backlight_device *bd)
> return ret;
> }
>
> +/**
> + * backlight_enable - Enable backlight
> + * @bd: the backlight device to enable
> + */
> +static inline int backlight_enable(struct backlight_device *bd)
> +{
> + if (!bd)
> + return 0;
> +
> + bd->props.power = FB_BLANK_UNBLANK;
> + bd->props.fb_blank = FB_BLANK_UNBLANK;
> + bd->props.state &= ~BL_CORE_FBBLANK;
> +
> + return backlight_update_status(bd);
> +}
> +
> +/**
> + * backlight_disable - Disable backlight
> + * @bd: the backlight device to disable
> + */
> +static inline int backlight_disable(struct backlight_device *bd)
> +{
> + if (!bd)
> + return 0;
> +
> + bd->props.power = FB_BLANK_POWERDOWN;
> + bd->props.fb_blank = FB_BLANK_POWERDOWN;
> + bd->props.state |= BL_CORE_FBBLANK;
> +
> + return backlight_update_status(bd);
> +}
> +
> extern struct backlight_device *backlight_device_register(const char *name,
> struct device *dev, void *devdata, const struct backlight_ops *ops,
> const struct backlight_properties *props);
> --
> 2.11.0
>
Den 22.01.2018 15.54, skrev Meghana Madhyastha:
> Use backlight_enable/disable helpers instead of changing
> the property and calling backlight_update_status for cleaner
> and simpler code and also to avoid repetitions.
>
> Signed-off-by: Meghana Madhyastha <[email protected]>
> ---
Reviewed-by: Noralf Trønnes <[email protected]>
> drivers/gpu/drm/panel/panel-innolux-p079zca.c | 6 ++----
> drivers/gpu/drm/panel/panel-jdi-lt070me05000.c | 6 ++----
> drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 10 ++--------
> drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 10 ++--------
> 4 files changed, 8 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> index 6ba93449f..4c1b29eec 100644
> --- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> +++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> @@ -45,8 +45,7 @@ static int innolux_panel_disable(struct drm_panel *panel)
> if (!innolux->enabled)
> return 0;
>
> - innolux->backlight->props.power = FB_BLANK_POWERDOWN;
> - backlight_update_status(innolux->backlight);
> + backlight_disable(innolux->backlight);
>
> err = mipi_dsi_dcs_set_display_off(innolux->link);
> if (err < 0)
> @@ -151,8 +150,7 @@ static int innolux_panel_enable(struct drm_panel *panel)
> if (innolux->enabled)
> return 0;
>
> - innolux->backlight->props.power = FB_BLANK_UNBLANK;
> - ret = backlight_update_status(innolux->backlight);
> + ret = backlight_enable(innolux->backlight);
> if (ret) {
> DRM_DEV_ERROR(panel->drm->dev,
> "Failed to enable backlight %d\n", ret);
> diff --git a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
> index 5b2340ef7..0a94ab79a 100644
> --- a/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
> +++ b/drivers/gpu/drm/panel/panel-jdi-lt070me05000.c
> @@ -192,8 +192,7 @@ static int jdi_panel_disable(struct drm_panel *panel)
> if (!jdi->enabled)
> return 0;
>
> - jdi->backlight->props.power = FB_BLANK_POWERDOWN;
> - backlight_update_status(jdi->backlight);
> + backlight_disable(jdi->backlight);
>
> jdi->enabled = false;
>
> @@ -289,8 +288,7 @@ static int jdi_panel_enable(struct drm_panel *panel)
> if (jdi->enabled)
> return 0;
>
> - jdi->backlight->props.power = FB_BLANK_UNBLANK;
> - backlight_update_status(jdi->backlight);
> + backlight_enable(jdi->backlight);
>
> jdi->enabled = true;
>
> diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> index 3cce3ca19..072c0fc79 100644
> --- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> +++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> @@ -96,10 +96,7 @@ static int sharp_panel_disable(struct drm_panel *panel)
> if (!sharp->enabled)
> return 0;
>
> - if (sharp->backlight) {
> - sharp->backlight->props.power = FB_BLANK_POWERDOWN;
> - backlight_update_status(sharp->backlight);
> - }
> + backlight_disable(sharp->backlight);
>
> sharp->enabled = false;
>
> @@ -263,10 +260,7 @@ static int sharp_panel_enable(struct drm_panel *panel)
> if (sharp->enabled)
> return 0;
>
> - if (sharp->backlight) {
> - sharp->backlight->props.power = FB_BLANK_UNBLANK;
> - backlight_update_status(sharp->backlight);
> - }
> + backlight_enable(sharp->backlight);
>
> sharp->enabled = true;
>
> diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> index 3aeb0bda4..8a5137963 100644
> --- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> +++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> @@ -117,10 +117,7 @@ static int sharp_nt_panel_disable(struct drm_panel *panel)
> if (!sharp_nt->enabled)
> return 0;
>
> - if (sharp_nt->backlight) {
> - sharp_nt->backlight->props.power = FB_BLANK_POWERDOWN;
> - backlight_update_status(sharp_nt->backlight);
> - }
> + backlight_disable(sharp_nt->backlight);
>
> sharp_nt->enabled = false;
>
> @@ -203,10 +200,7 @@ static int sharp_nt_panel_enable(struct drm_panel *panel)
> if (sharp_nt->enabled)
> return 0;
>
> - if (sharp_nt->backlight) {
> - sharp_nt->backlight->props.power = FB_BLANK_UNBLANK;
> - backlight_update_status(sharp_nt->backlight);
> - }
> + backlight_enable(sharp_nt->backlight);
>
> sharp_nt->enabled = true;
>
Den 22.01.2018 15.54, skrev Meghana Madhyastha:
> Use backlight_enable/disable helpers instead of changing
> the property and calling backlight_update_status for cleaner
> and simpler code and also to avoid repetitions.
>
> Signed-off-by: Meghana Madhyastha <[email protected]>
> ---
Reviewed-by: Noralf Trønnes<[email protected]>
> drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> index e065f7e10..ac9596251 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> @@ -87,11 +87,7 @@ static int panel_dpi_enable(struct omap_dss_device *dssdev)
> }
>
> gpiod_set_value_cansleep(ddata->enable_gpio, 1);
> -
> - if (ddata->backlight) {
> - ddata->backlight->props.power = FB_BLANK_UNBLANK;
> - backlight_update_status(ddata->backlight);
> - }
> + backlight_enable(ddata->backlight);
>
> dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
>
> @@ -106,10 +102,7 @@ static void panel_dpi_disable(struct omap_dss_device *dssdev)
> if (!omapdss_device_is_enabled(dssdev))
> return;
>
> - if (ddata->backlight) {
> - ddata->backlight->props.power = FB_BLANK_POWERDOWN;
> - backlight_update_status(ddata->backlight);
> - }
> + backlight_disable(ddata->backlight);
>
> gpiod_set_value_cansleep(ddata->enable_gpio, 0);
> regulator_disable(ddata->vcc_supply);
Den 22.01.2018 15.55, skrev Meghana Madhyastha:
> Replace of_find_backlight_by_node and of the code around it
> with of_find_backlight helper to avoid repetition of code.
>
> Signed-off-by: Meghana Madhyastha <[email protected]>
> ---
Reviewed-by: Noralf Trønnes<[email protected]>
> Changes in v18:
> -Fixed warnings resulting from passing device_node* to of_find_backlight.
> Fixed it by passing struct device* to of_find_backlight
>
> drivers/gpu/drm/panel/panel-innolux-p079zca.c | 24 ++++-----------------
> drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 28 +++++--------------------
> drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 27 ++++--------------------
> 3 files changed, 13 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> index 4c1b29eec..57df39b5c 100644
> --- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> +++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
> @@ -215,7 +215,6 @@ MODULE_DEVICE_TABLE(of, innolux_of_match);
> static int innolux_panel_add(struct innolux_panel *innolux)
> {
> struct device *dev = &innolux->link->dev;
> - struct device_node *np;
> int err;
>
> innolux->supply = devm_regulator_get(dev, "power");
> @@ -230,37 +229,22 @@ static int innolux_panel_add(struct innolux_panel *innolux)
> innolux->enable_gpio = NULL;
> }
>
> - np = of_parse_phandle(dev->of_node, "backlight", 0);
> - if (np) {
> - innolux->backlight = of_find_backlight_by_node(np);
> - of_node_put(np);
> + innolux->backlight = devm_of_find_backlight(dev);
>
> - if (!innolux->backlight)
> - return -EPROBE_DEFER;
> - }
> + if (IS_ERR(innolux->backlight))
> + return PTR_ERR(innolux->backlight);
>
> drm_panel_init(&innolux->base);
> innolux->base.funcs = &innolux_panel_funcs;
> innolux->base.dev = &innolux->link->dev;
>
> - err = drm_panel_add(&innolux->base);
> - if (err < 0)
> - goto put_backlight;
> -
> - return 0;
> -
> -put_backlight:
> - put_device(&innolux->backlight->dev);
> -
> - return err;
> + return drm_panel_add(&innolux->base);
> }
>
> static void innolux_panel_del(struct innolux_panel *innolux)
> {
> if (innolux->base.dev)
> drm_panel_remove(&innolux->base);
> -
> - put_device(&innolux->backlight->dev);
> }
>
> static int innolux_panel_probe(struct mipi_dsi_device *dsi)
> diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> index 072c0fc79..6bf8730f1 100644
> --- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> +++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
> @@ -318,8 +318,7 @@ MODULE_DEVICE_TABLE(of, sharp_of_match);
>
> static int sharp_panel_add(struct sharp_panel *sharp)
> {
> - struct device_node *np;
> - int err;
> + struct device *dev = &sharp->link1->dev;
>
> sharp->mode = &default_mode;
>
> @@ -327,30 +326,16 @@ static int sharp_panel_add(struct sharp_panel *sharp)
> if (IS_ERR(sharp->supply))
> return PTR_ERR(sharp->supply);
>
> - np = of_parse_phandle(sharp->link1->dev.of_node, "backlight", 0);
> - if (np) {
> - sharp->backlight = of_find_backlight_by_node(np);
> - of_node_put(np);
> + sharp->backlight = devm_of_find_backlight(dev);
>
> - if (!sharp->backlight)
> - return -EPROBE_DEFER;
> - }
> + if (IS_ERR(sharp->backlight))
> + return PTR_ERR(sharp->backlight);
>
> drm_panel_init(&sharp->base);
> sharp->base.funcs = &sharp_panel_funcs;
> sharp->base.dev = &sharp->link1->dev;
>
> - err = drm_panel_add(&sharp->base);
> - if (err < 0)
> - goto put_backlight;
> -
> - return 0;
> -
> -put_backlight:
> - if (sharp->backlight)
> - put_device(&sharp->backlight->dev);
> -
> - return err;
> + return drm_panel_add(&sharp->base);
> }
>
> static void sharp_panel_del(struct sharp_panel *sharp)
> @@ -358,9 +343,6 @@ static void sharp_panel_del(struct sharp_panel *sharp)
> if (sharp->base.dev)
> drm_panel_remove(&sharp->base);
>
> - if (sharp->backlight)
> - put_device(&sharp->backlight->dev);
> -
> if (sharp->link2)
> put_device(&sharp->link2->dev);
> }
> diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> index 8a5137963..494aa9b16 100644
> --- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> +++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
> @@ -253,8 +253,6 @@ static const struct drm_panel_funcs sharp_nt_panel_funcs = {
> static int sharp_nt_panel_add(struct sharp_nt_panel *sharp_nt)
> {
> struct device *dev = &sharp_nt->dsi->dev;
> - struct device_node *np;
> - int ret;
>
> sharp_nt->mode = &default_mode;
>
> @@ -271,39 +269,22 @@ static int sharp_nt_panel_add(struct sharp_nt_panel *sharp_nt)
> gpiod_set_value(sharp_nt->reset_gpio, 0);
> }
>
> - np = of_parse_phandle(dev->of_node, "backlight", 0);
> - if (np) {
> - sharp_nt->backlight = of_find_backlight_by_node(np);
> - of_node_put(np);
> + sharp_nt->backlight = devm_of_find_backlight(dev);
>
> - if (!sharp_nt->backlight)
> - return -EPROBE_DEFER;
> - }
> + if (IS_ERR(sharp_nt->backlight))
> + return PTR_ERR(sharp_nt->backlight);
>
> drm_panel_init(&sharp_nt->base);
> sharp_nt->base.funcs = &sharp_nt_panel_funcs;
> sharp_nt->base.dev = &sharp_nt->dsi->dev;
>
> - ret = drm_panel_add(&sharp_nt->base);
> - if (ret < 0)
> - goto put_backlight;
> -
> - return 0;
> -
> -put_backlight:
> - if (sharp_nt->backlight)
> - put_device(&sharp_nt->backlight->dev);
> -
> - return ret;
> + return drm_panel_add(&sharp_nt->base);
> }
>
> static void sharp_nt_panel_del(struct sharp_nt_panel *sharp_nt)
> {
> if (sharp_nt->base.dev)
> drm_panel_remove(&sharp_nt->base);
> -
> - if (sharp_nt->backlight)
> - put_device(&sharp_nt->backlight->dev);
> }
>
> static int sharp_nt_panel_probe(struct mipi_dsi_device *dsi)
Den 22.01.2018 15.56, skrev Meghana Madhyastha:
> Replace of_find_backlight_by_node and of the code around it
> with of_find_backlight helper to avoid repetition of code.
>
> Signed-off-by: Meghana Madhyastha <[email protected]>
> ---
> Changes in v18:
> -Fixed warnings resulting from passing device_node* to of_find_backlight.
> Fixed it by passing struct device* to of_find_backlight
>
> drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 33 ++++++++++------------------
> 1 file changed, 11 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> index ac9596251..93b7a176d 100644
> --- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> @@ -156,14 +156,14 @@ static struct omap_dss_driver panel_dpi_ops = {
> static int panel_dpi_probe_of(struct platform_device *pdev)
> {
> struct panel_drv_data *ddata = platform_get_drvdata(pdev);
> + struct device *dev = &pdev->dev;
> struct device_node *node = pdev->dev.of_node;
> - struct device_node *bl_node;
> struct omap_dss_device *in;
> int r;
> struct display_timing timing;
> struct gpio_desc *gpio;
>
> - gpio = devm_gpiod_get_optional(&pdev->dev, "enable", GPIOD_OUT_LOW);
> + gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
Please don't make unrelated changes like this. It clutters the patch.
You can just as well use &pdev->dev when getting the backlight also.
> if (IS_ERR(gpio))
> return PTR_ERR(gpio);
>
> @@ -175,47 +175,36 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
> * timing and order relative to the enable gpio. So for now it's just
> * ensured that the reset line isn't active.
> */
> - gpio = devm_gpiod_get_optional(&pdev->dev, "reset", GPIOD_OUT_LOW);
> + gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> if (IS_ERR(gpio))
> return PTR_ERR(gpio);
>
> - ddata->vcc_supply = devm_regulator_get(&pdev->dev, "vcc");
> + ddata->vcc_supply = devm_regulator_get(dev, "vcc");
> if (IS_ERR(ddata->vcc_supply))
> return PTR_ERR(ddata->vcc_supply);
>
> - bl_node = of_parse_phandle(node, "backlight", 0);
> - if (bl_node) {
> - ddata->backlight = of_find_backlight_by_node(bl_node);
> - of_node_put(bl_node);
> + ddata->backlight = of_find_backlight(dev);
Any reason you don't use the devm_ version here?
You do remove error_free_backlight...
With the devm_ version remember to drop the put_device in
panel_dpi_remove().
Noralf.
>
> - if (!ddata->backlight)
> - return -EPROBE_DEFER;
> - }
> + if (IS_ERR(ddata->backlight))
> + return PTR_ERR(ddata->backlight);
>
> r = of_get_display_timing(node, "panel-timing", &timing);
> if (r) {
> - dev_err(&pdev->dev, "failed to get video timing\n");
> - goto error_free_backlight;
> + dev_err(dev, "failed to get video timing\n");
> + return r;
> }
>
> videomode_from_timing(&timing, &ddata->vm);
>
> in = omapdss_of_find_source_for_first_ep(node);
> if (IS_ERR(in)) {
> - dev_err(&pdev->dev, "failed to find video source\n");
> - r = PTR_ERR(in);
> - goto error_free_backlight;
> + dev_err(dev, "failed to find video source\n");
> + return PTR_ERR(in);
> }
>
> ddata->in = in;
>
> return 0;
> -
> -error_free_backlight:
> - if (ddata->backlight)
> - put_device(&ddata->backlight->dev);
> -
> - return r;
> }
>
> static int panel_dpi_probe(struct platform_device *pdev)
Den 23.01.2018 11.00, skrev Daniel Thompson:
> On Mon, Jan 22, 2018 at 02:49:28PM +0000, Meghana Madhyastha wrote:
>> Add helper functions backlight_enable and backlight_disable to
>> enable/disable a backlight device. These helper functions can
>> then be used by different drm and tinydrm drivers to avoid
>> repetition of code and also to enforce a uniform and consistent
>> way to enable/disable a backlight device.
>>
>> Signed-off-by: Meghana Madhyastha <[email protected]>
>> ---
>> Acked-by: Daniel Thompson <[email protected]>
>> Reviewed-by: Noralf Trønnes <[email protected]>
>> Reviewed-by: Sean Paul<[email protected]>
> Just in case there's another spin... the additional tags are normally
> presented above the --- (otherwise they are unlikely to be copied into
> the git history when they are applied).
Indeed, they should be part of the commit message.
Noralf.
>
> Daniel.
>
>
>> include/linux/backlight.h | 32 ++++++++++++++++++++++++++++++++
>> 1 file changed, 32 insertions(+)
>>
>> diff --git a/include/linux/backlight.h b/include/linux/backlight.h
>> index af7003548..ace825e2c 100644
>> --- a/include/linux/backlight.h
>> +++ b/include/linux/backlight.h
>> @@ -130,6 +130,38 @@ static inline int backlight_update_status(struct backlight_device *bd)
>> return ret;
>> }
>>
>> +/**
>> + * backlight_enable - Enable backlight
>> + * @bd: the backlight device to enable
>> + */
>> +static inline int backlight_enable(struct backlight_device *bd)
>> +{
>> + if (!bd)
>> + return 0;
>> +
>> + bd->props.power = FB_BLANK_UNBLANK;
>> + bd->props.fb_blank = FB_BLANK_UNBLANK;
>> + bd->props.state &= ~BL_CORE_FBBLANK;
>> +
>> + return backlight_update_status(bd);
>> +}
>> +
>> +/**
>> + * backlight_disable - Disable backlight
>> + * @bd: the backlight device to disable
>> + */
>> +static inline int backlight_disable(struct backlight_device *bd)
>> +{
>> + if (!bd)
>> + return 0;
>> +
>> + bd->props.power = FB_BLANK_POWERDOWN;
>> + bd->props.fb_blank = FB_BLANK_POWERDOWN;
>> + bd->props.state |= BL_CORE_FBBLANK;
>> +
>> + return backlight_update_status(bd);
>> +}
>> +
>> extern struct backlight_device *backlight_device_register(const char *name,
>> struct device *dev, void *devdata, const struct backlight_ops *ops,
>> const struct backlight_properties *props);
>> --
>> 2.11.0
>>
On Tue, Jan 23, 2018 at 03:37:38PM +0100, Noralf Tr?nnes wrote:
>
> Den 22.01.2018 15.56, skrev Meghana Madhyastha:
> >Replace of_find_backlight_by_node and of the code around it
> >with of_find_backlight helper to avoid repetition of code.
> >
> >Signed-off-by: Meghana Madhyastha <[email protected]>
> >---
> >Changes in v18:
> >-Fixed warnings resulting from passing device_node* to of_find_backlight.
> > Fixed it by passing struct device* to of_find_backlight
> >
> > drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 33 ++++++++++------------------
> > 1 file changed, 11 insertions(+), 22 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> >index ac9596251..93b7a176d 100644
> >--- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> >+++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
> >@@ -156,14 +156,14 @@ static struct omap_dss_driver panel_dpi_ops = {
> > static int panel_dpi_probe_of(struct platform_device *pdev)
> > {
> > struct panel_drv_data *ddata = platform_get_drvdata(pdev);
> >+ struct device *dev = &pdev->dev;
> > struct device_node *node = pdev->dev.of_node;
> >- struct device_node *bl_node;
> > struct omap_dss_device *in;
> > int r;
> > struct display_timing timing;
> > struct gpio_desc *gpio;
> >- gpio = devm_gpiod_get_optional(&pdev->dev, "enable", GPIOD_OUT_LOW);
> >+ gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
>
> Please don't make unrelated changes like this. It clutters the patch.
> You can just as well use &pdev->dev when getting the backlight also.
I had made the change in order to be more consistent with how the other
drivers were doing this. Most of them had a variable struct device *dev.
However, I can undo this if necessary.
> > if (IS_ERR(gpio))
> > return PTR_ERR(gpio);
> >@@ -175,47 +175,36 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
> > * timing and order relative to the enable gpio. So for now it's just
> > * ensured that the reset line isn't active.
> > */
> >- gpio = devm_gpiod_get_optional(&pdev->dev, "reset", GPIOD_OUT_LOW);
> >+ gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> > if (IS_ERR(gpio))
> > return PTR_ERR(gpio);
> >- ddata->vcc_supply = devm_regulator_get(&pdev->dev, "vcc");
> >+ ddata->vcc_supply = devm_regulator_get(dev, "vcc");
> > if (IS_ERR(ddata->vcc_supply))
> > return PTR_ERR(ddata->vcc_supply);
> >- bl_node = of_parse_phandle(node, "backlight", 0);
> >- if (bl_node) {
> >- ddata->backlight = of_find_backlight_by_node(bl_node);
> >- of_node_put(bl_node);
> >+ ddata->backlight = of_find_backlight(dev);
>
> Any reason you don't use the devm_ version here?
> You do remove error_free_backlight...
>
> With the devm_ version remember to drop the put_device in
> panel_dpi_remove().
>
> Noralf.
>
> >- if (!ddata->backlight)
> >- return -EPROBE_DEFER;
> >- }
> >+ if (IS_ERR(ddata->backlight))
> >+ return PTR_ERR(ddata->backlight);
> > r = of_get_display_timing(node, "panel-timing", &timing);
> > if (r) {
> >- dev_err(&pdev->dev, "failed to get video timing\n");
> >- goto error_free_backlight;
> >+ dev_err(dev, "failed to get video timing\n");
> >+ return r;
> > }
> > videomode_from_timing(&timing, &ddata->vm);
> > in = omapdss_of_find_source_for_first_ep(node);
> > if (IS_ERR(in)) {
> >- dev_err(&pdev->dev, "failed to find video source\n");
> >- r = PTR_ERR(in);
> >- goto error_free_backlight;
> >+ dev_err(dev, "failed to find video source\n");
> >+ return PTR_ERR(in);
> > }
> > ddata->in = in;
> > return 0;
> >-
> >-error_free_backlight:
> >- if (ddata->backlight)
> >- put_device(&ddata->backlight->dev);
> >-
> >- return r;
> > }
> > static int panel_dpi_probe(struct platform_device *pdev)
>
Den 23.01.2018 17.55, skrev Meghana Madhyastha:
> On Tue, Jan 23, 2018 at 03:37:38PM +0100, Noralf Trønnes wrote:
>> Den 22.01.2018 15.56, skrev Meghana Madhyastha:
>>> Replace of_find_backlight_by_node and of the code around it
>>> with of_find_backlight helper to avoid repetition of code.
>>>
>>> Signed-off-by: Meghana Madhyastha <[email protected]>
>>> ---
>>> Changes in v18:
>>> -Fixed warnings resulting from passing device_node* to of_find_backlight.
>>> Fixed it by passing struct device* to of_find_backlight
>>>
>>> drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 33 ++++++++++------------------
>>> 1 file changed, 11 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
>>> index ac9596251..93b7a176d 100644
>>> --- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
>>> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
>>> @@ -156,14 +156,14 @@ static struct omap_dss_driver panel_dpi_ops = {
>>> static int panel_dpi_probe_of(struct platform_device *pdev)
>>> {
>>> struct panel_drv_data *ddata = platform_get_drvdata(pdev);
>>> + struct device *dev = &pdev->dev;
>>> struct device_node *node = pdev->dev.of_node;
>>> - struct device_node *bl_node;
>>> struct omap_dss_device *in;
>>> int r;
>>> struct display_timing timing;
>>> struct gpio_desc *gpio;
>>> - gpio = devm_gpiod_get_optional(&pdev->dev, "enable", GPIOD_OUT_LOW);
>>> + gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
>> Please don't make unrelated changes like this. It clutters the patch.
>> You can just as well use &pdev->dev when getting the backlight also.
> I had made the change in order to be more consistent with how the other
> drivers were doing this. Most of them had a variable struct device *dev.
> However, I can undo this if necessary.
It's best to be consistent with the coding style in the driver you're
changing. If you make an extra dev variable or not isn't that important,
unless the driver maintainer have a strict coding style for their driver.
I try to stay on the safe side, change as little as possible and do thing
the way it's done in the driver to increase the change of getting the
patch accepted as-is the first time around.
The important feedback from me is to remove the unrelated changes.
Noralf.
>
>>> if (IS_ERR(gpio))
>>> return PTR_ERR(gpio);
>>> @@ -175,47 +175,36 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
>>> * timing and order relative to the enable gpio. So for now it's just
>>> * ensured that the reset line isn't active.
>>> */
>>> - gpio = devm_gpiod_get_optional(&pdev->dev, "reset", GPIOD_OUT_LOW);
>>> + gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
>>> if (IS_ERR(gpio))
>>> return PTR_ERR(gpio);
>>> - ddata->vcc_supply = devm_regulator_get(&pdev->dev, "vcc");
>>> + ddata->vcc_supply = devm_regulator_get(dev, "vcc");
>>> if (IS_ERR(ddata->vcc_supply))
>>> return PTR_ERR(ddata->vcc_supply);
>>> - bl_node = of_parse_phandle(node, "backlight", 0);
>>> - if (bl_node) {
>>> - ddata->backlight = of_find_backlight_by_node(bl_node);
>>> - of_node_put(bl_node);
>>> + ddata->backlight = of_find_backlight(dev);
>> Any reason you don't use the devm_ version here?
>> You do remove error_free_backlight...
>>
>> With the devm_ version remember to drop the put_device in
>> panel_dpi_remove().
>>
>> Noralf.
>>
>>> - if (!ddata->backlight)
>>> - return -EPROBE_DEFER;
>>> - }
>>> + if (IS_ERR(ddata->backlight))
>>> + return PTR_ERR(ddata->backlight);
>>> r = of_get_display_timing(node, "panel-timing", &timing);
>>> if (r) {
>>> - dev_err(&pdev->dev, "failed to get video timing\n");
>>> - goto error_free_backlight;
>>> + dev_err(dev, "failed to get video timing\n");
>>> + return r;
>>> }
>>> videomode_from_timing(&timing, &ddata->vm);
>>> in = omapdss_of_find_source_for_first_ep(node);
>>> if (IS_ERR(in)) {
>>> - dev_err(&pdev->dev, "failed to find video source\n");
>>> - r = PTR_ERR(in);
>>> - goto error_free_backlight;
>>> + dev_err(dev, "failed to find video source\n");
>>> + return PTR_ERR(in);
>>> }
>>> ddata->in = in;
>>> return 0;
>>> -
>>> -error_free_backlight:
>>> - if (ddata->backlight)
>>> - put_device(&ddata->backlight->dev);
>>> -
>>> - return r;
>>> }
>>> static int panel_dpi_probe(struct platform_device *pdev)
Den 23.01.2018 18.41, skrev Noralf Trønnes:
>
> Den 23.01.2018 17.55, skrev Meghana Madhyastha:
>> On Tue, Jan 23, 2018 at 03:37:38PM +0100, Noralf Trønnes wrote:
>>> Den 22.01.2018 15.56, skrev Meghana Madhyastha:
>>>> Replace of_find_backlight_by_node and of the code around it
>>>> with of_find_backlight helper to avoid repetition of code.
>>>>
>>>> Signed-off-by: Meghana Madhyastha <[email protected]>
>>>> ---
>>>> Changes in v18:
>>>> -Fixed warnings resulting from passing device_node* to
>>>> of_find_backlight.
>>>> Fixed it by passing struct device* to of_find_backlight
>>>>
>>>> drivers/gpu/drm/omapdrm/displays/panel-dpi.c | 33
>>>> ++++++++++------------------
>>>> 1 file changed, 11 insertions(+), 22 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
>>>> b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
>>>> index ac9596251..93b7a176d 100644
>>>> --- a/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
>>>> +++ b/drivers/gpu/drm/omapdrm/displays/panel-dpi.c
>>>> @@ -156,14 +156,14 @@ static struct omap_dss_driver panel_dpi_ops = {
>>>> static int panel_dpi_probe_of(struct platform_device *pdev)
>>>> {
>>>> struct panel_drv_data *ddata = platform_get_drvdata(pdev);
>>>> + struct device *dev = &pdev->dev;
>>>> struct device_node *node = pdev->dev.of_node;
>>>> - struct device_node *bl_node;
>>>> struct omap_dss_device *in;
>>>> int r;
>>>> struct display_timing timing;
>>>> struct gpio_desc *gpio;
>>>> - gpio = devm_gpiod_get_optional(&pdev->dev, "enable",
>>>> GPIOD_OUT_LOW);
>>>> + gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
>>> Please don't make unrelated changes like this. It clutters the patch.
>>> You can just as well use &pdev->dev when getting the backlight also.
>> I had made the change in order to be more consistent with how the other
>> drivers were doing this. Most of them had a variable struct device *dev.
>> However, I can undo this if necessary.
>
> It's best to be consistent with the coding style in the driver you're
> changing. If you make an extra dev variable or not isn't that important,
> unless the driver maintainer have a strict coding style for their driver.
>
> I try to stay on the safe side, change as little as possible and do thing
> the way it's done in the driver to increase the change of getting the
> patch accepted as-is the first time around.
>
> The important feedback from me is to remove the unrelated changes.
>
and the use of the devm_ version ofc.
> Noralf.
>
>>
>>>> if (IS_ERR(gpio))
>>>> return PTR_ERR(gpio);
>>>> @@ -175,47 +175,36 @@ static int panel_dpi_probe_of(struct
>>>> platform_device *pdev)
>>>> * timing and order relative to the enable gpio. So for now
>>>> it's just
>>>> * ensured that the reset line isn't active.
>>>> */
>>>> - gpio = devm_gpiod_get_optional(&pdev->dev, "reset",
>>>> GPIOD_OUT_LOW);
>>>> + gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
>>>> if (IS_ERR(gpio))
>>>> return PTR_ERR(gpio);
>>>> - ddata->vcc_supply = devm_regulator_get(&pdev->dev, "vcc");
>>>> + ddata->vcc_supply = devm_regulator_get(dev, "vcc");
>>>> if (IS_ERR(ddata->vcc_supply))
>>>> return PTR_ERR(ddata->vcc_supply);
>>>> - bl_node = of_parse_phandle(node, "backlight", 0);
>>>> - if (bl_node) {
>>>> - ddata->backlight = of_find_backlight_by_node(bl_node);
>>>> - of_node_put(bl_node);
>>>> + ddata->backlight = of_find_backlight(dev);
>>> Any reason you don't use the devm_ version here?
>>> You do remove error_free_backlight...
>>>
>>> With the devm_ version remember to drop the put_device in
>>> panel_dpi_remove().
>>>
>>> Noralf.
>>>
>>>> - if (!ddata->backlight)
>>>> - return -EPROBE_DEFER;
>>>> - }
>>>> + if (IS_ERR(ddata->backlight))
>>>> + return PTR_ERR(ddata->backlight);
>>>> r = of_get_display_timing(node, "panel-timing", &timing);
>>>> if (r) {
>>>> - dev_err(&pdev->dev, "failed to get video timing\n");
>>>> - goto error_free_backlight;
>>>> + dev_err(dev, "failed to get video timing\n");
>>>> + return r;
>>>> }
>>>> videomode_from_timing(&timing, &ddata->vm);
>>>> in = omapdss_of_find_source_for_first_ep(node);
>>>> if (IS_ERR(in)) {
>>>> - dev_err(&pdev->dev, "failed to find video source\n");
>>>> - r = PTR_ERR(in);
>>>> - goto error_free_backlight;
>>>> + dev_err(dev, "failed to find video source\n");
>>>> + return PTR_ERR(in);
>>>> }
>>>> ddata->in = in;
>>>> return 0;
>>>> -
>>>> -error_free_backlight:
>>>> - if (ddata->backlight)
>>>> - put_device(&ddata->backlight->dev);
>>>> -
>>>> - return r;
>>>> }
>>>> static int panel_dpi_probe(struct platform_device *pdev)
>
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
On Mon, Jan 22, 2018 at 4:51 PM, Meghana Madhyastha
<[email protected]> wrote:
> Add of_find_backlight, a helper function which is a generic version
> of tinydrm_of_find_backlight that can be used by other drivers to avoid
> repetition of code and simplify things.
> +struct backlight_device *of_find_backlight(struct device *dev)
It looks strange that of_ prefixed function takes struct device
instead of struct device_node.
--
With Best Regards,
Andy Shevchenko