In D49452#1295289, @adrian wrote:

what do i need to check in dmesg / pciconf to see if i have something that can test this?

this is PCI_DEVICE_ID_AMD_1AH_M60H_ROOT in the linux driver, which selects the following case for choosing the IP blocks array:

rebase

Return error if we get unknown state from acpi_stype_to_sstate

Was committed in d8a8c890a1a3a003dbd12ec0235304db7fbe9e6e, I just messed up the "Differential Revision" tag.

Move unit check to probe and not attach

The "best" fix would be to move the storage of dsms_supported from struct dsm_set to some field in struct acpi_spmc_softc. That could be the occasion of grouping all struct dsm_set objects into a single array, and make it a constant (they would then form a cleanly separated specification for the driver). But since supporting multiple instances at this stage did not appear to be worth the trouble, I didn't require such a change.

• Fail attach if unit > 0
• Change device description

which leaves a lot to be desired

Proper cleanup when mcnt = 0

Don't honour SPMC min D-state. Just read _S3D as if we were entering S3.

even though power consumers should still be switched to D3

i think your stack is the wrong way around, right?

In D56026#1281776, @wulf wrote:

Radio button can not be correctly handled with hidmap. MS does strange things here assuming every event in report as a keypress. E.g. ASUS TUF always reported radio key as depressed that brokes Linux but works on Windows

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.

don't have access to hardware to test this in a while, but 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 revisions

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.