* [PATCH v6 0/2] drm/tinydrm: drm_of_find_backlight helper @ 2017-09-30 17:10 Meghana Madhyastha 2017-09-30 17:12 ` [PATCH v6 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c Meghana Madhyastha 2017-09-30 17:14 ` [PATCH v6 2/2] drm/tinydrm: Add devres versions of drm_of_find_backlight Meghana Madhyastha 0 siblings, 2 replies; 11+ messages in thread From: Meghana Madhyastha @ 2017-09-30 17:10 UTC (permalink / raw) To: daniel, noralf, outreachy-kernel, dri-devel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v6 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c 2017-09-30 17:10 [PATCH v6 0/2] drm/tinydrm: drm_of_find_backlight helper Meghana Madhyastha @ 2017-09-30 17:12 ` Meghana Madhyastha 2017-09-30 19:04 ` Noralf Trønnes 2017-10-02 19:58 ` kbuild test robot 2017-09-30 17:14 ` [PATCH v6 2/2] drm/tinydrm: Add devres versions of drm_of_find_backlight Meghana Madhyastha 1 sibling, 2 replies; 11+ messages in thread From: Meghana Madhyastha @ 2017-09-30 17:12 UTC (permalink / raw) To: daniel, noralf, outreachy-kernel, dri-devel 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v6 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c 2017-09-30 17:12 ` [PATCH v6 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c Meghana Madhyastha @ 2017-09-30 19:04 ` Noralf Trønnes 2017-10-01 4:14 ` Meghana Madhyastha 2017-10-02 19:58 ` kbuild test robot 1 sibling, 1 reply; 11+ messages in thread From: Noralf Trønnes @ 2017-09-30 19:04 UTC (permalink / raw) To: Meghana Madhyastha, daniel, outreachy-kernel, dri-devel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c 2017-09-30 19:04 ` Noralf Trønnes @ 2017-10-01 4:14 ` Meghana Madhyastha 2017-10-01 13:26 ` Noralf Trønnes 0 siblings, 1 reply; 11+ messages in thread From: Meghana Madhyastha @ 2017-10-01 4:14 UTC (permalink / raw) To: Noralf Trønnes, daniel, outreachy-kernel, dri-devel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c 2017-10-01 4:14 ` Meghana Madhyastha @ 2017-10-01 13:26 ` Noralf Trønnes 2017-10-01 13:34 ` Meghana Madhyastha 0 siblings, 1 reply; 11+ messages in thread From: Noralf Trønnes @ 2017-10-01 13:26 UTC (permalink / raw) To: Meghana Madhyastha, daniel, outreachy-kernel, dri-devel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c 2017-10-01 13:26 ` Noralf Trønnes @ 2017-10-01 13:34 ` Meghana Madhyastha 2017-10-01 14:47 ` Noralf Trønnes 0 siblings, 1 reply; 11+ messages in thread From: Meghana Madhyastha @ 2017-10-01 13:34 UTC (permalink / raw) To: Noralf Trønnes, daniel, outreachy-kernel, dri-devel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c 2017-10-01 13:34 ` Meghana Madhyastha @ 2017-10-01 14:47 ` Noralf Trønnes 2017-10-01 17:35 ` Meghana Madhyastha 0 siblings, 1 reply; 11+ messages in thread From: Noralf Trønnes @ 2017-10-01 14:47 UTC (permalink / raw) To: Meghana Madhyastha, daniel, outreachy-kernel, dri-devel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c 2017-10-01 14:47 ` Noralf Trønnes @ 2017-10-01 17:35 ` Meghana Madhyastha 0 siblings, 0 replies; 11+ messages in thread From: Meghana Madhyastha @ 2017-10-01 17:35 UTC (permalink / raw) To: Noralf Trønnes, daniel, outreachy-kernel, dri-devel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v6 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c 2017-09-30 17:12 ` [PATCH v6 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c Meghana Madhyastha 2017-09-30 19:04 ` Noralf Trønnes @ 2017-10-02 19:58 ` kbuild test robot 1 sibling, 0 replies; 11+ messages in thread From: kbuild test robot @ 2017-10-02 19:58 UTC (permalink / raw) To: Meghana Madhyastha; +Cc: outreachy-kernel, dri-devel, kbuild-all [-- 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v6 2/2] drm/tinydrm: Add devres versions of drm_of_find_backlight 2017-09-30 17:10 [PATCH v6 0/2] drm/tinydrm: drm_of_find_backlight helper Meghana Madhyastha 2017-09-30 17:12 ` [PATCH v6 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c Meghana Madhyastha @ 2017-09-30 17:14 ` Meghana Madhyastha 2017-10-02 20:16 ` kbuild test robot 1 sibling, 1 reply; 11+ messages in thread From: Meghana Madhyastha @ 2017-09-30 17:14 UTC (permalink / raw) To: daniel, noralf, outreachy-kernel, dri-devel 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v6 2/2] drm/tinydrm: Add devres versions of drm_of_find_backlight 2017-09-30 17:14 ` [PATCH v6 2/2] drm/tinydrm: Add devres versions of drm_of_find_backlight Meghana Madhyastha @ 2017-10-02 20:16 ` kbuild test robot 0 siblings, 0 replies; 11+ messages in thread From: kbuild test robot @ 2017-10-02 20:16 UTC (permalink / raw) To: Meghana Madhyastha; +Cc: outreachy-kernel, dri-devel, kbuild-all [-- 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-10-02 20:17 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-09-30 17:10 [PATCH v6 0/2] drm/tinydrm: drm_of_find_backlight helper Meghana Madhyastha 2017-09-30 17:12 ` [PATCH v6 1/2] drm/tinydrm: Move tinydrm_of_find_backlight into drm_of.c Meghana Madhyastha 2017-09-30 19:04 ` Noralf Trønnes 2017-10-01 4:14 ` Meghana Madhyastha 2017-10-01 13:26 ` Noralf Trønnes 2017-10-01 13:34 ` Meghana Madhyastha 2017-10-01 14:47 ` Noralf Trønnes 2017-10-01 17:35 ` Meghana Madhyastha 2017-10-02 19:58 ` kbuild test robot 2017-09-30 17:14 ` [PATCH v6 2/2] drm/tinydrm: Add devres versions of drm_of_find_backlight Meghana Madhyastha 2017-10-02 20:16 ` kbuild test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox