On Monday 08 December 2008, Subbrathnam, Swaminathan wrote: > 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.
That's because "busy" was intended to be a flag that effectively preserves a lock on the endpoint, when the driver had to drop the driver-global spinlock while issuing callbacks. Then, if the callback re-entered the driver -- usually to update the endpoint's queue by adding a new transfer request -- it would see that flag was set. It could then update the queue (mostly just list_add_tail) but **NOT** touch the endpoint's registers by e.g. starting a new transfer, or aborting one. Consider any kind of fault path cleanup, where there's state in the registers (endpoint, dma, etc) that is being managed by the code which issued the callback. Having the reentrant code crap all over the registers is a guaranteed way to cause all kinds of subtle bugs. Which is exactly why the "busy" flag was set up to have VERY focussed, and minimal, use: it's *only* set while the driver is defending aginst being re-entered like that, and only for the endpoint that needs that defense. And that's also why the code setting that flag needs to save and restore the previous value ... instead of having code in several places claim it somehow has a global understanding of how reentrance happens. > This way "busy" > field incorrectly reflected the status of the EP at a given > point in time. This patch fixes that. Seems to me you're wanting something other than what that flag indicates. Would a list_empty() test on the transfer queue be more like what you want? Or would you also need to care if there's un-transmitted data in a TX fifo? This doesn't seem like a "fix" to me, at least not without a lot better description of whatever problem is allegedly being addressed. The patch comment kind of vaguely says that it's making the flag do what it's supposed to do. But it doesn't say a word about why it's not *already* doing that, and just reading the diff suggests that it's adding new breakage... _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
