Re: spec violation of xHCI?

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