Sergei,
My response inlined.
Regards
Swami
(Type "pspproducts" in you web browser for PSP info)
http://dbdwss01.india.ti.com/pspproducts/
PSP downloads at :
http://software.ti.com/swcoe/intranet/reports/pds/PSP_releases.php
Office : +91-80-25048629
> -----Original Message-----
> From: Sergei Shtylyov [mailto:[EMAIL PROTECTED]
> Sent: Thursday, December 04, 2008 10:54 PM
> To: Sergei Shtylyov
> Cc: Subbrathnam, Swaminathan; [EMAIL PROTECTED];
> [EMAIL PROTECTED]; [email protected]
> Subject: Re: [PATCH 3/3] MUSB : Fix for STALL handling in musb gadget code
>
> Hello.
>
> Sergei Shtylyov wrote:
>
> >> If EP is busy, we have even less reason to do a giveback. This is
> >> not a real fix, just pallialtive. Actually, whether a STALL token has
> >> been sent or not shouldn't play any role in giving back URB, so the
> >> fragment being pacthed is just totally wrong. Other drivers just don't
> >> allow EP that is still active to be halted, and this driver should do
> >> the same.
>
> >> Your comment is plain wrong.
>
> > No, you code is.
>
> Even your proof case contradicts your own code.
>
> >> I think there are scenarios that would require requests to be given
> >> back during stall handling.
>
> >> I am laying one out below for
>
> >> 1. USB Device is in the process of transferring data to/from Host.
> >> 2. USB Device queues another request to send.
> >> 3. For some reason say Host/Device needs to STALL the endpoint
> >> with the in-flight IO.
>
> > This can't be done reliably with requests in flight. That's why the
> > other drivers don't allow halting EP with requests in flight.
>
> >> 4. On completion of the STALL (on SENTSTALL interrupt) driver
> >> would need to abort the current transfer and return the USB request
> >> with appropriate status. Here the EP state is busy handling the
> >> in-flight IO and the request that is active has to be
> returned/givenback.
>
> > This just cannot work reliably. There should be connection between
> > actually sending STALL tokens and URB completion because this cannot be
> > handled reliably.
>
> >> 5. Once the CLEAR Stall is completed the Device would need to
> >> process the next request that is queued if one exists and continue
> >> further normally.
>
> > The requests *can* be queued while the EP is halted, and that's what
> > the other drivers do. You just can't stall EP until the request queue
> > drains.
>
> Or at least until the current request's I/O is complete (this doesn't
> mean
> that it's given back as the driver employs early giveback).
<SWAMI> Driver does not employ early giveback. It is returned with error in
the context of reception of STALL interrupt. There will be no other interrupt
raised after this condition. The only other event is the request from Host to
clear the STALL over EP0.
>
> >> In the above scenario as you can see we need to abort the in-flight
> >> request (musb_ep->busy) on SENTSTALL interrupt and process the queued
> >> request in the context of clear stall. Consider SENTSTALL interrupt
> >> as a completion interrupt for the current in-flight request that needs
> >> to be completed with error.
>
> > I'm not seeing why we would need to abort an in-flight request.
> > Moreover, I'm not seeing a conection between the EP busy state and an
> > URB giveback. What your patch does is just avoiding a race between the
> > EP halt and queueing URB afterwards -- with the gadget not intending to
> > abort that URB but which gets aborted. This for example happens when the
> > file-backed storage gadget sends short data in response to the MODE
> > SENSE command and the necessairily halts the endpoint, then queues the
> > CSW but the MUSB gadget driver would prematurely giveback that URB afetr
> > having sent STALL. This results in host's failure to get any CSW and USB
> > reset finally ensues...
> > And now please tell me how can the driver differ the URB that you
> > want to abort from one that you must not abort?
>
> And also tell me how you fix helps the driver to abort in-flight
> requests
> when you specifically disallow it to do so.
<SWAMI> Pl. refer to the SENT Stall handling for further information. This is
the code that has been in the community for a very long time now. The patch
just adds check for busy status before returning the IO.
>
> WBR, Sergei
>
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
