From owner-cvs-src@FreeBSD.ORG Mon Jun 14 18:22:28 2004 Return-Path: Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 922B516A4CE; Mon, 14 Jun 2004 18:22:28 +0000 (GMT) Received: from harmony.village.org (rover.village.org [168.103.84.182]) by mx1.FreeBSD.org (Postfix) with ESMTP id EEA0B43D31; Mon, 14 Jun 2004 18:22:27 +0000 (GMT) (envelope-from imp@bsdimp.com) Received: from localhost (warner@rover2.village.org [10.0.0.1]) by harmony.village.org (8.12.11/8.12.11) with ESMTP id i5EIJEVG053789; Mon, 14 Jun 2004 12:19:14 -0600 (MDT) (envelope-from imp@bsdimp.com) Date: Mon, 14 Jun 2004 12:19:24 -0600 (MDT) Message-Id: <20040614.121924.35506950.imp@bsdimp.com> To: nate@root.org From: "M. Warner Losh" In-Reply-To: <20040614103813.I20782@root.org> References: <20040614165326.BF0CF16A4F1@hub.freebsd.org> <20040614103813.I20782@root.org> X-Mailer: Mew version 3.3 on Emacs 21.3 / Mule 5.0 (SAKAKI) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit cc: mux@FreeBSD.org cc: cvs-src@FreeBSD.org cc: src-committers@FreeBSD.org cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/usr.sbin/apm apm.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Jun 2004 18:22:28 -0000 In message: <20040614103813.I20782@root.org> Nate Lawson writes: : On Mon, 14 Jun 2004, Maxime Henrion wrote: : > mux 2004-06-14 16:53:20 UTC : > : > FreeBSD src repository : > : > Modified files: : > usr.sbin/apm apm.c : > Log: : > Factor out some duplicated code and fix some style(9) issues. : > : > Submitted by: Liam J. Foy : > : > Revision Changes Path : > 1.33 +61 -68 src/usr.sbin/apm/apm.c : : This looks fine. If you get a chance, it might be nice if you'd look into : this task: : : --- : Fix drivers and the apm compat interface -- Currently, the apm compat : interface expects byte values but the ABI used is a set of u_ints and an : int. Either the apm or acpi battery drivers (or both) are setting the : value to -1, which results in 0xffffffff being passed back as the current : state. Really, only 255 should be returned in this case. The apm userland : utility marks values >= 255 as "unknown" to work around this. But really : the underlying drivers should be fixed. This seems bogus based on what I know of the real APM interface. The APM interface doesn't have byte values. The underlying drivers are historically correct. : --- : Note that we can't change the ABI but fixing the sign-extension issue when : the kernel drivers fill out the structures would be helpful. apm has traditionally set the values to -1 for 'don't care': ... if (sc->bios.r.edx == 0xffff) /* Time is unknown */ app->ap_batt_time = -1; ... if (apm_bioscall()) { aip->ai_batteries = -1; /* Unknown */ aip->ai_capabilities = 0xff00; /* Unknown, with no bits set */ } else { ... Since apm in the kernel defines the apm interface, people emulating it should conform to that interface. apm(8) was bogusly broken in 1.32 to change the comparison against -1 to >= 255. Prior to that, it did the right thing because it checked ai_batteries against a cast -1. ap_batt_time already is an int, and is correct before and after the changes. We can change this to be 0xffffffff, of course, but apm(1) has to cope (and at one time did cope correctly) with this interface. If the apci emulation code isn't doing the right thing, it should be change to return a compatible value. 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 { should really be: + aip->ai_batteries = -1; + aip->ai_capabilities = 0xff00; to properly emulate the APM interface. Warner