Patch 'b8efdaf USB: EHCI: add check for wakeup/suspend race'
adds a check for possible race between suspend and wakeup interrupt,
and thereby it returns -EBUSY as error code if there's a wakeup
interrupt.
So the platform host controller should not proceed further with
its suspend callback, rather should return immediately to avoid
powering down the essential things, like phy.
Signed-off-by: Vivek Gautam <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Jingoo Han <[email protected]>
---
Based on 'usb-next' branch of Greg's usb tree.
drivers/usb/host/ehci-exynos.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
index d1d8c47..a4550eb 100644
--- a/drivers/usb/host/ehci-exynos.c
+++ b/drivers/usb/host/ehci-exynos.c
@@ -212,6 +212,8 @@ static int exynos_ehci_suspend(struct device *dev)
int rc;
rc = ehci_suspend(hcd, do_wakeup);
+ if (rc)
+ return rc;
if (exynos_ehci->otg)
exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
@@ -221,7 +223,7 @@ static int exynos_ehci_suspend(struct device *dev)
clk_disable_unprepare(exynos_ehci->clk);
- return rc;
+ return 0;
}
static int exynos_ehci_resume(struct device *dev)
--
1.7.10.4
Patch 'b8efdaf USB: EHCI: add check for wakeup/suspend race'
adds a check for possible race between suspend and wakeup interrupt,
and thereby it returns -EBUSY as error code if there's a wakeup
interrupt.
So the platform host controller should not proceed further with
its suspend callback, rather should return immediately to avoid
powering down the essential things, like phy.
Signed-off-by: Vivek Gautam <[email protected]>
Cc: Alan Stern <[email protected]>
Cc: Hauke Mehrtens <[email protected]>
Cc: Hans de Goede <[email protected]>
---
Based on 'usb-next' branch of Greg's usb tree.
drivers/usb/host/ehci-platform.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/ehci-platform.c b/drivers/usb/host/ehci-platform.c
index b3a0e11..60d3d1a 100644
--- a/drivers/usb/host/ehci-platform.c
+++ b/drivers/usb/host/ehci-platform.c
@@ -303,11 +303,13 @@ static int ehci_platform_suspend(struct device *dev)
int ret;
ret = ehci_suspend(hcd, do_wakeup);
+ if (ret)
+ return ret;
if (pdata->power_suspend)
pdata->power_suspend(pdev);
- return ret;
+ return 0;
}
static int ehci_platform_resume(struct device *dev)
--
1.7.10.4
On Wednesday, April 09, 2014 1:01 PM, Vivek Gautam wrote:
>
> Patch 'b8efdaf USB: EHCI: add check for wakeup/suspend race'
> adds a check for possible race between suspend and wakeup interrupt,
> and thereby it returns -EBUSY as error code if there's a wakeup
> interrupt.
> So the platform host controller should not proceed further with
> its suspend callback, rather should return immediately to avoid
> powering down the essential things, like phy.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> Cc: Alan Stern <[email protected]>
> Cc: Jingoo Han <[email protected]>
Acked-by: Jingoo Han <[email protected]>
Best regards,
Jingoo Han
> ---
>
> Based on 'usb-next' branch of Greg's usb tree.
>
> drivers/usb/host/ehci-exynos.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
> index d1d8c47..a4550eb 100644
> --- a/drivers/usb/host/ehci-exynos.c
> +++ b/drivers/usb/host/ehci-exynos.c
> @@ -212,6 +212,8 @@ static int exynos_ehci_suspend(struct device *dev)
> int rc;
>
> rc = ehci_suspend(hcd, do_wakeup);
> + if (rc)
> + return rc;
>
> if (exynos_ehci->otg)
> exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
> @@ -221,7 +223,7 @@ static int exynos_ehci_suspend(struct device *dev)
>
> clk_disable_unprepare(exynos_ehci->clk);
>
> - return rc;
> + return 0;
> }
>
> static int exynos_ehci_resume(struct device *dev)
> --
> 1.7.10.4
On Wed, 9 Apr 2014, Vivek Gautam wrote:
> Patch 'b8efdaf USB: EHCI: add check for wakeup/suspend race'
> adds a check for possible race between suspend and wakeup interrupt,
> and thereby it returns -EBUSY as error code if there's a wakeup
> interrupt.
> So the platform host controller should not proceed further with
> its suspend callback, rather should return immediately to avoid
> powering down the essential things, like phy.
>
> Signed-off-by: Vivek Gautam <[email protected]>
> Cc: Alan Stern <[email protected]>
> Cc: Jingoo Han <[email protected]>
> ---
>
> Based on 'usb-next' branch of Greg's usb tree.
>
> drivers/usb/host/ehci-exynos.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
> index d1d8c47..a4550eb 100644
> --- a/drivers/usb/host/ehci-exynos.c
> +++ b/drivers/usb/host/ehci-exynos.c
> @@ -212,6 +212,8 @@ static int exynos_ehci_suspend(struct device *dev)
> int rc;
>
> rc = ehci_suspend(hcd, do_wakeup);
> + if (rc)
> + return rc;
>
> if (exynos_ehci->otg)
> exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
> @@ -221,7 +223,7 @@ static int exynos_ehci_suspend(struct device *dev)
>
> clk_disable_unprepare(exynos_ehci->clk);
>
> - return rc;
> + return 0;
> }
>
> static int exynos_ehci_resume(struct device *dev)
The first hunk of this patch is correct, but the second hunk isn't
needed. A similar remark is true for the ehci-platform patch.
Alan Stern
On Thursday, April 10, 2014 3:32 AM, Alan Stern wrote:
> On Wed, 9 Apr 2014, Vivek Gautam wrote:
>
> > Patch 'b8efdaf USB: EHCI: add check for wakeup/suspend race'
> > adds a check for possible race between suspend and wakeup interrupt,
> > and thereby it returns -EBUSY as error code if there's a wakeup
> > interrupt.
> > So the platform host controller should not proceed further with
> > its suspend callback, rather should return immediately to avoid
> > powering down the essential things, like phy.
> >
> > Signed-off-by: Vivek Gautam <[email protected]>
> > Cc: Alan Stern <[email protected]>
> > Cc: Jingoo Han <[email protected]>
> > ---
> >
> > Based on 'usb-next' branch of Greg's usb tree.
> >
> > drivers/usb/host/ehci-exynos.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/host/ehci-exynos.c b/drivers/usb/host/ehci-exynos.c
> > index d1d8c47..a4550eb 100644
> > --- a/drivers/usb/host/ehci-exynos.c
> > +++ b/drivers/usb/host/ehci-exynos.c
> > @@ -212,6 +212,8 @@ static int exynos_ehci_suspend(struct device *dev)
> > int rc;
> >
> > rc = ehci_suspend(hcd, do_wakeup);
> > + if (rc)
> > + return rc;
> >
> > if (exynos_ehci->otg)
> > exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
> > @@ -221,7 +223,7 @@ static int exynos_ehci_suspend(struct device *dev)
> >
> > clk_disable_unprepare(exynos_ehci->clk);
> >
> > - return rc;
> > + return 0;
> > }
> >
> > static int exynos_ehci_resume(struct device *dev)
>
> The first hunk of this patch is correct, but the second hunk isn't
> needed. A similar remark is true for the ehci-platform patch.
Hi Alan,
Do you mean the following?
1st hunk
+ if (rc)
+ return rc;
2nd hunk
- return rc;
+ return 0;
Currently, the 'rc' will be always 'zero'; however, I don't
Have any objection, because the code might be modified later.
Best regards,
Jingoo Han
>
> Alan Stern
On Thu, 10 Apr 2014, Jingoo Han wrote:
> > > --- a/drivers/usb/host/ehci-exynos.c
> > > +++ b/drivers/usb/host/ehci-exynos.c
> > > @@ -212,6 +212,8 @@ static int exynos_ehci_suspend(struct device *dev)
> > > int rc;
> > >
> > > rc = ehci_suspend(hcd, do_wakeup);
> > > + if (rc)
> > > + return rc;
> > >
> > > if (exynos_ehci->otg)
> > > exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
> > > @@ -221,7 +223,7 @@ static int exynos_ehci_suspend(struct device *dev)
> > >
> > > clk_disable_unprepare(exynos_ehci->clk);
> > >
> > > - return rc;
> > > + return 0;
> > > }
> > >
> > > static int exynos_ehci_resume(struct device *dev)
> >
> > The first hunk of this patch is correct, but the second hunk isn't
> > needed. A similar remark is true for the ehci-platform patch.
>
> Hi Alan,
>
> Do you mean the following?
>
> 1st hunk
> + if (rc)
> + return rc;
>
> 2nd hunk
> - return rc;
> + return 0;
Yes, that's what I mean.
> Currently, the 'rc' will be always 'zero'; however, I don't
> Have any objection, because the code might be modified later.
Exactly. We should add the new "if" statement but leave the "return
rc" the way it is.
Alan Stern
Hi Alan,
On Thu, Apr 10, 2014 at 7:06 AM, Alan Stern <[email protected]> wrote:
> On Thu, 10 Apr 2014, Jingoo Han wrote:
>
>> > > --- a/drivers/usb/host/ehci-exynos.c
>> > > +++ b/drivers/usb/host/ehci-exynos.c
>> > > @@ -212,6 +212,8 @@ static int exynos_ehci_suspend(struct device *dev)
>> > > int rc;
>> > >
>> > > rc = ehci_suspend(hcd, do_wakeup);
>> > > + if (rc)
>> > > + return rc;
>> > >
>> > > if (exynos_ehci->otg)
>> > > exynos_ehci->otg->set_host(exynos_ehci->otg, &hcd->self);
>> > > @@ -221,7 +223,7 @@ static int exynos_ehci_suspend(struct device *dev)
>> > >
>> > > clk_disable_unprepare(exynos_ehci->clk);
>> > >
>> > > - return rc;
>> > > + return 0;
>> > > }
>> > >
>> > > static int exynos_ehci_resume(struct device *dev)
>> >
>> > The first hunk of this patch is correct, but the second hunk isn't
>> > needed. A similar remark is true for the ehci-platform patch.
>>
>> Hi Alan,
>>
>> Do you mean the following?
>>
>> 1st hunk
>> + if (rc)
>> + return rc;
>>
>> 2nd hunk
>> - return rc;
>> + return 0;
>
> Yes, that's what I mean.
>
>> Currently, the 'rc' will be always 'zero'; however, I don't
>> Have any objection, because the code might be modified later.
>
> Exactly. We should add the new "if" statement but leave the "return
> rc" the way it is.
Sure, i will update both the patches.
>
> Alan Stern
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html