Hi Alan
Cc Magnus, Rafael
> > > this patch enable to use pm_runtime_xxx() on ehci-platform
> > > if .config has CONFIG_PM_RUNTIME, otherwise, these are ignored
> > >
> > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> > > ---
> > > drivers/usb/host/ehci-platform.c | 6 ++++++
> > > 1 files changed, 6 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
> > > index dfe881a..89029eb 100644
> > > --- a/drivers/usb/host/ehci-platform.c
> > > +++ b/drivers/usb/host/ehci-platform.c
> > > @@ -122,6 +122,9 @@ static int __devinit ehci_platform_probe(struct platform_device *dev)
> > > if (err)
> > > goto err_iounmap;
> > >
> > > + pm_runtime_enable(&dev->dev);
> > > + pm_runtime_get_sync(&dev->dev);
> >
> > This line is peculiar. Why call pm_runtime_get_sync()? If the
> > controller was at low power before, the usb_add_hcd() call above would
> > have failed. If it was at full power before, this call isn't needed.
> > Also it leaves the runtime PM usage counter set to 1, which will
> > prevent the controller from runtime-suspending.
> >
> > I think what you want to do is more like this:
> >
> > pm_runtime_set_active(&dev->dev);
> > pm_runtime_enable(&dev->dev);
>
> Thank you for explaining this.
>
> Indeed pm_runtime_xxx should be called befor usb_add_hcd()
>
> But on our system (= SuperH CPU), EHCI/OHCI device are
> on-chip, and it needs power to use it.
> So, we need to call pm_rumtime_get_xxx() to enable power.
> Without this function, our EHCI/OHCI couldn't work.
>
> In this case, where should I call it ?
> Or should I create new ehci-xxx driver ?
I'm sorry for my confusing mail.
I would like to say is "can I call pm_runtime_get_xxx() ?"
pm_runtime_set_active(&dev->dev);
pm_runtime_enable(&dev->dev);
pm_runtime_get_xxx(&dev->dev);
usb_add_hcd()...
>
> > > +
> > > platform_set_drvdata(dev, hcd);
> > >
> > > return err;
> > > @@ -139,6 +142,9 @@ static int __devexit ehci_platform_remove(struct platform_device *dev)
> > > {
> > > struct usb_hcd *hcd = platform_get_drvdata(dev);
> > >
> > > + pm_runtime_put_sync(&dev->dev);
> > > + pm_runtime_disable(&dev->dev);
> >
> > These look wrong also. The put_sync will set the controller to low
> > power, which will cause usb_remove_hcd() below to fail.
> >
> > Probably you should have:
> >
> > pm_runtime_get_sync(&dev->dev);
> >
> > > usb_remove_hcd(hcd);
> >
> > Then here do:
> >
> > pm_runtime_disable(&dev->dev);
> > pm_runtime_put_nosuspend(&dev->dev);
> > pm_runtime_set_suspended(&dev->dev);
> >
> > > iounmap(hcd->regs);
> > > release_mem_region(hcd->rsrc_start, hcd->rsrc_len);
> >
> > Similar comments apply to your patch for ohci-platform.c.
> >
> > Alan Stern
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
> Best regards
> ---
> Kuninori Morimoto
Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
