Page MenuHomeFreeBSD

acpi_lid: correct Lid state after boot and resume
ClosedPublic

Authored by yuripv on Feb 17 2020, 2:47 AM.

Details

Summary
  1. Correct value after boot, triggered by a message on questions@. I have looked the file history, and see that it was implemented previously, and backed out as breaking pure GPE lid switches -- it should not be a problem now as the logic has changed, and we send notifications based only on current value, without comparing it to previous one.
  2. Correct value after resume, PR240881.
  3. Correct the sysctl description while here.
Test Plan

Check dev.acpi_lid.0.state value after:

  1. Both open and closed lid: normal boot, 1 open, 0 closed.
  2. Closed lid: suspend using acpiconf -s3, resume by opening the lid, check state (state = 1).
  3. Closed lid: suspend using acpiconf -s3, resume using a power button on dock (state = 0).
  4. Open lid: suspend using acpiconf -s3, resume using a power button (state = 1).

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

yuripv created this revision.Feb 17 2020, 2:47 AM
yuripv added inline comments.Feb 17 2020, 3:04 AM
sys/dev/acpica/acpi_lid.c
136 ↗(On Diff #68435)

Wonder if I should fix this description while here, it's somewhat misleading.

yuripv edited the summary of this revision. (Show Details)Feb 17 2020, 4:38 AM
hselasky added inline comments.
sys/dev/acpica/acpi_lid.c
136 ↗(On Diff #68435)

Please do.

yuripv updated this revision to Diff 68628.Feb 21 2020, 9:37 AM
yuripv edited the summary of this revision. (Show Details)

Correct sysctl description.

yuripv marked an inline comment as done.Feb 21 2020, 9:37 AM
yuripv added inline comments.
sys/dev/acpica/acpi_lid.c
136 ↗(On Diff #68435)

Looks better now?

yuripv marked an inline comment as done.Feb 21 2020, 9:38 AM
hselasky accepted this revision.Feb 21 2020, 9:48 AM
This revision is now accepted and ready to land.Feb 21 2020, 9:48 AM
cem added a subscriber: cem.Feb 21 2020, 4:31 PM
cem added inline comments.
head/sys/dev/acpica/acpi_lid.c
128

This is now inconsistent with acpi_lid_notify_status_changed(), which checks the return value of acpi_GetInteger(... "_LID" ...) with if (ACPI_FAILURE(status)).

(We should probably check the error return everywhere, or nowhere.)

hselasky added inline comments.Feb 21 2020, 5:43 PM
head/sys/dev/acpica/acpi_lid.c
128

I need to check in which cases we can expect an error.

For the time being, failure in these code paths is not critical and will likely block attach for the LID driver.