From: "Noralf Trønnes" <noralf@tronnes.org> To: Sean Paul <seanpaul@chromium.org>, Meghana Madhyastha <meghana.madhyastha@gmail.com> Cc: outreachy-kernel@googlegroups.com, daniel.thompson@linaro.org, dri-devel@lists.freedesktop.org Subject: Re: [Outreachy kernel] [PATCH v14 2/3] drm/tinydrm: Move tinydrm_of_find_backlight to backlight.c Date: Tue, 24 Oct 2017 18:45:34 +0200 [thread overview] Message-ID: <73f488c6-b2ef-e5a4-5ff6-17ef9758c8f0@tronnes.org> (raw) In-Reply-To: <20171024154202.5h63j3j3rvbadolo@art_vandelay> Den 24.10.2017 17.42, skrev Sean Paul: > On Sat, Oct 21, 2017 at 05:27:33PM +0530, Meghana Madhyastha wrote: >> Rename tinydrm_of_find_backlight to of_find_backlight and move >> it to linux/backlight.c so that it can be used by other drivers. >> >> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com> >> --- >> Changes in v14: >> -s/backlight_get/of_find_backlight/ >> >> drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 -------------------------- >> drivers/gpu/drm/tinydrm/mi0283qt.c | 3 +- >> drivers/video/backlight/backlight.c | 37 ++++++++++++++++++++++++ >> include/drm/tinydrm/tinydrm-helpers.h | 2 -- >> include/linux/backlight.h | 19 ++++++++++++ >> 5 files changed, 58 insertions(+), 43 deletions(-) >> >> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c >> index a42dee6..cb1a01a 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 7fd2691..53ab5a0 100644 >> --- a/drivers/gpu/drm/tinydrm/mi0283qt.c >> +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c >> @@ -12,6 +12,7 @@ >> #include <drm/tinydrm/ili9341.h> >> #include <drm/tinydrm/mipi-dbi.h> >> #include <drm/tinydrm/tinydrm-helpers.h> >> +#include <linux/backlight.h> >> #include <linux/delay.h> >> #include <linux/gpio/consumer.h> >> #include <linux/module.h> >> @@ -188,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 = of_find_backlight(dev); > Sorry for the follow-up spam, but are you missing the put_device somewhere? The > next patch uses devm_of_find_backlight. So AFAICT you're either leaking a reference > here, or you're closing an additional reference in the next patch. This is my fault, put_device() has been missing all along, so Meghana is plugging that hole, in the next patch :-) Noralf. > Sean > >> if (IS_ERR(mipi->backlight)) >> return PTR_ERR(mipi->backlight); >> >> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c >> index 8049e76..76d46c3 100644 >> --- a/drivers/video/backlight/backlight.c >> +++ b/drivers/video/backlight/backlight.c >> @@ -580,6 +580,43 @@ 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); >> + } >> + } >> + >> + return bd; >> +} >> +EXPORT_SYMBOL(of_find_backlight); >> + >> static void __exit backlight_class_exit(void) >> { >> class_destroy(backlight_class); >> diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h >> index f54fae0..0a4ddbc 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, >> diff --git a/include/linux/backlight.h b/include/linux/backlight.h >> index b88fabb..f98b684 100644 >> --- a/include/linux/backlight.h >> +++ b/include/linux/backlight.h >> @@ -159,6 +159,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); >> @@ -202,4 +212,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.7.4 >> >> -- >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. >> To post to this group, send email to outreachy-kernel@googlegroups.com. >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/678c3adb07b729acf6e80119ab9adec01d89842f.1508586429.git.meghana.madhyastha%40gmail.com. >> For more options, visit https://groups.google.com/d/optout. _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: "Noralf Trønnes" <noralf@tronnes.org> To: Sean Paul <seanpaul@chromium.org>, Meghana Madhyastha <meghana.madhyastha@gmail.com> Cc: daniel@ffwll.ch, outreachy-kernel@googlegroups.com, dri-devel@lists.freedesktop.org, daniel.thompson@linaro.org, jani.nikula@linux.intel.com Subject: Re: [Outreachy kernel] [PATCH v14 2/3] drm/tinydrm: Move tinydrm_of_find_backlight to backlight.c Date: Tue, 24 Oct 2017 18:45:34 +0200 [thread overview] Message-ID: <73f488c6-b2ef-e5a4-5ff6-17ef9758c8f0@tronnes.org> (raw) In-Reply-To: <20171024154202.5h63j3j3rvbadolo@art_vandelay> Den 24.10.2017 17.42, skrev Sean Paul: > On Sat, Oct 21, 2017 at 05:27:33PM +0530, Meghana Madhyastha wrote: >> Rename tinydrm_of_find_backlight to of_find_backlight and move >> it to linux/backlight.c so that it can be used by other drivers. >> >> Signed-off-by: Meghana Madhyastha <meghana.madhyastha@gmail.com> >> --- >> Changes in v14: >> -s/backlight_get/of_find_backlight/ >> >> drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 40 -------------------------- >> drivers/gpu/drm/tinydrm/mi0283qt.c | 3 +- >> drivers/video/backlight/backlight.c | 37 ++++++++++++++++++++++++ >> include/drm/tinydrm/tinydrm-helpers.h | 2 -- >> include/linux/backlight.h | 19 ++++++++++++ >> 5 files changed, 58 insertions(+), 43 deletions(-) >> >> diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c b/drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c >> index a42dee6..cb1a01a 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 7fd2691..53ab5a0 100644 >> --- a/drivers/gpu/drm/tinydrm/mi0283qt.c >> +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c >> @@ -12,6 +12,7 @@ >> #include <drm/tinydrm/ili9341.h> >> #include <drm/tinydrm/mipi-dbi.h> >> #include <drm/tinydrm/tinydrm-helpers.h> >> +#include <linux/backlight.h> >> #include <linux/delay.h> >> #include <linux/gpio/consumer.h> >> #include <linux/module.h> >> @@ -188,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 = of_find_backlight(dev); > Sorry for the follow-up spam, but are you missing the put_device somewhere? The > next patch uses devm_of_find_backlight. So AFAICT you're either leaking a reference > here, or you're closing an additional reference in the next patch. This is my fault, put_device() has been missing all along, so Meghana is plugging that hole, in the next patch :-) Noralf. > Sean > >> if (IS_ERR(mipi->backlight)) >> return PTR_ERR(mipi->backlight); >> >> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c >> index 8049e76..76d46c3 100644 >> --- a/drivers/video/backlight/backlight.c >> +++ b/drivers/video/backlight/backlight.c >> @@ -580,6 +580,43 @@ 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); >> + } >> + } >> + >> + return bd; >> +} >> +EXPORT_SYMBOL(of_find_backlight); >> + >> static void __exit backlight_class_exit(void) >> { >> class_destroy(backlight_class); >> diff --git a/include/drm/tinydrm/tinydrm-helpers.h b/include/drm/tinydrm/tinydrm-helpers.h >> index f54fae0..0a4ddbc 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, >> diff --git a/include/linux/backlight.h b/include/linux/backlight.h >> index b88fabb..f98b684 100644 >> --- a/include/linux/backlight.h >> +++ b/include/linux/backlight.h >> @@ -159,6 +159,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); >> @@ -202,4 +212,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.7.4 >> >> -- >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. >> To post to this group, send email to outreachy-kernel@googlegroups.com. >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/678c3adb07b729acf6e80119ab9adec01d89842f.1508586429.git.meghana.madhyastha%40gmail.com. >> For more options, visit https://groups.google.com/d/optout.
next prev parent reply other threads:[~2017-10-24 16:45 UTC|newest] Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-10-21 11:55 [PATCH v14 0/3] Move backlight helper functions from tinydrm-helpers to linux/backlight Meghana Madhyastha 2017-10-21 11:55 ` Meghana Madhyastha 2017-10-21 11:56 ` [PATCH v14 1/3] drm/tinydrm: Move helper functions from tinydrm-helpers to backlight.h Meghana Madhyastha 2017-10-21 11:56 ` Meghana Madhyastha 2017-10-24 15:37 ` [Outreachy kernel] " Sean Paul 2017-10-24 15:37 ` Sean Paul 2017-10-21 11:57 ` [PATCH v14 2/3] drm/tinydrm: Move tinydrm_of_find_backlight to backlight.c Meghana Madhyastha 2017-10-21 11:57 ` Meghana Madhyastha 2017-10-24 15:38 ` [Outreachy kernel] " Sean Paul 2017-10-24 15:38 ` Sean Paul 2017-10-24 15:56 ` Daniel Thompson 2017-10-24 15:56 ` Daniel Thompson 2017-10-25 7:39 ` Sean Paul 2017-10-25 7:39 ` Sean Paul 2017-10-27 15:12 ` Daniel Thompson 2017-10-27 15:12 ` Daniel Thompson 2017-10-24 15:42 ` Sean Paul 2017-10-24 15:42 ` Sean Paul 2017-10-24 16:45 ` Noralf Trønnes [this message] 2017-10-24 16:45 ` Noralf Trønnes 2017-12-06 10:43 ` Meghana Madhyastha 2017-12-07 15:44 ` Daniel Thompson 2017-12-07 16:01 ` Noralf Trønnes 2017-12-08 8:52 ` Daniel Vetter 2017-10-21 11:59 ` [PATCH v14 3/3] drm/tinydrm: Add devres versions of of_find_backlight Meghana Madhyastha 2017-10-21 11:59 ` Meghana Madhyastha 2017-10-23 10:34 ` [PATCH v14 0/3] Move backlight helper functions from tinydrm-helpers to linux/backlight Daniel Thompson 2017-10-23 10:34 ` Daniel Thompson 2017-12-09 14:09 ` Noralf Trønnes 2017-12-09 14:09 ` Noralf Trønnes 2017-12-11 13:17 ` Meghana Madhyastha 2017-12-11 14:12 ` Noralf Trønnes 2017-12-11 14:58 ` Meghana Madhyastha 2017-12-11 17:45 ` Noralf Trønnes 2017-12-11 17:56 ` Noralf Trønnes 2017-12-21 10:52 ` Noralf Trønnes 2017-12-21 13:05 ` Daniel Vetter 2017-12-21 13:44 ` Noralf Trønnes 2017-12-21 14:08 ` Daniel Vetter 2017-12-21 17:06 ` Noralf Trønnes 2018-01-09 13:50 ` Rob Herring 2017-12-26 6:39 ` Meghana Madhyastha 2018-01-02 17:23 ` Noralf Trønnes
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=73f488c6-b2ef-e5a4-5ff6-17ef9758c8f0@tronnes.org \ --to=noralf@tronnes.org \ --cc=daniel.thompson@linaro.org \ --cc=dri-devel@lists.freedesktop.org \ --cc=meghana.madhyastha@gmail.com \ --cc=outreachy-kernel@googlegroups.com \ --cc=seanpaul@chromium.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.