Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754151AbdGJO7U convert rfc822-to-8bit (ORCPT ); Mon, 10 Jul 2017 10:59:20 -0400 Received: from us01smtprelay-2.synopsys.com ([198.182.47.9]:60140 "EHLO smtprelay.synopsys.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753309AbdGJO7S (ORCPT ); Mon, 10 Jul 2017 10:59:18 -0400 From: Minas Harutyunyan To: Felipe Balbi , Minas Harutyunyan , John Youn , "Greg Kroah-Hartman" , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] usb: dwc2: gadget: On disconnect reset device address to zero Thread-Topic: [PATCH] usb: dwc2: gadget: On disconnect reset device address to zero Thread-Index: AQHS+YHTrvN+Vv7VDU6i+iB/EOmV5A== Date: Mon, 10 Jul 2017 14:59:13 +0000 Message-ID: <410670D7E743164D87FA6160E7907A56FDABFDBC@am04wembxa.internal.synopsys.com> References: <87d19875fc.fsf@linux.intel.com> <87a84c751h.fsf@linux.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.116.70.205] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 13997 Lines: 331 Hi Filipe, On 7/10/2017 6:14 PM, Felipe Balbi wrote: > > Hi again, > > Felipe Balbi writes: >> Hi, >> >> Minas Harutyunyan writes: >>> USB CV driver stack doesn't perform USB RESET after device disconnect/ >>> connect, so need to reset to zero DEVADDR field in DCFG to pass >>> enumeration again. >>> >>> Signed-off-by: Minas Harutyunyan >>> --- >>> drivers/usb/dwc2/gadget.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c >>> index 98a4a79e7f6e..deb3d901b99d 100644 >>> --- a/drivers/usb/dwc2/gadget.c >>> +++ b/drivers/usb/dwc2/gadget.c >>> @@ -3174,6 +3174,9 @@ void dwc2_hsotg_disconnect(struct dwc2_hsotg *hsotg) >>> return; >>> >>> hsotg->connected = 0; >>> + /* On disconnect reset device address to zero */ >>> + __bic32(hsotg->regs + DCFG, DCFG_DEVADDR_MASK); >>> + >> >> Which of the tests are you talking about? Which particular USB CV are >> you running? >> >> I don't remember ever seeing this with dwc3. How should I go about >> triggering this problem? If this was really the case, then we would have >> this on *all* UDCs. >> >> I just did a fresh install of USB 3 Gen X CV (that I just download from >> usb.org). Ran Chapter 9 tests against a HS dwc3 board I have around and >> I can't see the problem you're talking about. >> >> Here are all my non-endpoint interrupt events in order. Test passes >> fine. If disconnect, reconnect and run the tests again, then Reset will >> be driven on the bus which will cause address to be reset. >> >> Can you share more details about the problem you're facing? > > I've been looking at dwc2 for a while and I think this is a bug in > dwc2_hsotg_irq() on the branch for GINTSTS_USBRST. I don't have docs for > dwc2. > USB RESET not issuing by CV host stack after connect. Below dmesg: Disconnect [23984.039199] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04008428 00000400 (d0bc3cc4) retry 8 [23984.039204] dwc2 dwc2.1.auto: GINTSTS_ErlySusp [23984.042235] dwc2 dwc2.1.auto: gintsts=04008828 gintmsk=d0bc3cc4 [23984.042240] dwc2 dwc2.1.auto: USB SUSPEND [23984.042247] dwc2 dwc2.1.auto: DSTS=0x5f4c01 [23984.042250] dwc2 dwc2.1.auto: DSTS.Suspend Status=1 HWCFG4.Power Optimize=0 [23984.042260] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04008028 00000000 (d0bc3cc4) retry 8 [23984.272308] dwc2 dwc2.1.auto: gintsts=0480902c gintmsk=d0bc3cc4 [23984.272318] dwc2 dwc2.1.auto: ++OTG Interrupt gotgint=4 [b_peripheral] [23984.272321] dwc2 dwc2.1.auto: ++OTG Interrupt: Session End Detected++ (b_peripheral) [23984.272331] dwc2 dwc2.1.auto: complete: ep ffff880401a31b28 ep0, req ffff8803e495ef00, -108 => ffffffffa00541da [23984.272336] dwc2 dwc2.1.auto: dwc2_hsotg_complete_setup: failed -108 [23984.272346] dwc2 dwc2.1.auto: complete: ep ffff880401a30328 ep1out, req ffff88040b6d7c80, -108 => ffffffffa008865c [23984.272435] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04809028 00801000 (d0bc3cc4) retry 8 [23984.272437] dwc2 dwc2.1.auto: dwc2_hsotg_irq: USBRstDet [23984.272442] dwc2 dwc2.1.auto: dwc2_hsotg_irq: USBRst [23984.272446] dwc2 dwc2.1.auto: GNPTXSTS=00040300 [23984.272473] dwc2 dwc2.1.auto: dwc2_hsotg_ep_disable(ep ffff880401a30528) [23984.272478] dwc2 dwc2.1.auto: dwc2_hsotg_ep_disable: DxEPCTL=0x084a0200 [23984.272485] dwc2 dwc2.1.auto: dwc2_hsotg_ep_disable(ep ffff880401a30328) [23984.272490] dwc2 dwc2.1.auto: dwc2_hsotg_ep_stop_xfr: stopping transfer on ep1out [23984.272513] dwc2 dwc2.1.auto: dwc2_hsotg_ep_disable: DxEPCTL=0x08080200 [23984.272540] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04008028 00000000 (d0bc3cc4) retry 8 Connect [24010.664017] dwc2 dwc2.1.auto: gintsts=44008028 gintmsk=d0bc3cc4 [24010.664023] dwc2 dwc2.1.auto: Session request interrupt - lx_state=0 [24010.664035] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04008028 00000000 (d0bc3cc4) retry 8 Run test First try to enumerate [24021.649528] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 0400a028 00002000 (d0bc3cc4) retry 8 [24021.649536] dwc2 dwc2.1.auto: EnumDone (DSTS=0x0043e902) [24021.649539] dwc2 dwc2.1.auto: new device is full-speed [24021.649618] dwc2 dwc2.1.auto: dwc2_hsotg_enqueue_setup: queueing setup request [24021.649623] dwc2 dwc2.1.auto: ep0: req ffff8803e495ef00: 8@ffff88040ad1d228, noi=0, zero=0, snok=0 [24021.649631] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DxEPCTL=0x80028000, ep 0, dir out [24021.649636] dwc2 dwc2.1.auto: ureq->length:8 ureq->actual:0 [24021.649640] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: 1@8/8, 0x00080008 => 0x00000b10 [24021.649643] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: 0x00000000d9858800 => 0x00000b14 [24021.649645] dwc2 dwc2.1.auto: ep0 state:0 [24021.649648] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DxEPCTL=0x80028000 [24021.649654] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DXEPCTL=0x80028000 [24021.649664] dwc2 dwc2.1.auto: EP0: DIEPCTL0=0x00008000, DOEPCTL0=0x80028000 [24021.849430] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04088028 00080000 (d0bc3cc4) retry 8 [24021.849439] dwc2 dwc2.1.auto: dwc2_hsotg_irq: daint=00010000 [24021.849452] dwc2 dwc2.1.auto: dwc2_hsotg_epint: ep0(out) DxEPINT=0x00008009 [24021.849455] dwc2 dwc2.1.auto: dwc2_hsotg_epint: Setup/Timeout [24021.849462] dwc2 dwc2.1.auto: complete: ep ffff880401a31b28 ep0, req ffff8803e495ef00, 0 => ffffffffa00541da [24021.849468] dwc2 dwc2.1.auto: ctrl Type=80, Req=06, V=0100, I=0000, L=0008 [24021.849474] dwc2 dwc2.1.auto: ep0: req ffff88040d7c9b80: 8@ffff8803e9af6000, noi=0, zero=0, snok=0 [24021.849481] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DxEPCTL=0x00028000, ep 0, dir in [24021.849486] dwc2 dwc2.1.auto: ureq->length:8 ureq->actual:0 [24021.849490] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: 1@8/8, 0x00080008 => 0x00000910 [24021.849493] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: 0x00000000d9859000 => 0x00000914 [24021.849495] dwc2 dwc2.1.auto: ep0 state:1 [24021.849498] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DxEPCTL=0x84028000 [24021.849504] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DXEPCTL=0x80008000 [24021.849560] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04048028 00040000 (d0bc3cc4) retry 8 [24021.849567] dwc2 dwc2.1.auto: dwc2_hsotg_irq: daint=00000001 [24021.849578] dwc2 dwc2.1.auto: dwc2_hsotg_epint: ep0(in) DxEPINT=0x00000001 [24021.849585] dwc2 dwc2.1.auto: dwc2_hsotg_epint: XferCompl: DxEPCTL=0x00008000, DXEPTSIZ=00000000 [24021.849590] dwc2 dwc2.1.auto: dwc2_hsotg_complete_in: adjusting size done 0 => 8 [24021.849593] dwc2 dwc2.1.auto: req->length:8 req->actual:8 req->zero:0 [24021.849596] dwc2 dwc2.1.auto: Receiving zero-length packet on ep0 [24021.849629] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04088028 00080000 (d0bc3cc4) retry 8 [24021.849635] dwc2 dwc2.1.auto: dwc2_hsotg_irq: daint=00010000 [24021.849647] dwc2 dwc2.1.auto: dwc2_hsotg_epint: ep0(out) DxEPINT=0x00000001 [24021.849654] dwc2 dwc2.1.auto: dwc2_hsotg_epint: XferCompl: DxEPCTL=0x00028000, DXEPTSIZ=20000000 [24021.849658] dwc2 dwc2.1.auto: zlp packet received [24021.849661] dwc2 dwc2.1.auto: complete: ep ffff880401a31b28 ep0, req ffff88040d7c9b80, 0 => ffffffffa0076da7 [24021.849665] dwc2 dwc2.1.auto: dwc2_hsotg_enqueue_setup: queueing setup request [24021.849669] dwc2 dwc2.1.auto: ep0: req ffff8803e495ef00: 8@ffff88040ad1d228, noi=0, zero=0, snok=0 [24021.849674] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DxEPCTL=0x00028000, ep 0, dir out [24021.849679] dwc2 dwc2.1.auto: ureq->length:8 ureq->actual:0 [24021.849682] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: 1@8/8, 0x00080008 => 0x00000b10 [24021.849685] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: 0x00000000d9859800 => 0x00000b14 [24021.849687] dwc2 dwc2.1.auto: ep0 state:0 [24021.849689] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DxEPCTL=0x80028000 [24021.849696] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DXEPCTL=0x80028000 Second try to enumerate [24022.203534] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04008428 00000400 (d0bc3cc4) retry 8 [24022.203539] dwc2 dwc2.1.auto: GINTSTS_ErlySusp [24022.206591] dwc2 dwc2.1.auto: gintsts=04008828 gintmsk=d0bc3cc4 [24022.206595] dwc2 dwc2.1.auto: USB SUSPEND [24022.206602] dwc2 dwc2.1.auto: DSTS=0x426203 [24022.206605] dwc2 dwc2.1.auto: DSTS.Suspend Status=1 HWCFG4.Power Optimize=0 [24022.206615] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04008028 00000000 (d0bc3cc4) retry 8 [24022.432686] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04809028 00801000 (d0bc3cc4) retry 8 [24022.432691] dwc2 dwc2.1.auto: dwc2_hsotg_irq: USBRstDet [24022.432695] dwc2 dwc2.1.auto: dwc2_hsotg_irq: USBRst [24022.432700] dwc2 dwc2.1.auto: GNPTXSTS=00040300 [24022.487557] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 0400a028 00002000 (d0bc3cc4) retry 8 [24022.487564] dwc2 dwc2.1.auto: EnumDone (DSTS=0x00131000) [24022.487566] dwc2 dwc2.1.auto: new device is high-speed [24022.487645] dwc2 dwc2.1.auto: dwc2_hsotg_enqueue_setup: queueing setup request [24022.487647] dwc2 dwc2.1.auto: dwc2_hsotg_enqueue_setup already queued??? [24022.487654] dwc2 dwc2.1.auto: EP0: DIEPCTL0=0x00008000, DOEPCTL0=0x80028000 [24022.688563] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04088028 00080000 (d0bc3cc4) retry 8 [24022.688572] dwc2 dwc2.1.auto: dwc2_hsotg_irq: daint=00010000 [24022.688584] dwc2 dwc2.1.auto: dwc2_hsotg_epint: ep0(out) DxEPINT=0x00008009 [24022.688587] dwc2 dwc2.1.auto: dwc2_hsotg_epint: Setup/Timeout [24022.688593] dwc2 dwc2.1.auto: complete: ep ffff880401a31b28 ep0, req ffff8803e495ef00, 0 => ffffffffa00541da [24022.688598] dwc2 dwc2.1.auto: ctrl Type=80, Req=06, V=0100, I=0000, L=0012 [24022.688604] dwc2 dwc2.1.auto: ep0: req ffff88040d7c9b80: 18@ffff8803e9af6000, noi=0, zero=0, snok=0 [24022.688611] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DxEPCTL=0x00028000, ep 0, dir in [24022.688615] dwc2 dwc2.1.auto: ureq->length:18 ureq->actual:0 [24022.688619] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: 1@18/18, 0x00080012 => 0x00000910 [24022.688622] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: 0x00000000d985a000 => 0x00000914 [24022.688625] dwc2 dwc2.1.auto: ep0 state:1 [24022.688627] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DxEPCTL=0x84028000 [24022.688634] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DXEPCTL=0x80008000 [24022.688676] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04048028 00040000 (d0bc3cc4) retry 8 [24022.688682] dwc2 dwc2.1.auto: dwc2_hsotg_irq: daint=00000001 [24022.688694] dwc2 dwc2.1.auto: dwc2_hsotg_epint: ep0(in) DxEPINT=0x00000001 [24022.688701] dwc2 dwc2.1.auto: dwc2_hsotg_epint: XferCompl: DxEPCTL=0x00008000, DXEPTSIZ=00000000 [24022.688706] dwc2 dwc2.1.auto: dwc2_hsotg_complete_in: adjusting size done 0 => 18 [24022.688709] dwc2 dwc2.1.auto: req->length:18 req->actual:18 req->zero:0 [24022.688711] dwc2 dwc2.1.auto: Receiving zero-length packet on ep0 [24022.688755] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04088028 00080000 (d0bc3cc4) retry 8 [24022.688762] dwc2 dwc2.1.auto: dwc2_hsotg_irq: daint=00010000 [24022.688773] dwc2 dwc2.1.auto: dwc2_hsotg_epint: ep0(out) DxEPINT=0x00000001 [24022.688781] dwc2 dwc2.1.auto: dwc2_hsotg_epint: XferCompl: DxEPCTL=0x00028000, DXEPTSIZ=20000000 [24022.688785] dwc2 dwc2.1.auto: zlp packet received [24022.688788] dwc2 dwc2.1.auto: complete: ep ffff880401a31b28 ep0, req ffff88040d7c9b80, 0 => ffffffffa0076da7 [24022.688791] dwc2 dwc2.1.auto: dwc2_hsotg_enqueue_setup: queueing setup request [24022.688795] dwc2 dwc2.1.auto: ep0: req ffff8803e495ef00: 8@ffff88040ad1d228, noi=0, zero=0, snok=0 [24022.688801] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DxEPCTL=0x00028000, ep 0, dir out [24022.688805] dwc2 dwc2.1.auto: ureq->length:8 ureq->actual:0 [24022.688808] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: 1@8/8, 0x00080008 => 0x00000b10 [24022.688811] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: 0x00000000d985a800 => 0x00000b14 [24022.688813] dwc2 dwc2.1.auto: ep0 state:0 [24022.688816] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DxEPCTL=0x80028000 [24022.688822] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DXEPCTL=0x80028000 > Nowhere in that function is driver making sure DCFG_DEVADDR is cleared > to 0. In fact, there nowhere at all in the driver where you're making > sure to reset device address: > > $ git --no-pager grep --color -nH -e DCFG_DEVADDR_ -- drivers/usb/dwc2/ > drivers/usb/dwc2/gadget.c:1837: dcfg &= ~DCFG_DEVADDR_MASK; > drivers/usb/dwc2/gadget.c:1839: DCFG_DEVADDR_SHIFT) & DCFG_DEVADDR_MASK; > drivers/usb/dwc2/hw.h:426:#define DCFG_DEVADDR_MASK (0x7f << 4) > drivers/usb/dwc2/hw.h:427:#define DCFG_DEVADDR_SHIFT 4 > drivers/usb/dwc2/hw.h:428:#define DCFG_DEVADDR_LIMIT 0x7f > > If we look into gadget.c on those lines, here's what we have: > > static void dwc2_hsotg_process_control(struct dwc2_hsotg *hsotg, > struct usb_ctrlrequest *ctrl) > { > > [...] > > struct dwc2_hsotg_ep *ep0 = hsotg->eps_out[0]; > int ret = 0; > u32 dcfg; > > dev_dbg(hsotg->dev, > "ctrl Type=%02x, Req=%02x, V=%04x, I=%04x, L=%04x\n", > ctrl->bRequestType, ctrl->bRequest, ctrl->wValue, > ctrl->wIndex, ctrl->wLength); > > if (ctrl->wLength == 0) { > ep0->dir_in = 1; > hsotg->ep0_state = DWC2_EP0_STATUS_IN; > } else if (ctrl->bRequestType & USB_DIR_IN) { > ep0->dir_in = 1; > hsotg->ep0_state = DWC2_EP0_DATA_IN; > } else { > ep0->dir_in = 0; > hsotg->ep0_state = DWC2_EP0_DATA_OUT; > } > > if ((ctrl->bRequestType & USB_TYPE_MASK) == USB_TYPE_STANDARD) { > switch (ctrl->bRequest) { > case USB_REQ_SET_ADDRESS: > hsotg->connected = 1; > dcfg = dwc2_readl(hsotg->regs + DCFG); > dcfg &= ~DCFG_DEVADDR_MASK; > dcfg |= (le16_to_cpu(ctrl->wValue) << > DCFG_DEVADDR_SHIFT) & DCFG_DEVADDR_MASK; > dwc2_writel(dcfg, hsotg->regs + DCFG); > > dev_info(hsotg->dev, "new address %d\n", ctrl->wValue); > > ret = dwc2_hsotg_send_reply(hsotg, ep0, NULL, 0); > return; > > [...] > } > > So you only touch DEVADDR from SetAddress. That's clearly a bug > elsewhere in the driver and is *NOT* related to USB CV at all. Please > fix this driver properly instead of hacking around unrelated functions. > > Disconnect IRQ is *NOT* supposed to clear device address. That's a job > for Reset IRQ. >