apm problem
Liam J. Foy
liamfoy at sepulcrum.org
Wed Jun 23 20:27:55 GMT 2004
On Fri, 23 Jun 2000 20:58:05 +0100
"Liam J. Foy" <liamfoy at sepulcrum.org> wrote:
> On Wed, 23 Jun 2004 12:40:40 -0700 (PDT)
> Nate Lawson <nate at root.org> wrote:
>
> > On Wed, 16 Jun 2004, M. Warner Losh wrote:
> > > As it relates to acpi, however, there is one bug. First in
> > > acpi_capm_get_info(), if we can't get the battery info, we do:
> > >
> > > if (acpi_battery_get_battinfo(-1, &batt)) {
> > > aip->ai_batt_stat = 0xff; /* unknown */
> > > aip->ai_batt_life = 0xff; /* unknown */
> > > aip->ai_batt_time = -1; /* unknown */
> > > - aip->ai_batteries = 0;
> > > } else {
> > >
> > > instead, this should be:
> > > if (acpi_battery_get_battinfo(-1, &batt)) {
> > > aip->ai_batt_stat = 0xff; /* unknown */
> > > aip->ai_batt_life = 0xff; /* unknown */
> > > aip->ai_batt_time = -1; /* unknown */
> > > + aip->ai_batteries = -1; /* Unknown */ [*]
> > > } else {
> > >
> > > [*] or 0xffffffff instead of -1. 0 is clearly wrong, since it means
> > > no batteries, not the number of batteries cannot be determined.
> >
> > I agree with this. I'd like to use ~0 instead of (u_int)-1. Up to you
> > though. Please commit.
> >
> > > Finally, apm(8) needs the following patch, imho. If drivers are
> > > producing results that produce bad things, they should be fixed, not
> > > apm(8). I don't think that anything does actually produce them.
> > >
> > > Index: apm.c
> > > ===================================================================
> > > RCS file: /cache/ncvs/src/usr.sbin/apm/apm.c,v
> > > retrieving revision 1.32
> > > diff -u -r1.32 apm.c
> > > --- apm.c 27 May 2004 19:23:27 -0000 1.32
> > > +++ apm.c 16 Jun 2004 19:45:52 -0000
> > > @@ -34,6 +34,8 @@
> > >
> > > #define APMDEV "/dev/apm"
> > >
> > > +#define APM_UNKNOWN 0xff /* Unknown in APM BIOS spec */
> > > +
> > > #define xh(a) (((a) & 0xff00) >> 8)
> > > #define xl(a) ((a) & 0xff)
> > > #define APMERR(a) xh(a)
> > > @@ -156,7 +158,7 @@
> > > printf("APM version: %d.%d\n", aip->ai_major, aip->ai_minor);
> > > printf("APM Management: %s\n", aip->ai_status ? "Enabled" : "Disabled");
> > > printf("AC Line status: ");
> > > - if (aip->ai_acline >= 255)
> > > + if (aip->ai_acline == APM_UNKNOWN)
> > > printf("unknown");
> > > else if (aip->ai_acline > 1)
> > > printf("invalid value (0x%x)", aip->ai_acline);
> > > @@ -164,7 +166,7 @@
> > > printf("%s", line_msg[aip->ai_acline]);
> > > printf("\n");
> > > printf("Battery status: ");
> > > - if (aip->ai_batt_stat >= 255)
> > > + if (aip->ai_batt_stat == APM_UNKNOWN)
> > > printf("unknown");
> > > else if (aip->ai_batt_stat > 3)
> > > printf("invalid value (0x%x)", aip->ai_batt_stat);
> > > @@ -172,7 +174,7 @@
> > > printf("%s", batt_msg[aip->ai_batt_stat]);
> > > printf("\n");
> > > printf("Remaining battery life: ");
> > > - if (aip->ai_batt_life >= 255)
> > > + if (aip->ai_batt_life == APM_UNKNOWN)
> > > printf("unknown\n");
> > > else if (aip->ai_batt_life <= 100)
> > > printf("%d%%\n", aip->ai_batt_life);
> > > @@ -194,7 +196,7 @@
> > > }
> > > if (aip->ai_infoversion >= 1) {
> > > printf("Number of batteries: ");
> > > - if (aip->ai_batteries >= 255)
> > > + if (aip->ai_batteries == (u_int) -1)
> > > printf("unknown\n");
> > > else {
> > > u_int i;
> > > @@ -208,12 +210,12 @@
> > > continue;
> > > printf("Battery %d:\n", i);
> > > printf("\tBattery status: ");
> > > - if (aps.ap_batt_flag <= 255 &&
> > > + if (aps.ap_batt_flag != APM_UNKNOWN &&
> > > (aps.ap_batt_flag & APM_BATT_NOT_PRESENT)) {
> > > printf("not present\n");
> > > continue;
> > > }
> > > - if (aps.ap_batt_stat >= 255)
> > > + if (aps.ap_batt_stat != APM_UNKNOWN)
> > > printf("unknown\n");
> > > else if (aps.ap_batt_stat > 3)
> > > printf("invalid value (0x%x)\n",
> > > @@ -222,7 +224,7 @@
> > > printf("%s\n",
> > > batt_msg[aps.ap_batt_stat]);
> > > printf("\tRemaining battery life: ");
> > > - if (aps.ap_batt_life >= 255)
> > > + if (aps.ap_batt_life == (u_int)-1)
> > > printf("unknown\n");
> > > else if (aps.ap_batt_life <= 100)
> > > printf("%d%%\n", aps.ap_batt_life);
> >
> > Please commit this patch after deciding whether you want to go with ~0 or
> > (u_int)-1.
>
> We decided to go with -1. The apm.c patch currently will not apply due to
> the re-structure of apm. The following patch will:
>
> http://liamfoy.kerneled.org/apm.diff
>
> After more digging, apm -l should return 255(stated in man page and acpi spec).
> The following patch will make it work:
>
> --- /usr/src/sys/dev/acpica/acpi_cmbat.c Tue Jun 22 16:40:35 2004
> +++ /hd2/acpi_cmbat.c Tue Jun 22 17:02:18 2004
> @@ -449,7 +449,7 @@
> static int bat_units = 0;
> static struct acpi_cmbat_softc **bat = NULL;
>
> - cap = min = -1;
> + cap = min = 255;
> batt_stat = ACPI_BATT_STAT_NOT_PRESENT;
> error = 0;
>
> @@ -545,7 +545,7 @@
>
> /* Battery life */
> if (valid_units == 0) {
> - cap = -1;
> + cap = 255;
> batt_stat = ACPI_BATT_STAT_NOT_PRESENT;
> } else {
> cap = total_cap / valid_units;
> @@ -649,7 +649,7 @@
> }
>
> if (!sc->present) {
> - battinfo->cap = -1;
> + battinfo->cap = 255;
> battinfo->min = -1;
> battinfo->state = ACPI_BATT_STAT_NOT_PRESENT;
> } else {
Do any of you guys have any opinions/comments on the
acpi_cmbat.c patch?
>
> I think all the patches need commiting.
>
> >
> > -Nate
> > _______________________________________________
> > freebsd-acpi at freebsd.org mailing list
> > http://lists.freebsd.org/mailman/listinfo/freebsd-acpi
> > To unsubscribe, send any mail to "freebsd-acpi-unsubscribe at freebsd.org"
>
>
> --
> -Liam J. Foy
> http://liamfoy.kerneled.org
> "Now I wish it would rain down on me"
> _______________________________________________
> freebsd-acpi at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-acpi
> To unsubscribe, send any mail to "freebsd-acpi-unsubscribe at freebsd.org"
--
-Liam J. Foy
http://liamfoy.kerneled.org
"Now I wish it would rain down on me"
More information about the freebsd-acpi
mailing list