2014-10-06 19:44:29

by Rabin Vincent

[permalink] [raw]
Subject: Re: [PATCH] phy: omap-usb2: Enable runtime PM of omap-usb2 phy properly

On Fri, Sep 26, 2014 at 02:17:47PM +0100, Oussama Ghorbel wrote:
> You may find merge conflict since v3.17-rc5.
> If necessary I can send a re-based version of this patch on v3.17-rc5 for
> seamless integration.

Looks like pandaboard USB is still broken on v3.17 due to this bug.
Please do send an updated patch based on latest mainline so that Kishon
can apply it easily when he gets around to it.

> On 09/24/2014 05:15 PM, Oussama Ghorbel wrote:
> >The USB OTG port does not work since v3.16 on omap platform.
> >This is a regression introduced by the commit
> >eb82a3d846fa (phy: omap-usb2: Balance pm_runtime_enable() on probe failure
> > and remove).
> >This because the call to pm_runtime_enable() function is moved after the
> >call to devm_phy_create() function, which has side effect since later in
> >the subsequent calls of devm_phy_create() there is a check with
> >pm_runtime_enabled() to configure few things.
> >
> >Signed-off-by: Oussama Ghorbel <[email protected]>
> >---
> > drivers/phy/phy-omap-usb2.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
> >index 34b3961..3fb9293 100644
> >--- a/drivers/phy/phy-omap-usb2.c
> >+++ b/drivers/phy/phy-omap-usb2.c
> >@@ -262,14 +262,16 @@ static int omap_usb2_probe(struct platform_device *pdev)
> > otg->phy = &phy->phy;
> > platform_set_drvdata(pdev, phy);
> >+ pm_runtime_enable(phy->dev);
> > generic_phy = devm_phy_create(phy->dev, &ops, NULL);
> >- if (IS_ERR(generic_phy))
> >+ if (IS_ERR(generic_phy)) {
> >+ pm_runtime_disable(phy->dev);
> > return PTR_ERR(generic_phy);
> >+ }
> > phy_set_drvdata(generic_phy, phy);
> >- pm_runtime_enable(phy->dev);
> > phy_provider = devm_of_phy_provider_register(phy->dev,
> > of_phy_simple_xlate);
> > if (IS_ERR(phy_provider)) {


2014-10-07 11:04:23

by Oussama Ghorbel

[permalink] [raw]
Subject: [PATCHv2] phy: omap-usb2: Enable runtime PM of omap-usb2 phy properly

The USB OTG port does not work since v3.16 on omap platform.
This is a regression introduced by the commit
eb82a3d846fa (phy: omap-usb2: Balance pm_runtime_enable() on probe failure
and remove).
This because the call to pm_runtime_enable() function is moved after the
call to devm_phy_create() function, which has side effect since later in
the subsequent calls of devm_phy_create() there is a check with
pm_runtime_enabled() to configure few things.

Signed-off-by: Oussama Ghorbel <[email protected]>
---
drivers/phy/phy-omap-usb2.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index 93d7835..acc13f8 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -262,14 +262,16 @@ static int omap_usb2_probe(struct platform_device *pdev)
otg->phy = &phy->phy;

platform_set_drvdata(pdev, phy);
+ pm_runtime_enable(phy->dev);

generic_phy = devm_phy_create(phy->dev, NULL, &ops, NULL);
- if (IS_ERR(generic_phy))
+ if (IS_ERR(generic_phy)) {
+ pm_runtime_disable(phy->dev);
return PTR_ERR(generic_phy);
+ }

phy_set_drvdata(generic_phy, phy);

- pm_runtime_enable(phy->dev);
phy_provider = devm_of_phy_provider_register(phy->dev,
of_phy_simple_xlate);
if (IS_ERR(phy_provider)) {
--
1.8.3.2

2014-10-07 17:11:20

by Rabin Vincent

[permalink] [raw]
Subject: Re: [PATCHv2] phy: omap-usb2: Enable runtime PM of omap-usb2 phy properly

On Tue, Oct 07, 2014 at 12:02:51PM +0100, Oussama Ghorbel wrote:
> The USB OTG port does not work since v3.16 on omap platform.
> This is a regression introduced by the commit
> eb82a3d846fa (phy: omap-usb2: Balance pm_runtime_enable() on probe failure
> and remove).
> This because the call to pm_runtime_enable() function is moved after the
> call to devm_phy_create() function, which has side effect since later in
> the subsequent calls of devm_phy_create() there is a check with
> pm_runtime_enabled() to configure few things.
>
> Signed-off-by: Oussama Ghorbel <[email protected]>

Tested-by: Rabin Vincent <[email protected]>

Thanks.

2014-10-08 08:18:10

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCHv2] phy: omap-usb2: Enable runtime PM of omap-usb2 phy properly

Hi Oussama,

On 10/07/2014 02:02 PM, Oussama Ghorbel wrote:
> The USB OTG port does not work since v3.16 on omap platform.
> This is a regression introduced by the commit
> eb82a3d846fa (phy: omap-usb2: Balance pm_runtime_enable() on probe failure
> and remove).
> This because the call to pm_runtime_enable() function is moved after the
> call to devm_phy_create() function, which has side effect since later in
> the subsequent calls of devm_phy_create() there is a check with
> pm_runtime_enabled() to configure few things.
>
> Signed-off-by: Oussama Ghorbel <[email protected]>

Acked-by: Roger Quadros <[email protected]>

Could you please send the patch to
Cc: <[email protected]> [3.16+]

That way it will get into 3.16 onwards. Thanks.

cheers,
-roger

2014-10-08 10:55:18

by Oussama Ghorbel

[permalink] [raw]
Subject: Re: [PATCHv2] phy: omap-usb2: Enable runtime PM of omap-usb2 phy properly

Hi Roger,

Should I resend this v2 version of the patch to [email protected]
which is suitable for v3.17 and which will require a very tiny adaptation?
Or I should resend the first version of this patch which is suitable for
3.16 but might seems confusing a little sine I'm resend the outdated
version.

Thanks,
Oussama
On 10/08/2014 09:17 AM, Roger Quadros wrote:
> Hi Oussama,
>
> On 10/07/2014 02:02 PM, Oussama Ghorbel wrote:
>> The USB OTG port does not work since v3.16 on omap platform.
>> This is a regression introduced by the commit
>> eb82a3d846fa (phy: omap-usb2: Balance pm_runtime_enable() on probe failure
>> and remove).
>> This because the call to pm_runtime_enable() function is moved after the
>> call to devm_phy_create() function, which has side effect since later in
>> the subsequent calls of devm_phy_create() there is a check with
>> pm_runtime_enabled() to configure few things.
>>
>> Signed-off-by: Oussama Ghorbel <[email protected]>
> Acked-by: Roger Quadros <[email protected]>
>
> Could you please send the patch to
> Cc: <[email protected]> [3.16+]
>
> That way it will get into 3.16 onwards. Thanks.
>
> cheers,
> -roger
>

2014-10-08 11:43:10

by Roger Quadros

[permalink] [raw]
Subject: Re: [PATCHv2] phy: omap-usb2: Enable runtime PM of omap-usb2 phy properly

Oussama,

On 10/08/2014 01:54 PM, Oussama Ghorbel wrote:
> Hi Roger,
>
> Should I resend this v2 version of the patch to [email protected] which is suitable for v3.17 and which will require a very tiny adaptation?
> Or I should resend the first version of this patch which is suitable for 3.16 but might seems confusing a little sine I'm resend the outdated version.

If it is a trivial conflict then I think you can just send the patch which applies to the most recent kernel.
Greg can clarify if I'm wrong as he maintains the stable trees.

cheers,
-roger

>
> Thanks,
> Oussama
> On 10/08/2014 09:17 AM, Roger Quadros wrote:
>> Hi Oussama,
>>
>> On 10/07/2014 02:02 PM, Oussama Ghorbel wrote:
>>> The USB OTG port does not work since v3.16 on omap platform.
>>> This is a regression introduced by the commit
>>> eb82a3d846fa (phy: omap-usb2: Balance pm_runtime_enable() on probe failure
>>> and remove).
>>> This because the call to pm_runtime_enable() function is moved after the
>>> call to devm_phy_create() function, which has side effect since later in
>>> the subsequent calls of devm_phy_create() there is a check with
>>> pm_runtime_enabled() to configure few things.
>>>
>>> Signed-off-by: Oussama Ghorbel <[email protected]>
>> Acked-by: Roger Quadros <[email protected]>
>>
>> Could you please send the patch to
>> Cc: <[email protected]> [3.16+]
>>
>> That way it will get into 3.16 onwards. Thanks.
>>
>> cheers,
>> -roger
>>
>

2014-10-08 13:27:16

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCHv2] phy: omap-usb2: Enable runtime PM of omap-usb2 phy properly

