Page MenuHomeFreeBSD

pci: Fix dependency on ACPICA for non-ACPI builds
ClosedPublic

Authored by obiwac on Aug 8 2025, 1:34 PM.
Tags
None
Referenced Files
F132447134: D51823.diff
Fri, Oct 17, 12:45 AM
Unknown Object (File)
Sat, Oct 11, 9:20 AM
Unknown Object (File)
Sat, Oct 11, 7:01 AM
Unknown Object (File)
Fri, Oct 10, 11:44 PM
Unknown Object (File)
Fri, Oct 10, 11:44 PM
Unknown Object (File)
Fri, Oct 10, 11:44 PM
Unknown Object (File)
Fri, Oct 10, 5:21 PM
Unknown Object (File)
Thu, Oct 9, 10:32 PM

Details

Summary

Commit 84bbfc32a3f4 introduced a dependency on ACPICA for non-ACPI builds. This fixes that.

While here, print "D3hot" for D3hot in ACPI code instead of just "D3", as it was unclear whether that referred to D3hot or D3cold and was inconsistent with the PCI_POWERSTATE_D3 and ACPI_D_STATE_D3 defines.

Sponsored by: The FreeBSD Foundation.
Fixes: 84bbfc32a3f4 ("acpi_powerres: D3cold support")

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

obiwac requested review of this revision.Aug 8 2025, 1:34 PM
sys/dev/pci/pcivar.h
526

PCI_POWERSTATE_D3 is PCI_POWERSTATE_D3_COLD but we print PCI_POWERSTATE_D3_HOT as "D3"?

Print D3hot instead of just D3.

obiwac added inline comments.
sys/dev/pci/pcivar.h
526

Yeah I believe we should print D3hot. Let me just double-check the ACPICA change because this was a while ago for me.

If this is wrong then I need to change this in acpi_d_state_to_str too.

A better commit message would be:

pci: Fix dependency on ACPI

While here, print "D3hot" for D3hot in ACPI code instead of just "D3".

Fixes:		84bbfc32a3f47bd2e32741247afe516cbeff7789
obiwac retitled this revision from pci: Fix dependency on ACPI to pci: Fix dependency on ACPICA for non-ACPI builds.Aug 8 2025, 6:25 PM
obiwac edited the summary of this revision. (Show Details)

thanks, will use your commit message on commit :)

I'm guessing I can shorten the hash and add the commit title in "Fixes:"?

I'm guessing I can shorten the hash and add the commit title in "Fixes:"?

I usually just use the first twelve characters of the hash and no commit title. If you include the title, it has to be ("in parentheses and double quotes").

Correctly-formatted commit message would be:

Commit 84bbfc32a3f4 ("acpi_powerres: D3cold support") introduced a
dependency on ACPICA for non-ACPI builds. This fixes that.

Uses the opportunity to print "D3hot" for D3hot in ACPI code instead of
just "D3".

Fixes:		84bbfc32a3f4 ("acpi_powerres: D3cold support")
Sponsored by:	The FreeBSD Foundation

But also this doesn't fully explain *why* we should print D3hot rather than D3, namely that it was inconsistent in whether D3 meant hot or cold when unqualified. Please explain that in your own words, as the commit message should be the "why", not just the "what" (because, after all, the "what" is already there in the form of the diff).

In D51823#1184158, @des wrote:

I'm guessing I can shorten the hash and add the commit title in "Fixes:"?

I usually just use the first twelve characters of the hash and no commit title. If you include the title, it has to be ("in parentheses and double quotes").

Please always use the subject for a given hash at least once in the commit message. It's fine to elide in the body if Fixes: mentions it, for example, but not including the subject at all can be a problem for short hash collisions (maybe 12 is fine, but maybe it isn't), and also makes it easier to cross-reference things if we ever have to change VCS again (c.f. how annoying it is to have to find the corresponding git commit for an SVN revision mentioned in a commit message). It's also just more human-friendly; I don't remember commit hashes, but I might remember a commit from its subject.

obiwac edited the summary of this revision. (Show Details)

Thanks for your comments Jessica. Is the description as it stands now to your liking? I will wrap at 72 columns and format the metadata correctly on commit.

That looks a lot better now, thanks.

This revision is now accepted and ready to land.Aug 8 2025, 8:10 PM

Please commit this change: it not being fixed in resulting in deterministic kernel build failures on powerpc*, risc64, etc.
CC: @cperciva