Re: spec violation of xHCI?

From: Hans Petter Selasky <hps_at_bitfrost.no>
Date: Wed, 11 Dec 2013 13:50:50 +0100
> On 12/11/13 13:44, Hans Petter Selasky wrote:
>> On 12/11/13 12:12, Kohji Okuno wrote:
>>>> On 12/11/13 11:12, Kohji Okuno wrote:
>>>>> Hi,
>>>>>
>>>>> I think the xHCI host controller driver has a spec violation.
>>>>>
>>>>> Could you refer to
>>>>> ``Table 126: Offset 0Ch – Link TRB Field Definitions''
>>>>> in  xHCI_Specification_for_USB.pdf(Revision 1.0)?
>>>>>
>>>>> The following is an excerpt about the CHAIN ​​BIT.
>>>>>
>>>>>     Chain bit (CH). Set to ‘1’ by software to associate this TRB with
>>>>>     the next TRB on the Ring. A Transfer Descriptor (TD) is defined as
>>>>>     one or more TRBs. The Chain bit is used to identify the TRBs that
>>>>>     comprise a TD. Refer to section 4.11.7 for more information on Link
>>>>>     TRB placement within a TD. On a Command Ring this bit is ignored by
>>>>>     the xHC.
>>>>>
>>>>>
>>>>> I think that we should add XHCI_TRB_3_CHAIN_BIT to line 1895.
>>>>> How do you think?
>>>>>
>>>>
>>>> Hi Kohji,
>>>>
>>>> The double word written at line 1895 does not set the "chain bit"
>>>> because this
>>>> is the end of a transfer descriptor, TD. I'm unsure how hardware
>>>> interprets
>>>> this bit, if setting the bit on the previous TRB makes the next one
>>>> connect to
>>>> the previous one, or the other way around. If setting this bit makes
>>>> the TRB
>>>> connect to the previous one, you are correct. Else the current code is
>>>> correct.
>>>
>>> Hi, HPS,
>>>
>>> Thank you for your comment.
>>>
>>> I think that this (line 1895) is not the end of a transfer descriptor.
>>> When the device driver needs a Zero Length Packet, this is not the
>>> end. And, If xfer has nframes, this is not the end, too.
>>>
>>> Regards,
>>>   Kohji Okuno
>>>
>>
>> Hi Kohji,
>>
>> Yes, you are right that if nframes is greater than one, and/or if a ZLP
>> needs to be sent this is not the end of the USB transfers. Are we sure
>> that if the XHCI_TRB_3_CHAIN_BIT is added at line 1895, that we will
>> receive a completion TRB-event for each of the nframes, or will the
>> chain bit result in loss of TRB completion events?
>>
>> Does setting this bit have any impact on performance?
>>
>> Thank you!
>>
>> --HPS
> 
> Some more thoughts:
> 
> The code in question handle all four USB transfer types. Are you saying that
> the CHAIN bit should be set for isochronous transfers too, so all the packets
> send in all the intervals are chained together? Or is this only for BULK
> traffic you want to add the CHAIN bit?
> 
> Thank you!
> 
> --HPS

Hi HPS,

All link trbs which are not the end need CHAIN bit, I think.
But, this is errata in xHCI ver 0.95. So, linux has quirk for chain
bit. Could you check linux codes?

Regards,
 Kohji Okuno