On Wed, Oct 08, 2014 at 02:42:56PM +0300, Roger Quadros wrote:
> Oussama,
>
> On 10/08/2014 01:54 PM, Oussama Ghorbel wrote:
> > Hi Roger,
> >
> > Should I resend this v2 version of the patch to [email protected] which is suitable for v3.17 and which will require a very tiny adaptation?
> > Or I should resend the first version of this patch which is suitable for 3.16 but might seems confusing a little sine I'm resend the outdated version.
>
> If it is a trivial conflict then I think you can just send the patch
> which applies to the most recent kernel.
> Greg can clarify if I'm wrong as he maintains the stable trees.

Please read Documentation/stable_kernel_rules.txt for how to submit
patches to the stable trees. Note, it has to be in Linus's tree _first_
before you ever worry about the stable trees. Do that first please.

thanks,

greg k-h

2014-10-08 13:50:23

by Oussama Ghorbel

[permalink] [raw]
Subject: Re: [PATCHv2] phy: omap-usb2: Enable runtime PM of omap-usb2 phy properly

Nothing to send then.

Oussama
On 10/08/2014 02:26 PM, Greg KH wrote:
> On Wed, Oct 08, 2014 at 02:42:56PM +0300, Roger Quadros wrote:
>> Oussama,
>>
>> On 10/08/2014 01:54 PM, Oussama Ghorbel wrote:
>>> Hi Roger,
>>>
>>> Should I resend this v2 version of the patch to [email protected] which is suitable for v3.17 and which will require a very tiny adaptation?
>>> Or I should resend the first version of this patch which is suitable for 3.16 but might seems confusing a little sine I'm resend the outdated version.
>> If it is a trivial conflict then I think you can just send the patch
>> which applies to the most recent kernel.
>> Greg can clarify if I'm wrong as he maintains the stable trees.
> Please read Documentation/stable_kernel_rules.txt for how to submit
> patches to the stable trees. Note, it has to be in Linus's tree _first_
> before you ever worry about the stable trees. Do that first please.
>
> thanks,
>
> greg k-h

