Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1510503 > unrolled thread

Re: [PATCH v2 0/3] usb: musb: da8xx: Fix few issues

Started byDavid Lechner <david@lechnology.com>
First post2016-10-27 19:20 +0200
Last post2016-10-28 20:00 +0200
Articles 5 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v2 0/3] usb: musb: da8xx: Fix few issues David Lechner <david@lechnology.com> - 2016-10-27 19:20 +0200
    Re: [PATCH v2 0/3] usb: musb: da8xx: Fix few issues David Lechner <david@lechnology.com> - 2016-10-27 20:50 +0200
      Re: [PATCH v2 0/3] usb: musb: da8xx: Fix few issues Alexandre Bailon <abailon@baylibre.com> - 2016-10-28 11:40 +0200
        Re: [PATCH v2 0/3] usb: musb: da8xx: Fix few issues Alexandre Bailon <abailon@baylibre.com> - 2016-10-28 12:20 +0200
        Re: [PATCH v2 0/3] usb: musb: da8xx: Fix few issues David Lechner <david@lechnology.com> - 2016-10-28 20:00 +0200

#1510503 — Re: [PATCH v2 0/3] usb: musb: da8xx: Fix few issues

FromDavid Lechner <david@lechnology.com>
Date2016-10-27 19:20 +0200
SubjectRe: [PATCH v2 0/3] usb: musb: da8xx: Fix few issues
Message-ID<swWvn-4Ob-7@gated-at.bofh.it>
On 10/26/2016 05:58 AM, Alexandre Bailon wrote:
> Currently, the USB OTG of the da8xx doesn't work.
> This series intend to fix them.
>
> Change in v2:
> * Fix the error path da8xx_musb_init()
>
> Alexandre Bailon (3):
>   usb: musb: da8xx: Call earlier clk_prepare_enable()
>   phy: da8xx-usb: Configure CFGCHIP2 to support OTG workaround
>   usb: musb: da8xx: Only execute the OTG workaround when phy in OTG mode
>
>  drivers/phy/phy-da8xx-usb.c | 17 ++++++++++++-----
>  drivers/usb/musb/da8xx.c    | 28 +++++++++++++++++++---------
>  2 files changed, 31 insertions(+), 14 deletions(-)
>

I have found another problem with peripheral mode. When we force 
peripheral mode, the glue layer currently uses CFGCHIP2 to override the 
VBUS and ID. This causes it to not be able to detect disconnection 
because the VBUS is overridden.

Here is a patch to fix the problem. I have tested this on LEGO 
MINDSTORMS EV3 (AM1808). This works because the ID pin is internally 
pulled up on the SoC, so we don't need to override it.

---

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 2bc12a2..33daa3b 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -374,9 +374,7 @@ static int da8xx_musb_set_mode(struct musb *musb, u8 
musb_mode)
         case MUSB_HOST:         /* Force VBUS valid, ID = 0 */
                 phy_mode = PHY_MODE_USB_HOST;
                 break;
-       case MUSB_PERIPHERAL:   /* Force VBUS valid, ID = 1 */
-               phy_mode = PHY_MODE_USB_DEVICE;
-               break;
+       case MUSB_PERIPHERAL:
         case MUSB_OTG:          /* Don't override the VBUS/ID 
comparators */
                 phy_mode = PHY_MODE_USB_OTG;
                 break;

---

If this works for other SoCs/boards, I think we should make this change. 
If it doesn't work, we could work around the VBUS problem by polling 
VBUSSENSE in CFGCHIP2. But, I like the simple solution above better.

[toc] | [next] | [standalone]


#1510574

