* [PATCH] omapdss: use devm_clk_get()
@ 2013-04-08 8:56 Archit Taneja
2013-04-10 8:13 ` Tomi Valkeinen
0 siblings, 1 reply; 7+ messages in thread
From: Archit Taneja @ 2013-04-08 8:56 UTC (permalink / raw)
To: tomi.valkeinen; +Cc: linux-omap, linux-fbdev, Archit Taneja
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;
}
static void dss_put_clocks(void)
{
if (dss.dpll4_m4_ck)
clk_put(dss.dpll4_m4_ck);
- clk_put(dss.dss_clk);
}
static int dss_runtime_get(void)
diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c
index 7292364..c7e0bf7 100644
--- a/drivers/video/omap2/dss/hdmi.c
+++ b/drivers/video/omap2/dss/hdmi.c
@@ -804,7 +804,7 @@ static int hdmi_get_clocks(struct platform_device *pdev)
{
struct clk *clk;
- clk = clk_get(&pdev->dev, "sys_clk");
+ clk = devm_clk_get(&pdev->dev, "sys_clk");
if (IS_ERR(clk)) {
DSSERR("can't get sys_clk\n");
return PTR_ERR(clk);
@@ -815,12 +815,6 @@ static int hdmi_get_clocks(struct platform_device *pdev)
return 0;
}
-static void hdmi_put_clocks(void)
-{
- if (hdmi.sys_clk)
- clk_put(hdmi.sys_clk);
-}
-
#if defined(CONFIG_OMAP4_DSS_HDMI_AUDIO)
int hdmi_compute_acr(u32 sample_freq, u32 *n, u32 *cts)
{
@@ -1100,7 +1094,7 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
r = hdmi_panel_init();
if (r) {
DSSERR("can't init panel\n");
- goto err_panel_init;
+ return r;
}
dss_debugfs_create_file("hdmi", hdmi_dump_regs);
@@ -1110,10 +1104,6 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev)
hdmi_probe_pdata(pdev);
return 0;
-
-err_panel_init:
- hdmi_put_clocks();
- return r;
}
static int __exit hdmi_remove_child(struct device *dev, void *data)
@@ -1135,8 +1125,6 @@ static int __exit omapdss_hdmihw_remove(struct platform_device *pdev)
pm_runtime_disable(&pdev->dev);
- hdmi_put_clocks();
-
return 0;
}
diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c
index 006caf3..c27ab6f 100644
--- a/drivers/video/omap2/dss/venc.c
+++ b/drivers/video/omap2/dss/venc.c
@@ -721,7 +721,7 @@ static int venc_get_clocks(struct platform_device *pdev)
struct clk *clk;
if (dss_has_feature(FEAT_VENC_REQUIRES_TV_DAC_CLK)) {
- clk = clk_get(&pdev->dev, "tv_dac_clk");
+ clk = devm_clk_get(&pdev->dev, "tv_dac_clk");
if (IS_ERR(clk)) {
DSSERR("can't get tv_dac_clk\n");
return PTR_ERR(clk);
@@ -735,12 +735,6 @@ static int venc_get_clocks(struct platform_device *pdev)
return 0;
}
-static void venc_put_clocks(void)
-{
- if (venc.tv_dac_clk)
- clk_put(venc.tv_dac_clk);
-}
-
static struct omap_dss_device * __init venc_find_dssdev(struct platform_device *pdev)
{
struct omap_dss_board_info *pdata = pdev->dev.platform_data;
@@ -886,7 +880,6 @@ static int __init omap_venchw_probe(struct platform_device *pdev)
err_panel_init:
err_runtime_get:
pm_runtime_disable(&pdev->dev);
- venc_put_clocks();
return r;
}
@@ -904,7 +897,6 @@ static int __exit omap_venchw_remove(struct platform_device *pdev)
venc_uninit_output(pdev);
pm_runtime_disable(&pdev->dev);
- venc_put_clocks();
return 0;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] omapdss: use devm_clk_get()
2013-04-08 8:56 [PATCH] omapdss: use devm_clk_get() Archit Taneja
@ 2013-04-10 8:13 ` Tomi Valkeinen
2013-04-10 8:49 ` Archit Taneja
0 siblings, 1 reply; 7+ messages in thread
From: Tomi Valkeinen @ 2013-04-10 8:13 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 1708 bytes --]
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?
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] omapdss: use devm_clk_get()
2013-04-10 8:13 ` Tomi Valkeinen
@ 2013-04-10 8:49 ` Archit Taneja
2013-04-10 9:19 ` Tomi Valkeinen
0 siblings, 1 reply; 7+ messages in thread
From: Archit Taneja @ 2013-04-10 8:49 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
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?
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.
Archit
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] omapdss: use devm_clk_get()
2013-04-10 8:49 ` Archit Taneja
@ 2013-04-10 9:19 ` Tomi Valkeinen
2013-04-10 9:40 ` Archit Taneja
0 siblings, 1 reply; 7+ messages in thread
From: Tomi Valkeinen @ 2013-04-10 9:19 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
[-- 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 --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] omapdss: use devm_clk_get()
2013-04-10 9:19 ` Tomi Valkeinen
@ 2013-04-10 9:40 ` Archit Taneja
2013-04-10 9:32 ` Tomi Valkeinen
2013-04-10 9:41 ` Archit Taneja
0 siblings, 2 replies; 7+ messages in thread
From: Archit Taneja @ 2013-04-10 9:40 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
On Wednesday 10 April 2013 02:49 PM, Tomi Valkeinen wrote:
> 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.
Ah ok, I remember it now. One of the DPLL's divider's output is DSS_FCK.
We get the parent's rate and iterate through all the possible hsdivider
values to get the closest pixel clock. We don't/can't change
dpll4_m4_clk as such, we just need to know it's rate for our calculations.
So yes, we need dpll4_m4_clk, we just need to rename it to a better
thing. Like dss_clk_parent.
Anyway,
Archit
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] omapdss: use devm_clk_get()
2013-04-10 9:40 ` Archit Taneja
@ 2013-04-10 9:32 ` Tomi Valkeinen
2013-04-10 9:41 ` Archit Taneja
1 sibling, 0 replies; 7+ messages in thread
From: Tomi Valkeinen @ 2013-04-10 9:32 UTC (permalink / raw)
To: Archit Taneja; +Cc: linux-omap, linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 732 bytes --]
On 2013-04-10 12:28, Archit Taneja wrote:
> Ah ok, I remember it now. One of the DPLL's divider's output is DSS_FCK.
> We get the parent's rate and iterate through all the possible hsdivider
> values to get the closest pixel clock. We don't/can't change
> dpll4_m4_clk as such, we just need to know it's rate for our calculations.
>
> So yes, we need dpll4_m4_clk, we just need to rename it to a better
> thing. Like dss_clk_parent.
Right. And the field name in struct dss_features, "clk_name" is rather
vague.
At some point we should fix the omap clk framework so that we can set
the dss fck directly. Presuming there's no good reason to have it as it
is now.
I'll apply the patch as it is now.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] omapdss: use devm_clk_get()
2013-04-10 9:40 ` Archit Taneja
2013-04-10 9:32 ` Tomi Valkeinen
@ 2013-04-10 9:41 ` Archit Taneja
1 sibling, 0 replies; 7+ messages in thread
From: Archit Taneja @ 2013-04-10 9:41 UTC (permalink / raw)
To: Tomi Valkeinen; +Cc: linux-omap, linux-fbdev
On Wednesday 10 April 2013 02:58 PM, Archit Taneja wrote:
> On Wednesday 10 April 2013 02:49 PM, Tomi Valkeinen wrote:
>> 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.
>
> Ah ok, I remember it now. One of the DPLL's divider's output is DSS_FCK.
> We get the parent's rate and iterate through all the possible hsdivider
> values to get the closest pixel clock. We don't/can't change
> dpll4_m4_clk as such, we just need to know it's rate for our calculations.
>
> So yes, we need dpll4_m4_clk, we just need to rename it to a better
> thing. Like dss_clk_parent.
>
> Anyway,
Sorry, hit the send button before completing the message. I wanted to
add that we could push this patch as it is.
Archit
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-04-10 9:41 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-08 8:56 [PATCH] omapdss: use devm_clk_get() Archit Taneja
2013-04-10 8:13 ` Tomi Valkeinen
2013-04-10 8:49 ` Archit Taneja
2013-04-10 9:19 ` Tomi Valkeinen
2013-04-10 9:40 ` Archit Taneja
2013-04-10 9:32 ` Tomi Valkeinen
2013-04-10 9:41 ` Archit Taneja
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).