On Tue, 31 Jul 2012, Kuninori Morimoto wrote: > 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()... The problem is that the ehci-platform driver currently doesn't know anything about clocks or power sources. We need to add that information in a generic fashion, somehow. Until that's done, you can't really use ehci-platform as your driver. So for example, if ehci-platform _did_ know about clocks and power sources, then before calling usb_add_hcd() you would do: ehci_platform_enable_clocks_and_power(dev); pm_runtime_set_active(&dev->dev); pm_runtime_enable(&dev->dev); But you definitely cannot call pm_runtime_get_aync() or anything similar, because the EHCI core resume routine assumes the controller's clocks are already running and its power supply is at full power. Can you figure out a good way to add clock and power information to the usb_ehci_pdata structure? Something that will work with a large variety of SoC EHCI controllers? 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
