On Fri, Apr 13, 2012 at 03:09:41PM +0530, ABRAHAM, KISHON VIJAY wrote:
> Hi,
>
> On Tue, Apr 10, 2012 at 9:48 PM, Felipe Balbi <balbi@xxxxxx> wrote:
> > From: Grazvydas Ignotas <notasas@xxxxxxxxx>
> >
> > When runtime_pm was originally added, it was done in rather confusing
> > way: omap2430_musb_init() (called from musb_init_controller) would do
> > runtime_pm_get_sync() and musb_init_controller() itself would do
> > runtime_pm_put to balance it out. This is not only confusing but also
> > wrong if non-omap2430 glue layer is used.
> >
> > This confusion resulted in commit 772aed45b604 "usb: musb: fix
> > pm_runtime mismatch", that removed runtime_pm_put() from
> > musb_init_controller as that looked unbalanced, and also happened to
> > fix unrelated isp1704_charger crash. However this broke runtime PM
> > functionality (musb is now always powered, even without gadget active).
> >
> > Avoid these confusing runtime pm dependences by making
> > musb_init_controller() and omap2430_musb_init() do their own runtime
> > get/put pairs; also cover error paths. Remove unneeded runtime_pm_put
> > in omap2430_remove too. isp1704_charger crash that motivated
> > 772aed45b604 will be fixed by following patch.
> >
> > Cc: Felipe Contreras <felipe.contreras@xxxxxxxxx>
> > Signed-off-by: Grazvydas Ignotas <notasas@xxxxxxxxx>
> > Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> > ---
> > drivers/usb/musb/musb_core.c | 9 ++++++++-
> > drivers/usb/musb/omap2430.c | 2 +-
> > 2 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> > index 0f8b829..2392146 100644
> > --- a/drivers/usb/musb/musb_core.c
> > +++ b/drivers/usb/musb/musb_core.c
> > @@ -1904,7 +1904,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
> >
> > if (!musb->isr) {
> > status = -ENODEV;
> > - goto fail3;
> > + goto fail2;
> > }
> >
> > if (!musb->xceiv->io_ops) {
> > @@ -1912,6 +1912,8 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
> > musb->xceiv->io_ops = &musb_ulpi_access;
> > }
> >
> > + pm_runtime_get_sync(musb->controller);
> > +
> Move this get_sync above musb_platform_init() and remove the get_sync
> in omap2430_musb_init().
>
> > #ifndef CONFIG_MUSB_PIO_ONLY
> > if (use_dma && dev->dma_mask) {
> > struct dma_controller *c;
> > @@ -2023,6 +2025,8 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
> > goto fail5;
> > #endif
> >
> > + pm_runtime_put(musb->controller);
> > +
> > dev_info(dev, "USB %s mode controller at %p using %s, IRQ %d\n",
> > ({char *s;
> > switch (musb->board_mode) {
> > @@ -2047,6 +2051,9 @@ fail4:
> > musb_gadget_cleanup(musb);
> >
> > fail3:
> > + pm_runtime_put_sync(musb->controller);
> > +
> > +fail2:
> > if (musb->irq_wake)
> > device_init_wakeup(dev, 0);
> > musb_platform_exit(musb);
> > diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
> > index 11b571e..3dfd122 100644
> > --- a/drivers/usb/musb/omap2430.c
> > +++ b/drivers/usb/musb/omap2430.c
> > @@ -333,6 +333,7 @@ static int omap2430_musb_init(struct musb *musb)
> >
> > setup_timer(&musb_idle_timer, musb_do_idle, (unsigned long) musb);
> >
> > + pm_runtime_put_noidle(musb->controller);
> This is not needed if you move the get_sync above musb_platform_init()
> (musb_core.c) and remove the get_sync in omap2430_musb_init().
> Though your patch doesn't do any harm, we can avoid redundant calls to
> runtime_get and runtime_put.
pull request has already been sent, if you care enough, please send
another patch which will go into v3.5 merge window. Also, make sure to
test it to avoid regressions ;-)
--
balbi
Attachment:
signature.asc
Description: Digital signature
