USB4 initial work on suspend routine

[obiwac]

Summary

Initial work on a suspend routine for USB4 on v1.0 host interfaces. At the moment I'm not quite concerned about resume, as it would already be quite nice to be able to suspend USB4 routers as this is a prerequisite for S0i3 on AMD.

This will also work on v2.0, but we might want to handle this differently in the future for these hosts.

Test Plan

Tested on AMD Pink Sardine controller (1022:1668/9), which is host interface version 1.0. The SLPR (sleep ready) bit is being set by the hardware.

[imp]

sys/dev/thunderbolt/router.c

378

Why don't we want these events?

393

Do we want to be verbose about being in this state to see if we need to implement this or if we can punt? Or is it a 'gotta do eventually'?

395

50ms is a long time to wait here...

407

And we wait 50ms multiple times. 100 if I'm reading the code right 5 seconds sleeping in a suspend path seems potentially unwise if we have to do this for multiple bridges. Is there a way to avoid that?

418

This will prevent the whole system from shutting down.... how likely is this timeout?

[obiwac]

sys/dev/thunderbolt/router.c

378

Sorry, I should make this clearer.

The initial work on USB4 is meant to get suspend working well enough for the firmware on AMD systems to enter S0i3, since these are handed over to the OS in a powered-on state. Currently, I want to make sure we don't wake up for any reasons as I'm not handling resume and am not intending to handle resume in the near future.

393

I think leaving as is fine; the spec just mentions that, while we can wait for ROP_CMPLT on v2, polling as on v1 routers is also fine. So more of a "gotta do eventually".

395

This comes from tSetSR and is the time between setting SLP and SLPR being set by the router.

407

Actually, I think tSetSR is the maximum time the router has to set SLPR after SLP. I will double check this tomorrow, because this means we can immediately poll once every maybe 5 ms a maximum of 10 times.

418

I'm guessing this is unlikeley but if this is the case then I'll downgrade this to a simple warning I guess.

[obiwac]

Don't touch router defines; this should be done in D49452.

[obiwac]

Rebase

[ngie]

The changes seem reasonable. My general concerns with the change are just related to documentation, follow up and potential future collaboration (especially if I start helping out in this space in the next few months).

sys/dev/thunderbolt/nhi.c

400

I assume that the only reason why we don't have resume code after this change is that we don't actually properly initialize NHI today. Is that correct? If so, this warrants another TODO and follow up issue.

sys/dev/thunderbolt/router.c

378

Sorry, I should make this clearer.

The initial work on USB4 is meant to get suspend working well enough for the firmware on AMD systems to enter S0i3, since these are handed over to the OS in a powered-on state. Currently, I want to make sure we don't wake up for any reasons as I'm not handling resume and am not intending to handle resume in the near future.

I seems like it deserves documentation in the manpage and comment, as well as a tracking issue: this helps avoid the scenario where a random user will file a bug saying the feature doesn't work ("it's already documented clearly here") and helps encourage those who have the knowledge/capability to collaborate with enhancements/fixes.

403

Please use NHI_SLPR_WAIT_US instead of 50 ms here (using another constant to convert from microseconds to milliseconds is reasonable because the constant is expressed in microseconds).

416–417

Please keep the string constant/format strings intact so it's greppable by humans.

424–426

Please file a follow up issue here.

[emaste]

sys/dev/thunderbolt/nhi.c

400

Fair point, either way there should be a comment here explaining why nhi_resume does nothing.

sys/dev/thunderbolt/router.c

416–417

As an aside, we should note this in style(9) if we don't already