diff mbox series

[1/3,v2] usb: dwc2: Improve gadget state disconnection handling

Message ID 1508794370-29982-2-git-send-email-john.stultz@linaro.org
State New
Headers show
Series dwc2 fixes for edge cases on hikey | expand

Commit Message

John Stultz Oct. 23, 2017, 9:32 p.m. UTC
In the earlier commit dad3f793f20f ("usb: dwc2: Make sure we
disconnect the gadget state"), I was trying to fix up the
fact that we somehow weren't disconnecting the gadget state,
so that when the OTG port was plugged in the second time we
would get warnings about the state tracking being wrong.

(This seems to be due to a quirk of the HiKey board where
we do not ever get any otg interrupts, particularly the session
end detected signal. Instead we only see status change
interrupt.)

The fix there was somewhat simple, as it just made sure to
call dwc2_hsotg_disconnect() before we connected things up
in OTG mode, ensuring the state handling didn't throw errors.

But in looking at a different issue I was seeing with UDC
state handling, I realized that it would be much better
to call dwc2_hsotg_disconnect when we get the state change
signal moving to host mode.

Thus, this patch removes the earlier disconnect call I added
and moves it (and the needed locking) to the host mode
transition.

Cc: Wei Xu <xuwei5@hisilicon.com>
Cc: Guodong Xu <guodong.xu@linaro.org>
Cc: Amit Pundir <amit.pundir@linaro.org>
Cc: YongQin Liu <yongqin.liu@linaro.org>
Cc: John Youn <johnyoun@synopsys.com>
Cc: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>
Cc: Douglas Anderson <dianders@chromium.org>
Cc: Chen Yu <chenyu56@huawei.com>
Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: linux-usb@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>

---
v2: Remove the extra dwc2_hsotg_core_init_disconnected() call
    I had added, as suggested by Minas.
---
 drivers/usb/dwc2/hcd.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

-- 
2.7.4

Comments

Minas Harutyunyan Oct. 24, 2017, 8:59 a.m. UTC | #1
On 10/24/2017 1:33 AM, John Stultz wrote:
> In the earlier commit dad3f793f20f ("usb: dwc2: Make sure we

> disconnect the gadget state"), I was trying to fix up the

> fact that we somehow weren't disconnecting the gadget state,

> so that when the OTG port was plugged in the second time we

> would get warnings about the state tracking being wrong.

> 

> (This seems to be due to a quirk of the HiKey board where

> we do not ever get any otg interrupts, particularly the session

> end detected signal. Instead we only see status change

> interrupt.)

> 

> The fix there was somewhat simple, as it just made sure to

> call dwc2_hsotg_disconnect() before we connected things up

> in OTG mode, ensuring the state handling didn't throw errors.

> 

> But in looking at a different issue I was seeing with UDC

> state handling, I realized that it would be much better

> to call dwc2_hsotg_disconnect when we get the state change

> signal moving to host mode.

> 

> Thus, this patch removes the earlier disconnect call I added

> and moves it (and the needed locking) to the host mode

> transition.

> 

> Cc: Wei Xu <xuwei5@hisilicon.com>

> Cc: Guodong Xu <guodong.xu@linaro.org>

> Cc: Amit Pundir <amit.pundir@linaro.org>

> Cc: YongQin Liu <yongqin.liu@linaro.org>

> Cc: John Youn <johnyoun@synopsys.com>

> Cc: Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>

> Cc: Douglas Anderson <dianders@chromium.org>

> Cc: Chen Yu <chenyu56@huawei.com>

> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>

> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

> Cc: linux-usb@vger.kernel.org

> Signed-off-by: John Stultz <john.stultz@linaro.org>

> ---

> v2: Remove the extra dwc2_hsotg_core_init_disconnected() call

>      I had added, as suggested by Minas.

> ---

>   drivers/usb/dwc2/hcd.c | 7 +++++--

>   1 file changed, 5 insertions(+), 2 deletions(-)

> 

> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c

> index c263114..9bd60ec 100644

> --- a/drivers/usb/dwc2/hcd.c

> +++ b/drivers/usb/dwc2/hcd.c

> @@ -3277,7 +3277,6 @@ static void dwc2_conn_id_status_change(struct work_struct *work)

>   		dwc2_core_init(hsotg, false);

>   		dwc2_enable_global_interrupts(hsotg);

>   		spin_lock_irqsave(&hsotg->lock, flags);

> -		dwc2_hsotg_disconnect(hsotg);

>   		dwc2_hsotg_core_init_disconnected(hsotg, false);

>   		spin_unlock_irqrestore(&hsotg->lock, flags);

>   		dwc2_hsotg_core_connect(hsotg);

> @@ -3296,8 +3295,12 @@ static void dwc2_conn_id_status_change(struct work_struct *work)

>   		if (count > 250)

>   			dev_err(hsotg->dev,

>   				"Connection id status change timed out\n");

> -		hsotg->op_state = OTG_STATE_A_HOST;

>   

> +		spin_lock_irqsave(&hsotg->lock, flags);

> +		dwc2_hsotg_disconnect(hsotg);

> +		spin_unlock_irqrestore(&hsotg->lock, flags);

> +

> +		hsotg->op_state = OTG_STATE_A_HOST;

>   		/* Initialize the Core for Host mode */

>   		dwc2_core_init(hsotg, false);

>   		dwc2_enable_global_interrupts(hsotg);

> 


This patch is required for the HiKey platform, because the assertion of 
the "Connector ID status change" interrupt is different: asserting on B 
connector unplug and goes to Host mode.
No any side effect on SNPS HAPS-DX platform where ConnIDStsChng 
assertion is correct.

Acked-by: Minas Harutyunyan <hminas@synopsys.com>

Tested-by: Minas Harutyunyan <hminas@synopsys.com>


Thanks,
Minas
diff mbox series

Patch

diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
index c263114..9bd60ec 100644
--- a/drivers/usb/dwc2/hcd.c
+++ b/drivers/usb/dwc2/hcd.c
@@ -3277,7 +3277,6 @@  static void dwc2_conn_id_status_change(struct work_struct *work)
 		dwc2_core_init(hsotg, false);
 		dwc2_enable_global_interrupts(hsotg);
 		spin_lock_irqsave(&hsotg->lock, flags);
-		dwc2_hsotg_disconnect(hsotg);
 		dwc2_hsotg_core_init_disconnected(hsotg, false);
 		spin_unlock_irqrestore(&hsotg->lock, flags);
 		dwc2_hsotg_core_connect(hsotg);
@@ -3296,8 +3295,12 @@  static void dwc2_conn_id_status_change(struct work_struct *work)
 		if (count > 250)
 			dev_err(hsotg->dev,
 				"Connection id status change timed out\n");
-		hsotg->op_state = OTG_STATE_A_HOST;
 
+		spin_lock_irqsave(&hsotg->lock, flags);
+		dwc2_hsotg_disconnect(hsotg);
+		spin_unlock_irqrestore(&hsotg->lock, flags);
+
+		hsotg->op_state = OTG_STATE_A_HOST;
 		/* Initialize the Core for Host mode */
 		dwc2_core_init(hsotg, false);
 		dwc2_enable_global_interrupts(hsotg);