Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752430AbcLGA1Z (ORCPT ); Tue, 6 Dec 2016 19:27:25 -0500 Received: from smtprelay4.synopsys.com ([198.182.47.9]:56816 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751180AbcLGA1Y (ORCPT ); Tue, 6 Dec 2016 19:27:24 -0500 Subject: Re: [RFC][PATCH 1/3 v2] usb: dwc2: Add extcon support to dwc2 driver To: John Stultz , John Youn References: <1481011582-7162-1-git-send-email-john.stultz@linaro.org> <1481011582-7162-2-git-send-email-john.stultz@linaro.org> <4be239ba-b2f1-a951-d558-7469bf835556@synopsys.com> From: John Youn CC: lkml , Wei Xu , Guodong Xu , Amit Pundir , "Rob Herring" , Douglas Anderson , "Chen Yu" , Kishon Vijay Abraham I , "Felipe Balbi" , Greg Kroah-Hartman , "linux-usb@vger.kernel.org" Message-ID: <5c4f7288-c0b1-f770-2141-067f4724e95f@synopsys.com> Date: Tue, 6 Dec 2016 16:26:22 -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: 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: 4164 Lines: 99 On 12/6/2016 4:05 PM, John Stultz wrote: > On Tue, Dec 6, 2016 at 3:17 PM, John Youn wrote: >> On 12/6/2016 12:06 AM, John Stultz wrote: >>> This patch wires up extcon support to the dwc2 driver >>> so that devices that use a modern generic phy driver >>> and don't use the usb-phy infrastructure can still >>> signal connect/disconnect events. >>> >>> 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 >>> --- >>> v2: >>> * Move extcon logic from generic phy to dwc2 driver, as >>> suggested by Kishon. >>> --- >>> arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 11 +++++++++ >>> drivers/usb/dwc2/core.h | 16 ++++++++++++ >>> drivers/usb/dwc2/core_intr.c | 25 +++++++++++++++++++ >>> drivers/usb/dwc2/hcd.c | 23 +++++++++++++++++ >>> drivers/usb/dwc2/platform.c | 41 +++++++++++++++++++++++++++++++ >>> 5 files changed, 116 insertions(+) >>> >>> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi >>> index 17839db..8a86a11 100644 >>> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi >>> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi >>> @@ -732,6 +732,16 @@ >>> regulator-always-on; >>> }; >>> >>> + usb_vbus: usb-vbus { >>> + compatible = "linux,extcon-usb-gpio"; >>> + id-gpio = <&gpio2 6 1>; >>> + }; >>> + >>> + usb_id: usb-id { >>> + compatible = "linux,extcon-usb-gpio"; >>> + id-gpio = <&gpio2 5 1>; >>> + }; >>> + >> >> So you are using extcon-usb-gpio driver to detect both the ID pin and >> VBUS status correct? Do you need the VBUS one? It doesn't look like >> you are using it. > > Not at the moment. Apologies for my ignorance, I'm not totally > familiar with the usage of the vbus vs id pins, so I'm somewhat > following what I was seeing from other drivers. I know with a usb OTG > to usb A adapter, you get the vbus signal but not the id signal, but I > don't quite see what should be done differently in that case (as it > seems to work ok). Hi John, The ID pin indicates which end of the cable is connected to the controller port, determining whether it should take the role of an A-device or B-device. If this is not visible to the controller (thus the controller would not give CONNIDSTSCHNG interrupt), that is why you would need to hook up the extcon module. I'm thinking this shouldn't be needed for you since you can see this interrupt. > >> Also, do you really need this at all? Wasn't your system previously >> able to detect the ID pin change correctly via the connection id >> status change interrupt? This would only be needed if that were not >> the case. > > So it can be made work w/o this, but we needed other hacks because the > usb-gadget disconnect logic never triggered when the cable was > unplugged. The controller would jump over to host mode, then when we > re-plugged in the usb-gadget cable, it would fail often as we never > got a disconnect signal. That's why earlier I was using this hack to > force gadget disconnect before the reset was called: > https://lkml.org/lkml/2016/10/20/26 Other than the triggering WARN_ON() in fifo init, is there any other negative effects? We are revisiting this fifo init code and I think the fifo init is not necessary for USB_RESET purposes. This should get rid of a race condition where the EP's are not disabled before attempting to initialize their FIFO's. Which should get rid of the WARN_ON(). If this is the only issue, then this will probably resolve it. Regards, John