On Sun, Mar 21, 2021 at 02:02:00PM +0100, Mark Kettenis wrote: > > Date: Sun, 21 Mar 2021 01:02:53 +0100 > > From: Klemens Nanni <k...@openbsd.org> > > > > apm(4/arm64) merely provides an all zero/unknown stub for those values, > > e.g. apm(8) output is useless. > > > > Hardware sensors however provide the information: > > > > $ sysctl hw.sensors > > hw.sensors.rktemp0.temp0=32.22 degC (CPU) > > hw.sensors.rktemp0.temp1=33.89 degC (GPU) > > hw.sensors.cwfg0.volt0=3.76 VDC (battery voltage) > > hw.sensors.cwfg0.raw0=259 (battery remaining minutes) > > hw.sensors.cwfg0.percent0=58.00% (battery percent) > > > > Diff below simply copies them over using apm_setinfohook() > > (I've looked at sys/arch/loongson/dev/kb3310.c which does the same): > > > > $ apm > > Battery state: high, 58% remaining, 259 minutes life estimate > > A/C adapter state: not known > > Performance adjustment mode: auto (408 MHz) > > > > Feedback? OK? > > This doesn't scale. What exactly does not scale? The way how various drivers hook into apm(4)? I'm not familiar enough (yet) with the framework(s) to judge their overall design and/or interop strategy off hand.
> The whole apm(4) interface is too closely tied to the original APM > BIOS model. Even with acpi(4) it is a bit of a square peg for a round > hole, for example on machines with more than one battery. Right, there is hardware which doesn't fit this model and our apm* is not exactly up to speed with those, but I still don't see actual issues (as per above). > I'm not even sure apm(8) should bother reporting some sort of battery > status. apm* showing battery information seems only naturally to me since that is the driver/daemon/tool one issues power commands with, often based on such information - think of `apmd -z 15': given that I want my box to safely suspend around 15% remaining battery, it seems only consequential that apm* is the same tool I query such information with. > But if it does, my suggestion would be to make it use the > sensors framework. That would need some work though such that drivers > can mark sensors as providing battery information. Maybe add a > SENSOR_FBATTERY flag? You mean apm(8) directly using sensor_find(9) or so to look for suitable sensors instead of relaying values throuh apm(4)? It sounds simpler in principal but that's just my naive guess; I'll take a curious look into whether/how this could work.