In D54881#1290568, @olce wrote:

although, personally, I wouldn't move the SPDX tag; I've never understood why it's not systematically in the first line, where it is easy to find to know the license without reading the text; having it after the text but before other comments looks to me as an unnecessary and inconvenient complication).

This is not my idea, it's the recommendation of the style guide. (if it was my idea, no one would listen :P) -- https://docs.freebsd.org/en/articles/license-guide/

Also while I agree that we tend to have a style of not having underscores in driver names, and intelpmc exists on OpenBSD, I think this convention is really bad and looks a lot like cargo-culting (as I really do not see any significant advantage of it). Incidentally, we already have hwpstate_intel(4) and hwpstate_amd(4), and I'd wish all future drivers to rather use that kind of convention, which makes names easy to read.

For me, I just prefer we do whatever is most consistent. I like the traditional convention because its terse and consistent. I proposed several options, but I don't like using the exact name as the different linux driver, especially when it has an unnecessary and seemingly word at the end, and it's actively counterproductive in a google search because it doesn't use the same language as the datasheets, and produces entirely unrelated results except the confusingly similarly named linux driver.

@ziaee, the reason I put it on the second line was to keep it grepable. If you run head -n3 on driver files, you almost always land in the license header instead of anything useful, for example:
here and sys/fs/ext2fs/ext2_dir.h and many others.

Some files don't even have a license header at all, e.g. sys/arm64/arm64/bus_machdep.c.

PS: there are more files with the SPDX header , and my IDE show me warning because it don't follow the conventions most codebase had
coreboot
linux

The copyright line becomes problematic when there are multiple authors, since the license block keeps getting pushed further down.

The FreeBSD Project aims to produce a complete, BSD-licensed operating system allowing consumers of the system to produce derivative products without constraint or further license obligations.

A fork will inevitably change the structure once they start adding more contributors. Checking if someone altered the version, is not diffable.

I will update this diff and thermal once i get the naming is final.

In D54881#1290624, @guest-seuros wrote:

@ziaee, the reason I put it on the second line was to keep it grepable. If you run head -n3 on driver files, you almost always land in the license header instead of anything useful, for example:
here and sys/fs/ext2fs/ext2_dir.h and many others.

Some files don't even have a license header at all, e.g. sys/arm64/arm64/bus_machdep.c.

PS: there are more files with the SPDX header , and my IDE show me warning because it don't follow the conventions most codebase had
coreboot
linux

The copyright line becomes problematic when there are multiple authors, since the license block keeps getting pushed further down.

The FreeBSD Project aims to produce a complete, BSD-licensed operating system allowing consumers of the system to produce derivative products without constraint or further license obligations.

A fork will inevitably change the structure once they start adding more contributors. Checking if someone altered the version, is not diffable.

---

I will update this diff and thermal once i get the naming is final.

Again, I did not come up with this and I am not giving legal advice, I am just pointing out what is in style.9 and the license guide. All our docs request to use this style. Here are some links for your convenience.

https://man.freebsd.org/cgi/man.cgi?query=style&apropos=0&sektion=0&manpath=FreeBSD+16.0-CURRENT&format=html

https://docs.freebsd.org/en/articles/license-guide/

for LTR_IGNORE.. each bit maps to an IP block (SPT has 18: SPA, SPB, SATA, ect).
A clear bit means that agent is asserting LTR requirements that can block S0ix entry.
When slp_s0_residency stops incrementing, the register tells you which device is in charge.

Linux exposes the same data in debugfs via pmc_core_ltr_show() for exactly this reason. I will add a comment in the source with the SPT bit map.

PS: I have other generations's support already tested. I will submit additional diffs once this driver land. (extra code used there was removed from this diff)

In D54881#1294250, @guest-seuros wrote:

rename to intelpmc for consistency with hwpmc/intelspi

I'd like moving into the easier-to-read direction. Could you please rename it back to intel_pmc? That's also more consistent with acpi_spmc(4).

In D54881#1294254, @guest-seuros wrote:

for LTR_IGNORE.. each bit maps to an IP block (SPT has 18: SPA, SPB, SATA, ect).
A clear bit means that agent is asserting LTR requirements that can block S0ix entry.

From my reading of Linux code, not exactly. It rather means that the corresponding IP *can* block S0ix entry. Do you confirm?

When slp_s0_residency stops incrementing, the register tells you which device is in charge.

From my reading of Linux code, it doesn't do that. It's rather that, if you don't get into S0ix, you can try to ignore IPs that would prevent from doing that.

So it seems to me that exposing LTR_IGNORE doesn't really make sense without write support (you'll have to change the corresponding sysctl).

