diff mbox

[v3,1/3] ARM: omap: clk: add clk_prepare and clk_unprepare

Message ID 1341224669-15231-2-git-send-email-rnayak@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Rajendra Nayak July 2, 2012, 10:24 a.m. UTC
As part of Common Clk Framework (CCF) the clk_enable() operation
was split into a clk_prepare() which could sleep, and a clk_enable()
which should never sleep. Similarly the clk_disable() was
split into clk_disable() and clk_unprepare(). This was
needed to handle complex cases where in a clk gate/ungate
would require a slow and a fast part to be implemented.
None of the clocks below seem to be in the 'complex' clocks
category and are just simple clocks which are enabled/disabled
through simple register writes.
Most of the instances also seem to be called in non-atomic
context which means its safe to move all of those from
using a clk_enable() to clk_prepare_enable() and clk_disable() to
clk_disable_unprepare().
For a few others where there is a possibility they get called from
an interrupt or atomic context, there is an additonal clk_prepare()
done before a clk_enable() and a clk_unprepare()
after a clk_disable().
This is in preparation of OMAP moving to CCF.

Based on initial changes from Mike turquette.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
 arch/arm/mach-omap2/board-apollon.c    |    4 ++--
 arch/arm/mach-omap2/board-h4.c         |    6 +++---
 arch/arm/mach-omap2/board-omap4panda.c |    2 +-
 arch/arm/mach-omap2/clock3xxx.c        |    8 ++++----
 arch/arm/mach-omap2/display.c          |    4 ++--
 arch/arm/mach-omap2/gpmc.c             |    2 +-
 arch/arm/mach-omap2/omap_hwmod.c       |    3 +++
 arch/arm/mach-omap2/pm24xx.c           |    2 ++
 arch/arm/mach-omap2/usb-fs.c           |    4 ++--
 9 files changed, 20 insertions(+), 15 deletions(-)

Comments

Paul Walmsley July 30, 2012, 10:31 p.m. UTC | #1
Hi

On Mon, 2 Jul 2012, Rajendra Nayak wrote:

> As part of Common Clk Framework (CCF) the clk_enable() operation
> was split into a clk_prepare() which could sleep, and a clk_enable()
> which should never sleep. Similarly the clk_disable() was
> split into clk_disable() and clk_unprepare(). This was
> needed to handle complex cases where in a clk gate/ungate
> would require a slow and a fast part to be implemented.
> None of the clocks below seem to be in the 'complex' clocks
> category and are just simple clocks which are enabled/disabled
> through simple register writes.
> Most of the instances also seem to be called in non-atomic
> context which means its safe to move all of those from
> using a clk_enable() to clk_prepare_enable() and clk_disable() to
> clk_disable_unprepare().
> For a few others where there is a possibility they get called from
> an interrupt or atomic context, there is an additonal clk_prepare()
> done before a clk_enable() and a clk_unprepare()
> after a clk_disable().
> This is in preparation of OMAP moving to CCF.
> 
> Based on initial changes from Mike turquette.
> 
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>

Looking at this one, it seems basically okay except for this part:

> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 7731936..f904993 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -608,6 +608,7 @@ static int _init_main_clk(struct omap_hwmod *oh)
>  			   oh->name, oh->main_clk);
>  		return -EINVAL;
>  	}
> +	clk_prepare(oh->_clk);
>  
>  	if (!oh->_clk->clkdm)
>  		pr_warning("omap_hwmod: %s: missing clockdomain for %s.\n",
> @@ -645,6 +646,7 @@ static int _init_interface_clks(struct omap_hwmod *oh)
>  			ret = -EINVAL;
>  		}
>  		os->_clk = c;
> +		clk_prepare(os->_clk);
>  	}
>  
>  	return ret;
> @@ -672,6 +674,7 @@ static int _init_opt_clks(struct omap_hwmod *oh)
>  			ret = -EINVAL;
>  		}
>  		oc->_clk = c;
> +		clk_prepare(oc->_clk);
>  	}
>  
>  	return ret;

