On Sat, Sep 14, 2024 at 10:04:14AM +0800, Peter Chen wrote: > On 24-09-13 23:25:13, Xu Yang wrote: > > On Fri, Sep 13, 2024 at 05:53:44PM +0800, Peter Chen wrote: > > > On 24-09-13 15:11:33, Xu Yang wrote: > > > > On Fri, Sep 13, 2024 at 09:20:45AM +0800, Peter Chen wrote: > > > > > On 24-09-12 12:51:49, Xu Yang wrote: > > > > > > Currently, the deivice controller has below limitations: > > > > > > 1. can't generate short packet interrupt if IOC not set in dTD. So if one > > > > > > request span more than one dTDs and only the last dTD set IOC, the usb > > > > > > request will pending there if no more data comes. > > > > > > 2. the controller can't accurately deliver data to differtent usb requests > > > > > > in some cases due to short packet. For example: one usb request span 3 > > > > > > dTDs, then if the controller received a short packet the next packet > > > > > > will go to 2nd dTD of current request rather than the first dTD of next > > > > > > request. > > > > > > > > > > > > > > > > Are there any IP errata for it? > > > > > > > > No. It's decided by hw IP design. This old design may not suit current > > > > requirements. > > > > > > > > > > > > > > > To let the device controller work properly, one usb request should only > > > > > > correspond to one dTD. Then every dTD will set IOC. In theory, each dTD > > > > > > support up to 20KB data transfer if the offset is 0. Due to we cannot > > > > > > predetermine the offset, this will limit the usb request length to max > > > > > > 16KB. This should be fine since most of the user transfer data based on > > > > > > this size policy. > > > > > > > > > > > > Although these limitations found on OUT eps, we can put the request to IN > > > > > > eps too, this will benefit the following patches. > > > > > > > > > > Since IN endpoints have not found the problem, please limit the changes > > > > > only for OUT endpoints. > > > > > > > > This 1st patch is mainly used to serve the 2nd patch which may impact > > > > both IN and OUT eps. > > > ... > > > > Because it's hard to judge whether a request is > > > > suit for transfer if it spans more dTDs. So it's needed for both eps. > > > > > > Sorry, I do not understand you above words. First, you may know this > > > request is for IN or OUT, second, according to TD size and data buffer > > > address, you may know you use one or more dTDs. > > > > If req.num_sgs = 0, then we can know how many TDs need to transfer data. > > > > For example: > > req.buf = 0xA0001800 req.length = 40KB > > > > - TD1 addr:0xA0001800 size:18KB > > - TD2 addr:0xA0017000 size:20KB > > - TD3 addr:0xA002D000 size:2KB > > > > We basically won't meet issue for non-sg case. The only expection is that > > received short packet on TD1 (or TD2). Then the next data packet will go > > to TD2. But it should go to TD1 of next request. > > > > But if num_sgs > 0, we need to check validity of each sg entry due to above > > limitations. > > > > For example: > > req.num_sgs = 3 req.length = 40KB > > > > - sg1.addr = 0xA0001800 length = 18KB -> TD1 > > - sg2.addr = 0xA0016000 length = 20KB -> TD2 > > - sg3.addr = 0xA0028800 length = 2KB -> TD3 > > > > This request can be safty used to transfer data. But we can also meet > > previous short packet issue. > > > > req.num_sgs = 5 req.length = 10B + 20KB > > > > - sg1.addr = 0xA0001800 length = 10B -> TD1 > > - sg2.addr = 0xA0016000 length = 6KB -> TD2 > > - sg3.addr = 0xA0028800 length = 6KB -> TD3 > > - sg4.addr = 0xA003A000 length = 4KB -> TD3 > > - sg5.addr = 0xA004C000 length = 4KB -> TD3 > > > Sorry for reply late. > With your the 2nd patch, you could make end of sg1.addr is PAGE aligned, The 2nd patch is used to create a liner buffer for rest of the invalid sg entries, in this case, sg1 is the first invalid entry and sg1 - sg5 will be bounced together. So it's not what you think, only sg1 is bounced. > in that case, the sg1 and sg2 could be at the one TD. sg1 is at the > first dTD, and sg2 at the 2nd & 3rd dTD. If that could be done, the Yeah, I have considered this way. It could not be done. If only bounce sg1, the usb controller should only transfer 10 bytes to/from sg1.addr. But TD doesn't have a param to control it. As a result, data may be inconsistent since usb controller may transfer illegal data. Thanks, Xu Yang > host may not see short packet, anyway, you could confirm through > analyser. > > Peter > > > This request can't be used to transfer data since sg1 + sg2 can't > > form a data packet. The host will see a short packet (100 bytes). > > > > req.num_sgs = 5 req.length = 20KB + 10B > > > > - sg1.addr = 0xA0001800 length = 2KB -> TD1 > > - sg2.addr = 0xA0016400 length = 5KB -> TD2 > > - sg3.addr = 0xA0028800 length = 8KB -> TD3 > > - sg4.addr = 0xA003A800 length = 5KB -> TD4 > > - sg5.addr = 0xA004C200 length = 10B -> TD5 > > > > Compared to previous request, it need 5 TDs even though req.length > > are same. Most of the sg entries can't share same TD since their > > address is not page aligned. For high-speed isoc eps, sg1 + sg2 can't > > form a 3KB DATA2 + DATA1 + DATA0 data sequence too. > > > > Therefore, it's a bit complicated to validate request if num_sgs > 0, > > especially when req.length is larger than 16KB (1 TD size). > > > > When add such condition, each of the sg entry must follow below > > requirements: > > 1. the end address of the first sg buffer must be 4KB aligned. > > 2. the start and end address of the middle sg buffer must be 4KB aligned. > > 3. the start address of the last sg buffer must be 4KB aligned. > > > > So it will be more easy to validate the request. > > > > Hope this will help you understand the motivation of 1st patch. > > > > Thanks, > > Xu Yang
