Messages in this thread Patch in this message | | | Subject | Re: [PATCH v16 09/10] drm/panel: Use of_find_backlight helper | From | Noralf Trønnes <> | Date | Thu, 18 Jan 2018 15:51:12 +0100 |
| |
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.madhyastha@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;
| |