Seems to me that the strategy here of calling clk_prepare() right after 
the clk_get() is only going to work as long as clk_prepare() is a no-op. 
Right now this code is called early in the boot before many subsystems are 
available.

So if we make a change like this one, it seems like we would basically 
expect it to break once we start doing anything meaningful with 
clk_prepare(), like using clk_prepare() for voltage scaling or something 
that requires I2C?  We'd also probably want to mark this with some kind of 
"HACK" comment.
 


- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Turquette July 30, 2012, 10:42 p.m. UTC | #2
On Mon, Jul 30, 2012 at 3:31 PM, Paul Walmsley <paul@pwsan.com> wrote:
> So if we make a change like this one, it seems like we would basically
> expect it to break once we start doing anything meaningful with
> clk_prepare(), like using clk_prepare() for voltage scaling or something
> that requires I2C?  We'd also probably want to mark this with some kind of
> "HACK" comment.

Hi Paul,

Did you have anything in mind to start using clk_prepare?  I still
hope to get rid of it some day and replace that call with a
combination of clk_enable and a check like clk_enable_can_sleep.

Regards,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux July 30, 2012, 11:02 p.m. UTC | #3
On Mon, Jul 30, 2012 at 03:42:13PM -0700, Turquette, Mike wrote:
> On Mon, Jul 30, 2012 at 3:31 PM, Paul Walmsley <paul@pwsan.com> wrote:
> > So if we make a change like this one, it seems like we would basically
> > expect it to break once we start doing anything meaningful with
> > clk_prepare(), like using clk_prepare() for voltage scaling or something
> > that requires I2C?  We'd also probably want to mark this with some kind of
> > "HACK" comment.
> 
> Hi Paul,
> 
> Did you have anything in mind to start using clk_prepare?  I still
> hope to get rid of it some day and replace that call with a
> combination of clk_enable and a check like clk_enable_can_sleep.

Don't you dare.

We arrived at the clk_prepare()/clk_enable() split after lots of
discussion between different platform maintainers and their
requirements.

Shoving crap like "clk_enable_can_sleep()" down into drivers is
totally and utterly idiotic.  We've had the situation *already*
where some drivers can be used on some platforms but not on others
because of differences in clk_enable() expectations.

Don't go back there.  clk_prepare() and clk_enable() are separate
calls for a very good reason.  One is sleepable, the other can be
called from any atomic contexts.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mike Turquette July 31, 2012, 12:31 a.m. UTC | #4
On Mon, Jul 30, 2012 at 4:02 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Jul 30, 2012 at 03:42:13PM -0700, Turquette, Mike wrote:
>> On Mon, Jul 30, 2012 at 3:31 PM, Paul Walmsley <paul@pwsan.com> wrote:
>> > So if we make a change like this one, it seems like we would basically
>> > expect it to break once we start doing anything meaningful with
>> > clk_prepare(), like using clk_prepare() for voltage scaling or something
>> > that requires I2C?  We'd also probably want to mark this with some kind of
>> > "HACK" comment.
>>
>> Hi Paul,
>>
>> Did you have anything in mind to start using clk_prepare?  I still
>> hope to get rid of it some day and replace that call with a
>> combination of clk_enable and a check like clk_enable_can_sleep.
>
> Don't you dare.
>
> We arrived at the clk_prepare()/clk_enable() split after lots of
> discussion between different platform maintainers and their
> requirements.
>
> Shoving crap like "clk_enable_can_sleep()" down into drivers is
> totally and utterly idiotic.  We've had the situation *already*
> where some drivers can be used on some platforms but not on others
> because of differences in clk_enable() expectations.
>

How does having a dynamic run-time check cause a generic driver to run
on "some platforms but not on others"?

> Don't go back there.  clk_prepare() and clk_enable() are separate
> calls for a very good reason.  One is sleepable, the other can be
> called from any atomic contexts.

