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.
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
Attachment:
signature.asc
Description: PGP signature