FromDavid Lechner <david@lechnology.com>
Date2016-10-27 20:50 +0200
Message-ID<swXUu-5B8-9@gated-at.bofh.it>
In reply to#1510503
On 10/27/2016 12:16 PM, David Lechner wrote:
> On 10/26/2016 05:58 AM, Alexandre Bailon wrote:
>> Currently, the USB OTG of the da8xx doesn't work.
>> This series intend to fix them.
>>
>> Change in v2:
>> * Fix the error path da8xx_musb_init()
>>
>> Alexandre Bailon (3):
>>   usb: musb: da8xx: Call earlier clk_prepare_enable()
>>   phy: da8xx-usb: Configure CFGCHIP2 to support OTG workaround
>>   usb: musb: da8xx: Only execute the OTG workaround when phy in OTG mode
>>
>>  drivers/phy/phy-da8xx-usb.c | 17 ++++++++++++-----
>>  drivers/usb/musb/da8xx.c    | 28 +++++++++++++++++++---------
>>  2 files changed, 31 insertions(+), 14 deletions(-)
>>
>
> I have found another problem with peripheral mode. When we force
> peripheral mode, the glue layer currently uses CFGCHIP2 to override the
> VBUS and ID. This causes it to not be able to detect disconnection
> because the VBUS is overridden.
>
> Here is a patch to fix the problem. I have tested this on LEGO
> MINDSTORMS EV3 (AM1808). This works because the ID pin is internally
> pulled up on the SoC, so we don't need to override it.
>
> ---
>
> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
> index 2bc12a2..33daa3b 100644
> --- a/drivers/usb/musb/da8xx.c
> +++ b/drivers/usb/musb/da8xx.c
> @@ -374,9 +374,7 @@ static int da8xx_musb_set_mode(struct musb *musb, u8
> musb_mode)
>         case MUSB_HOST:         /* Force VBUS valid, ID = 0 */
>                 phy_mode = PHY_MODE_USB_HOST;
>                 break;
> -       case MUSB_PERIPHERAL:   /* Force VBUS valid, ID = 1 */
> -               phy_mode = PHY_MODE_USB_DEVICE;
> -               break;
> +       case MUSB_PERIPHERAL:
>         case MUSB_OTG:          /* Don't override the VBUS/ID
> comparators */
>                 phy_mode = PHY_MODE_USB_OTG;
>                 break;
>
> ---
>
> If this works for other SoCs/boards, I think we should make this change.
> If it doesn't work, we could work around the VBUS problem by polling
> VBUSSENSE in CFGCHIP2. But, I like the simple solution above better.

I have realized that due to the way my device is wired, I can actually 
use OTG mode and it will behave exactly as peripheral mode because the 
ID pin is not connected. So, maybe this patch is not needed after all.

[toc] | [prev] | [next] | [standalone]


#1510992

FromAlexandre Bailon <abailon@baylibre.com>
Date2016-10-28 11:40 +0200
Message-ID<sxbNL-6sc-5@gated-at.bofh.it>
In reply to#1510574
On 10/27/2016 08:44 PM, David Lechner wrote:
> On 10/27/2016 12:16 PM, David Lechner wrote:
>> On 10/26/2016 05:58 AM, Alexandre Bailon wrote:
>>> Currently, the USB OTG of the da8xx doesn't work.
>>> This series intend to fix them.
>>>
>>> Change in v2:
>>> * Fix the error path da8xx_musb_init()
>>>
>>> Alexandre Bailon (3):
>>>   usb: musb: da8xx: Call earlier clk_prepare_enable()
>>>   phy: da8xx-usb: Configure CFGCHIP2 to support OTG workaround
>>>   usb: musb: da8xx: Only execute the OTG workaround when phy in OTG mode
>>>
>>>  drivers/phy/phy-da8xx-usb.c | 17 ++++++++++++-----
>>>  drivers/usb/musb/da8xx.c    | 28 +++++++++++++++++++---------
>>>  2 files changed, 31 insertions(+), 14 deletions(-)
>>>
>>
>> I have found another problem with peripheral mode. When we force
>> peripheral mode, the glue layer currently uses CFGCHIP2 to override the
>> VBUS and ID. This causes it to not be able to detect disconnection
>> because the VBUS is overridden.
How have you found it ? Does it cause any issues ?
I mean I had to enable traces to see that disconnect was not happening.
>>
>> Here is a patch to fix the problem. I have tested this on LEGO
>> MINDSTORMS EV3 (AM1808). This works because the ID pin is internally
>> pulled up on the SoC, so we don't need to override it.
Actually, I'm wonder if that if not related to VBUS sensing.
May be we should set CFGCHIP2_VBDTCTEN in device mode.
>>
>> ---
>>
>> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
>> index 2bc12a2..33daa3b 100644
>> --- a/drivers/usb/musb/da8xx.c
>> +++ b/drivers/usb/musb/da8xx.c
>> @@ -374,9 +374,7 @@ static int da8xx_musb_set_mode(struct musb *musb, u8
>> musb_mode)
>>         case MUSB_HOST:         /* Force VBUS valid, ID = 0 */
>>                 phy_mode = PHY_MODE_USB_HOST;
>>                 break;
>> -       case MUSB_PERIPHERAL:   /* Force VBUS valid, ID = 1 */
>> -               phy_mode = PHY_MODE_USB_DEVICE;
>> -               break;
>> +       case MUSB_PERIPHERAL:
>>         case MUSB_OTG:          /* Don't override the VBUS/ID
>> comparators */
>>                 phy_mode = PHY_MODE_USB_OTG;
>>                 break;
>>
>> ---
>>
>> If this works for other SoCs/boards, I think we should make this change.
>> If it doesn't work, we could work around the VBUS problem by polling
>> VBUSSENSE in CFGCHIP2. But, I like the simple solution above better.
> 
> I have realized that due to the way my device is wired, I can actually
> use OTG mode and it will behave exactly as peripheral mode because the
> ID pin is not connected. So, maybe this patch is not needed after all.
> 
Actually, I'm sure that is related to ID
I have the same issue except the disconnect is called when I use the
OTG mode.