PS: I have other generations's support already tested. I will submit additional diffs once this driver land. (extra code used there was removed from this diff)

I'm eager to see that, but first of all, let's make sure this base part is completely correct and usable.

Sorry for the long delay before coming back to this. I'm now quite in a hurry to get this in, so that I can work on some other CPU/chipset generation. A comment a while ago indicated that you had more generations in stock, let's also examine that just after this one is done. I can help polishing and committing them if you do not have time to handle them.

A number of earlier inline comments, which for some reason were not added to the then current diff (probable pilot error on my side), have not been handled. I've added them back (except the mask one, where apparently I was wrong).

Please also see the new inline comments.

I do not really understand why you added "firmware access lock" detection. To my knowledge, after consulting the doc, there is no real lock mechanism for the PMC memory map, and no global firmware mechanism to read lock (there is the possibility for the BIOS to block reading 2 8-byte ranges, but this is designed to prevent reading password and other system sensitive info, and arguably it would be a bug if this used to block reading SLP_S0_RES or anything else exposed by the PMC). And there's nothing similar in Linux code either (contrary to what you stated in comment #1276647; AI hallucination?). Do you have any concrete evidence that this mechanism is really needed? Barring that, I'd just drop it.

Hopefully, after this new round, we will be there, or almost.

I'd also advise to disclose the AI used (for anything) with an "Assisted by:" tag.

You forgot to put this in the release notes, and omitted that I worked with you on this in the commit log. Both of these make my life harder. Please do not do this.

i wrote that the driver is still not complete there are many other generations to work on like D59596. < and this is not the last one.

A release note doesn't break the build. We can even add it in a separate commit.

Alternatively, i will refrain from writing any man page and you handle it. wdyt ?

In D54881#1368942, @seuros wrote:

i wrote that the driver is still not complete there are many other generations to work on like D59596. < and this is not the last one.

A release note doesn't break the build. We can even add it in a separate commit.

Honestly, in terms of things that can go wrong, breaking the build is not that big of a deal.
You rushed and made a mistake. I am pointing it out so that you can take a little more time.
Please add it as you said. Had you done as I asked it would be a lot less work for you and cleaner, and I would not have to be here arguing with you about it.

Alternatively, i will refrain from writing any man page and you handle it. wdyt ?

I think this is a bit rude actually. To me this is reading like you don't want to do it anymore because I asked you to fix a few details, and even showed you how, and you wanted to rush or something. You are writing perfectly fine doc, I just gave you some small editorial corrections so that the doc follows project standards. I think if you do not want to fix these small things, you definitely do not want to spend the much greater amount of time to teach me what your driver is doing so that I can write it for you. Which is also a giant waste of my time honestly. All of the corrections I told you on this review, I told you on previous reviews. And, I wrote them in style guides, and I told you where they were. So, this is all a waste of time, and your proposal sounds like you think I am wasting your time, so you want to waste even more time. It makes me a bit sad.

Please add it as you said. Had you done as I asked it would be a lot less work for you and cleaner, and I would not have to be here arguing with you about it.

I will add it in the other diff.

I think this is a bit rude actually. To me this is reading like you don't want to do it anymore because I asked you to fix a few details, and even showed you how, and you wanted to rush or something. You are writing perfectly fine doc, I just gave you some small editorial corrections so that the doc follows project standards. I think if you do not want to fix these small things, you definitely do not want to spend the much greater amount of time to teach me what your driver is doing so that I can write it for you. Which is also a giant waste of my time honestly. All of the corrections I told you on this review, I told you on previous reviews. And, I wrote them in style guides, and I told you where they were. So, this is all a waste of time, and your proposal sounds like you think I am wasting your time, so you want to waste even more time. It makes me a bit sad.

That was not a statement, some people like to do things on their own. I myself push changes on people PRs in various of the project i maintain, especially when i want them to land exactly like i want.
If you want to arc patch locally , edit it and update it, i'm totally fine with it.

the apple_bce driver shipped without a manpage, and you and ngie authored it. So, i though that maybe i'm stepping on your tasks.
And no, I will spend time to explain you the driver, even give you ssh access or do video call with you if needed so we ship documentation correctly.
My issue is that this driver is super old Jan 26 2026 , i revisited so many time. Also there are 12 generations of on top.. which mean that the narrative and man page is going to change multiple time. The end users are going to read the last version, not the history.
Also for me writing man pages is harder than the driver itself, not because can't write. But because i don't know when to stop explaining things.
So my flow is that i copy a similar man page and edit it.

So i apologies if that sounded like an attack..

Anyway, this driver works continue on D59596, and there will be more. I will resume it later.