Page MenuHomeFreeBSD

xHCI Debug Capability support for USB 3.0
Needs ReviewPublic

Authored by bms on Jan 12 2017, 6:19 PM.

Details

Summary

This first revision introduces detection only. The second corrects detection based on Sec. 7.6.9 of Intel xHCI v1.1 spec

Test Plan

Primary development around NEC uPD720202, PCIe 1-lane and ExpressCard/34 adapters are available. Detection also returns positive results on AMD Hudson series FCH on ASUS A88XM-PLUS for DbC.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6754
Build 6969: arc lint + arc unit

Event Timeline

bms retitled this revision from to xHCI Debug Capability support for USB 3.0.
bms updated this object.
bms edited the test plan for this revision. (Show Details)
cem added inline comments.
sys/dev/usb/controller/xhci_dbc.c
29–34
If either <sys/types.h> or
<sys/param.h> is needed, include it before other include files.
(<sys/param.h> includes <sys/types.h>; do not include both.) The
remaining kernel headers should be sorted alphabetically.
74–77

Just a single space between type and name; don't initialize (or especially call functions) in declarations. temp, eecp, noff could share a line.

79

sc_dbc_off/noff are an unsigned integers; s/-1/UINT32_MAX/

96

sc_dbc_off is an unsigned integer; s/-1/UINT32_MAX/

102

UINT32_MAX again

sys/dev/usb/controller/xhci_pci.c
103–104

This could probably go in independent of the debug patch.

324–326

Any err here is discarded by the halt_controller() call.

Hi,

I have a style script which I want you to run the new code through before putting it in:

Run the changes through this utility:
/usr/src/tools/tools/indent_wrapper

Also make sure the XHCI driver in sys/boot/usb still builds.

--HPS

sys/dev/usb/controller/xhci_dbc.c
79

Use -1U here.

102

-1U ??

bms edited edge metadata.

Catch up with Sec. 7.6.9 of xHCI v1.1: DbC context sizes (DbC Info and Endpoint) are always 64 bytes, regardless of whether or not the host controller uses 32 byte contexts for ordinary endpoints (HCCPARAMS1).

This allows DbC capability to be detected on both of the AMD A88X Hudson FCH's USB 3.0 root hubs (which use discrete xhci HCD functions; 2 root hubs, 2 ports each).

Style changes from hselasky@.

cem requested changes to this revision.Jan 13 2017, 4:49 PM
cem added a reviewer: cem.
This revision now requires changes to proceed.Jan 13 2017, 4:49 PM
bms removed a reviewer: cem.
bms updated this object.
bms marked an inline comment as done.Jan 13 2017, 4:58 PM

I actually find spelling out the bits in hex is more useful here - it's a driver and it's accessing a register, so although UINT32_MAX might well represent the value (and be equivalent to it), stylistically spelling out those bits makes it crystal clear that we're acting on a register.

However, I defer to Hans' shorthand of -1U here; I'd rather be consistent with sys/dev/usb on style.

sys/dev/usb/controller/xhci_dbc.c
102

I actually find spelling out the bits in hex is more useful here - it's a driver and it's accessing a register, so although UINT32_MAX might well represent the value (and be equivalent to it), stylistically spelling out those bits makes it crystal clear that we're acting on a register.

However, I defer to Hans' shorthand of -1U here; I'd rather be consistent with sys/dev/usb on style.

bms marked an inline comment as done.Jan 13 2017, 5:00 PM

It's probably also worth pointing out that because a lot of things in USB are generally named in Hungarian notation, some concessions already get made to that in/around dev/usb/*.

Style changes cem@ and unsigned integer constants.

sys/dev/usb/controller/xhci_dbc.h
81

It is probably not clever to pack structures which are used by DMA. Try to use __aligned() instead. Because on ARM platforms everything packet will be accessed byte-by-byte.