[toc] | [prev] | [next] | [standalone]


#1511041

FromAlexandre Bailon <abailon@baylibre.com>
Date2016-10-28 12:20 +0200
Message-ID<sxcqt-6Y9-23@gated-at.bofh.it>
In reply to#1510992
On 10/28/2016 11:31 AM, Alexandre Bailon wrote:
> On 10/27/2016 08:44 PM, David Lechner wrote:
>> On 10/27/2016 12:16 PM, David Lechner wrote:
>>> On 10/26/2016 05:58 AM, Alexandre Bailon wrote:
>>>> Currently, the USB OTG of the da8xx doesn't work.
>>>> This series intend to fix them.
>>>>
>>>> Change in v2:
>>>> * Fix the error path da8xx_musb_init()
>>>>
>>>> Alexandre Bailon (3):
>>>>   usb: musb: da8xx: Call earlier clk_prepare_enable()
>>>>   phy: da8xx-usb: Configure CFGCHIP2 to support OTG workaround
>>>>   usb: musb: da8xx: Only execute the OTG workaround when phy in OTG mode
>>>>
>>>>  drivers/phy/phy-da8xx-usb.c | 17 ++++++++++++-----
>>>>  drivers/usb/musb/da8xx.c    | 28 +++++++++++++++++++---------
>>>>  2 files changed, 31 insertions(+), 14 deletions(-)
>>>>
>>>
>>> I have found another problem with peripheral mode. When we force
>>> peripheral mode, the glue layer currently uses CFGCHIP2 to override the
>>> VBUS and ID. This causes it to not be able to detect disconnection
>>> because the VBUS is overridden.
> How have you found it ? Does it cause any issues ?
> I mean I had to enable traces to see that disconnect was not happening.
>>>
>>> Here is a patch to fix the problem. I have tested this on LEGO
>>> MINDSTORMS EV3 (AM1808). This works because the ID pin is internally
>>> pulled up on the SoC, so we don't need to override it.
> Actually, I'm wonder if that if not related to VBUS sensing.
> May be we should set CFGCHIP2_VBDTCTEN in device mode.
>>>
>>> ---
>>>
>>> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
>>> index 2bc12a2..33daa3b 100644
>>> --- a/drivers/usb/musb/da8xx.c
>>> +++ b/drivers/usb/musb/da8xx.c
>>> @@ -374,9 +374,7 @@ static int da8xx_musb_set_mode(struct musb *musb, u8
>>> musb_mode)
>>>         case MUSB_HOST:         /* Force VBUS valid, ID = 0 */
>>>                 phy_mode = PHY_MODE_USB_HOST;
>>>                 break;
>>> -       case MUSB_PERIPHERAL:   /* Force VBUS valid, ID = 1 */
>>> -               phy_mode = PHY_MODE_USB_DEVICE;
>>> -               break;
>>> +       case MUSB_PERIPHERAL:
>>>         case MUSB_OTG:          /* Don't override the VBUS/ID
>>> comparators */
>>>                 phy_mode = PHY_MODE_USB_OTG;
>>>                 break;
>>>
>>> ---
>>>
>>> If this works for other SoCs/boards, I think we should make this change.
I will try it but for me, it should be good.
It is actually similar to what is done in TI BSP.
>>> If it doesn't work, we could work around the VBUS problem by polling
>>> VBUSSENSE in CFGCHIP2. But, I like the simple solution above better.
Actually, I don't think we need to do any polling.
We can just catch the suspend interrupt and use VBUSSENSE to
differentiate a real suspend from a disconnect.
>>
>> I have realized that due to the way my device is wired, I can actually
>> use OTG mode and it will behave exactly as peripheral mode because the
>> ID pin is not connected. So, maybe this patch is not needed after all.
>>
> Actually, I'm sure that is related to ID
> I have the same issue except the disconnect is called when I use the
> OTG mode.
> 

