linux-kernel - Re: [PATCH] davinci: vpif: add pm

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5159234E.8080105@ti.com>
Date:	Mon, 1 Apr 2013 11:33:58 +0530
From:	Sekhar Nori <nsekhar@...com>
To:	Prabhakar Lad <prabhakar.csengg@...il.com>
CC:	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	DLOS <davinci-linux-open-source@...ux.davincidsp.com>,
	LMML <linux-media@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Mauro Carvalho Chehab <mchehab@...hat.com>,
	Hans Verkuil <hans.verkuil@...co.com>,
	Sakari Ailus <sakari.ailus@....fi>
Subject: Re: [PATCH] davinci: vpif: add pm_runtime support

On 3/28/2013 3:50 PM, Prabhakar Lad wrote:
> Hi Laurent,
> 
> On Thu, Mar 28, 2013 at 3:40 PM, Laurent Pinchart
> <laurent.pinchart@...asonboard.com> wrote:
>> Hi Prabhakar,
>>
>> On Thursday 28 March 2013 15:36:11 Prabhakar Lad wrote:
>>> On Thu, Mar 28, 2013 at 2:39 PM, Laurent Pinchart wrote:
>>>> On Thursday 28 March 2013 14:20:32 Prabhakar lad wrote:
>>>>> From: Lad, Prabhakar <prabhakar.csengg@...il.com>
>>>>>
>>>>> Add pm_runtime support to the TI Davinci VPIF driver.
>>>>> Along side this patch replaces clk_get() with devm_clk_get()
>>>>> to simplify the error handling.
>>>>>
>>>>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@...il.com>
>>>>> ---
>>>>>
>>>>>  drivers/media/platform/davinci/vpif.c |   21 +++++++++++----------
>>>>>  1 files changed, 11 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/drivers/media/platform/davinci/vpif.c
>>>>> b/drivers/media/platform/davinci/vpif.c index 28638a8..7d14625 100644
>>>>> --- a/drivers/media/platform/davinci/vpif.c
>>>>> +++ b/drivers/media/platform/davinci/vpif.c
>>
>> [snip]
>>
>>>>> @@ -439,12 +440,17 @@ static int vpif_probe(struct platform_device *pdev)
>>>>>               goto fail;
>>>>>       }
>>>>>
>>>>> -     vpif_clk = clk_get(&pdev->dev, "vpif");
>>>>> +     vpif_clk = devm_clk_get(&pdev->dev, "vpif");
>>>>>       if (IS_ERR(vpif_clk)) {
>>>>>               status = PTR_ERR(vpif_clk);
>>>>>               goto clk_fail;
>>>>>       }
>>>>>
>>>>> -     clk_prepare_enable(vpif_clk);
>>>>> +     clk_put(vpif_clk);
>>>>
>>>> Why do you need to call clk_put() here ?
>>>
>>> The above check is to see if the clock is provided, once done
>>> we free it using clk_put().
>>
>> In that case you shouldn't use devm_clk_get(), otherwise clk_put() will be
>> called again automatically at remove() time.
>>
> Yes agreed it should be clk_get() only.
> 
>>>>> +     pm_runtime_enable(&pdev->dev);
>>>>> +     pm_runtime_resume(&pdev->dev);
>>>>> +
>>>>> +     pm_runtime_get(&pdev->dev);
>>>>
>>>> Does runtime PM automatically handle your clock ? If so can't you remove
>>>> clock handling from the driver completely ?
>>>
>>> Yes  pm runtime take care of enabling/disabling the clocks
>>> so that we don't have to do it in drivers. I believe clock
>>> handling is removed with this patch, with just  devm_clk_get() remaining ;)
>>
>> When is the clk_get() call expected to fail ? If the clock is provided by the
>> SoC and always available, can't the check be removed completely ?
>>
> Yes I agree with you it can be removed completely assuming the clock
> is always available from the Soc.
> But may be I need feedback from others Hans/Sekhar what do you suggest ?

Unless you need the clk handle to get the clock rate or something, you
should simply rely on runtime PM calls to enable clocks for you and not
have any clk API call at all in your driver.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/