Re: Fwd: Re: PXA27x UDC driver GIT repository

On Tue, Sep 04, 2007 at 01:01:45PM +0400, Yuri Tikhonov wrote:

>  Oops, my previous e-mail in this topic was surely addressed to you as to the 
> author of the pxa27x UDC driver.
>  Though the David's and others comments are very welcomed too : )

:)

>  I investigated the problem and found the following reference in the PXA270
> Developers Manual document:
> "
>  12.6.8.5 Sent STALL and Force STALL
>  ...
>  Note: Users must clear the SST bit before the USB host controller requests
> more data from IN endpoints or invalid or corrupted data may be sent to the
> host.
>  ...
> "

Continuing to read the data sheet we found:

   ...
   To force the UDC to issue a STALL handshake in response to all IN
   token requests or all OUT tokens, set the FST bit. The FST bit is
   cleared and the SST bit is set when the STALL handshake has actually
   been sent to the USB host controller and the STALL state entered (this
   may be delayed if the UDC is active when the FST bit is set).
   ...

>  What follows is the patch (also attached) which modifies the pxa27x UDC
> driver according to the note mentioned above. Using this patch I successfully
> run the UDC driver with the file-storage gadget on its top and mounted the
> backing file exported by the gadget on my host PC.
> 
>  Perhaps this is not the most effective and elegant way of fixing the bug
> discovered, or  there are some other places in the UDC driver where the
> corresponding fix has to be applied too. What do you think ?

Since the original code says:

        /* FST bit is the same for control, bulk in, bulk out, interrupt in */
        *ep->reg_udccsr = UDCCSR_FST | UDCCSR_FEF;

        /* ep0 needs special care */
        if (!ep->desc) {
                start_udc_watchdog(ep->udc);
                ep->udc->req_pending = 0;
                ep->udc->ep0state = EP0_STALL;

                /* and bulk/intr endpoints like dropping stalls too */
        } else {
                unsigned i;
                for (i = 0; i < 1000; i += 20) {
                        if (*ep->reg_udccsr & UDCCSR_SST)
                                break;
                        udelay(20);
                }
        }

It seems to me that after setting FST bit in order to force a STALL,
the driver waits for SST bit assert that means the STALL handshake has
actually been sent to the USB host controller and the STALL state
entered.

That's why I think that we should clear SST bit _after_ the "for"
cycle ends. That is:

	/* FST bit is the same for control, bulk in, bulk out, interrupt in */
	*ep->reg_udccsr = UDCCSR_FST | UDCCSR_FEF;

        /* ep0 needs special care */
	if (!ep->desc) {
                start_udc_watchdog(ep->udc);
                ep->udc->req_pending = 0;
                ep->udc->ep0state = EP0_STALL;

		/* and bulk/intr endpoints like dropping stalls too */
	} else {
                unsigned i;
		for (i = 0; i < 1000; i += 20) {
			if (*ep->reg_udccsr & UDCCSR_SST)
				break;
			udelay(20);
		}
+
+               /* According to UM we must clear the SST bit
+                * before the USB host controller requests more
+                * data from IN endpoints; do it right now.
+                */
+               *ep->reg_udccsr = UDCCSR_SST;

	}

What do you think about that? Can you please test this solution also?
I'm very busy now and I cannot do it myself.

Thanks,

Rodolfo

-- 

GNU/Linux Solutions                  e-mail:    [email protected]
Linux Device Driver                             [email protected]
Embedded Systems                     		[email protected]
UNIX programming                     phone:     +39 349 2432127

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel