[PATCH 06/11] USB: OHCI: Properly handle ohci-exynos suspend

2013-10-03 Thread Majunath Goudar
From: Manjunath Goudar 

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 
Signed-off-by: Manjunath Goudar 
Acked-by: Alan Stern 
Cc: Arnd Bergmann 
Cc: Greg KH 
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

2013-10-03 Thread Greg KH
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 wrote:
> > 
> > > 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


Re: [PATCH 06/11] USB: OHCI: Properly handle ohci-exynos suspend

2013-10-03 Thread Bartlomiej Zolnierkiewicz
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 wrote:
> > 
> > > 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 R&D 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

2013-10-03 Thread Alan Stern
On Thu, 3 Oct 2013, manju goudar wrote:

> On Wed, Oct 2, 2013 at 9:22 PM, Alan Stern wrote:
> 
> > 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

2013-10-02 Thread Alan Stern
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


Re: [PATCH 06/11] USB: OHCI: Properly handle ohci-exynos suspend

2013-10-02 Thread Bartlomiej Zolnierkiewicz

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 R&D 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

2013-10-02 Thread Alan Stern
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

2013-10-02 Thread Bartlomiej Zolnierkiewicz

Hi,

On Wednesday, October 02, 2013 03:45:42 PM Majunath Goudar wrote:
> From: Manjunath Goudar 
> 
> 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 
> Signed-off-by: Manjunath Goudar 
> Acked-by: Alan Stern 
> Cc: Arnd Bergmann 
> Cc: Greg KH 
> 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 R&D 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


[PATCH 06/11] USB: OHCI: Properly handle ohci-exynos suspend

2013-10-02 Thread Majunath Goudar
From: Manjunath Goudar 

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 
Signed-off-by: Manjunath Goudar 
Acked-by: Alan Stern 
Cc: Arnd Bergmann 
Cc: Greg KH 
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

2013-07-22 Thread Manjunath Goudar
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 
Acked-by: Alan Stern 
Cc: Arnd Bergmann 
Cc: Greg KH 
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