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 ?
> > +
> > 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
--
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
