> 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.
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.
I'm not even sure apm(8) should bother reporting some sort of battery
status. 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?
> Index: dev/fdt/cwfg.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/fdt/cwfg.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 cwfg.c
> --- dev/fdt/cwfg.c 10 Jun 2020 17:51:21 -0000 1.1
> +++ dev/fdt/cwfg.c 20 Mar 2021 23:43:13 -0000
> @@ -32,12 +32,15 @@
> #include <sys/malloc.h>
> #include <sys/sensors.h>
>
> +#include <machine/apmvar.h>
> #include <machine/fdt.h>
>
> #include <dev/ofw/openfirm.h>
>
> #include <dev/i2c/i2cvar.h>
>
> +#include "apm.h"
> +
> #define VERSION_REG 0x00
> #define VCELL_HI_REG 0x02
> #define VCELL_HI_MASK 0x3f
> @@ -119,6 +122,18 @@ struct cfdriver cwfg_cd = {
> NULL, "cwfg", DV_DULL
> };
>
> +int cwfg_apminfo(struct apm_power_info *info);
> +#if NAPM > 0
> +struct apm_power_info cwfg_power;
> +
> +int
> +cwfg_apminfo(struct apm_power_info *info)
> +{
> + bcopy(&cwfg_power, info, sizeof(*info));
> + return 0;
> +}
> +#endif
> +
> int
> cwfg_match(struct device *parent, void *match, void *aux)
> {
> @@ -202,6 +217,12 @@ cwfg_attach(struct device *parent, struc
>
> sensor_task_register(sc, cwfg_update_sensors, 5);
>
> +#if NAPM > 0
> + /* make sure we have the apm state initialized before apm attaches */
> + cwfg_update_sensors(sc);
> + apm_setinfohook(cwfg_apminfo);
> +#endif
> +
> printf("\n");
> }
>
> @@ -324,6 +345,7 @@ cwfg_update_sensors(void *arg)
> uint32_t vcell, rtt, tmp;
> uint8_t val;
> int error, n;
> + u_char bat_percent;
>
> if ((error = cwfg_lock(sc)) != 0)
> return;
> @@ -348,6 +370,7 @@ cwfg_update_sensors(void *arg)
> if ((error = cwfg_read(sc, SOC_HI_REG, &val)) != 0)
> goto done;
> if (val != 0xff) {
> + bat_percent = val;
> sc->sc_sensor[CWFG_SENSOR_SOC].value = val * 1000;
> sc->sc_sensor[CWFG_SENSOR_SOC].flags &= ~SENSOR_FINVALID;
> }
> @@ -363,6 +386,14 @@ cwfg_update_sensors(void *arg)
> sc->sc_sensor[CWFG_SENSOR_RTT].value = rtt;
> sc->sc_sensor[CWFG_SENSOR_RTT].flags &= ~SENSOR_FINVALID;
> }
> +
> +#if NAPM > 0
> + cwfg_power.battery_state = bat_percent > sc->sc_alert_level ?
> + APM_BATT_HIGH : APM_BATT_LOW;
> + cwfg_power.ac_state = APM_AC_UNKNOWN;
> + cwfg_power.battery_life = bat_percent;
> + cwfg_power.minutes_left = sc->sc_sensor[CWFG_SENSOR_RTT].value;
> +#endif
>
> done:
> cwfg_unlock(sc);
>
>
