[PATCH] usb: dwc3: turn off VBUS when leaving host mode

[PATCH] usb: dwc3: turn off VBUS when leaving host mode

[Bin Liu]

VBUS should be turned off when leaving the host mode.
Set GCTL_PRTCAP to device mode in teardown to de-assert DRVVBUS pin to
turn off VBUS power.

Fixes: 5f94adfeed97 ("usb: dwc3: core: refactor mode initialization to its own function")
Signed-off-by: Bin Liu <b-liu@ti.com>
---
 drivers/usb/dwc3/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 97d6ae3c4df2..76ac9cd54e64 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1201,6 +1201,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
 		break;
 	case USB_DR_MODE_HOST:
 		dwc3_host_exit(dwc);
+		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);
 		break;
 	case USB_DR_MODE_OTG:
 		dwc3_drd_exit(dwc);
-- 
2.17.1

Re: [PATCH] usb: dwc3: turn off VBUS when leaving host mode

[Roger Quadros]

Bin,

On 13/11/2019 16:15, Bin Liu wrote:
> VBUS should be turned off when leaving the host mode.
> Set GCTL_PRTCAP to device mode in teardown to de-assert DRVVBUS pin to
> turn off VBUS power.

Doesn't DRVVBUS pin de-assert when root hub driver turns off port power?
Which should happen when XHCI host driver is unregistered as part of
dwc3_host_exit()?

cheers,
-roger

> 
> Fixes: 5f94adfeed97 ("usb: dwc3: core: refactor mode initialization to its own function")
> Signed-off-by: Bin Liu <b-liu@ti.com>
> ---
>   drivers/usb/dwc3/core.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 97d6ae3c4df2..76ac9cd54e64 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1201,6 +1201,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
>   		break;
>   	case USB_DR_MODE_HOST:
>   		dwc3_host_exit(dwc);
> +		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>   		break;
>   	case USB_DR_MODE_OTG:
>   		dwc3_drd_exit(dwc);
> 

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH] usb: dwc3: turn off VBUS when leaving host mode

[Bin Liu]

Roger,

On Thu, Nov 14, 2019 at 10:48:36AM +0200, Roger Quadros wrote:
> Bin,
> 
> On 13/11/2019 16:15, Bin Liu wrote:
> > VBUS should be turned off when leaving the host mode.
> > Set GCTL_PRTCAP to device mode in teardown to de-assert DRVVBUS pin to
> > turn off VBUS power.
> 
> Doesn't DRVVBUS pin de-assert when root hub driver turns off port power?
> Which should happen when XHCI host driver is unregistered as part of
> dwc3_host_exit()?

No, DRVVBUS is still 3.3v after 'modprbe -r dwc3-omap' on TI AM57x
platform if this patch is not applied.

-Bin.

Re: [PATCH] usb: dwc3: turn off VBUS when leaving host mode

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 859 bytes --]

Hi,

Bin Liu <b-liu@ti.com> writes:

> VBUS should be turned off when leaving the host mode.
> Set GCTL_PRTCAP to device mode in teardown to de-assert DRVVBUS pin to
> turn off VBUS power.
>
> Fixes: 5f94adfeed97 ("usb: dwc3: core: refactor mode initialization to its own function")
> Signed-off-by: Bin Liu <b-liu@ti.com>
> ---
>  drivers/usb/dwc3/core.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 97d6ae3c4df2..76ac9cd54e64 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -1201,6 +1201,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
>  		break;
>  	case USB_DR_MODE_HOST:
>  		dwc3_host_exit(dwc);
> +		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);

seems like this should be done as part of dwc3_host_exit()

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] usb: dwc3: turn off VBUS when leaving host mode

[Roger Quadros]

