* [PATCH 1/2] drm/fsl-dcu: remove unneeded 'ret' assignment
@ 2016-12-28 16:48 Fabio Estevam
2016-12-28 16:48 ` [PATCH 2/2] drm/fsl-dcu: check for clk_prepare_enable() error Fabio Estevam
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Fabio Estevam @ 2016-12-28 16:48 UTC (permalink / raw)
To: stefan; +Cc: Fabio Estevam, dri-devel
From: Fabio Estevam <fabio.estevam@nxp.com>
When devm_kzalloc() fails there is no need to assign an error code
to the 'ret' variable as it will not be used after jumping to the
'err_node_put' label, so just remove the assignment.
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
drivers/gpu/drm/fsl-dcu/fsl_tcon.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_tcon.c b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
index 3194e54..2fbb7ee 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
@@ -72,10 +72,8 @@ struct fsl_tcon *fsl_tcon_init(struct device *dev)
return NULL;
tcon = devm_kzalloc(dev, sizeof(*tcon), GFP_KERNEL);
- if (!tcon) {
- ret = -ENOMEM;
+ if (!tcon)
goto err_node_put;
- }
ret = fsl_tcon_init_regmap(dev, tcon, np);
if (ret) {
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/2] drm/fsl-dcu: check for clk_prepare_enable() error
2016-12-28 16:48 [PATCH 1/2] drm/fsl-dcu: remove unneeded 'ret' assignment Fabio Estevam
@ 2016-12-28 16:48 ` Fabio Estevam
2016-12-28 18:38 ` Gabriel Krisman Bertazi
2016-12-28 18:49 ` [PATCH 1/2] drm/fsl-dcu: remove unneeded 'ret' assignment Gabriel Krisman Bertazi
2017-02-08 5:11 ` Stefan Agner
2 siblings, 1 reply; 8+ messages in thread
From: Fabio Estevam @ 2016-12-28 16:48 UTC (permalink / raw)
To: stefan; +Cc: Fabio Estevam, dri-devel
From: Fabio Estevam <fabio.estevam@nxp.com>
clk_prepare_enable() may fail, so we should better check its return
value.
Also place the of_node_put() function right after clk_prepare_enable(),
in order to avoid calling of_node_put() twice in case clk_prepare_enable()
fails.
Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
---
drivers/gpu/drm/fsl-dcu/fsl_tcon.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_tcon.c b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
index 2fbb7ee..b3d70a6 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
@@ -87,9 +87,13 @@ struct fsl_tcon *fsl_tcon_init(struct device *dev)
goto err_node_put;
}
- of_node_put(np);
- clk_prepare_enable(tcon->ipg_clk);
+ ret = clk_prepare_enable(tcon->ipg_clk);
+ if (ret) {
+ dev_err(dev, "Couldn't enable the TCON clock\n");
+ goto err_node_put;
+ }
+ of_node_put(np);
dev_info(dev, "Using TCON in bypass mode\n");
return tcon;
--
2.7.4
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH 2/2] drm/fsl-dcu: check for clk_prepare_enable() error
2016-12-28 16:48 ` [PATCH 2/2] drm/fsl-dcu: check for clk_prepare_enable() error Fabio Estevam
@ 2016-12-28 18:38 ` Gabriel Krisman Bertazi
2016-12-28 18:44 ` Fabio Estevam
0 siblings, 1 reply; 8+ messages in thread
From: Gabriel Krisman Bertazi @ 2016-12-28 18:38 UTC (permalink / raw)
To: Fabio Estevam; +Cc: Fabio Estevam, dri-devel
Fabio Estevam <festevam@gmail.com> writes:
> From: Fabio Estevam <fabio.estevam@nxp.com>
>
> clk_prepare_enable() may fail, so we should better check its return
> value.
>
> Also place the of_node_put() function right after clk_prepare_enable(),
> in order to avoid calling of_node_put() twice in case clk_prepare_enable()
> fails.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> drivers/gpu/drm/fsl-dcu/fsl_tcon.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_tcon.c b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
> index 2fbb7ee..b3d70a6 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
> @@ -87,9 +87,13 @@ struct fsl_tcon *fsl_tcon_init(struct device *dev)
> goto err_node_put;
> }
>
> - of_node_put(np);
> - clk_prepare_enable(tcon->ipg_clk);
> + ret = clk_prepare_enable(tcon->ipg_clk);
> + if (ret) {
> + dev_err(dev, "Couldn't enable the TCON clock\n");
> + goto err_node_put;
> + }
This leaks tcon if clk_prepare_enable fails.
>
> + of_node_put(np);
> dev_info(dev, "Using TCON in bypass mode\n");
>
> return tcon;
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 2/2] drm/fsl-dcu: check for clk_prepare_enable() error
2016-12-28 18:38 ` Gabriel Krisman Bertazi
@ 2016-12-28 18:44 ` Fabio Estevam
2016-12-28 18:53 ` Gabriel Krisman Bertazi
0 siblings, 1 reply; 8+ messages in thread
From: Fabio Estevam @ 2016-12-28 18:44 UTC (permalink / raw)
To: Gabriel Krisman Bertazi; +Cc: Fabio Estevam, DRI mailing list
On Wed, Dec 28, 2016 at 4:38 PM, Gabriel Krisman Bertazi
<krisman@collabora.co.uk> wrote:
> This leaks tcon if clk_prepare_enable fails.
No, it does not as tcon is allocated via devm_kzalloc().
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/fsl-dcu: check for clk_prepare_enable() error
2016-12-28 18:44 ` Fabio Estevam
@ 2016-12-28 18:53 ` Gabriel Krisman Bertazi
2016-12-29 7:12 ` Stefan Agner
0 siblings, 1 reply; 8+ messages in thread
From: Gabriel Krisman Bertazi @ 2016-12-28 18:53 UTC (permalink / raw)
To: Fabio Estevam; +Cc: Fabio Estevam, DRI mailing list
Fabio Estevam <festevam@gmail.com> writes:
> On Wed, Dec 28, 2016 at 4:38 PM, Gabriel Krisman Bertazi
> <krisman@collabora.co.uk> wrote:
>
>> This leaks tcon if clk_prepare_enable fails.
>
> No, it does not as tcon is allocated via devm_kzalloc().
Agreed. But I think devm_kzalloc() only drops the memory once the
device is removed, which is not the case here, since failing
fsl_tcon_init is not fatal and the initialization continues in this
case.
--
Gabriel Krisman Bertazi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] drm/fsl-dcu: check for clk_prepare_enable() error
2016-12-28 18:53 ` Gabriel Krisman Bertazi
@ 2016-12-29 7:12 ` Stefan Agner
0 siblings, 0 replies; 8+ messages in thread
From: Stefan Agner @ 2016-12-29 7:12 UTC (permalink / raw)
To: Gabriel Krisman Bertazi; +Cc: Fabio Estevam, DRI mailing list
On 2016-12-28 19:53, Gabriel Krisman Bertazi wrote:
> Fabio Estevam <festevam@gmail.com> writes:
>
>> On Wed, Dec 28, 2016 at 4:38 PM, Gabriel Krisman Bertazi
>> <krisman@collabora.co.uk> wrote:
>>
>>> This leaks tcon if clk_prepare_enable fails.
>>
>> No, it does not as tcon is allocated via devm_kzalloc().
>
> Agreed. But I think devm_kzalloc() only drops the memory once the
> device is removed, which is not the case here, since failing
> fsl_tcon_init is not fatal and the initialization continues in this
> case.
That is probably true, but does it matter? I guess it is anyway an
unlikely case, so why bother?
--
Stefan
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] drm/fsl-dcu: remove unneeded 'ret' assignment
2016-12-28 16:48 [PATCH 1/2] drm/fsl-dcu: remove unneeded 'ret' assignment Fabio Estevam
2016-12-28 16:48 ` [PATCH 2/2] drm/fsl-dcu: check for clk_prepare_enable() error Fabio Estevam
@ 2016-12-28 18:49 ` Gabriel Krisman Bertazi
2017-02-08 5:11 ` Stefan Agner
2 siblings, 0 replies; 8+ messages in thread
From: Gabriel Krisman Bertazi @ 2016-12-28 18:49 UTC (permalink / raw)
To: Fabio Estevam; +Cc: Fabio Estevam, dri-devel
Fabio Estevam <festevam@gmail.com> writes:
> From: Fabio Estevam <fabio.estevam@nxp.com>
>
> When devm_kzalloc() fails there is no need to assign an error code
> to the 'ret' variable as it will not be used after jumping to the
> 'err_node_put' label, so just remove the assignment.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
> ---
> drivers/gpu/drm/fsl-dcu/fsl_tcon.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_tcon.c b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
> index 3194e54..2fbb7ee 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
> @@ -72,10 +72,8 @@ struct fsl_tcon *fsl_tcon_init(struct device *dev)
> return NULL;
>
> tcon = devm_kzalloc(dev, sizeof(*tcon), GFP_KERNEL);
> - if (!tcon) {
> - ret = -ENOMEM;
> + if (!tcon)
> goto err_node_put;
> - }
Hi Fabio,
I'm curious if it is ok to not have a tcon even for devices that support
it? If we hit -ENOMEM here, I suppose we're likely hit it too for
drm_dev_alloc and give up the initialization, so either way
Reviewed-by: Gabriel Krisman Bertazi <krisman@collabora.co.uk>
>
> ret = fsl_tcon_init_regmap(dev, tcon, np);
> if (ret) {
--
Gabriel Krisman Bertazi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH 1/2] drm/fsl-dcu: remove unneeded 'ret' assignment
2016-12-28 16:48 [PATCH 1/2] drm/fsl-dcu: remove unneeded 'ret' assignment Fabio Estevam
2016-12-28 16:48 ` [PATCH 2/2] drm/fsl-dcu: check for clk_prepare_enable() error Fabio Estevam
2016-12-28 18:49 ` [PATCH 1/2] drm/fsl-dcu: remove unneeded 'ret' assignment Gabriel Krisman Bertazi
@ 2017-02-08 5:11 ` Stefan Agner
2 siblings, 0 replies; 8+ messages in thread
From: Stefan Agner @ 2017-02-08 5:11 UTC (permalink / raw)
To: Fabio Estevam; +Cc: Fabio Estevam, dri-devel
On 2016-12-28 08:48, Fabio Estevam wrote:
> From: Fabio Estevam <fabio.estevam@nxp.com>
>
> When devm_kzalloc() fails there is no need to assign an error code
> to the 'ret' variable as it will not be used after jumping to the
> 'err_node_put' label, so just remove the assignment.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@nxp.com>
Applied both to my drm-fsl-dcu tree.
--
Stefan
> ---
> drivers/gpu/drm/fsl-dcu/fsl_tcon.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
> b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
> index 3194e54..2fbb7ee 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
> @@ -72,10 +72,8 @@ struct fsl_tcon *fsl_tcon_init(struct device *dev)
> return NULL;
>
> tcon = devm_kzalloc(dev, sizeof(*tcon), GFP_KERNEL);
> - if (!tcon) {
> - ret = -ENOMEM;
> + if (!tcon)
> goto err_node_put;
> - }
>
> ret = fsl_tcon_init_regmap(dev, tcon, np);
> if (ret) {
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-02-08 5:11 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-28 16:48 [PATCH 1/2] drm/fsl-dcu: remove unneeded 'ret' assignment Fabio Estevam
2016-12-28 16:48 ` [PATCH 2/2] drm/fsl-dcu: check for clk_prepare_enable() error Fabio Estevam
2016-12-28 18:38 ` Gabriel Krisman Bertazi
2016-12-28 18:44 ` Fabio Estevam
2016-12-28 18:53 ` Gabriel Krisman Bertazi
2016-12-29 7:12 ` Stefan Agner
2016-12-28 18:49 ` [PATCH 1/2] drm/fsl-dcu: remove unneeded 'ret' assignment Gabriel Krisman Bertazi
2017-02-08 5:11 ` Stefan Agner
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.