Re: [PATCH] omapdss: use devm_clk_get()

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 2818 bytes --]

On 2013-04-10 11:37, Archit Taneja wrote:
> On Wednesday 10 April 2013 01:43 PM, Tomi Valkeinen wrote:
>> On 2013-04-08 11:55, Archit Taneja wrote:
>>> Use devm_clk_get() instead of clk_get() for dss, and for outputs hdmi
>>> and venc.
>>> This reduces reduces code and simplifies error handling.
>>>
>>> Signed-off-by: Archit Taneja <archit@ti.com>
>>> ---
>>>   drivers/video/omap2/dss/dss.c  |   18 +++---------------
>>>   drivers/video/omap2/dss/hdmi.c |   16 ++--------------
>>>   drivers/video/omap2/dss/venc.c |   10 +---------
>>>   3 files changed, 6 insertions(+), 38 deletions(-)
>>>
>>> diff --git a/drivers/video/omap2/dss/dss.c
>>> b/drivers/video/omap2/dss/dss.c
>>> index 054c2a2..645b3bc 100644
>>> --- a/drivers/video/omap2/dss/dss.c
>>> +++ b/drivers/video/omap2/dss/dss.c
>>> @@ -767,13 +767,11 @@ int dss_dpi_select_source(enum omap_channel
>>> channel)
>>>   static int dss_get_clocks(void)
>>>   {
>>>       struct clk *clk;
>>> -    int r;
>>>
>>> -    clk = clk_get(&dss.pdev->dev, "fck");
>>> +    clk = devm_clk_get(&dss.pdev->dev, "fck");
>>>       if (IS_ERR(clk)) {
>>>           DSSERR("can't get clock fck\n");
>>> -        r = PTR_ERR(clk);
>>> -        goto err;
>>> +        return PTR_ERR(clk);
>>>       }
>>>
>>>       dss.dss_clk = clk;
>>> @@ -782,8 +780,7 @@ static int dss_get_clocks(void)
>>>           clk = clk_get(NULL, dss.feat->clk_name);
>>>           if (IS_ERR(clk)) {
>>>               DSSERR("Failed to get %s\n", dss.feat->clk_name);
>>> -            r = PTR_ERR(clk);
>>> -            goto err;
>>> +            return PTR_ERR(clk);
>>>           }
>>>       } else {
>>>           clk = NULL;
>>> @@ -792,21 +789,12 @@ static int dss_get_clocks(void)
>>>       dss.dpll4_m4_ck = clk;
>>>
>>>       return 0;
>>> -
>>> -err:
>>> -    if (dss.dss_clk)
>>> -        clk_put(dss.dss_clk);
>>> -    if (dss.dpll4_m4_ck)
>>> -        clk_put(dss.dpll4_m4_ck);
>>> -
>>> -    return r;
>>>   }
>>
>> Why didn't you use devm_clk_get for the dpll4_m4_ck clock also?
> 
> clk_get of dpll4_m4_ck isn't tied to a device:
> 
>     clk = clk_get(NULL, dss.feat->clk_name);
> 
> We can't use devm_clk_get() if we don't tie it to a device, right?

Ah, true.

> I think the dss.dss_clk clock above is same as the dpll4_m4_ck for all
> OMAPs. We could probably remove the dpll4_m4_clk all together, and use
> dss_clk to get the rate of DSS_FCK coming from PRCM.

Hmm, no, if I recall right, dpll4_m4_clk is the parent of dss clock (or
parent of the parent of...), and the dss_fck_multiplier affects the
resulting dss fck. Or something like that. It's a bit messy, especially
as we can't just use the dss fck to change the rate, we need to change
the rate at the parent for some reason.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]