On Fri, Mar 08, 2013 at 05:38:53PM +0800, Peter Chen wrote:
> On Thu, Mar 07, 2013 at 09:58:52AM +0100, Peter Bestler wrote:
> > Hi,
> > 
> > We try to get our device (based on p2020rdb) usb 2.0 compliant. We ran the 
> > usb30cv test suite (version 1.0.1.2, chapter 9 tests for usb 2.0 devices) 
> > on win7 with g_zero and g_serial. We access the device via an usb 3.0 hcd 
> > from intel. Our device runs the 3.2.35-rt52 kernel. I spotted the following 
> > problem with ch9setaddress in fsl_udc_core.c.
> > 
> > All tests passed on single execution. At running it in batch mode the first 
> > test after switching from default to adressed state failed. The subsequent 
> > tests passed. It doesn't depend on the selected tests, only on the switch 
> > over from default to adressed. It fails with our custom gadgetfs driver 
> > too. Another device with Intel PXA25x and the same setup passes all tests.
> > 
> > With the total phase usbsniffer i spotted the following behavior:
> > The test issues a setAddress and receives an ACK, 125 us afterwards the 
> > host issues a getDescriptor (setuptx) request, which fails at setuptx. The 
> > USB sniffer reports invalid PID sequence.
> > 
> > For debugging purpose I delayed the dma_map_single in ep0_prime_status  
> > (which does the ACK, right?) by 2 milliseconds. And all tests are passing 
> > in batch mode. It's quite the same sequence on the bus, but between 
> > setAdress and its ACK is a delay of 3 ms.
> > 
> > I think delaying the ACK in set request isn't the way to go. I think we set 
> > the address too early; we have to wait until the status phase of the set 
> > address has finished. My understanding is that the device has to respond to 
> > address 0 until the complete status phase of setAddress is passed (is this 
> > correct).
> > 
> > Has anybody ran the usb30cv on fsl_udc recently? 
> > 
> > After applying the patch f79a60b8785 none of the tests run anymore. Did I 
> > miss anything here?  How can we fix this issue?
> > 
> > Best regards 
> > 
> > Peter
> > 
> > --- Patch for debugging ---
> > diff --git a/drivers/usb/gadget/fsl_udc_core.c 
> > b/drivers/usb/gadget/fsl_udc_core.c
> > index 55abfb6..fdbfd25 100644
> > --- a/drivers/usb/gadget/fsl_udc_core.c
> > +++ b/drivers/usb/gadget/fsl_udc_core.c
> > @@ -1285,6 +1285,7 @@ static int ep0_prime_status(struct fsl_udc *udc, int 
> > direction)
> >         req->req.complete = NULL;
> >         req->dtd_count = 0;
> >  
> > +       udelay(2000);
> >         req->req.dma = dma_map_single(ep->udc->gadget.dev.parent,
> >                         req->req.buf, req->req.length,
> >                         ep_is_in(ep) ? DMA_TO_DEVICE : DMA_FROM_DEVICE);
> > 
> > 
> > 
> > 
> 
> Please check your datasheet if your controller has USBADRA bit
> at DEVICEADDR, if it exists, it means your controller supports
> advance store device address function. Please have a try for 
> below change, if it fixes your problem, please give a tested-by,
> then, I can make change for chipidea and fsl-core driver.
> 
> diff --git a/drivers/usb/gadget/fsl_udc_core.c 
> b/drivers/usb/gadget/fsl_udc_core.c
> index 04d5fef..a75c884 100644
> --- a/drivers/usb/gadget/fsl_udc_core.c
> +++ b/drivers/usb/gadget/fsl_udc_core.c
> @@ -1335,10 +1335,14 @@ static void udc_reset_ep_queue(struct fsl_udc *udc, 
> u8 pipe)
>   */
>  static void ch9setaddress(struct fsl_udc *udc, u16 value, u16 index, u16 
> length)
>  {
> -     /* Save the new address to device struct */
>       udc->device_address = (u8) value;
> +     /* Set the new address */
> +     fsl_writel(((u32)value << USB_DEVICE_ADDRESS_BIT_POS)| 
> +                     (1 << USB_DEVICE_ADDRESS_BIT_POS),

do you mean:

(u32) ((value << USB_DEVICE_ADDRESS_BIT_POS) |
        (1 << USB_DEVICE_ADDRESS_BIT_POS))

??

Also, why do you need the extra (1 << USB_DEVICE_ADDRESS_BIT_POS) ?

You'd be making all addresses odd, no ?

-- 
balbi

Attachment: signature.asc
Description: Digital signature

Reply via email to