nit: I think a blank first line is no longer style(9). Ditto elsewhere.

Ditto later diff -- it's been a while, but I think the whitespace is excessive for style(9).

These constants work out to a full second of waiting. Do we really need that long?

Do we need to unmap smu_space in this failure path?

Yeah, IIRC we first said the blank line was not necessary, but style(9) now says it should not be present, and also should be removed if there's significant changes to a file.

Mirroring the Linux constants for these here, but I don't know what bounds are reasonable in practice (SMU documentation is sparse). Probably you're right and 1 sec is excessive.

I would add a header like amdsmu_reg.h and put hw-defined constants there.

I do not understand this (I generally do not understand newbus, but this case might be even more special).

You are looking for pci device, and then you add an synthetic child. Why not attach the driver directly to the (pseudo-)pci device?

Not too useful comment(s)

I find it helpful to wrap the bus_space_read/write into driver-specific helpers. You might look at sys/x86/iommu/dmar.h and amdiommu.h for examples.

Esp. this one

The PCI device we look for is the root for other drivers such as amdsmn and amdtemp (and is already claimed by hostb). amdsmn and amdtemp do the same thing.

Mainly added this because I don't think it's obvious that we're waiting for a result because this is how to wait for the SMU to be ready to receive commands.

Perhaps CPUID_* is not the clearest name and I should call this PCI_DEVICE_ID_AMD_*_ROOT like in sys/dev/amdsmn/amdsmn.c.

I think so yes

These constants come from the Linux driver. Fairly sure they are just choices by the driver, I doubt the SMU would ever take 1 full second to respond. But perhaps its best just to keep in line with what AMD are doing on Linux.

Move them to _reg.h

If they are not something describing hardware, but providing a parameters for the driver, the _reg.h is not the right place for them. Move to amdsmu.h.