From: Zhou Zhu <zzhu3@marvell.com>
To: linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] video: mmp: drop needless devm cleanup
Date: Tue, 24 Sep 2013 07:55:13 +0000 [thread overview]
Message-ID: <52414561.1010000@marvell.com> (raw)
In-Reply-To: <5241406B.8090106@ti.com>
On 09/24/2013 03:34 PM, Tomi Valkeinen wrote:
> On 23/09/13 19:19, Russell King - ARM Linux wrote:
>> On Mon, Sep 23, 2013 at 06:13:10PM +0200, Uwe Kleine-König wrote:
>>> The nice thing about devm_* is that the driver doesn't need to free the
>>> resources but the driver core takes care about that. This also
>>> simplifies the error path quite a bit and removes the wrong check for a
>>> clock pointer being NULL.
>>>
>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>>> ---
>>> drivers/video/mmp/hw/mmp_ctrl.c | 17 ++---------------
>>> 1 file changed, 2 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/video/mmp/hw/mmp_ctrl.c b/drivers/video/mmp/hw/mmp_ctrl.c
>>> index 75dca19..6ac7552 100644
>>> --- a/drivers/video/mmp/hw/mmp_ctrl.c
>>> +++ b/drivers/video/mmp/hw/mmp_ctrl.c
>>> @@ -514,7 +514,7 @@ static int mmphw_probe(struct platform_device *pdev)
>>> if (IS_ERR(ctrl->clk)) {
>>> dev_err(ctrl->dev, "unable to get clk %s\n", mi->clk_name);
>>> ret = -ENOENT;
>>> - goto failed_get_clk;
>>> + goto failed;
>>> }
>>> clk_prepare_enable(ctrl->clk);
>>>
>>> @@ -551,21 +551,8 @@ failed_path_init:
>>> path_deinit(path_plat);
>>> }
>>>
>>> - if (ctrl->clk) {
>>> - devm_clk_put(ctrl->dev, ctrl->clk);
>>> - clk_disable_unprepare(ctrl->clk);
>>
>> And this patch also fixes the above: disabling/unpreparing _after_ putting
>> the thing - which was quite silly... :)
>
> Hmm, I wonder if that causes any issues... I.e. should this patch go for
> 3.12, or is 3.13 fine?
>
> Tomi
>
It would cause oops if probe failed due to some reason - although it
would almost never happen so we missed it.
Thank you for finding it out.
--
Thanks, -Zhou
WARNING: multiple messages have this Message-ID (diff)
From: zzhu3@marvell.com (Zhou Zhu)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] video: mmp: drop needless devm cleanup
Date: Tue, 24 Sep 2013 15:55:13 +0800 [thread overview]
Message-ID: <52414561.1010000@marvell.com> (raw)
In-Reply-To: <5241406B.8090106@ti.com>
On 09/24/2013 03:34 PM, Tomi Valkeinen wrote:
> On 23/09/13 19:19, Russell King - ARM Linux wrote:
>> On Mon, Sep 23, 2013 at 06:13:10PM +0200, Uwe Kleine-K?nig wrote:
>>> The nice thing about devm_* is that the driver doesn't need to free the
>>> resources but the driver core takes care about that. This also
>>> simplifies the error path quite a bit and removes the wrong check for a
>>> clock pointer being NULL.
>>>
>>> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
>>> ---
>>> drivers/video/mmp/hw/mmp_ctrl.c | 17 ++---------------
>>> 1 file changed, 2 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/drivers/video/mmp/hw/mmp_ctrl.c b/drivers/video/mmp/hw/mmp_ctrl.c
>>> index 75dca19..6ac7552 100644
>>> --- a/drivers/video/mmp/hw/mmp_ctrl.c
>>> +++ b/drivers/video/mmp/hw/mmp_ctrl.c
>>> @@ -514,7 +514,7 @@ static int mmphw_probe(struct platform_device *pdev)
>>> if (IS_ERR(ctrl->clk)) {
>>> dev_err(ctrl->dev, "unable to get clk %s\n", mi->clk_name);
>>> ret = -ENOENT;
>>> - goto failed_get_clk;
>>> + goto failed;
>>> }
>>> clk_prepare_enable(ctrl->clk);
>>>
>>> @@ -551,21 +551,8 @@ failed_path_init:
>>> path_deinit(path_plat);
>>> }
>>>
>>> - if (ctrl->clk) {
>>> - devm_clk_put(ctrl->dev, ctrl->clk);
>>> - clk_disable_unprepare(ctrl->clk);
>>
>> And this patch also fixes the above: disabling/unpreparing _after_ putting
>> the thing - which was quite silly... :)
>
> Hmm, I wonder if that causes any issues... I.e. should this patch go for
> 3.12, or is 3.13 fine?
>
> Tomi
>
It would cause oops if probe failed due to some reason - although it
would almost never happen so we missed it.
Thank you for finding it out.
--
Thanks, -Zhou
next prev parent reply other threads:[~2013-09-24 7:55 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-09-23 15:45 [PATCH] clk: fix function name in devm_clk_put kernel-doc comment Uwe Kleine-König
2013-09-23 15:53 ` [PATCH] serial: clps711x: drop needless devm_clk_put Uwe Kleine-König
2013-09-23 15:53 ` Uwe Kleine-König
2013-09-23 16:00 ` [PATCH] remoteproc/davinci: " Uwe Kleine-König
2014-02-24 15:07 ` Ohad Ben-Cohen
2013-09-23 16:13 ` [PATCH] video: mmp: drop needless devm cleanup Uwe Kleine-König
2013-09-23 16:13 ` Uwe Kleine-König
2013-09-23 16:19 ` Russell King - ARM Linux
2013-09-23 16:19 ` Russell King - ARM Linux
2013-09-24 7:34 ` Tomi Valkeinen
2013-09-24 7:34 ` Tomi Valkeinen
2013-09-24 7:55 ` Zhou Zhu [this message]
2013-09-24 7:55 ` Zhou Zhu
2013-09-26 8:43 ` Tomi Valkeinen
2013-09-26 8:43 ` Tomi Valkeinen
2013-09-23 16:28 ` [PATCH] clk: fix function name in devm_clk_put kernel-doc comment Russell King - ARM Linux
2013-09-24 18:12 ` [PATCH] ASoC: kirkwood: simplify clock handling Uwe Kleine-König
2013-09-24 18:12 ` Uwe Kleine-König
2013-09-24 18:38 ` Russell King - ARM Linux
2013-09-24 18:38 ` Russell King - ARM Linux
2013-09-24 19:04 ` Jean-Francois Moine
2013-09-24 19:04 ` Jean-Francois Moine
2013-09-24 19:05 ` Russell King - ARM Linux
2013-09-24 19:05 ` Russell King - ARM Linux
2013-09-24 19:24 ` Uwe Kleine-König
2013-09-24 19:24 ` Uwe Kleine-König
2013-09-24 18:42 ` [PATCH] [media] marvell-ccic: simplify and fix clk handling (a bit) Uwe Kleine-König
2013-09-24 18:59 ` [PATCH v2] " Uwe Kleine-König
2013-09-25 7:15 ` Jonathan Corbet
2013-09-26 2:47 ` Libin Yang
2013-09-26 8:13 ` Uwe Kleine-König
2013-09-26 8:24 ` Russell King - ARM Linux
2013-09-26 10:08 ` Libin Yang
2013-09-26 10:03 ` Libin Yang
2013-09-24 19:20 ` [PATCH] [RFC] devm: drop devm_clk_put Uwe Kleine-König
2013-09-24 19:20 ` Uwe Kleine-König
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52414561.1010000@marvell.com \
--to=zzhu3@marvell.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.