apm(8): Remove support for APM BIOS
Needs ReviewPublic
Actions

Authored by jhb on Wed, Sep 23, 7:06 PM.

Details

Reviewers

Summary

Drop support for queries and commands that are not supported by ACPI's
/dev/apm interface. This includes dropping support for
enabling/disabling APM BIOS used by /etc/rc.d/apm.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 77224
Build 74107: arc lint + arc unit

Event Timeline

jhb created this revision.Wed, Sep 23, 7:06 PM

Herald added a subscriber: ziaee. · View Herald TranscriptWed, Sep 23, 7:06 PM

jhb requested review of this revision.Wed, Sep 23, 7:06 PM

jhb added a parent revision: D59940: acpi/apm: Store the ACPI softc in si_drv1.

Harbormaster completed remote builds in B77224: Diff 187532.Wed, Sep 23, 7:06 PM

jhb added a child revision: D59942: acpi: Remove support for /dev/apmctl and apmd(8) compatibility.Wed, Sep 23, 7:07 PM

Tested on my X1 Carbon which still outputs correct battery info.

imp accepted this revision.Wed, Sep 23, 9:09 PM

imp added inline comments.

usr.sbin/apm/apm.c
109	I'm glad to see you kept this sutff.... It's required for Emacs to display battery stuff right

This revision is now accepted and ready to land.Wed, Sep 23, 9:09 PM

ziaee added inline comments.Wed, Sep 23, 9:17 PM

usr.sbin/apm/apm.8
24–26	Can we have the introductory sentence explain what it is now, not what it was? I think this is more appropriate for HISTORY.
74	(or something)

ziaee added inline comments.Wed, Sep 23, 9:33 PM

usr.sbin/apm/apm.8
13	i386 is no more in 16.0 and this only works on amd64 now, right?
17	(or something)

jhb added inline comments.Thu, Sep 24, 1:54 PM

usr.sbin/apm/apm.8
13	Well, an i386 world can still be built in 16.0. Only the kernel is being removed entirely in 16.0, world is still going to be around.
17	Control still matters (you can still use this to initiate a suspend), but I did miss updating this and can figure out something.
24–26	Except that the name is "APM" which comes from "APM BIOS", unlike, say, `acpiconf` which also reports this information via ACPI ioctls. I'm not sure we can backronym it to "ACPI power management" either. This tool only exists as a compatibility shim. Possibly it should be deprecated entirely, but I personally use it frequently on my own laptop. I guess it might be fine to move that to history though.
usr.sbin/apm/apm.c
109	This is the thing I actually use all the time. It's debatable if outputting the APM version and enable/disable status is truly meaningful, but I worried about breaking tools that might be parsing this output.

ziaee added inline comments.Thu, Sep 24, 4:55 PM

usr.sbin/apm/apm.8
24–26	This tool only exists as a compatibility shim I did not know that. What is the "canon" thing I am supposed to do to check my battery and if my charger is working on acpi/amd64? I'm not sure we can backronym it to "ACPI power management" either. If the shoe fits... :)

jhb added inline comments.Fri, Sep 25, 2:14 PM

usr.sbin/apm/apm.8
24–26	The canonical tool for ACPI would be `acpiconf` for battery status, and probably a sysctl for A/C line status, but it's not packaged up as neatly as apm(8). If one changed apm(8) to use ACPI ioctls on /dev/acpi0 and trim some of the irrelevant bits you could probably make this an ACPI tool instead. That's a bit more work, but this commit is a step in that direction.

Still like the commit.
I'd be tempted to remove apm -Z and apm -z entirely, to be honest, since it's a poor fit with ACPI and zzz has replaced it.

usr.sbin/apm/apm.8
24–26	In reality, this program exists primarily to report state of the battery to emacs. :) Also, apm(8) doesn't manage any power state at all, it just reports it. You used to manage things with it, but that functionality has been gone for decades. Sure, it lets you suspend. But that's not power management... and the acpiconf utility provides better interface and zzz provides a more complete apm -z. The .Nm utility reports battery and power line state using the old APM interfaces. yes, that skips over the ability to suspend, but that's a footnote, at best, for how this utility is used today.