Re: [RFC PATCH 19/20] xhci: v1.0 scatterlist enqueue support (td-fragment rework)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 26, 2014 at 3:55 AM, David Laight <David.Laight@xxxxxxxxxx> wrote:
> From: Dan Williams
>> v1.0 hosts require that TD-fragments (portions of a TD that do not end
>> on a MPB boundary) not cross a TRB segment boundary. This constraint is
>> in addition to the constraint that a TRB may not specify a transfer that
>> crosses a 64K boundary. This enabling permits the driver to accept
>> scatterlists of nearly any geometry.  "Nearly" because there is one
>> unlikely remaining degenerate case of a driver submitting a transfer
>> that consumes all the TRBs in a segment before hitting an MBP boundary.
>> That case is trapped and the transfer is rejected.
>>
>> Given the multi-dimensional constraints of queuing TRBs from a
>> scattelist, this implementation does not attempt to pre-calculate the
>> number TRBs in a TD.  Instead it attempts a dry-run of enqueuing the
>> TRBs to the ring.
>
> Presumably not really a 'dry run', it is just willing to back up to
> the buffer than contained the previous boundary.

In the first pass we don't modify the enqueue pointer or any other
ring state (outside of ring expansion).  If we encounter an error in
the first pass we can simply return and no ring tracking state will
have been modified.  That's why I consider it a dry-run.

>> If it discovers a TD-fragment straddling a segment
>> boundary it backs up to the last MBP boundary, inserts a link-trb at
>> that boundary, and restarts enqueuing in the next segment.
>
> You have to be careful doing that.
> At least some controllers don't like following a LINK and then finding
> that they can't process the 'linked-to' TRB.
> This means that the ownership of the LINK TRB can't be changed until
> the ownership of the rest of the TRBs has been set (the controller could
> be active and process the LINK at the end of the previous transfer).
>
> This is why I ended up with a patch that moved the ownership setting
> out of the callers.

I believe I have this case covered, and the existing code was careful
to not link until there was a valid TRB in the next segement.

>> A side
>> effect of not pre-calculating the number of required TRBs is that the
>> ring is now expanded as the scatterlist is walked, rather than in
>> prepare_ring().
>>
>> To simplify the math and forgo the need to track (union xhci_trb *) and
>> (struct xhci_segment *) pointers, modulo-power-of-2 ring indexes are
>> used.  A small portion of the patch is adding infrastructure to convert
>> from a (struct xhci_ring_pointer *) to an integer index.
>>
>> Glossary of acronyms:
>> TRB: Transfer Request Buffer, 16-byte xhci-hardware scatterlist entry
>>
>> TD: Transfer Descriptor, the set of trbs that comprise a transfer
>>
>> TRB segment: A contiguous allocation of TRBs.  They are of size
>>   PAGE_SIZE in the xhci driver.  Each segment ends with a link TRB
>>   pointing to the next segment, but the link trb may appear at any TRB
>>   boundary in the segment.
>>
>> Ring: A linked list of segments.
>>
>> MBP: Max Burst Packet, is the minimum amount of data hardware expects to
>>   transfer before the end of a segment (assuming the TD spans a segment
>>   boundary).
>
> I think you can ignore MBP, and only align to 1k boundaries.
> However, I suspect that all that happens is the host controller splits
> the transfer as the ring end - so provided it is at a 1k boundary
> the target device can't tell the difference.
> Indeed the 'end of TRB list' flag could be set on the 1k boundary.
>
> 1k boundaries were enough to make the Ge card I had (I don't have
> them any more) work on the Intel Ivy bridge USB3.
>

That's great, but the spec says:

"If the TD Transfer Size is an even multiple of the MBP then all TD
Fragments shall define exact multiples of MBP data bytes. If not, then
the only last TD Fragment shall define less than MBP data (or the
Residue) bytes"

If you convince the spec author to add "...just kidding ignore that",
then I'll relax the implementation. :-)
--
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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux