Den 18.01.2018 13.07, skrev Meghana Madhyastha:
On Tue, Jan 16, 2018 at 06:08:53PM +0100, Noralf Trønnes wrote:
Den 16.01.2018 11.36, skrev Meghana Madhyastha:
Replace of_find_backlight_by_node and of the code around it
with of_find_backlight helper to avoid repetition of code.

Signed-off-by: Meghana Madhyastha <meghana.madhyas...@gmail.com>
---
  drivers/gpu/drm/panel/panel-innolux-p079zca.c   | 10 +++-------
  drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c | 10 +++-------
  drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c | 10 +++-------
  3 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-innolux-p079zca.c 
b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
index 4c1b29eec..a69b0530f 100644
--- a/drivers/gpu/drm/panel/panel-innolux-p079zca.c
+++ b/drivers/gpu/drm/panel/panel-innolux-p079zca.c
@@ -230,14 +230,10 @@ static int innolux_panel_add(struct innolux_panel 
*innolux)
                innolux->enable_gpio = NULL;
        }
-       np = of_parse_phandle(dev->of_node, "backlight", 0);
-       if (np) {
-               innolux->backlight = of_find_backlight_by_node(np);
-               of_node_put(np);
+       innolux->backlight = devm_of_find_backlight(np);
This driver isn't broken like tinydrm was, so it does put the backlight
device on cleanup like it should. So when you're using the devm_ version,
the result is a double put. Just remove the put_device() in
innolux_panel_add/del() and you're good.
I have a quick question here. So devm_of_find_backlight automatically
does a put_device on detachment of the driver. However in this case,
put_device is called when panel_add is not successful (i.e when it
returns a negative value here). So is devm's release mechanism still
invoked here ?

As long as the error is propagated back up the chain, which it is in this
case, devres_release_all() is called and the resources are released.

Driver callchain passing up the error:
innolux_panel_driver.probe = innolux_panel_probe <- innolux_panel_add <- devm_of_find_backlight

This is mipi_dsi setting up the probe hook and calling into it's own version:

drivers/gpu/drm/drm_mipi_dsi.c:
int mipi_dsi_driver_register_full(struct mipi_dsi_driver *drv,
                  struct module *owner)
{
...
    if (drv->probe)
        drv->driver.probe = mipi_dsi_drv_probe;
...
}

static int mipi_dsi_drv_probe(struct device *dev)
{
    struct mipi_dsi_driver *drv = to_mipi_dsi_driver(dev->driver);
    struct mipi_dsi_device *dsi = to_mipi_dsi_device(dev);

    return drv->probe(dsi);
}

This is the device-driver core that initiates the probing:

drivers/base/dd.c
static int really_probe(struct device *dev, struct device_driver *drv)
{
...
    } else if (drv->probe) {
        ret = drv->probe(dev);
        if (ret)
            goto probe_failed;
    }
...
probe_failed:
...
    devres_release_all(dev);
...
}

        err = drm_panel_add(&innolux->base);
        if (err < 0)
                goto put_backlight;

        return 0;

        put_backlight:
                put_device(&innolux->backlight->dev);
        return err;

If so then I should change this to
        err = drm_panel_add(&innolux->base)
        return err;

Yes, and you can drop the variable:

        return drm_panel_add(&innolux->base);

Noralf.


Thanks and regards,
Meghana

I haven't checked the other drivers.

Noralf.

PS:
Give people a few days (one week?) to respond before respinning a new
version, so we avoid a fragmented discussion.

-               if (!innolux->backlight)
-                       return -EPROBE_DEFER;
-       }
+       if (IS_ERR(innolux->backlight))
+               return PTR_ERR(innolux->backlight);
        drm_panel_init(&innolux->base);
        innolux->base.funcs = &innolux_panel_funcs;
diff --git a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c 
b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
index 1512ec4f3..d5450c9d9 100644
--- a/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-lq101r1sx01.c
@@ -329,14 +329,10 @@ static int sharp_panel_add(struct sharp_panel *sharp)
        if (IS_ERR(sharp->supply))
                return PTR_ERR(sharp->supply);
-       np = of_parse_phandle(sharp->link1->dev.of_node, "backlight", 0);
-       if (np) {
-               sharp->backlight = of_find_backlight_by_node(np);
-               of_node_put(np);
+       sharp->backlight = devm_of_find_backlight(np);
-               if (!sharp->backlight)
-                       return -EPROBE_DEFER;
-       }
+       if (IS_ERR(sharp->backlight))
+               return PTR_ERR(sharp->backlight);
        drm_panel_init(&sharp->base);
        sharp->base.funcs = &sharp_panel_funcs;
diff --git a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c 
b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
index a6af3257f..db31d8268 100644
--- a/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
+++ b/drivers/gpu/drm/panel/panel-sharp-ls043t1le01.c
@@ -273,14 +273,10 @@ static int sharp_nt_panel_add(struct sharp_nt_panel 
*sharp_nt)
                gpiod_set_value(sharp_nt->reset_gpio, 0);
        }
-       np = of_parse_phandle(dev->of_node, "backlight", 0);
-       if (np) {
-               sharp_nt->backlight = of_find_backlight_by_node(np);
-               of_node_put(np);
+       sharp_nt->backlight = devm_of_find_backlight(np);
-               if (!sharp_nt->backlight)
-                       return -EPROBE_DEFER;
-       }
+       if (IS_ERR(sharp_nt->backlight))
+               return PTR_ERR(sharp_nt->backlight);
        drm_panel_init(&sharp_nt->base);
        sharp_nt->base.funcs = &sharp_nt_panel_funcs;

Reply via email to