Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752193AbcLGDwg (ORCPT ); Tue, 6 Dec 2016 22:52:36 -0500 Received: from smtprelay4.synopsys.com ([198.182.47.9]:33576 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751347AbcLGDwf (ORCPT ); Tue, 6 Dec 2016 22:52:35 -0500 Subject: Re: [RFC][PATCH] HACK: usb: dwc2: Workaround case where GOTGCTL state is wrong To: John Stultz , lkml References: <1481075278-17600-1-git-send-email-john.stultz@linaro.org> From: John Youn CC: Wei Xu , Guodong Xu , "Amit Pundir" , Rob Herring , John Youn , Douglas Anderson , Chen Yu , Kishon Vijay Abraham I , Felipe Balbi , Greg Kroah-Hartman , "linux-usb@vger.kernel.org" Message-ID: <30669194-dc9c-503b-b84d-a7cfd23bbfa6@synopsys.com> Date: Tue, 6 Dec 2016 19:52:32 -0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: <1481075278-17600-1-git-send-email-john.stultz@linaro.org> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.10.186.99] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3219 Lines: 97 On 12/6/2016 5:48 PM, John Stultz wrote: > Hey John, > Just wanted to send this by you, as it seems something is > slightly off with the GOTGCTL state when removing a otg adapter > cable. The following seems to work around the issue I'm seeing. > > Let me know if you have any thoughts on this. > thanks > -john > > > When removing a USB-A to USB-otg adapter cable, we get a change > status irq, and then in dwc2_conn_id_status_change, we > erroniously see the GOTGCTL_CONID_B flag set. This causes us to This is the correct behavior for an OTG controller. When you unplug a cable or plug in the B end of a cable, the ID pin floats, indicating it is a B-Device. When you plug in an A-cable, which is what your adapter is, it will ground the pin, meaning A-device. > get stuck in the "while (!dwc2_is_device_mode(hsotg))" loop, > spitting out "Waiting for Peripheral Mode, Mode=Host" warnings > until it fails out many seconds later. This is weird. Once the ID pin goes to B, the core should become a peripheral and this should be reflected in the status registers. > > This patch works around the issue by re-reading the GOTGCTL > state to check if the GOTGCTL_CONID_B is still set and if not > restarting the change status logic. This also seems weird. The connector id status shouldn't go back to A, assuming you've left the cable unplugged. Is the controller supposed to work in both peripheral and host modes? > > I suspect this isn't the best solution, but it seems to work > well for me. > The workaround seems fine, but still, this indicates that something wrong is going on somwhere. You can add my ack: Acked-by: John Youn Regards, John > Feedback would be greatly appreciated! > > Cc: Wei Xu > Cc: Guodong Xu > Cc: Amit Pundir > Cc: Rob Herring > Cc: John Youn > Cc: Douglas Anderson > Cc: Chen Yu > Cc: Kishon Vijay Abraham I > Cc: Felipe Balbi > Cc: Greg Kroah-Hartman > Cc: linux-usb@vger.kernel.org > Signed-off-by: John Stultz > --- > drivers/usb/dwc2/hcd.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c > index 143da47..6d6802a 100644 > --- a/drivers/usb/dwc2/hcd.c > +++ b/drivers/usb/dwc2/hcd.c > @@ -3203,7 +3203,7 @@ static void dwc2_conn_id_status_change(struct work_struct *work) > dev_dbg(hsotg->dev, "gotgctl=%0x\n", gotgctl); > dev_dbg(hsotg->dev, "gotgctl.b.conidsts=%d\n", > !!(gotgctl & GOTGCTL_CONID_B)); > - > +again: > /* B-Device connector (Device Mode) */ > if (gotgctl & GOTGCTL_CONID_B) { > /* Wait for switch to device mode */ > @@ -3219,6 +3219,9 @@ static void dwc2_conn_id_status_change(struct work_struct *work) > dwc2_is_host_mode(hsotg) ? "Host" : > "Peripheral"); > usleep_range(20000, 40000); > + gotgctl = dwc2_readl(hsotg->regs + GOTGCTL); > + if (!(gotgctl & GOTGCTL_CONID_B)) > + goto again; > if (++count > 250) > break; > } >