diff mbox

[2/2] usb: dwc3: exynos: use clk_prepare_enable and clk_disable_unprepare

Message ID 1363257898-17504-3-git-send-email-gautam.vivek@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vivek Gautam March 14, 2013, 10:44 a.m. UTC
Convert clk_enable/clk_disable to clk_prepare_enable/clk_disable_unprepare
calls as required by common clock framework.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
CC: Felipe Balbi <balbi@ti.com>
CC: Kukjin Kim <kgene.kim@samsung.com>
---
 drivers/usb/dwc3/dwc3-exynos.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

Comments

Felipe Balbi March 14, 2013, 10:53 a.m. UTC | #1
Hi,

On Thu, Mar 14, 2013 at 04:14:58PM +0530, Vivek Gautam wrote:
> Convert clk_enable/clk_disable to clk_prepare_enable/clk_disable_unprepare
> calls as required by common clock framework.
> 
> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> CC: Felipe Balbi <balbi@ti.com>
> CC: Kukjin Kim <kgene.kim@samsung.com>
> ---
>  drivers/usb/dwc3/dwc3-exynos.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> index 66ca9ac..b03f609 100644
> --- a/drivers/usb/dwc3/dwc3-exynos.c
> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> @@ -129,7 +129,7 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>  	exynos->dev	= dev;
>  	exynos->clk	= clk;
>  
> -	clk_enable(exynos->clk);
> +	clk_prepare_enable(exynos->clk);

eventually we need to pass this clock handling to dwc3/core.c. Just make
sure it's optional since not all platforms need it.

I guess the best way would be to handle clocks via
->runtime_suspend()/->runtime_resume() ??
Vivek Gautam March 15, 2013, 6:06 a.m. UTC | #2
Hi,


On Thu, Mar 14, 2013 at 4:23 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Thu, Mar 14, 2013 at 04:14:58PM +0530, Vivek Gautam wrote:
>> Convert clk_enable/clk_disable to clk_prepare_enable/clk_disable_unprepare
>> calls as required by common clock framework.
>>
>> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
>> CC: Felipe Balbi <balbi@ti.com>
>> CC: Kukjin Kim <kgene.kim@samsung.com>
>> ---
>>  drivers/usb/dwc3/dwc3-exynos.c |    6 +++---
>>  1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
>> index 66ca9ac..b03f609 100644
>> --- a/drivers/usb/dwc3/dwc3-exynos.c
>> +++ b/drivers/usb/dwc3/dwc3-exynos.c
>> @@ -129,7 +129,7 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
>>       exynos->dev     = dev;
>>       exynos->clk     = clk;
>>
>> -     clk_enable(exynos->clk);
>> +     clk_prepare_enable(exynos->clk);
>
> eventually we need to pass this clock handling to dwc3/core.c. Just make
> sure it's optional since not all platforms need it.
>
True, as of now i could see only exynos platform getting a device
clock for dwc3-glue.
So, if not all platforms need to do this, why should we plan to move
this to dwc3/core.c ?

> I guess the best way would be to handle clocks via
> ->runtime_suspend()/->runtime_resume() ??

Right, but there was a doubt actually if you can please clear that.
In device probe, after enabling runtime_pm we would need to
'pm_runtime_get_sync' the device.
Thereby, in runtime_resume the clocks will be enabled.
Now as soon as the device probe finishes, the device will go in
suspend state, calling runtime_suspend
and the clocks would be disabled.
Now would it be possible for the controller to detect any connect/disconnect.

Pardon me if there is something very silly i am missing out.

>
> --
> balbi
Felipe Balbi March 15, 2013, 7:42 a.m. UTC | #3
Hi,

On Fri, Mar 15, 2013 at 11:36:00AM +0530, Vivek Gautam wrote:
> >> Convert clk_enable/clk_disable to clk_prepare_enable/clk_disable_unprepare
> >> calls as required by common clock framework.
> >>
> >> Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
> >> CC: Felipe Balbi <balbi@ti.com>
> >> CC: Kukjin Kim <kgene.kim@samsung.com>
> >> ---
> >>  drivers/usb/dwc3/dwc3-exynos.c |    6 +++---
> >>  1 files changed, 3 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
> >> index 66ca9ac..b03f609 100644
> >> --- a/drivers/usb/dwc3/dwc3-exynos.c
> >> +++ b/drivers/usb/dwc3/dwc3-exynos.c
> >> @@ -129,7 +129,7 @@ static int dwc3_exynos_probe(struct platform_device *pdev)
> >>       exynos->dev     = dev;
> >>       exynos->clk     = clk;
> >>
> >> -     clk_enable(exynos->clk);
> >> +     clk_prepare_enable(exynos->clk);
> >
> > eventually we need to pass this clock handling to dwc3/core.c. Just make
> > sure it's optional since not all platforms need it.
> >
> True, as of now i could see only exynos platform getting a device
> clock for dwc3-glue.
> So, if not all platforms need to do this, why should we plan to move
> this to dwc3/core.c ?

what if dwc3.ko's probe fail ? Clock will be left enabled ;-)

> > I guess the best way would be to handle clocks via
> > ->runtime_suspend()/->runtime_resume() ??
> 
> Right, but there was a doubt actually if you can please clear that.
> In device probe, after enabling runtime_pm we would need to
> 'pm_runtime_get_sync' the device.
> Thereby, in runtime_resume the clocks will be enabled.
> Now as soon as the device probe finishes, the device will go in
> suspend state, calling runtime_suspend
> and the clocks would be disabled.
> Now would it be possible for the controller to detect any
> connect/disconnect.

it depends on how you have configured your core in coreConsultant and
how you're implementing the actual IP. If you have retention flip-flops
then gating clocks (but not cutting Vcc) will not loose context and, if
PHYs are still enabled, you will see new connect events.

But that part of PM optimization has to be done as a last step, as it
tends to break things apart.
diff mbox

Patch

diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index 66ca9ac..b03f609 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -129,7 +129,7 @@  static int dwc3_exynos_probe(struct platform_device *pdev)
 	exynos->dev	= dev;
 	exynos->clk	= clk;
 
-	clk_enable(exynos->clk);
+	clk_prepare_enable(exynos->clk);
 
 	if (node) {
 		ret = of_platform_populate(node, NULL, NULL, dev);
@@ -146,7 +146,7 @@  static int dwc3_exynos_probe(struct platform_device *pdev)
 	return 0;
 
 err2:
-	clk_disable(clk);
+	clk_disable_unprepare(clk);
 err1:
 	return ret;
 }
@@ -158,7 +158,7 @@  static int dwc3_exynos_remove(struct platform_device *pdev)
 	platform_device_unregister(exynos->usb2_phy);
 	platform_device_unregister(exynos->usb3_phy);
 
-	clk_disable(exynos->clk);
+	clk_disable_unprepare(exynos->clk);
 
 	return 0;
 }