From: Alan Stern > On Wed, 15 Jan 2014, David Laight wrote: > > > I have a theory, I'll try to write a non-invasive patch. > > > > In spite of the comments and some code paths inc_enq() is only > > called by (and has only ever been called by) queue_trb(), so > > is only called for transfer and command rings. > > > > It takes care to avoid advancing past the LINK TRB. > > The coding would be much simpler if it did, and I postulate > > that the reason for this was that some hardware didn't like it. > > In particular doing so with TRB_CHAIN set could well cause issues. > > > > So assume that the ASMedia controller doesn't like LINK TRB if the > > link-to trb can't be processed. If the hardware test vectors > > don't test this is could easily be a bug. > > > > Now, under normal conditions the controller will be idle and > > won't look at the ring until the doorbell is rung - by which > > time the ownership bits are set through to the end of the command. > > > > But consider what happens if the controller is active and > > is looking for work.... > > > > prepare_ring() will change the ownership bit of any NOP TRB > > it writes and of the LINK TRB. The ownership on the first > > transfer TRB is set much later. > > > > This makes it not impossible for the controller to find a TRB > > it doesn't own after processing a LINK TRB. > > Doesn't this mean you shouldn't change the ownership of a LINK TRB > until after you change the ownership of the TRB it points to? That is what I assume. In practise this means that the 'first_trb' (whose ownership is set last) has to be the one that is valid when prepare_ring() is called. The plan for the patch is: - Save the enq pointer in prepare_ring (in the ep_ring structure). - When writing a trb set the ownership unless it is the saved one (ignoring the value set by the caller). - At the end invert the ownership on the saved entry. This is made slightly more complex by the way the isoc code abuses prepare_ring() and prepare_transfer(). If I get the code right a lot of stuff can get deleted. For instance the 'more_trbs_coming' flag to inc_enc() could cause the ownership of the first entry be inverted and the correct doorbell rung. I also think the doorbell address and value are fixed properties of a ring and could usefully be saved in the ring structure. David -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
