From owner-freebsd-acpi@FreeBSD.ORG Wed Jun 23 20:27:55 2004 Return-Path: Delivered-To: freebsd-acpi@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 2645E16A4CE for ; Wed, 23 Jun 2004 20:27:55 +0000 (GMT) Received: from moutvdomng.kundenserver.de (moutvdom.kundenserver.de [212.227.126.249]) by mx1.FreeBSD.org (Postfix) with ESMTP id 8BEEB43D5C for ; Wed, 23 Jun 2004 20:27:54 +0000 (GMT) (envelope-from liamfoy@sepulcrum.org) Received: from [212.227.126.224] (helo=mrvdomng.kundenserver.de) by moutvdomng.kundenserver.de with esmtp (Exim 3.35 #1) id 1BdEL3-0000tZ-00; Wed, 23 Jun 2004 22:27:21 +0200 Received: from [217.43.129.99] (helo=liamfoy.ath.cx) by mrvdomng.kundenserver.de with esmtp (Exim 3.35 #1) id 1BdEL2-0004i4-00; Wed, 23 Jun 2004 22:27:20 +0200 From: "Liam J. Foy" To: "Liam J. Foy" Message-Id: <20000623212553.19a28a13.liamfoy@sepulcrum.org> In-Reply-To: <20000623205805.4c299e4e.liamfoy@sepulcrum.org> References: <20040616171408.0f88c928.liamfoy@sepulcrum.org> <20040616.135044.85075412.imp@bsdimp.com> <20040623123827.O86825@root.org> <20000623205805.4c299e4e.liamfoy@sepulcrum.org> X-Mailer: Sylpheed version 0.9.10 (GTK+ 1.2.10; i386-portbld-freebsd5.2) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit cc: acpi@freebsd.org Subject: Re: apm problem X-BeenThere: freebsd-acpi@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: ACPI and power management development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Date: Wed, 23 Jun 2004 20:27:55 -0000 X-Original-Date: Fri, 23 Jun 2000 21:25:53 +0100 X-List-Received-Date: Wed, 23 Jun 2004 20:27:55 -0000 On Fri, 23 Jun 2000 20:58:05 +0100 "Liam J. Foy" wrote: > On Wed, 23 Jun 2004 12:40:40 -0700 (PDT) > Nate Lawson 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@freebsd.org mailing list > > http://lists.freebsd.org/mailman/listinfo/freebsd-acpi > > To unsubscribe, send any mail to "freebsd-acpi-unsubscribe@freebsd.org" > > > -- > -Liam J. Foy > http://liamfoy.kerneled.org > "Now I wish it would rain down on me" > _______________________________________________ > freebsd-acpi@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-acpi > To unsubscribe, send any mail to "freebsd-acpi-unsubscribe@freebsd.org" -- -Liam J. Foy http://liamfoy.kerneled.org "Now I wish it would rain down on me"