Sergei,
The patches do address the issues for which they were submitted.
1. Busy field update patch ensures that "busy" field reflects the
current status of the associated EP. In previous implementation busy is
predominantly set to 0 except for a brief time in giveback and nuke functions.
This way "busy" field incorrectly reflected the status of the EP at a given
point in time. This patch fixes that.
2. Stall handling is done correctly from the underlying driver
perspective. Pl. refer to my reply to Alan on the problem scenarios and what
this patch addresses. As I see it the USB applications can be state based (MSC
- Command, Data, Response) or stateless (RNDIS - Ethernet streaming) for
example. There could be other classes of apps that fall in between. Now
having the above scenario
1. If the gadget application invokes stall on the EP on its own
i). If the application is state based it is the responsibility
of the application to take appropriate steps, which might be to cancel all or
some requests before invoking the STALL on the EP.
ii). If the application is state less then it can either choose
not to cancel the requests or cancel all/part of the outstanding requests
before invoking the STALL on the EP. There could be a scenario were there may
not be a need to STALL at all.
2. If the host issues the request for the EP to be stalled
i). If the application is state based, on getting the -EPIPE
status on the cancelled request the application can choose to cancel all the
outstanding IO's and then start a new transfer. This way application re-sets
its state machine to the correct state to respond to the stall event from the
host.
ii). If the application is state less then it can choose to
ignore the -EPIPE error on the cancelled request and continue operation as
normal.
In all the above scenario's we let the application decide on the course
of action. The underlying driver just provides the feedback through -EPIPE
status on the In-Flight IO or if no IO is active just re-sets the EP to a clean
state. The application for its part chooses to cancel all or none as
appropriate.
The fixes and contribution's does not become invalid just because you
choose to repeatedly question them.
Regards
Swami
> -----Original Message-----
> From: Sergei Shtylyov [mailto:[EMAIL PROTECTED]
> Sent: Monday, December 08, 2008 12:04 AM
> To: Sergei Shtylyov
> Cc: Subbrathnam, Swaminathan; [EMAIL PROTECTED];
> [EMAIL PROTECTED]; [email protected]
> Subject: Re: [PATCH] MUSB : Fix for busy field update in the EP structure.
>
> Hello, I wrote:
>
> >> This patch updates the "busy" field to reflect the current status of
> the
> >> endpoint. This fix is critical to enable the request queuing logic to
> >> prevent
> >> a queued request from overwriting/interfering in-flight request.
>
> > Frankly speaking, I'm not seeing how a newly queued request can
> > overwrite anything as it's only started being *at the head of the
> > request queue* (which it won't be unless it's queued onto an already
> > empty queue with the last request has been already done). Moreover,
> > ep->busy will have already been set by musb_g_givebck() iff the queue()
> > method is called from the request completion handler, so
> > musb_ep_restart() cannotr be called from there at this time. What this
> > is really fixing is your broken logic in the prior patch about which so
> > many spears have been broken already, if I don't mistake, and in this
> > case these patches should be merged.
> > But I do think that your whole approach to stalling/aborting in flight
> > request is wrong and that's what should be fixed instead.
>
> Felipe, I'm seeing both the questionable patches already queued by you
> despite of the patches being inconsistent and fixing the issues in an
> inconsisnt way. Despite of this, I'd like to restate my request to
> Swaminathan
> to provide a failure scenario that this patch is allegedly fixing.
>
> WBR, Sergei
>
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