On 18/11/2019 09:07, Felipe Balbi wrote:
> Hi,
> 
> Bin Liu <b-liu@ti.com> writes:
> 
>> VBUS should be turned off when leaving the host mode.
>> Set GCTL_PRTCAP to device mode in teardown to de-assert DRVVBUS pin to
>> turn off VBUS power.
>>
>> Fixes: 5f94adfeed97 ("usb: dwc3: core: refactor mode initialization to its own function")
>> Signed-off-by: Bin Liu <b-liu@ti.com>
>> ---
>>   drivers/usb/dwc3/core.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 97d6ae3c4df2..76ac9cd54e64 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -1201,6 +1201,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
>>   		break;
>>   	case USB_DR_MODE_HOST:
>>   		dwc3_host_exit(dwc);
>> +		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> 
> seems like this should be done as part of dwc3_host_exit()
> 

That wouldn't work well with OTG case as dwc3_host_exit(dwc) is
called when switching roles and we don't want PRTCAP
to change from DWC3_GCTL_PRTCAP_OTG.

cheers,
-roger
-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH] usb: dwc3: turn off VBUS when leaving host mode

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1225 bytes --]


Hi,

Roger Quadros <rogerq@ti.com> writes:
> On 18/11/2019 09:07, Felipe Balbi wrote:
>> Hi,
>> 
>> Bin Liu <b-liu@ti.com> writes:
>> 
>>> VBUS should be turned off when leaving the host mode.
>>> Set GCTL_PRTCAP to device mode in teardown to de-assert DRVVBUS pin to
>>> turn off VBUS power.
>>>
>>> Fixes: 5f94adfeed97 ("usb: dwc3: core: refactor mode initialization to its own function")
>>> Signed-off-by: Bin Liu <b-liu@ti.com>
>>> ---
>>>   drivers/usb/dwc3/core.c | 1 +
>>>   1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>> index 97d6ae3c4df2..76ac9cd54e64 100644
>>> --- a/drivers/usb/dwc3/core.c
>>> +++ b/drivers/usb/dwc3/core.c
>>> @@ -1201,6 +1201,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
>>>   		break;
>>>   	case USB_DR_MODE_HOST:
>>>   		dwc3_host_exit(dwc);
>>> +		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>> 
>> seems like this should be done as part of dwc3_host_exit()
>> 
>
> That wouldn't work well with OTG case as dwc3_host_exit(dwc) is
> called when switching roles and we don't want PRTCAP
> to change from DWC3_GCTL_PRTCAP_OTG.

if (port != OTG)
  set_prtcap(DEVICE)

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] usb: dwc3: turn off VBUS when leaving host mode

[Bin Liu]

Hi Felipe,

On Mon, Dec 02, 2019 at 09:41:42AM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Roger Quadros <rogerq@ti.com> writes:
> > On 18/11/2019 09:07, Felipe Balbi wrote:
> >> Hi,
> >> 
> >> Bin Liu <b-liu@ti.com> writes:
> >> 
> >>> VBUS should be turned off when leaving the host mode.
> >>> Set GCTL_PRTCAP to device mode in teardown to de-assert DRVVBUS pin to
> >>> turn off VBUS power.
> >>>
> >>> Fixes: 5f94adfeed97 ("usb: dwc3: core: refactor mode initialization to its own function")
> >>> Signed-off-by: Bin Liu <b-liu@ti.com>
> >>> ---
> >>>   drivers/usb/dwc3/core.c | 1 +
> >>>   1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> >>> index 97d6ae3c4df2..76ac9cd54e64 100644
> >>> --- a/drivers/usb/dwc3/core.c
> >>> +++ b/drivers/usb/dwc3/core.c
> >>> @@ -1201,6 +1201,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
> >>>   		break;
> >>>   	case USB_DR_MODE_HOST:
> >>>   		dwc3_host_exit(dwc);
> >>> +		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> >> 
> >> seems like this should be done as part of dwc3_host_exit()
> >> 
> >
> > That wouldn't work well with OTG case as dwc3_host_exit(dwc) is
> > called when switching roles and we don't want PRTCAP
> > to change from DWC3_GCTL_PRTCAP_OTG.
> 
> if (port != OTG)
>   set_prtcap(DEVICE)

During init, the PRTCAP is set in dwc3_core_init_mode() besides
dwc3_{host,gadget,drd}_init(). So for tearing down setting it in
dwc3_core_exit_mode() would make the code logic symmetric and easy to
understand.

