[Outreachy kernel] [PATCH v14 2/3] drm/tinydrm: Move tinydrm_of_find_backlight to backlight.c
Daniel Vetter
daniel at ffwll.ch
Fri Dec 8 08:52:34 UTC 2017
On Thu, Dec 07, 2017 at 05:01:21PM +0100, Noralf Trønnes wrote:
>
> Den 07.12.2017 16.44, skrev Daniel Thompson:
> > On 06/12/17 10:43, Meghana Madhyastha wrote:
> > > On Tue, Oct 24, 2017 at 06:45:34PM +0200, Noralf Trønnes wrote:
> > > >
> > > > 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 at 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.
> > >
> > > Is there anything else that needs to be done for this patch ? If not,
> > > and it is ready, can this be acked ? It seems to have been stuck for
> > > some time now and I will definitely fix anything that needs to be fixed
> > > in this patch ASAP.
> >
> > I've lost track a little but have two questions:
> >
> > 1. Is there any pending feedback? If not a resend might be sensible.
> >
> > 2. Which maintainers are you expecting to pick up the patch? It looks to
> > me like lots of people are be missing from the To: fields (compared
> > to what I think get_maintainer.pl would give you) meaning the patches
> > won't get picked up.
> >
>
> I'm a bit confused since the individual patches are touching both the
> drm and backlight subsystem. Will this be merged through drm?
> I'd expect one patch to add the backlight stuff in that subsystem
> and then some to do the drm side.
Get an ack for the backlight patch to land through drm, then push the
entire pile into drm-misc. topic branches for individual patches are imo
overkill, and backlight doesn't seem like a fast-moving subsystem.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the dri-devel
mailing list