Date: Wed, 11 Dec 2013 20:12:13 +0900 (JST) From: Kohji Okuno <okuno.kohji@jp.panasonic.com> To: hps@bitfrost.no Cc: freebsd-current@FreeBSD.org, okuno.kohji@jp.panasonic.com, freebsd-usb@freebsd.org Subject: Re: spec violation of xHCI? Message-ID: <20131211.201213.2095490882413924223.okuno.kohji@jp.panasonic.com> In-Reply-To: <52A844D6.7050203@bitfrost.no> References: <20131211.191212.1888965979017331164.okuno.kohji@jp.panasonic.com> <52A844D6.7050203@bitfrost.no>
next in thread | previous in thread | raw e-mail | index | archive | help
> 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
>>
>> src/sys/dev/usb/controller/xhci.c:
>> 1879 /* fill out link TRB */
>> 1880
>> 1881 if (td_next != NULL) {
>> 1882 /* link the current TD with the next one */
>> 1883 td->td_trb[x].qwTrb0 = htole64((uint64_t)td_next->td_self);
>> 1884 DPRINTF("LINK=0x%08llx\n", (long long)td_next->td_self);
>> 1885 } else {
>> 1886 /* this field will get updated later */
>> 1887 DPRINTF("NOLINK\n");
>> 1888 }
>> 1889
>> 1890 dword = XHCI_TRB_2_IRQ_SET(0);
>> 1891
>> 1892 td->td_trb[x].dwTrb2 = htole32(dword);
>> 1893
>> 1894 dword = XHCI_TRB_3_TYPE_SET(XHCI_TRB_TYPE_LINK) |
>> 1895 XHCI_TRB_3_CYCLE_BIT | XHCI_TRB_3_IOC_BIT;
>> 1896
>> 1897 td->td_trb[x].dwTrb3 = htole32(dword);
>> 1898
>> 1899 td->alt_next = td_alt_next;
>>
>> --
>> Best regards,
>> Kohji Okuno
>>
>> _______________________________________________
>> freebsd-usb@freebsd.org mailing list
>> http://lists.freebsd.org/mailman/listinfo/freebsd-usb
>> To unsubscribe, send any mail to "freebsd-usb-unsubscribe@freebsd.org"
>>
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20131211.201213.2095490882413924223.okuno.kohji>