Also it turns out that setting PRTCAP is required for OTG mode too to
de-assert DRVVBUS. If left GCTL[PRTCAP] to OTG, grounding the ID pin
would make the controller to assert DRVVBUS without any software
involved. So the fix should be the following. Please let me know you
comments.

-Bin.

-----------8<------------
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 97d6ae3c4df2..cede7a8e3605 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1209,6 +1209,9 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
                /* do nothing */
                break;
        }
+
+       /* de-assert DRVVBUS for HOST and OTG mode */
+       dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);
 }

 static void dwc3_get_properties(struct dwc3 *dwc)

> 
> -- 
> balbi

Re: [PATCH] usb: dwc3: turn off VBUS when leaving host mode

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1963 bytes --]


Hi,

Bin Liu <b-liu@ti.com> writes:
>> >>> VBUS should be turned off when leaving the host mode.
>> >>> Set GCTL_PRTCAP to device mode in teardown to de-assert DRVVBUS pin to
>> >>> turn off VBUS power.
>> >>>
>> >>> Fixes: 5f94adfeed97 ("usb: dwc3: core: refactor mode initialization to its own function")
>> >>> Signed-off-by: Bin Liu <b-liu@ti.com>
>> >>> ---
>> >>>   drivers/usb/dwc3/core.c | 1 +
>> >>>   1 file changed, 1 insertion(+)
>> >>>
>> >>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> >>> index 97d6ae3c4df2..76ac9cd54e64 100644
>> >>> --- a/drivers/usb/dwc3/core.c
>> >>> +++ b/drivers/usb/dwc3/core.c
>> >>> @@ -1201,6 +1201,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
>> >>>   		break;
>> >>>   	case USB_DR_MODE_HOST:
>> >>>   		dwc3_host_exit(dwc);
>> >>> +		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);
>> >> 
>> >> seems like this should be done as part of dwc3_host_exit()
>> >> 
>> >
>> > That wouldn't work well with OTG case as dwc3_host_exit(dwc) is
>> > called when switching roles and we don't want PRTCAP
>> > to change from DWC3_GCTL_PRTCAP_OTG.
>> 
>> if (port != OTG)
>>   set_prtcap(DEVICE)
>
> During init, the PRTCAP is set in dwc3_core_init_mode() besides
> dwc3_{host,gadget,drd}_init(). So for tearing down setting it in
> dwc3_core_exit_mode() would make the code logic symmetric and easy to
> understand.

Fair enough, makes sense.

> Also it turns out that setting PRTCAP is required for OTG mode too to

But, then, how do we go back to OTG?

> de-assert DRVVBUS. If left GCTL[PRTCAP] to OTG, grounding the ID pin
> would make the controller to assert DRVVBUS without any software
> involved. So the fix should be the following. Please let me know you
> comments.

But that's expected, no? If port is OTG, then it must obey ID pin rules
specified by OTG. IIRC, dwc3 is OTG 2.x compliant, at least the version
TI uses.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] usb: dwc3: turn off VBUS when leaving host mode

[Bin Liu]