2014-10-29 23:09:25

by Rabin Vincent

[permalink] [raw]
Subject: Re: [PATCHv2] phy: omap-usb2: Enable runtime PM of omap-usb2 phy properly

Unless I'm missing something, this patch appears to have still not been
picked up. It would be nice if it can go in for 3.18 so that we have
working USB on pandaboard again at least in that release.

Tony, would you mind carrying it as OMAP maintainer since we haven't
heard anything from Kishon (the PHY maintainer) about this? It's been
acked by Roger (whose patch introduced the problem).

Thanks.

(leaving the patch intact below)

On Tue, Oct 07, 2014 at 12:02:51PM +0100, Oussama Ghorbel wrote:
> The USB OTG port does not work since v3.16 on omap platform.
> This is a regression introduced by the commit
> eb82a3d846fa (phy: omap-usb2: Balance pm_runtime_enable() on probe failure
> and remove).
> This because the call to pm_runtime_enable() function is moved after the
> call to devm_phy_create() function, which has side effect since later in
> the subsequent calls of devm_phy_create() there is a check with
> pm_runtime_enabled() to configure few things.
>
> Signed-off-by: Oussama Ghorbel <[email protected]>
> ---
> drivers/phy/phy-omap-usb2.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
> index 93d7835..acc13f8 100644
> --- a/drivers/phy/phy-omap-usb2.c
> +++ b/drivers/phy/phy-omap-usb2.c
> @@ -262,14 +262,16 @@ static int omap_usb2_probe(struct platform_device *pdev)
> otg->phy = &phy->phy;
>
> platform_set_drvdata(pdev, phy);
> + pm_runtime_enable(phy->dev);
>
> generic_phy = devm_phy_create(phy->dev, NULL, &ops, NULL);
> - if (IS_ERR(generic_phy))
> + if (IS_ERR(generic_phy)) {
> + pm_runtime_disable(phy->dev);
> return PTR_ERR(generic_phy);
> + }
>
> phy_set_drvdata(generic_phy, phy);
>
> - pm_runtime_enable(phy->dev);
> phy_provider = devm_of_phy_provider_register(phy->dev,
> of_phy_simple_xlate);
> if (IS_ERR(phy_provider)) {
> --
> 1.8.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-10-30 05:16:33

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCHv2] phy: omap-usb2: Enable runtime PM of omap-usb2 phy properly



On Thursday 30 October 2014 04:39 AM, Rabin Vincent wrote:
> Unless I'm missing something, this patch appears to have still not been
> picked up. It would be nice if it can go in for 3.18 so that we have
> working USB on pandaboard again at least in that release.
>
> Tony, would you mind carrying it as OMAP maintainer since we haven't
> heard anything from Kishon (the PHY maintainer) about this? It's been
> acked by Roger (whose patch introduced the problem).

