The sensor framework generally does not know where a sensor is. A sensor could reside on some device which has been plugged in, rather than be the primary sensor.
But the users of apm are only hoping for best effort. meaning "some information", not "perfect information". Many PC BIOS have lied in these fields before also, yet all the apm command users survived. So which is better: (1) try to emit some information for the people who quicky-use the apm interface (2) change apm to not print those lines on architectures where we are unsure. I think (1) is acceptable for a tool which has never promised perfect accuracy. I suspect hard-wiring this to one driver is going to be better than scanning the sensor list and heuristically determining which specific sensors to look at, because the only good selector now is strcmp(sensor->desc, "battery remaining minutes") yuck. Mark Kettenis <mark.kette...@xs4all.nl> wrote: > > Date: Sun, 21 Mar 2021 17:22:05 +0100 > > From: Klemens Nanni <k...@openbsd.org> > > > > 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. > > Since there is no firmware abstraction like on i386 (and to some > extent amd64) there will be many drivers that provide battery > informarion on armv7/arm64. > > > > 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. > > apmd(8) and apm(8) are not the same tool though. Having a deamon to > run scripts on certain power-state changes makes some sense, but I'd > argue that sensorsd(8) could be a reasonable place to implement this > as well. > > The need to run apmd(8) in order to suspend/hibernate is a bit of an > historical wart. And we already removed powermanagement state support > from apmd(8). These days, users should probably just use > /etc/sysctl.conf to set the powermanagement state instead of starting > apmd(8) with the appropriate -A, -H or -L option. > > > > 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)? > > That's a possibility. But having apm(4) do that might be a useful > middle ground as it would mean you don't have to change the userland > tools just yet. My main concern with your diff is having to add APM > hooks in all the drivers... > > > It sounds simpler in principal but that's just my naive guess; > > I'll take a curious look into whether/how this could work. >