On Tue, Dec 10, 2019 at 01:50:58PM +0200, Felipe Balbi wrote:
> 
> Hi,
> 
> Bin Liu <b-liu@ti.com> writes:
> >> >>> VBUS should be turned off when leaving the host mode.
> >> >>> Set GCTL_PRTCAP to device mode in teardown to de-assert DRVVBUS pin to
> >> >>> turn off VBUS power.
> >> >>>
> >> >>> Fixes: 5f94adfeed97 ("usb: dwc3: core: refactor mode initialization to its own function")
> >> >>> Signed-off-by: Bin Liu <b-liu@ti.com>
> >> >>> ---
> >> >>>   drivers/usb/dwc3/core.c | 1 +
> >> >>>   1 file changed, 1 insertion(+)
> >> >>>
> >> >>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> >> >>> index 97d6ae3c4df2..76ac9cd54e64 100644
> >> >>> --- a/drivers/usb/dwc3/core.c
> >> >>> +++ b/drivers/usb/dwc3/core.c
> >> >>> @@ -1201,6 +1201,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
> >> >>>   		break;
> >> >>>   	case USB_DR_MODE_HOST:
> >> >>>   		dwc3_host_exit(dwc);
> >> >>> +		dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> >> >> 
> >> >> seems like this should be done as part of dwc3_host_exit()
> >> >> 
> >> >
> >> > That wouldn't work well with OTG case as dwc3_host_exit(dwc) is
> >> > called when switching roles and we don't want PRTCAP
> >> > to change from DWC3_GCTL_PRTCAP_OTG.
> >> 
> >> if (port != OTG)
> >>   set_prtcap(DEVICE)
> >
> > During init, the PRTCAP is set in dwc3_core_init_mode() besides
> > dwc3_{host,gadget,drd}_init(). So for tearing down setting it in
> > dwc3_core_exit_mode() would make the code logic symmetric and easy to
> > understand.
> 
> Fair enough, makes sense.
> 
> > Also it turns out that setting PRTCAP is required for OTG mode too to
> 
> But, then, how do we go back to OTG?

This new setting (PRTCAP = DEVICE) is in dwc3_core_exit_mode() which is
called in platform driver .remove(). So PRTCAP will be set to OTG for
dr_mode = 'otg' in dwc3_core_init_mode() at the next time in the
platform driver .probe().

> 
> > de-assert DRVVBUS. If left GCTL[PRTCAP] to OTG, grounding the ID pin
> > would make the controller to assert DRVVBUS without any software
> > involved. So the fix should be the following. Please let me know you
> > comments.
> 
> But that's expected, no? If port is OTG, then it must obey ID pin rules
> specified by OTG. IIRC, dwc3 is OTG 2.x compliant, at least the version
> TI uses.

It is expected when the usb drivers are bind to the controller. I am
trying to solve the issue that DRVVBUS is still asserted on both host
port and otg port with ID pin grounded when the drivers are unbind from
the controller (after modprobe -r dwc3-omap, DRVVBUS should be
de-asserted).

-Bin.

Re: [PATCH] usb: dwc3: turn off VBUS when leaving host mode

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 897 bytes --]


Hi,

Bin Liu <b-liu@ti.com> writes:
>> > de-assert DRVVBUS. If left GCTL[PRTCAP] to OTG, grounding the ID pin
>> > would make the controller to assert DRVVBUS without any software
>> > involved. So the fix should be the following. Please let me know you
>> > comments.
>> 
>> But that's expected, no? If port is OTG, then it must obey ID pin rules
>> specified by OTG. IIRC, dwc3 is OTG 2.x compliant, at least the version
>> TI uses.
>
> It is expected when the usb drivers are bind to the controller. I am
> trying to solve the issue that DRVVBUS is still asserted on both host
> port and otg port with ID pin grounded when the drivers are unbind from
> the controller (after modprobe -r dwc3-omap, DRVVBUS should be
> de-asserted).

I see. Now I understand the scenario. Thanks for the explanation, Bin.

Care to send an up-to-date patch so I can queue it?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[PATCH v2] usb: dwc3: turn off VBUS when leaving host mode

[Bin Liu]

VBUS should be turned off when leaving the host mode.
Set GCTL_PRTCAP to device mode in teardown to de-assert DRVVBUS pin to
turn off VBUS power.

Fixes: 5f94adfeed97 ("usb: dwc3: core: refactor mode initialization to its own function")
Cc: stable@vger.kernel.org
Signed-off-by: Bin Liu <b-liu@ti.com>
---
v2: cover dr_mode == OTG too.

 drivers/usb/dwc3/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index f561c6c9e8a9..1d85c42b9c67 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -1246,6 +1246,9 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
 		/* do nothing */
 		break;
 	}
+
+	/* de-assert DRVVBUS for HOST and OTG mode */
+	dwc3_set_prtcap(dwc, DWC3_GCTL_PRTCAP_DEVICE);
 }
 
 static void dwc3_get_properties(struct dwc3 *dwc)
-- 
2.17.1