| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 18 Jan 2018 15:51:12 +0100
From: Noralf Trønnes <noralf@...nnes.org>
To: Meghana Madhyastha <meghana.madhyastha@...il.com>,
Lee Jones <lee.jones@...aro.org>,
Daniel Thompson <daniel.thompson@...aro.org>,
Jingoo Han <jingoohan1@...il.com>,
Thierry Reding <thierry.reding@...il.com>,
Tomi Valkeinen <tomi.valkeinen@...com>,
Daniel Vetter <daniel.vetter@...el.com>,
Sean Paul <seanpaul@...omium.org>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v16 09/10] drm/panel: Use of_find_backlight helper
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@...il.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;
Powered by blists - more mailing lists