Can confirm this works on my machine too! Although sometimes when I press the button fast enough, it doesn't register. It always does when I hold it down for more than a fraction of a second though. Assuming this will be fixed once we're able to pass GPIO interrupts to iichid.

i confirm I saw this work on his machine

If you commit the rest of the ICM code you got from hselasky, I'll work on making it function on my TB3 devices.

If you remove all of the TB3 PCI IDs from the NHI and PCIB drivers, the driver is basically useless for TB3 since it won't probe and attach to anything but generic USB4-capable controllers like the "Pink Sardine" ones (as-is).

I have not looked into this yet, but I think this revision causes S0i3 entry to fail for some reason (even though power consumers should still be switched to D3). So changing status to planned changes.

logic looks sound!

It would be nice to describe somewhere how the new states map to ACPI sleep states.

thanks for this!

overall looks good

i'm sorry, I must resign from this revision. I wish you the best of luck in getting this accepted into posix!

In D55653#1273804, @kib wrote:

Why is this needed? Size is end-start.
Note that there are huge amount of scripts used by people who parse procstat vm output based on the text format, with the fixed assignment of the columns. The output layout for text is kind of ABI, due to this.

If you insist that it is must be added, it probably fine for structured output like json/xml, but even less useful because its parsing already requires some full language that can do end-start. For the text columnar format, the new column perhaps requires a new option to enable it.

In D52861#1273704, @scottl wrote:

Maybe I’m confused, I was referring to tb_pcib.c.

ah, right, sorry, I didn't see there were other reviews

formatting

In D48721#1273445, @olce wrote:

I have a question related to amdsmu but not the change here: Given the logic in amdsmu_cmd(), I concur that the SMU will always return again the same result when re-reading SMU_REG_RESPONSE without any intervening command (except perhaps on a previous timeout)?

In D55562#1271617, @ngie wrote:

Hmm... atopcase(4) still panicked with a similar issue (once I figured out that I needed to add ig4 to my KERNCONF). This may have also occurred because my Mac had low battery and the hardware sends out events at the hardware level to OSes to suspend the host (there's always a chance that this is a bug with atopcase(4)).

I think we should consider renaming these constants, perhaps doing something as radical as POWER_SSTATE_TRANSITION_* => POWER_*, or perhaps keeping STATE (instead of SSTATE), before they are made publicly available (after which, we will have to provide them (almost) indefinitely).

Remove comment about overriding next D-state, as it was confusing and was anyway explained elsewhere.

rebase

In D52861#1273277, @scottl wrote:

Either return the PCI IDs in the tb_pcib driver, or remove the entire driver. With this review, the entire driver is dead code.

In D55635#1273245, @scottl wrote:

What I meant by "kinda recommending" is that if you're going to support TB3 PCI IDs, then bring back the ICM code until you've written an HCM to replace it. Otherwise you're relying on people to find the right option in their BIOS to disable the ICM challenge-response authorization exchange with devices.

In D52861#1273246, @scottl wrote:

I must decline being a reviewer or otherwise being involved in the freebsd code. Happy to discuss publicly available information regarding the tech in general.

I think that if you dig deep into the linux nhi and thunderbolt code you'll see some TB3 vs USB4 differences. I know what I know based on my time at Intel and my access to the TB3 specs at the time, obviously I can't share those with you.

looks good, thanks for finding/fixing this!

If I had access to the specs, I would like to get ICM/HCM working (which includes investing time in making that functionality in TB3 work). It's mildly annoying how I need to have separate RJ-45 dongles for communication between TB nodes when (in reality) I could just get 2 hosts to talk directly with each other over another IP-like protocol using a TB3/4 capable cable.

I assume that TR has an ICM, but if you've removed the ICM code from freebsd then that's moot. I would kinda recommend bringing the ICM code back and supporting TR and ICL controllers with it until you've written a comprehensive HCM. Just stay away from AR host controllers.

The relationship between TB3 and USB4 isn't as simple as you suggest. There are optional vs mandatory feature considerations, there are differences in the bus properties and device config registers, there are differences in the necessary behavior of the Connection Manager, not to mention that an HCM is mandatory for USB4 but not for TB3, and there are even differences in the NHI programming interface. A better comparison is TB4 and USB4, their programming interfaces and functional attributes are more closely related, with TB4 being a superset of USB4.

completely remove last remnants of ICM support, and assume we're always just using HCM

One thing I found interesting in this exercise is that the PCI IDs are flip flopped between FreeBSD and Linux: the NHI PCI IDs on FreeBSD are the TB PCIB PCI IDs and vice versa.

I'm not entirely sure if titan ridge supports having a HCM (USB4) or if it is ICM-only (linux uses its ICM it would seem), and short of checking if the progif is USB4 (0x40) or just trying it out, I don't know how to be sure a device supports HCM. In any case, D52861 will end up removing support for non-generic USB4 NHIs.

style

Check trans bounds while still unsigned

casting to enum power_sstate_transition can make uint32_t signed

• move FWRITE check outside switch()
• check trans is in enum power_sstate_transition

• uint32_t for PIOTRANSITION
• check writable for PIOTRANSITION
• style

I thought about making this a sysctl and am still open to that, but having this as a device node does give us room to expand the interface in the future. No specific plans, but we may e.g. want to be able to report state change events.

Rebase

Rebase

haven't done a hugely in-depth review yet. Overall looks pretty good!

update names

update HISTORY & names

update history section

remove LIBRARY

remove LIBRARY

use posix 2024 as standard

use posix 2024 as standard