Hi again, Felipe Balbi <[email protected]> writes: > Hi, > > Minas Harutyunyan <[email protected]> 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 <[email protected]> >> --- >> 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.
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.
--
pbalbi
signature.asc
Description: PGP signature
