Re: [PATCH] usb: dwc2: gadget: On disconnect reset device address to zero

From: Minas Harutyunyan
Date: Mon Jul 10 2017 - 10:59:24 EST

Next message: Greg Kroah-Hartman: "Re: [PATCH] regmap: regmap-w1: Fix build troubles"
Previous message: Rob Herring: "Re: [PATCH v3 2/3] dt-bindings: cpufreq: move MediaTek cpufreq dt-bindings document to proper place"
In reply to: Felipe Balbi: "Re: [PATCH] usb: dwc2: gadget: On disconnect reset device address to zero"
Next in thread: Felipe Balbi: "Re: [PATCH] usb: dwc2: gadget: On disconnect reset device address to zero"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Filipe,

On 7/10/2017 6:14 PM, Felipe Balbi wrote:
>
> Hi again,
>
> Felipe Balbi <balbi@xxxxxxxxxx> writes:
>> Hi,
>>
>> Minas Harutyunyan <Minas.Harutyunyan@xxxxxxxxxxxx> 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 <hminas@xxxxxxxxxxxx>
>>> ---
>>> 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.
>

Next message: Greg Kroah-Hartman: "Re: [PATCH] regmap: regmap-w1: Fix build troubles"
Previous message: Rob Herring: "Re: [PATCH v3 2/3] dt-bindings: cpufreq: move MediaTek cpufreq dt-bindings document to proper place"
In reply to: Felipe Balbi: "Re: [PATCH] usb: dwc2: gadget: On disconnect reset device address to zero"
Next in thread: Felipe Balbi: "Re: [PATCH] usb: dwc2: gadget: On disconnect reset device address to zero"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]