| Message ID | 1371114661-2207-11-git-send-email-manjunath.goudar@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 13 June 2013 14:41, Manjunath Goudar <manjunath.goudar@linaro.org> wrote: > Suspend scenario in case of ohci-spear glue was not > properly handled as it was not suspending generic part > of ohci controller.Calling explicitly the ohci_suspend() Add space after full stop. > routine in spear_ohci_hcd_drv_suspend() will ensure proper > handling of suspend scenario. > > V2: > -Incase ohci_suspend() fails, return right away without > executing further. You added changelog at wrong place. This will get commited once somebody applies this patch. > Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Alan Stern <stern@rowland.harvard.edu> > Cc: Greg KH <greg@kroah.com> > Cc: linux-usb@vger.kernel.org > --- Add it here. > drivers/usb/host/ohci-spear.c | 16 +++++++++++++--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/host/ohci-spear.c b/drivers/usb/host/ohci-spear.c > index 31ff3fc..2ff867b 100644 > --- a/drivers/usb/host/ohci-spear.c > +++ b/drivers/usb/host/ohci-spear.c > @@ -130,12 +130,22 @@ static int spear_ohci_hcd_drv_remove(struct platform_device *pdev) > } > > #if defined(CONFIG_PM) > -static int spear_ohci_hcd_drv_suspend(struct platform_device *dev, > +static int spear_ohci_hcd_drv_suspend(struct platform_device *pdev, > pm_message_t message) > { > - struct usb_hcd *hcd = platform_get_drvdata(dev); > + struct usb_hcd *hcd = platform_get_drvdata(pdev); > struct ohci_hcd *ohci = hcd_to_ohci(hcd); > struct spear_ohci *sohci_p = to_spear_ohci(hcd); > + bool do_wakeup = device_may_wakeup(&pdev->dev); > + int ret; > + > + ret = ohci_suspend(hcd, do_wakeup); > + if (ret == 0 && do_wakeup && HCD_WAKEUP_PENDING(hcd)) { maybe s/ret == 0/!ret I thought ret == 0 is the success case and not error case. > + ohci_resume(hcd, false); > + ret = -EBUSY; > + } > + if (ret) > + return ret; > > if (time_before(jiffies, ohci->next_statechange)) > msleep(5); > @@ -143,7 +153,7 @@ static int spear_ohci_hcd_drv_suspend(struct platform_device *dev, > > clk_disable_unprepare(sohci_p->clk); > > - return 0; > + return ret; > } > > static int spear_ohci_hcd_drv_resume(struct platform_device *dev) > -- > 1.7.9.5 > > > _______________________________________________ > linaro-dev mailing list > linaro-dev@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-dev
On 13 June 2013 14:49, Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 13 June 2013 14:41, Manjunath Goudar <manjunath.goudar@linaro.org> > wrote: > > Suspend scenario in case of ohci-spear glue was not > > properly handled as it was not suspending generic part > > of ohci controller.Calling explicitly the ohci_suspend() > > Add space after full stop. > > > routine in spear_ohci_hcd_drv_suspend() will ensure proper > > handling of suspend scenario. > > > > V2: > > -Incase ohci_suspend() fails, return right away without > > executing further. > > You added changelog at wrong place. This will get commited once > somebody applies this patch. > > > Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Alan Stern <stern@rowland.harvard.edu> > > Cc: Greg KH <greg@kroah.com> > > Cc: linux-usb@vger.kernel.org > > --- > > Add it here. > > > drivers/usb/host/ohci-spear.c | 16 +++++++++++++--- > > 1 file changed, 13 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/usb/host/ohci-spear.c > b/drivers/usb/host/ohci-spear.c > > index 31ff3fc..2ff867b 100644 > > --- a/drivers/usb/host/ohci-spear.c > > +++ b/drivers/usb/host/ohci-spear.c > > @@ -130,12 +130,22 @@ static int spear_ohci_hcd_drv_remove(struct > platform_device *pdev) > > } > > > > #if defined(CONFIG_PM) > > -static int spear_ohci_hcd_drv_suspend(struct platform_device *dev, > > +static int spear_ohci_hcd_drv_suspend(struct platform_device *pdev, > > pm_message_t message) > > { > > - struct usb_hcd *hcd = platform_get_drvdata(dev); > > + struct usb_hcd *hcd = platform_get_drvdata(pdev); > > struct ohci_hcd *ohci = hcd_to_ohci(hcd); > > struct spear_ohci *sohci_p = to_spear_ohci(hcd); > > + bool do_wakeup = device_may_wakeup(&pdev->dev); > > + int ret; > > + > > + ret = ohci_suspend(hcd, do_wakeup); > > + if (ret == 0 && do_wakeup && HCD_WAKEUP_PENDING(hcd)) { > > maybe s/ret == 0/!ret > > I thought ret == 0 is the success case and not error case yes ret == 0 is success when do_wakeup=false and HCD_WAKEUP_PENDING() return 0 or any one return 0 otherwise it is fail only. > > + ohci_resume(hcd, false); > > + ret = -EBUSY; > > + } > > + if (ret) > > + return ret; > > > > if (time_before(jiffies, ohci->next_statechange)) > > msleep(5); > > @@ -143,7 +153,7 @@ static int spear_ohci_hcd_drv_suspend(struct > platform_device *dev, > > > > clk_disable_unprepare(sohci_p->clk); > > > > - return 0; > > + return ret; > > } > > > > static int spear_ohci_hcd_drv_resume(struct platform_device *dev) > > -- > > 1.7.9.5 > > > > > > _______________________________________________ > > linaro-dev mailing list > > linaro-dev@lists.linaro.org > > http://lists.linaro.org/mailman/listinfo/linaro-dev >
On Thu, 13 Jun 2013, Viresh Kumar wrote: > > + bool do_wakeup = device_may_wakeup(&pdev->dev); > > + int ret; > > + > > + ret = ohci_suspend(hcd, do_wakeup); > > + if (ret == 0 && do_wakeup && HCD_WAKEUP_PENDING(hcd)) { > > maybe s/ret == 0/!ret I tend to prefer using ! for boolean values and == 0 for numeric values. Just a matter of personal taste. > I thought ret == 0 is the success case and not error case. > > > + ohci_resume(hcd, false); > > + ret = -EBUSY; > > + } Yes, 0 is the success case. You must not have seen the explanation for this code; it is needed to handle a race. If the suspend succeeds but a wakeup request has already arrived, we need to undo the suspend. Obviously, if the suspend failed then there's no need to undo anything. Alan Stern
diff --git a/drivers/usb/host/ohci-spear.c b/drivers/usb/host/ohci-spear.c index 31ff3fc..2ff867b 100644 --- a/drivers/usb/host/ohci-spear.c +++ b/drivers/usb/host/ohci-spear.c @@ -130,12 +130,22 @@ static int spear_ohci_hcd_drv_remove(struct platform_device *pdev) } #if defined(CONFIG_PM) -static int spear_ohci_hcd_drv_suspend(struct platform_device *dev, +static int spear_ohci_hcd_drv_suspend(struct platform_device *pdev, pm_message_t message) { - struct usb_hcd *hcd = platform_get_drvdata(dev); + struct usb_hcd *hcd = platform_get_drvdata(pdev); struct ohci_hcd *ohci = hcd_to_ohci(hcd); struct spear_ohci *sohci_p = to_spear_ohci(hcd); + bool do_wakeup = device_may_wakeup(&pdev->dev); + int ret; + + ret = ohci_suspend(hcd, do_wakeup); + if (ret == 0 && do_wakeup && HCD_WAKEUP_PENDING(hcd)) { + ohci_resume(hcd, false); + ret = -EBUSY; + } + if (ret) + return ret; if (time_before(jiffies, ohci->next_statechange)) msleep(5); @@ -143,7 +153,7 @@ static int spear_ohci_hcd_drv_suspend(struct platform_device *dev, clk_disable_unprepare(sohci_p->clk); - return 0; + return ret; } static int spear_ohci_hcd_drv_resume(struct platform_device *dev)
Suspend scenario in case of ohci-spear glue was not properly handled as it was not suspending generic part of ohci controller.Calling explicitly the ohci_suspend() routine in spear_ohci_hcd_drv_suspend() will ensure proper handling of suspend scenario. V2: -Incase ohci_suspend() fails, return right away without executing further. Signed-off-by: Manjunath Goudar <manjunath.goudar@linaro.org> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Alan Stern <stern@rowland.harvard.edu> Cc: Greg KH <greg@kroah.com> Cc: linux-usb@vger.kernel.org --- drivers/usb/host/ohci-spear.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)