[toc] | [prev] | [next] | [standalone]


#1511316

FromDavid Lechner <david@lechnology.com>
Date2016-10-28 20:00 +0200
Message-ID<sxjBE-2YP-21@gated-at.bofh.it>
In reply to#1510992
On 10/28/2016 04:31 AM, Alexandre Bailon wrote:
> On 10/27/2016 08:44 PM, David Lechner wrote:
>> On 10/27/2016 12:16 PM, David Lechner wrote:
>>> On 10/26/2016 05:58 AM, Alexandre Bailon wrote:
>>>> Currently, the USB OTG of the da8xx doesn't work.
>>>> This series intend to fix them.
>>>>
>>>> Change in v2:
>>>> * Fix the error path da8xx_musb_init()
>>>>
>>>> Alexandre Bailon (3):
>>>>   usb: musb: da8xx: Call earlier clk_prepare_enable()
>>>>   phy: da8xx-usb: Configure CFGCHIP2 to support OTG workaround
>>>>   usb: musb: da8xx: Only execute the OTG workaround when phy in OTG mode
>>>>
>>>>  drivers/phy/phy-da8xx-usb.c | 17 ++++++++++++-----
>>>>  drivers/usb/musb/da8xx.c    | 28 +++++++++++++++++++---------
>>>>  2 files changed, 31 insertions(+), 14 deletions(-)
>>>>
>>>
>>> I have found another problem with peripheral mode. When we force
>>> peripheral mode, the glue layer currently uses CFGCHIP2 to override the
>>> VBUS and ID. This causes it to not be able to detect disconnection
>>> because the VBUS is overridden.
> How have you found it ? Does it cause any issues ?
> I mean I had to enable traces to see that disconnect was not happening.

I am also using the device tree patch series. I specified dr_mode = 
"peripheral" in my device tree because the device is only wired for use 
as a peripheral port.

I have actually known about this issue for a long time. See
<https://github.com/ev3dev/ev3dev/issues/244> and 
<http://e2e.ti.com/support/embedded/linux/f/354/p/65332/237094#2370942>


>>>
>>> Here is a patch to fix the problem. I have tested this on LEGO
>>> MINDSTORMS EV3 (AM1808). This works because the ID pin is internally
>>> pulled up on the SoC, so we don't need to override it.
> Actually, I'm wonder if that if not related to VBUS sensing.
> May be we should set CFGCHIP2_VBDTCTEN in device mode.

Yes, as we discussed in another thread, we should set CFGCHIP2_VBDTCTEN 
and CFGCHIP2_SESNDEN *always* regardless of mode. I still have the 
problem described above with these two enabled if and only if we are 
also setting CFGCHIP2_OTG_FORCE_PERIPHERAL.

>>>
>>> ---
>>>
>>> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
>>> index 2bc12a2..33daa3b 100644
>>> --- a/drivers/usb/musb/da8xx.c
>>> +++ b/drivers/usb/musb/da8xx.c
>>> @@ -374,9 +374,7 @@ static int da8xx_musb_set_mode(struct musb *musb, u8
>>> musb_mode)
>>>         case MUSB_HOST:         /* Force VBUS valid, ID = 0 */
>>>                 phy_mode = PHY_MODE_USB_HOST;
>>>                 break;
>>> -       case MUSB_PERIPHERAL:   /* Force VBUS valid, ID = 1 */
>>> -               phy_mode = PHY_MODE_USB_DEVICE;
>>> -               break;
>>> +       case MUSB_PERIPHERAL:
>>>         case MUSB_OTG:          /* Don't override the VBUS/ID
>>> comparators */
>>>                 phy_mode = PHY_MODE_USB_OTG;
>>>                 break;
>>>
>>> ---
>>>
>>> If this works for other SoCs/boards, I think we should make this change.
>>> If it doesn't work, we could work around the VBUS problem by polling
>>> VBUSSENSE in CFGCHIP2. But, I like the simple solution above better.
>>
>> I have realized that due to the way my device is wired, I can actually
>> use OTG mode and it will behave exactly as peripheral mode because the
>> ID pin is not connected. So, maybe this patch is not needed after all.
>>
> Actually, I'm sure that is related to ID

The ID pin on my device is not connected, so I don't think this has to 
do with ID. It is always high (internally pulled up).

> I have the same issue except the disconnect is called when I use the
> OTG mode.
>

This is exactly the behavior I am seeing.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web