Two calls exist because of context differences.  But in practice they
do the same thing: cause a line to toggle at a rate.  If a run-time
check exists and the framework could handle this gracefully, why would
you still choose the split api?

Regards,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Saravana Kannan July 31, 2012, 8:21 a.m. UTC | #5
On 07/30/2012 05:31 PM, Turquette, Mike wrote:
> On Mon, Jul 30, 2012 at 4:02 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Mon, Jul 30, 2012 at 03:42:13PM -0700, Turquette, Mike wrote:
>>> On Mon, Jul 30, 2012 at 3:31 PM, Paul Walmsley <paul@pwsan.com> wrote:
>>>> So if we make a change like this one, it seems like we would basically
>>>> expect it to break once we start doing anything meaningful with
>>>> clk_prepare(), like using clk_prepare() for voltage scaling or something
>>>> that requires I2C?  We'd also probably want to mark this with some kind of
>>>> "HACK" comment.
>>>
>>> Hi Paul,
>>>
>>> Did you have anything in mind to start using clk_prepare?  I still
>>> hope to get rid of it some day and replace that call with a
>>> combination of clk_enable and a check like clk_enable_can_sleep.
>>
>> Don't you dare.
>>
>> We arrived at the clk_prepare()/clk_enable() split after lots of
>> discussion between different platform maintainers and their
>> requirements.
>>
>> Shoving crap like "clk_enable_can_sleep()" down into drivers is
>> totally and utterly idiotic.  We've had the situation *already*
>> where some drivers can be used on some platforms but not on others
>> because of differences in clk_enable() expectations.
>>
>
> How does having a dynamic run-time check cause a generic driver to run
> on "some platforms but not on others"?
>
>> Don't go back there.  clk_prepare() and clk_enable() are separate
>> calls for a very good reason.  One is sleepable, the other can be
>> called from any atomic contexts.
>
> Two calls exist because of context differences.  But in practice they
> do the same thing: cause a line to toggle at a rate.  If a run-time
> check exists and the framework could handle this gracefully, why would
> you still choose the split api?

No. IMO, the two calls exist because getting a line to toggle at a rate 
can involve both slow and fast steps. It's not just a either/or or a 
random choice to allow doing it in one context vs. another.

Drivers shouldn't care what the actual steps are. Having drivers care 
how long each steps take or what they are (by using the can_sleep APIs) 
in each architecture/platform is a bad abstraction.

The main point of the clock prepare/enable/disable/unprepare APIs is 
about power management. So, the drivers just need to know when they have 
enough time to do the quick part of the enable/disable and the slow part 
of the enable/disable (prepare/unprepare) and make the calls in those 
locations in code/execution flow. That, IMO is the ideal abstraction.

If drivers need to ensure the clock is fully gated for functional 
correctness, then they need to take the time (meaning, postpone the 
action to a non-atomic context) and do the full gating by completely 
unpreparing the clock.

I have heard this idea about removing the clk_prepare/unprepare API too 
many times and it makes me uncomfortable. I would really prefer we (the 
community) take this discussion to the end and put an end to it. We 
either agree to stick with the clk_prepare APIs or figure out the newer 
APIs. I don't want to keep having to deal with the "we really should be 
removing the clk_prepare() APIs" wrench thrown into the discussion every 
time we discuss anything related to a locking issue.

