USB4 Pink Sardine controller support
Needs ReviewPublic
Actions

Authored by obiwac_gmail.com on Mar 22 2025, 1:36 AM.

Details

Reviewers

mohammad_fbsdphab_thelightbird.com
imp
jhb
emaste

Summary

Add support for the AMD Pink Sardine controller (1022:1668/9) and fix issue where we weren't getting interrupts for more than the first command sent on the ring (which presumably was not an issue for the hardware Scott was testing on).

I also added a couple comments as notes-to-self which I'll come back to.

Test Plan

Testing on AMD Pink Sardine controller (1022:1668/9), which is host interface version 1.0.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 63046
Build 59930: arc lint + arc unit

Event Timeline

obiwac_gmail.com created this revision.Mar 22 2025, 1:36 AM

Herald added a subscriber: imp. · View Herald TranscriptMar 22 2025, 1:36 AM

obiwac_gmail.com requested review of this revision.Mar 22 2025, 1:36 AM

Harbormaster completed remote builds in B63046: Diff 152525.Mar 22 2025, 1:36 AM

obiwac_gmail.com added a parent revision: D49450: USB4 base.Mar 22 2025, 1:36 AM

obiwac_gmail.com mentioned this in D49452: USB4 host reset (& writing to router config space).Mar 22 2025, 1:45 AM

obiwac_gmail.com mentioned this in D49450: USB4 base.Mar 22 2025, 3:21 AM

obiwac_gmail.com added reviewers: imp, jhb, emaste.Mar 22 2025, 3:25 AM

imp added inline comments.Mar 25 2025, 3:11 AM

sys/dev/thunderbolt/nhi.c
1061	which standard?
1066	The question would be "does it work on all platforms" more than if it's strictly necessary on some. If this is right on all the we should so say. If there's some reason this isn't needed on all platforms that isn't some have it by default, we should document them if we can.
sys/dev/thunderbolt/tb_pcib.c
111	Will we need to add this for every bridge? Is there some way to look at something other than IDs to match this device?

obiwac_gmail.com added inline comments.Mar 25 2025, 3:20 PM

sys/dev/thunderbolt/nhi.c
1061	USB4 spec, should I specifiy this here?
1066	It will work on all platforms as we don't actually use the interrupt status register and this just clears them. Its just that on Pink Sardine this register needs to be cleared and I'm not sure if this is the case on others. I'll change this comment.

emaste added inline comments.Mar 25 2025, 5:22 PM

sys/dev/thunderbolt/nhi.c
1061	Yes IMO we should have a reference. Something like "see USB4 V2 12.6.3.4.1" upon use here, and a reference somewhere in this file with the full title (something like `Universal Serial Bus 4 (USB4) Specification, Version 2.0, September 2024`)

imp added inline comments.Mar 25 2025, 7:21 PM

sys/dev/thunderbolt/nhi.c
1061	Yea, have this at the top, if possible. We have some version skew in some drivers that started with, for example, nvme 1.2 and the section numbers have changed now that we're to 2.1. Having a URL at the top would be good too.