Re: [PATCH 06/11] USB: OHCI: Properly handle ohci-exynos suspend
On Thu, 3 Oct 2013, manju goudar wrote:
On Wed, Oct 2, 2013 at 9:22 PM, Alan Stern st...@rowland.harvard.eduwrote:
On Wed, 2 Oct 2013, Bartlomiej Zolnierkiewicz wrote:
Hi,
On Wednesday, October 02, 2013 10:38:58 AM Alan Stern wrote:
On Wed, 2 Oct 2013, Bartlomiej Zolnierkiewicz wrote:
Maybe it would make sense to cleanup ohci_suspend() first (before
adding
new ohci_suspend() users) and remove unused do_wakeup parameter?
Not possible. The do_wakeup parameter is part of a function prototype
shared by other callback routines (such as ehci_suspend()) that _do_
use the parameter.
If you mean ohci-pci.c usage (which is currently the only usage of
ohci_suspend() looking at the latest -next kernel) than it is enough
to add a simple wrapper for it in ohci-pci.c:
...
static int ohci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
{
ohci_suspend(hcd);
}
...
ohci_pci_hc_driver.pci_suspend = ohci_pci_suspend;
...
Ah, now I see your point. Yes, it's true; that parameter could be
eliminated.
Manjunath, would you like to update your patch series to get rid of the
do_wakeup argument to ohci_suspend()?
Yes I will do. I think we can also rid of ehci_suspend() do_wakeup
argument.
Arrgh! Manjunath, I was wrong. I'm sorry to make you do all this
extra work -- your original patch series was correct.
Bartlomiej, we both failed to notice that the 1/11 patch in the
original series adds a usage of do_wakeup. Therefore that argument
cannot be removed.
Greg, please ignore Manjunath's V2 series (sent today) and merge the
original 11-patch series posted on October 2.
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/11] USB: OHCI: Properly handle ohci-exynos suspend
On Thursday, October 03, 2013 10:31:53 AM Alan Stern wrote:
On Thu, 3 Oct 2013, manju goudar wrote:
On Wed, Oct 2, 2013 at 9:22 PM, Alan Stern st...@rowland.harvard.eduwrote:
On Wed, 2 Oct 2013, Bartlomiej Zolnierkiewicz wrote:
Hi,
On Wednesday, October 02, 2013 10:38:58 AM Alan Stern wrote:
On Wed, 2 Oct 2013, Bartlomiej Zolnierkiewicz wrote:
Maybe it would make sense to cleanup ohci_suspend() first (before
adding
new ohci_suspend() users) and remove unused do_wakeup parameter?
Not possible. The do_wakeup parameter is part of a function prototype
shared by other callback routines (such as ehci_suspend()) that _do_
use the parameter.
If you mean ohci-pci.c usage (which is currently the only usage of
ohci_suspend() looking at the latest -next kernel) than it is enough
to add a simple wrapper for it in ohci-pci.c:
...
static int ohci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
{
ohci_suspend(hcd);
}
...
ohci_pci_hc_driver.pci_suspend = ohci_pci_suspend;
...
Ah, now I see your point. Yes, it's true; that parameter could be
eliminated.
Manjunath, would you like to update your patch series to get rid of the
do_wakeup argument to ohci_suspend()?
Yes I will do. I think we can also rid of ehci_suspend() do_wakeup
argument.
Arrgh! Manjunath, I was wrong. I'm sorry to make you do all this
extra work -- your original patch series was correct.
Bartlomiej, we both failed to notice that the 1/11 patch in the
original series adds a usage of do_wakeup. Therefore that argument
cannot be removed.
Uhh, indeed. Sorry about that.
Greg, please ignore Manjunath's V2 series (sent today) and merge the
original 11-patch series posted on October 2.
Yep.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung RD Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/11] USB: OHCI: Properly handle ohci-exynos suspend
On Thu, Oct 03, 2013 at 10:31:53AM -0400, Alan Stern wrote:
On Thu, 3 Oct 2013, manju goudar wrote:
On Wed, Oct 2, 2013 at 9:22 PM, Alan Stern st...@rowland.harvard.eduwrote:
On Wed, 2 Oct 2013, Bartlomiej Zolnierkiewicz wrote:
Hi,
On Wednesday, October 02, 2013 10:38:58 AM Alan Stern wrote:
On Wed, 2 Oct 2013, Bartlomiej Zolnierkiewicz wrote:
Maybe it would make sense to cleanup ohci_suspend() first (before
adding
new ohci_suspend() users) and remove unused do_wakeup parameter?
Not possible. The do_wakeup parameter is part of a function prototype
shared by other callback routines (such as ehci_suspend()) that _do_
use the parameter.
If you mean ohci-pci.c usage (which is currently the only usage of
ohci_suspend() looking at the latest -next kernel) than it is enough
to add a simple wrapper for it in ohci-pci.c:
...
static int ohci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
{
ohci_suspend(hcd);
}
...
ohci_pci_hc_driver.pci_suspend = ohci_pci_suspend;
...
Ah, now I see your point. Yes, it's true; that parameter could be
eliminated.
Manjunath, would you like to update your patch series to get rid of the
do_wakeup argument to ohci_suspend()?
Yes I will do. I think we can also rid of ehci_suspend() do_wakeup
argument.
Arrgh! Manjunath, I was wrong. I'm sorry to make you do all this
extra work -- your original patch series was correct.
Bartlomiej, we both failed to notice that the 1/11 patch in the
original series adds a usage of do_wakeup. Therefore that argument
cannot be removed.
Greg, please ignore Manjunath's V2 series (sent today) and merge the
original 11-patch series posted on October 2.
I no longer have these. Manjunath, can you please resend the correct
series that I should apply so that it is obvious which is your latest
version?
thanks,
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/11] USB: OHCI: Properly handle ohci-exynos suspend
From: Manjunath Goudar manjunath.gou...@linaro.org
Suspend scenario in case of ohci-exynos glue was not
properly handled as it was not suspending generic part
of ohci controller. Alan Stern suggested, properly handle
ohci-exynos suspend scenario.
Calling explicitly the ohci_suspend() routine in
exynos_ohci_suspend() will ensure proper handling of suspend
scenario.
Signed-off-by: Manjunath Goudar manjunath.gou...@linaro.org
Signed-off-by: Manjunath Goudar csmanjuvi...@gmail.com
Acked-by: Alan Stern st...@rowland.harvard.edu
Cc: Arnd Bergmann a...@arndb.de
Cc: Greg KH g...@kroah.com
Cc: linux-usb@vger.kernel.org
---
drivers/usb/host/ohci-exynos.c | 20 +---
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 3e4bc74..f5f372e 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -203,24 +203,15 @@ static int exynos_ohci_suspend(struct device *dev)
struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
struct ohci_hcd *ohci = hcd_to_ohci(hcd);
struct platform_device *pdev = to_platform_device(dev);
+ bool do_wakeup = device_may_wakeup(dev);
unsigned long flags;
int rc = 0;
- /*
-* Root hub was already suspended. Disable irq emission and
-* mark HW unaccessible, bail out if RH has been resumed. Use
-* the spinlock to properly synchronize with possible pending
-* RH suspend or resume activity.
-*/
- spin_lock_irqsave(ohci-lock, flags);
- if (ohci-rh_state != OHCI_RH_SUSPENDED
- ohci-rh_state != OHCI_RH_HALTED) {
- rc = -EINVAL;
- goto fail;
- }
-
- clear_bit(HCD_FLAG_HW_ACCESSIBLE, hcd-flags);
+ rc = ohci_suspend(hcd, do_wakeup);
+ if (rc)
+ return rc;
+ spin_lock_irqsave(ohci-lock, flags);
if (exynos_ohci-otg)
exynos_ohci-otg-set_host(exynos_ohci-otg, hcd-self);
@@ -228,7 +219,6 @@ static int exynos_ohci_suspend(struct device *dev)
clk_disable_unprepare(exynos_ohci-clk);
-fail:
spin_unlock_irqrestore(ohci-lock, flags);
return rc;
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/11] USB: OHCI: Properly handle ohci-exynos suspend
From: Manjunath Goudar manjunath.gou...@linaro.org
Suspend scenario in case of ohci-exynos glue was not
properly handled as it was not suspending generic part
of ohci controller. Alan Stern suggested, properly handle
ohci-exynos suspend scenario.
Calling explicitly the ohci_suspend() routine in
exynos_ohci_suspend() will ensure proper handling of suspend
scenario.
Signed-off-by: Manjunath Goudar manjunath.gou...@linaro.org
Signed-off-by: Manjunath Goudar csmanjuvi...@gmail.com
Acked-by: Alan Stern st...@rowland.harvard.edu
Cc: Arnd Bergmann a...@arndb.de
Cc: Greg KH g...@kroah.com
Cc: linux-usb@vger.kernel.org
---
drivers/usb/host/ohci-exynos.c | 20 +---
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 3e4bc74..f5f372e 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -203,24 +203,15 @@ static int exynos_ohci_suspend(struct device *dev)
struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
struct ohci_hcd *ohci = hcd_to_ohci(hcd);
struct platform_device *pdev = to_platform_device(dev);
+ bool do_wakeup = device_may_wakeup(dev);
unsigned long flags;
int rc = 0;
- /*
-* Root hub was already suspended. Disable irq emission and
-* mark HW unaccessible, bail out if RH has been resumed. Use
-* the spinlock to properly synchronize with possible pending
-* RH suspend or resume activity.
-*/
- spin_lock_irqsave(ohci-lock, flags);
- if (ohci-rh_state != OHCI_RH_SUSPENDED
- ohci-rh_state != OHCI_RH_HALTED) {
- rc = -EINVAL;
- goto fail;
- }
-
- clear_bit(HCD_FLAG_HW_ACCESSIBLE, hcd-flags);
+ rc = ohci_suspend(hcd, do_wakeup);
+ if (rc)
+ return rc;
+ spin_lock_irqsave(ohci-lock, flags);
if (exynos_ohci-otg)
exynos_ohci-otg-set_host(exynos_ohci-otg, hcd-self);
@@ -228,7 +219,6 @@ static int exynos_ohci_suspend(struct device *dev)
clk_disable_unprepare(exynos_ohci-clk);
-fail:
spin_unlock_irqrestore(ohci-lock, flags);
return rc;
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/11] USB: OHCI: Properly handle ohci-exynos suspend
Hi,
On Wednesday, October 02, 2013 03:45:42 PM Majunath Goudar wrote:
From: Manjunath Goudar manjunath.gou...@linaro.org
Suspend scenario in case of ohci-exynos glue was not
properly handled as it was not suspending generic part
of ohci controller. Alan Stern suggested, properly handle
ohci-exynos suspend scenario.
Calling explicitly the ohci_suspend() routine in
exynos_ohci_suspend() will ensure proper handling of suspend
scenario.
Signed-off-by: Manjunath Goudar manjunath.gou...@linaro.org
Signed-off-by: Manjunath Goudar csmanjuvi...@gmail.com
Acked-by: Alan Stern st...@rowland.harvard.edu
Cc: Arnd Bergmann a...@arndb.de
Cc: Greg KH g...@kroah.com
Cc: linux-usb@vger.kernel.org
---
drivers/usb/host/ohci-exynos.c | 20 +---
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index 3e4bc74..f5f372e 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -203,24 +203,15 @@ static int exynos_ohci_suspend(struct device *dev)
struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
struct ohci_hcd *ohci = hcd_to_ohci(hcd);
struct platform_device *pdev = to_platform_device(dev);
+ bool do_wakeup = device_may_wakeup(dev);
unsigned long flags;
int rc = 0;
- /*
- * Root hub was already suspended. Disable irq emission and
- * mark HW unaccessible, bail out if RH has been resumed. Use
- * the spinlock to properly synchronize with possible pending
- * RH suspend or resume activity.
- */
- spin_lock_irqsave(ohci-lock, flags);
- if (ohci-rh_state != OHCI_RH_SUSPENDED
- ohci-rh_state != OHCI_RH_HALTED) {
- rc = -EINVAL;
- goto fail;
- }
-
- clear_bit(HCD_FLAG_HW_ACCESSIBLE, hcd-flags);
+ rc = ohci_suspend(hcd, do_wakeup);
Maybe it would make sense to cleanup ohci_suspend() first (before adding
new ohci_suspend() users) and remove unused do_wakeup parameter?
+ if (rc)
+ return rc;
+ spin_lock_irqsave(ohci-lock, flags);
if (exynos_ohci-otg)
exynos_ohci-otg-set_host(exynos_ohci-otg, hcd-self);
@@ -228,7 +219,6 @@ static int exynos_ohci_suspend(struct device *dev)
clk_disable_unprepare(exynos_ohci-clk);
-fail:
spin_unlock_irqrestore(ohci-lock, flags);
return rc;
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung RD Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/11] USB: OHCI: Properly handle ohci-exynos suspend
On Wed, 2 Oct 2013, Bartlomiej Zolnierkiewicz wrote: Maybe it would make sense to cleanup ohci_suspend() first (before adding new ohci_suspend() users) and remove unused do_wakeup parameter? Not possible. The do_wakeup parameter is part of a function prototype shared by other callback routines (such as ehci_suspend()) that _do_ use the parameter. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/11] USB: OHCI: Properly handle ohci-exynos suspend
Hi,
On Wednesday, October 02, 2013 10:38:58 AM Alan Stern wrote:
On Wed, 2 Oct 2013, Bartlomiej Zolnierkiewicz wrote:
Maybe it would make sense to cleanup ohci_suspend() first (before adding
new ohci_suspend() users) and remove unused do_wakeup parameter?
Not possible. The do_wakeup parameter is part of a function prototype
shared by other callback routines (such as ehci_suspend()) that _do_
use the parameter.
If you mean ohci-pci.c usage (which is currently the only usage of
ohci_suspend() looking at the latest -next kernel) than it is enough
to add a simple wrapper for it in ohci-pci.c:
...
static int ohci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
{
ohci_suspend(hcd);
}
...
ohci_pci_hc_driver.pci_suspend = ohci_pci_suspend;
...
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung RD Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/11] USB: OHCI: Properly handle ohci-exynos suspend
On Wed, 2 Oct 2013, Bartlomiej Zolnierkiewicz wrote:
Hi,
On Wednesday, October 02, 2013 10:38:58 AM Alan Stern wrote:
On Wed, 2 Oct 2013, Bartlomiej Zolnierkiewicz wrote:
Maybe it would make sense to cleanup ohci_suspend() first (before adding
new ohci_suspend() users) and remove unused do_wakeup parameter?
Not possible. The do_wakeup parameter is part of a function prototype
shared by other callback routines (such as ehci_suspend()) that _do_
use the parameter.
If you mean ohci-pci.c usage (which is currently the only usage of
ohci_suspend() looking at the latest -next kernel) than it is enough
to add a simple wrapper for it in ohci-pci.c:
...
static int ohci_pci_suspend(struct usb_hcd *hcd, bool do_wakeup)
{
ohci_suspend(hcd);
}
...
ohci_pci_hc_driver.pci_suspend = ohci_pci_suspend;
...
Ah, now I see your point. Yes, it's true; that parameter could be
eliminated.
Manjunath, would you like to update your patch series to get rid of the
do_wakeup argument to ohci_suspend()?
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 06/11] USB: OHCI: Properly handle ohci-exynos suspend
Suspend scenario in case of ohci-exynos glue was not
properly handled as it was not suspending generic part
of ohci controller. Calling explicitly the ohci_suspend()
routine in exynos_ohci_suspend() will ensure proper
handling of suspend scenario.
Signed-off-by: Manjunath Goudar manjunath.gou...@linaro.org
Acked-by: Alan Stern st...@rowland.harvard.edu
Cc: Arnd Bergmann a...@arndb.de
Cc: Greg KH g...@kroah.com
Cc: linux-usb@vger.kernel.org
V2:
-Incase ohci_suspend() fails, return right away without
executing further.
V3:
-rid of unwanted code from ohci_hcd_s3c2410_drv_suspend() which already
ohci_suspend() does it.
-Aligned variable do_wakeup and ret.
V4:
-The do_wakeup and rc variable alignment is removed.
---
drivers/usb/host/ohci-exynos.c | 20 +---
1 file changed, 5 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/host/ohci-exynos.c b/drivers/usb/host/ohci-exynos.c
index ae6068d..17de3dd 100644
--- a/drivers/usb/host/ohci-exynos.c
+++ b/drivers/usb/host/ohci-exynos.c
@@ -203,24 +203,15 @@ static int exynos_ohci_suspend(struct device *dev)
struct exynos_ohci_hcd *exynos_ohci = to_exynos_ohci(hcd);
struct ohci_hcd *ohci = hcd_to_ohci(hcd);
struct platform_device *pdev = to_platform_device(dev);
+ bool do_wakeup = device_may_wakeup(dev);
unsigned long flags;
int rc = 0;
- /*
-* Root hub was already suspended. Disable irq emission and
-* mark HW unaccessible, bail out if RH has been resumed. Use
-* the spinlock to properly synchronize with possible pending
-* RH suspend or resume activity.
-*/
- spin_lock_irqsave(ohci-lock, flags);
- if (ohci-rh_state != OHCI_RH_SUSPENDED
- ohci-rh_state != OHCI_RH_HALTED) {
- rc = -EINVAL;
- goto fail;
- }
-
- clear_bit(HCD_FLAG_HW_ACCESSIBLE, hcd-flags);
+ rc = ohci_suspend(hcd, do_wakeup);
+ if (rc)
+ return rc;
+ spin_lock_irqsave(ohci-lock, flags);
if (exynos_ohci-otg)
exynos_ohci-otg-set_host(exynos_ohci-otg, hcd-self);
@@ -228,7 +219,6 @@ static int exynos_ohci_suspend(struct device *dev)
clk_disable_unprepare(exynos_ohci-clk);
-fail:
spin_unlock_irqrestore(ohci-lock, flags);
return rc;
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