Thanks,
Saravana
Russell King - ARM Linux July 31, 2012, 9:23 a.m. UTC | #6
On Mon, Jul 30, 2012 at 05:31:23PM -0700, Turquette, Mike wrote:
> On Mon, Jul 30, 2012 at 4:02 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Mon, Jul 30, 2012 at 03:42:13PM -0700, Turquette, Mike wrote:
> >> On Mon, Jul 30, 2012 at 3:31 PM, Paul Walmsley <paul@pwsan.com> wrote:
> >> > So if we make a change like this one, it seems like we would basically
> >> > expect it to break once we start doing anything meaningful with
> >> > clk_prepare(), like using clk_prepare() for voltage scaling or something
> >> > that requires I2C?  We'd also probably want to mark this with some kind of
> >> > "HACK" comment.
> >>
> >> Hi Paul,
> >>
> >> Did you have anything in mind to start using clk_prepare?  I still
> >> hope to get rid of it some day and replace that call with a
> >> combination of clk_enable and a check like clk_enable_can_sleep.
> >
> > Don't you dare.
> >
> > We arrived at the clk_prepare()/clk_enable() split after lots of
> > discussion between different platform maintainers and their
> > requirements.
> >
> > Shoving crap like "clk_enable_can_sleep()" down into drivers is
> > totally and utterly idiotic.  We've had the situation *already*
> > where some drivers can be used on some platforms but not on others
> > because of differences in clk_enable() expectations.
> >
> 
> How does having a dynamic run-time check cause a generic driver to run
> on "some platforms but not on others"?

What you're asking is for drivers which use clk_prepare() and clk_enable()
correctly to be littered with:

-	clk_prepare(clk);
+	if (clk_enable_can_sleep(clk))
+		clk_enable(clk);

and, in atomic contexts:

-	clk_enable(clk);
+	if (!clk_enable_can_sleep(clk))
+		clk_enable(clk);

which is total bollocks - drivers should not need to know about this
clk_enable_can_sleep() crap.

> Two calls exist because of context differences.  But in practice they
> do the same thing: cause a line to toggle at a rate.  If a run-time
> check exists and the framework could handle this gracefully, why would
> you still choose the split api?

How can it handle it gracefully?

Take a look at something like amba-pl011.c for a clue.  Remove the
clk_prepare() and clk_unprepare() calls.  Then consider a clk
implementation which does not support clk_enable()/clk_disable() from
atomic contexts.  Now think about how the pl011 driver enable the clock
for console output from atomic contexts?  Answer, it can't.

We've been here before with this stuff, and this is exactly why we split
clk_enable() into the two separate calls we have today.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Russell King - ARM Linux July 31, 2012, 9:27 a.m. UTC | #7
On Tue, Jul 31, 2012 at 01:21:24AM -0700, Saravana Kannan wrote:
> I have heard this idea about removing the clk_prepare/unprepare API too  
> many times and it makes me uncomfortable. I would really prefer we (the  
> community) take this discussion to the end and put an end to it. We  
> either agree to stick with the clk_prepare APIs or figure out the newer  
> APIs. I don't want to keep having to deal with the "we really should be  
> removing the clk_prepare() APIs" wrench thrown into the discussion every  
> time we discuss anything related to a locking issue.

Well, part of the motivation to remove it comes from the fact that we
have this "clk_prepare_enable()" thing which _everyone_ simply converts
their existing clk_enable() call to without _thinking_ one little bit
about the rest of the picture.

It's that which is making the justification for getting rid of the split
API soo easy.  If people put more thought into it then the separation of
the two calls would be much clearer, and there would be more of a
justification to prevent its removal.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Walmsley July 31, 2012, 6 p.m. UTC | #8
Hi Mike,

On Mon, 30 Jul 2012, Turquette, Mike wrote:

> On Mon, Jul 30, 2012 at 3:31 PM, Paul Walmsley <paul@pwsan.com> wrote:
>
> > So if we make a change like this one, it seems like we would basically 
> > expect it to break once we start doing anything meaningful with 
> > clk_prepare(), like using clk_prepare() for voltage scaling or 
> > something that requires I2C?  We'd also probably want to mark this 
> > with some kind of "HACK" comment.
> 
> Did you have anything in mind to start using clk_prepare?  I still hope 
> to get rid of it some day and replace that call with a combination of 
> clk_enable and a check like clk_enable_can_sleep.

Nothing that requires immediate attention.  Mostly am just trying to think 
through how to reconcile the clock and device management code that we have 
with what other SoC programmers are apparently implementing in their 
platform code.


