Hi, >-----Original Message----- >From: Felipe Balbi [mailto:balbi@xxxxxx] >Sent: Thursday, February 10, 2011 7:56 PM >To: Hema HK >Cc: linux-omap@xxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; >Felipe Balbi; Tony Lindgren; Kevin Hilman; Cousson, Benoit; >Paul Walmsley >Subject: Re: [PATCH 5/5 v6] usb: musb: Using runtime pm APIs for musb. > >Hi, > >On Thu, Feb 10, 2011 at 07:38:01PM +0530, Hema HK wrote: >> Calling runtime pm APIs pm_runtime_put_sync() and >pm_runtime_get_sync() >> for enabling/disabling the clocks, sysconfig settings. >> >> Enable clock, configure no-idle/standby when active and >configure force idle/standby >> and disable clock when idled. This is taken care by the >runtime framework when >> driver calls the pm_runtime_get_sync and pm_runtime_put_sync APIs. > >does it have to be the _sync() ?? Yes. Because immediately after this I access the registers. > >> Index: linux-2.6/drivers/usb/musb/omap2430.c >> =================================================================== >> --- linux-2.6.orig/drivers/usb/musb/omap2430.c >> +++ linux-2.6/drivers/usb/musb/omap2430.c >> @@ -33,6 +33,8 @@ >> #include <linux/io.h> >> #include <linux/platform_device.h> >> #include <linux/dma-mapping.h> >> +#include <linux/pm_runtime.h> >> +#include <linux/err.h> >> >> #include "musb_core.h" >> #include "omap2430.h" >> @@ -40,7 +42,6 @@ >> struct omap2430_glue { >> struct device *dev; >> struct platform_device *musb; >> - struct clk *clk; >> }; >> #define glue_to_musb(g) platform_get_drvdata(g->musb) >> >> @@ -216,20 +217,12 @@ static inline void omap2430_low_level_ex >> l = musb_readl(musb->mregs, OTG_FORCESTDBY); >> l |= ENABLEFORCE; /* enable MSTANDBY */ >> musb_writel(musb->mregs, OTG_FORCESTDBY, l); >> - >> - l = musb_readl(musb->mregs, OTG_SYSCONFIG); >> - l |= ENABLEWAKEUP; /* enable wakeup */ >> - musb_writel(musb->mregs, OTG_SYSCONFIG, l); >> } >> >> static inline void omap2430_low_level_init(struct musb *musb) >> { >> u32 l; >> >> - l = musb_readl(musb->mregs, OTG_SYSCONFIG); >> - l &= ~ENABLEWAKEUP; /* disable wakeup */ >> - musb_writel(musb->mregs, OTG_SYSCONFIG, l); >> - >> l = musb_readl(musb->mregs, OTG_FORCESTDBY); >> l &= ~ENABLEFORCE; /* disable MSTANDBY */ >> musb_writel(musb->mregs, OTG_FORCESTDBY, l); >> @@ -309,21 +302,6 @@ static int omap2430_musb_init(struct mus >> >> omap2430_low_level_init(musb); >> >> - l = musb_readl(musb->mregs, OTG_SYSCONFIG); >> - l &= ~ENABLEWAKEUP; /* disable wakeup */ >> - l &= ~NOSTDBY; /* remove possible nostdby */ >> - l |= SMARTSTDBY; /* enable smart standby */ >> - l &= ~AUTOIDLE; /* disable auto idle */ >> - l &= ~NOIDLE; /* remove possible noidle */ >> - l |= SMARTIDLE; /* enable smart idle */ >> - /* >> - * MUSB AUTOIDLE don't work in 3430. >> - * Workaround by Richard Woodruff/TI >> - */ >> - if (!cpu_is_omap3430()) >> - l |= AUTOIDLE; /* enable auto idle */ > >is this taken care somewhere else ? Yes. In HWMOD data structure, there is a flag defined. > >> @@ -431,28 +395,30 @@ static int __init omap2430_probe(struct >> pdev->num_resources); >> if (ret) { >> dev_err(&pdev->dev, "failed to add resources\n"); >> - goto err4; >> + goto err2; >> } >> >> ret = platform_device_add_data(musb, pdata, sizeof(*pdata)); >> if (ret) { >> dev_err(&pdev->dev, "failed to add platform_data\n"); >> - goto err4; >> + goto err2; >> } >> >> ret = platform_device_add(musb); >> if (ret) { >> dev_err(&pdev->dev, "failed to register musb device\n"); >> - goto err4; >> + goto err2; >> } >> >> - return 0; >> + pm_runtime_enable(&pdev->dev); >> + if (pm_runtime_get_sync(&pdev->dev) < 0) { > >you have the status variable, so how about: > >status = pm_runtime_get_sync(&pdev->dev); >if (status < 0) { > >?? > Can be done. >> + dev_err(&pdev->dev, "pm_runtime_get_sync FAILED"); >> + pm_runtime_disable(&pdev->dev); > >move the pm_runtime_disable() to err3, maybe. Then just call goto err3 >here. Yes. Regards, Hema > >-- >balbi > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
