arm: omap: usb: clock enable typo fix in usbhs driver

Commit Message

Munegowda, Keshava July 19, 2011, 2:11 p.m. UTC

From: Keshava Munegowda <[e-mail]>

The usbhs_disable function was invoking clk_enable api
instead of clk_disable; The clk_disable is called to
disble the port clocks of usbhs

Signed-off-by: Keshava Munegowda <[e-mail]>
---
 drivers/mfd/omap-usb-host.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Comments

Felipe Balbi July 22, 2011, 12:22 p.m. UTC | #1

Hi,

On Tue, Jul 19, 2011 at 07:41:43PM +0530, Keshava Munegowda wrote:
> From: Keshava Munegowda <[e-mail]>

$SUBJECT is quite weird. First, this is not a patch on arch/arm/*omap*/
so arm: omap: is already wrong. Also, "enable typo fix" is quite werid.

Something like: "mfd: omap-usb-host: fix clock API usage on
usbhs_disable()" would read better.

> The usbhs_disable function was invoking clk_enable api
> instead of clk_disable; The clk_disable is called to
                        ^ this semicolon should be a period.

ok, you're missing the actual problem here. How about you extend it to
read as:

usbhs_disable function was invoking clk_enable() instead of
clk_disable(), thus only increasing the clock usage counter and
preventing this particular clock from being ever turned off.

Because of that, the power domain where the USB Host subsystem is
located, would never reach lower power states.

> disble the port clocks of usbhs
    ^^ disable.

Munegowda, Keshava July 22, 2011, 12:27 p.m. UTC | #2

On Fri, Jul 22, 2011 at 5:52 PM, Felipe Balbi <[e-mail]> wrote:
> Hi,
>
> On Tue, Jul 19, 2011 at 07:41:43PM +0530, Keshava Munegowda wrote:
>> From: Keshava Munegowda <[e-mail]>
>
> $SUBJECT is quite weird. First, this is not a patch on arch/arm/*omap*/
> so arm: omap: is already wrong. Also, "enable typo fix" is quite werid.
>
> Something like: "mfd: omap-usb-host: fix clock API usage on
> usbhs_disable()" would read better.
>
>> The usbhs_disable function was invoking clk_enable api
>> instead of clk_disable; The clk_disable is called to
>                        ^ this semicolon should be a period.
>
> ok, you're missing the actual problem here. How about you extend it to
> read as:
>
> usbhs_disable function was invoking clk_enable() instead of
> clk_disable(), thus only increasing the clock usage counter and
> preventing this particular clock from being ever turned off.
>
> Because of that, the power domain where the USB Host subsystem is
> located, would never reach lower power states.
>
>> disble the port clocks of usbhs
>    ^^ disable.
>
> --
> balbi

Thanks I will send the updated patch soon.

>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 1717144..29601e7 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -998,9 +998,9 @@  static void usbhs_disable(struct device *dev)
 
 	if (is_omap_usbhs_rev2(omap)) {
 		if (is_ehci_tll_mode(pdata->port_mode[0]))
-			clk_enable(omap->usbtll_p1_fck);
+			clk_disable(omap->usbtll_p1_fck);
 		if (is_ehci_tll_mode(pdata->port_mode[1]))
-			clk_enable(omap->usbtll_p2_fck);
+			clk_disable(omap->usbtll_p2_fck);
 		clk_disable(omap->utmi_p2_fck);
 		clk_disable(omap->utmi_p1_fck);
 	}