- Paul
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mach-omap2/board-apollon.c b/arch/arm/mach-omap2/board-apollon.c
index 502c31e..1d8c693 100644
--- a/arch/arm/mach-omap2/board-apollon.c
+++ b/arch/arm/mach-omap2/board-apollon.c
@@ -205,7 +205,7 @@  static inline void __init apollon_init_smc91x(void)
 		return;
 	}
 
-	clk_enable(gpmc_fck);
+	clk_prepare_enable(gpmc_fck);
 	rate = clk_get_rate(gpmc_fck);
 
 	eth_cs = APOLLON_ETH_CS;
@@ -249,7 +249,7 @@  static inline void __init apollon_init_smc91x(void)
 		gpmc_cs_free(APOLLON_ETH_CS);
 	}
 out:
-	clk_disable(gpmc_fck);
+	clk_disable_unprepare(gpmc_fck);
 	clk_put(gpmc_fck);
 }
 
diff --git a/arch/arm/mach-omap2/board-h4.c b/arch/arm/mach-omap2/board-h4.c
index 876becf..a273af0 100644
--- a/arch/arm/mach-omap2/board-h4.c
+++ b/arch/arm/mach-omap2/board-h4.c
@@ -267,9 +267,9 @@  static inline void __init h4_init_debug(void)
 		return;
 	}
 
-	clk_enable(gpmc_fck);
+	clk_prepare_enable(gpmc_fck);
 	rate = clk_get_rate(gpmc_fck);
-	clk_disable(gpmc_fck);
+	clk_disable_unprepare(gpmc_fck);
 	clk_put(gpmc_fck);
 
 	if (is_gpmc_muxed())
@@ -313,7 +313,7 @@  static inline void __init h4_init_debug(void)
 		gpmc_cs_free(eth_cs);
 
 out:
-	clk_disable(gpmc_fck);
+	clk_disable_unprepare(gpmc_fck);
 	clk_put(gpmc_fck);
 }
 
diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
index 982fb26..f0ea558 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -172,7 +172,7 @@  static void __init omap4_ehci_init(void)
 		return;
 	}
 	clk_set_rate(phy_ref_clk, 19200000);
-	clk_enable(phy_ref_clk);
+	clk_prepare_enable(phy_ref_clk);
 
 	/* disable the power to the usb hub prior to init and reset phy+hub */
 	ret = gpio_request_array(panda_ehci_gpios,
diff --git a/arch/arm/mach-omap2/clock3xxx.c b/arch/arm/mach-omap2/clock3xxx.c
index 794d827..4c1591a 100644
--- a/arch/arm/mach-omap2/clock3xxx.c
+++ b/arch/arm/mach-omap2/clock3xxx.c
@@ -64,15 +64,15 @@  void __init omap3_clk_lock_dpll5(void)
 
 	dpll5_clk = clk_get(NULL, "dpll5_ck");
 	clk_set_rate(dpll5_clk, DPLL5_FREQ_FOR_USBHOST);
-	clk_enable(dpll5_clk);
+	clk_prepare_enable(dpll5_clk);
 
 	/* Program dpll5_m2_clk divider for no division */
 	dpll5_m2_clk = clk_get(NULL, "dpll5_m2_ck");
-	clk_enable(dpll5_m2_clk);
+	clk_prepare_enable(dpll5_m2_clk);
 	clk_set_rate(dpll5_m2_clk, DPLL5_FREQ_FOR_USBHOST);
 
-	clk_disable(dpll5_m2_clk);
-	clk_disable(dpll5_clk);
+	clk_disable_unprepare(dpll5_m2_clk);
+	clk_disable_unprepare(dpll5_clk);
 	return;
 }
 
diff --git a/arch/arm/mach-omap2/display.c b/arch/arm/mach-omap2/display.c
index 5fb47a1..e5f8e48 100644
--- a/arch/arm/mach-omap2/display.c
+++ b/arch/arm/mach-omap2/display.c
@@ -471,7 +471,7 @@  int omap_dss_reset(struct omap_hwmod *oh)
 
 	for (i = oh->opt_clks_cnt, oc = oh->opt_clks; i > 0; i--, oc++)
 		if (oc->_clk)
