Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 18 Jan 2018 17:37:32 +0530
From:   Meghana Madhyastha <meghana.madhyastha@gmail.com>
To:     Noralf =?iso-8859-1?Q?Tr=F8nnes?= <noralf@tronnes.org>,
        Lee Jones <lee.jones@linaro.org>,
        Daniel Thompson <daniel.thompson@linaro.org>,
        Jingoo Han <jingoohan1@gmail.com>,
        Thierry Reding <thierry.reding@gmail.com>,
        Tomi Valkeinen <tomi.valkeinen@ti.com>,
        Daniel Vetter <daniel.vetter@intel.com>,
        Sean Paul <seanpaul@chromium.org>,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v16 09/10] drm/panel: Use of_find_backlight helper
Message-ID: <20180118120732.GC4864@meghana-HP-Pavilion-Notebook>
References: <cover.1516098341.git.meghana.madhyastha@gmail.com>
 <d44db943ccd6680c060060b263b2ec8c44e9ff1b.1516098341.git.meghana.madhyastha@gmail.com>
 <af25028c-8134-8178-d3fa-2cb76baaf3aa@tronnes.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <af25028c-8134-8178-d3fa-2cb76baaf3aa@tronnes.org>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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 ?

	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;

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;
>