I'll send this in this -rc cycle.

Thanks
Kishon

>
> Thanks.
>
> (leaving the patch intact below)
>
> On Tue, Oct 07, 2014 at 12:02:51PM +0100, Oussama Ghorbel wrote:
>> The USB OTG port does not work since v3.16 on omap platform.
>> This is a regression introduced by the commit
>> eb82a3d846fa (phy: omap-usb2: Balance pm_runtime_enable() on probe failure
>> and remove).
>> This because the call to pm_runtime_enable() function is moved after the
>> call to devm_phy_create() function, which has side effect since later in
>> the subsequent calls of devm_phy_create() there is a check with
>> pm_runtime_enabled() to configure few things.
>>
>> Signed-off-by: Oussama Ghorbel <[email protected]>
>> ---
>> drivers/phy/phy-omap-usb2.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
>> index 93d7835..acc13f8 100644
>> --- a/drivers/phy/phy-omap-usb2.c
>> +++ b/drivers/phy/phy-omap-usb2.c
>> @@ -262,14 +262,16 @@ static int omap_usb2_probe(struct platform_device *pdev)
>> otg->phy = &phy->phy;
>>
>> platform_set_drvdata(pdev, phy);
>> + pm_runtime_enable(phy->dev);
>>
>> generic_phy = devm_phy_create(phy->dev, NULL, &ops, NULL);
>> - if (IS_ERR(generic_phy))
>> + if (IS_ERR(generic_phy)) {
>> + pm_runtime_disable(phy->dev);
>> return PTR_ERR(generic_phy);
>> + }
>>
>> phy_set_drvdata(generic_phy, phy);
>>
>> - pm_runtime_enable(phy->dev);
>> phy_provider = devm_of_phy_provider_register(phy->dev,
>> of_phy_simple_xlate);
>> if (IS_ERR(phy_provider)) {
>> --
>> 1.8.3.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-11-03 11:21:52

by Kishon Vijay Abraham I

[permalink] [raw]
Subject: Re: [PATCHv2] phy: omap-usb2: Enable runtime PM of omap-usb2 phy properly

Hi Greg,

On Tuesday 07 October 2014 04:32 PM, Oussama Ghorbel wrote:
> The USB OTG port does not work since v3.16 on omap platform.
> This is a regression introduced by the commit
> eb82a3d846fa (phy: omap-usb2: Balance pm_runtime_enable() on probe failure
> and remove).
> This because the call to pm_runtime_enable() function is moved after the
> call to devm_phy_create() function, which has side effect since later in
> the subsequent calls of devm_phy_create() there is a check with
> pm_runtime_enabled() to configure few things.

This is the only fix for this -rc cycle in the PHY susbsystem. So can you take
this directly? Or else I can prepare a pull request. Let me know.

Acked-by: Kishon Vijay Abraham I <[email protected]>

Thanks
Kishon
>
> Signed-off-by: Oussama Ghorbel <[email protected]>
> ---
> drivers/phy/phy-omap-usb2.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
> index 93d7835..acc13f8 100644
> --- a/drivers/phy/phy-omap-usb2.c
> +++ b/drivers/phy/phy-omap-usb2.c
> @@ -262,14 +262,16 @@ static int omap_usb2_probe(struct platform_device *pdev)
> otg->phy = &phy->phy;
>
> platform_set_drvdata(pdev, phy);
> + pm_runtime_enable(phy->dev);
>
> generic_phy = devm_phy_create(phy->dev, NULL, &ops, NULL);
> - if (IS_ERR(generic_phy))
> + if (IS_ERR(generic_phy)) {
> + pm_runtime_disable(phy->dev);
> return PTR_ERR(generic_phy);
> + }
>
> phy_set_drvdata(generic_phy, phy);
>
> - pm_runtime_enable(phy->dev);
> phy_provider = devm_of_phy_provider_register(phy->dev,
> of_phy_simple_xlate);
> if (IS_ERR(phy_provider)) {
>

2014-11-03 23:29:17

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCHv2] phy: omap-usb2: Enable runtime PM of omap-usb2 phy properly

On Mon, Nov 03, 2014 at 04:51:41PM +0530, Kishon Vijay Abraham I wrote:
> Hi Greg,
>
> On Tuesday 07 October 2014 04:32 PM, Oussama Ghorbel wrote:
> > The USB OTG port does not work since v3.16 on omap platform.
> > This is a regression introduced by the commit
> > eb82a3d846fa (phy: omap-usb2: Balance pm_runtime_enable() on probe failure
> > and remove).
> > This because the call to pm_runtime_enable() function is moved after the
> > call to devm_phy_create() function, which has side effect since later in
> > the subsequent calls of devm_phy_create() there is a check with
> > pm_runtime_enabled() to configure few things.
>
> This is the only fix for this -rc cycle in the PHY susbsystem. So can you take
> this directly? Or else I can prepare a pull request. Let me know.
>
> Acked-by: Kishon Vijay Abraham I <[email protected]>

I don't have this in my archives anymore, please resend so that I can
apply it from an email.

thanks,

greg k-h

2014-11-11 15:10:37

by Enric Balletbo Serra

[permalink] [raw]
Subject: Re: [PATCHv2] phy: omap-usb2: Enable runtime PM of omap-usb2 phy properly

Hi all,

2014-11-04 0:29 GMT+01:00 Greg KH <[email protected]>:
> On Mon, Nov 03, 2014 at 04:51:41PM +0530, Kishon Vijay Abraham I wrote:
>> Hi Greg,
>>
>> On Tuesday 07 October 2014 04:32 PM, Oussama Ghorbel wrote:
>> > The USB OTG port does not work since v3.16 on omap platform.
>> > This is a regression introduced by the commit
>> > eb82a3d846fa (phy: omap-usb2: Balance pm_runtime_enable() on probe failure
>> > and remove).
>> > This because the call to pm_runtime_enable() function is moved after the
>> > call to devm_phy_create() function, which has side effect since later in
>> > the subsequent calls of devm_phy_create() there is a check with
>> > pm_runtime_enabled() to configure few things.
>>
>> This is the only fix for this -rc cycle in the PHY susbsystem. So can you take
>> this directly? Or else I can prepare a pull request. Let me know.
>>
>> Acked-by: Kishon Vijay Abraham I <[email protected]>
>
> I don't have this in my archives anymore, please resend so that I can
> apply it from an email.
>
> thanks,
>
> greg k-h

I'm trying to use the OTG port as HOST on OMAP3-based platform
(IGEPv2), I saw that this patch is already applied in kernel
3.18.0-rc4 so I expected it to work, but I've some problems. To
explain the situation let me propose some test cases.

The first tests I made were configuring the MUSB OTG as HOST only
mode,In this mode test case A fails and test case B and C succeed.

USB Mode Selection (HOST only mode)
Test case A) Boot the system :
mode : b_idle
vbus : Vbus off, timeout 1100 msec
1) Ground ID-pin
mode : a_idle
vbus : Vbus off, timeout 1100 msec
2) Plug pendrive
mode : a_idle
vbus : Vbus off, timeout 1100 msec
Failed: Pendrive is not detected.