-			clk_enable(oc->_clk);
+			clk_prepare_enable(oc->_clk);
 
 	dispc_disable_outputs();
 
@@ -498,7 +498,7 @@  int omap_dss_reset(struct omap_hwmod *oh)
 
 	for (i = oh->opt_clks_cnt, oc = oh->opt_clks; i > 0; i--, oc++)
 		if (oc->_clk)
-			clk_disable(oc->_clk);
+			clk_disable_unprepare(oc->_clk);
 
 	r = (c == MAX_MODULE_SOFTRESET_WAIT) ? -ETIMEDOUT : 0;
 
diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 2286410..a33f89d 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -749,7 +749,7 @@  static int __init gpmc_init(void)
 		BUG();
 	}
 
-	clk_enable(gpmc_l3_clk);
+	clk_prepare_enable(gpmc_l3_clk);
 
 	l = gpmc_read_reg(GPMC_REVISION);
 	printk(KERN_INFO "GPMC revision %d.%d\n", (l >> 4) & 0x0f, l & 0x0f);
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 7731936..f904993 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -608,6 +608,7 @@  static int _init_main_clk(struct omap_hwmod *oh)
 			   oh->name, oh->main_clk);
 		return -EINVAL;
 	}
+	clk_prepare(oh->_clk);
 
 	if (!oh->_clk->clkdm)
 		pr_warning("omap_hwmod: %s: missing clockdomain for %s.\n",
@@ -645,6 +646,7 @@  static int _init_interface_clks(struct omap_hwmod *oh)
 			ret = -EINVAL;
 		}
 		os->_clk = c;
+		clk_prepare(os->_clk);
 	}
 
 	return ret;
@@ -672,6 +674,7 @@  static int _init_opt_clks(struct omap_hwmod *oh)
 			ret = -EINVAL;
 		}
 		oc->_clk = c;
+		clk_prepare(oc->_clk);
 	}
 
 	return ret;
diff --git a/arch/arm/mach-omap2/pm24xx.c b/arch/arm/mach-omap2/pm24xx.c
index 2edeffc..8eee8bc 100644
--- a/arch/arm/mach-omap2/pm24xx.c
+++ b/arch/arm/mach-omap2/pm24xx.c
@@ -340,11 +340,13 @@  int __init omap2_pm_init(void)
 		printk(KERN_ERR "could not get osc_ck\n");
 		return -ENODEV;
 	}
+	clk_prepare(osc_ck);
 
 	if (cpu_is_omap242x()) {
 		emul_ck = clk_get(NULL, "emul_ck");
 		if (IS_ERR(emul_ck)) {
 			printk(KERN_ERR "could not get emul_ck\n");
+			clk_unprepare(osc_ck);
 			clk_put(osc_ck);
 			return -ENODEV;
 		}
diff --git a/arch/arm/mach-omap2/usb-fs.c b/arch/arm/mach-omap2/usb-fs.c
index 1481078..cff7d24 100644
--- a/arch/arm/mach-omap2/usb-fs.c
+++ b/arch/arm/mach-omap2/usb-fs.c
@@ -344,7 +344,7 @@  void __init omap2_usbfs_init(struct omap_usb_config *pdata)
 	if (IS_ERR(ick))
 		return;
 
-	clk_enable(ick);
+	clk_prepare_enable(ick);
 	pdata->usb0_init = omap2_usb0_init;
 	pdata->usb1_init = omap2_usb1_init;
 	pdata->usb2_init = omap2_usb2_init;
@@ -352,7 +352,7 @@  void __init omap2_usbfs_init(struct omap_usb_config *pdata)
 	ohci_device_init(pdata);
 	otg_device_init(pdata);
 	omap_otg_init(pdata);
-	clk_disable(ick);
+	clk_disable_unprepare(ick);
 	clk_put(ick);
 }