Hi, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes: >> Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes: >> > On Mon, 2 May 2016, Mathias Nyman wrote: >> > >> >> The current implemenentation restart the sent pattern for each entry in >> >> the sg list. The receiving end expects a continuous pattern, and test >> >> will fail unless scatterilst entries happen to be aligned with the >> >> pattern >> > >> > Ah. We didn't spot this earlier because for non-xHCI controllers, the >> > scatterlist entries _must_ be aligned with the maxpacket size and >> > therefore with the pattern. >> >> interesting. We actually found a similar issue with XHCI. scatterlist >> has to be aligned to wMaxPacketSize but only before a link TRB. Mathias >> has been working on a solution which involves memcpy()ing enough bytes >> to align to wMaxPacketSize before the link TRB (it's very infrequent as >> we have 256 TRBs per segment on XHCI), but if you know of a nicer way, >> we're all ears :-) > > You should be able to handle this without memcpy'ing anything. > > If the individual scatterlist entries are large enough, you can simply "if" being the keyword here :-) We check for that, yes :-) > end the ring segment early. Either put a link TRB before the physical > segment end, at the last point where the cumulative transfer size is > divisible by maxpacket, or else fill out from there to the end of the > segment with no-op TRBs. Common case: Each scatterlist entry is a no-op TRBs was tried before, but it didn't work well and patch was reverted. > multiple of 512 bytes and the maxpacket size is 1024. Then you either that's not common case for testusb. One of the test cases (see below) exercises exactly small sg entries: # try to trigger short OUT processing bugs echo "test 7a: $COUNT scatterlists, variable size/short entries" do_test -t 7 -v 579 BUFLEN=4096 echo "test 7b: $COUNT scatterlists, variable size/bigger entries" do_test -t 7 -v 41 BUFLEN=64 echo "test 7c: $COUNT scatterlists, variable size/micro entries" do_test -t 7 -v 63 (above is from tools/usb/hcd-tests.sh) > have to split the last entry in two or move it completely into the next > ring segment. right, this is easy enough to do and that we (well, Mathias) already have working :-) > This approach doesn't work quite as well if the scatterlist entries are > very small. For instance, if they are 8 bytes or smaller then you > might have to fill out the segment with 128 or more no-op TRBs, which > is not so good if the segment can hold only 256 TRBs. I suppose we > could simply rule this out by requiring SG transfers to have a minimum > entry size of 128 bytes, or something like that. that's not a good idea, IMO. HCD drivers should be robust enough in these situations. >> /me goes dig EHCI > > Not sure that will help. The same issue could arise there, if the > scatterlist entries were smaller than 512 bytes. The driver doesn't > handle this case properly, but it works out okay because the case never > comes up. see testusb :-) You did mention, on another thread, that you ran testusb. And, btw, years ago I used to run these tests on daily basis on MUSB. It also seems that Synopsys' XHCI (part of dwc3 when configured as dual-role) is immune to this link TRB alignment limitation because I was running hcd-tests.sh also on a daily basis on TI's AM437x SK and IDK boards (one as host, one as peripheral). -- balbi
Attachment:
signature.asc
Description: PGP signature