Test case B) Boot with ID-pin grounded :
mode : a_idle
vbus : Vbus off, timeout 1100 msec
1) Plug pendrive
mode : a_host
vbus : Vbus off, timeout 1100 msec
Success: Pendrive is detected

Test case C) Boot with ID-pin grounded and pendrive
mode : a_host
vbus : Vbus off, timeout 1100 msec
Success: Pendrive is detected

The second tests I made were configuring the MUSB OTG as Dual Role
mode,In this mode all test cases failed.

MUSB Mode Selection (Dual Role mode)
Test case A) Boot the system :
mode : b_idle
vbus : Vbus off, timeout 1100 msec
1) Ground ID-pin:
mode : a_idle
mode : Vbus off, timeout 1100 msec
2) Plug pendrive
mode : a_idle
vbus : Vbus off, timeout 1100 msec
Failed: Pendrive is not detected.

Test case B) Boot with ID-pin grounded :
mode : b_idle
vbus : Vbus off, timeout 1100 msec
1) Plug pendrive
mode : b_idle
vbus : Vbus off, timeout 1100 msec
Failed: Pendrive is not detected.

Test case C) Boot with ID-pin grounded and pendrive
mode : b_idle
vbus : Vbus off, timeout 1100 msec
Failed: Pendrive is not detected.

I got the mode and vbus values from sysfs:

cat /sys/bus/platform/drivers/musb-hdrc/musb-hdrc.0.auto/mode
cat /sys/bus/platform/drivers/musb-hdrc/musb-hdrc.0.auto/vbus

Did anybody test these test cases ?

I'm a bit surprised with the vbus status because always reports Vbus
off, although pendrive is working. I'm missing something ?

Note that if you see at the mode transitions something looks wrong.
For example, in Dual Role Mode test case B and C the mode is b_idle
instead of a_idle.

Thanks,
Enric