From owner-freebsd-acpi@FreeBSD.ORG Tue Nov 16 07:14:47 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 D161916A4D0 for ; Tue, 16 Nov 2004 07:14:47 +0000 (GMT) Received: from harmony.village.org (rover.village.org [168.103.84.182]) by mx1.FreeBSD.org (Postfix) with ESMTP id 1813F43D3F for ; Tue, 16 Nov 2004 07:14:47 +0000 (GMT) (envelope-from imp@bsdimp.com) Received: from localhost (harmony.village.org [10.0.0.6]) by harmony.village.org (8.13.1/8.13.1) with ESMTP id iAG7ERQe029782; Tue, 16 Nov 2004 00:14:27 -0700 (MST) (envelope-from imp@bsdimp.com) Date: Tue, 16 Nov 2004 00:14:46 -0700 (MST) Message-Id: <20041116.001446.66168349.imp@bsdimp.com> To: nate@root.org From: "M. Warner Losh" In-Reply-To: <4199A260.3020001@root.org> References: <20041115.231816.133541642.imp@bsdimp.com> <4199A260.3020001@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: acpi@FreeBSD.org Subject: Re: Minor improvement to acpiconf 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: , X-List-Received-Date: Tue, 16 Nov 2004 07:14:48 -0000 In message: <4199A260.3020001@root.org> Nate Lawson writes: : M. Warner Losh wrote: : > acpiconf -i 0 now prints more information about the battery from the bst: : > Rate of discharge : > Present Capacity : > Current Voltage : ------------------------------------------------- : > : > --- /dell/imp/FreeBSD/src/usr.sbin/acpi/acpiconf/acpiconf.c Wed Aug 18 16:14:43 2004 : > +++ ./acpiconf.c Mon Nov 15 23:12:50 2004 : > @@ -45,8 +45,8 @@ : > : > static int acpifd; : > : > -static int : > -acpi_init() : > +static void : > +acpi_init(void) : > { : : Why the change to void if it still returns 0? See other mail. There's no return there at all... : > acpifd = open(ACPIDEV, O_RDWR); : > if (acpifd == -1){ : > @@ -117,6 +117,17 @@ : > printf("Type:\t\t\t%s\n", battio.bif.type); : > printf("OEM info:\t\t%s\n", battio.bif.oeminfo); : > : > + if (ioctl(acpifd, ACPIIO_CMBAT_GET_BST, &battio) == -1) : > + err(EX_IOERR, "get battery info (%d) failed", num); : > + : > + if (battio.bst.state != ACPI_BATT_STAT_NOT_PRESENT) { : : Prefer positive logic. Most common path first is generally the logic I prefer... : > + printf("State:\t\t\tPresent\n"); : > + printf("Rate:\t\t\t%d mWh\n", battio.bst.rate); : > + printf("Cap:\t\t\t%d mWh\n", battio.bst.cap); : > + printf("Volt:\t\t\t%d mV\n", battio.bst.volt); : : I agree with these except for a slight misgiving about "cap". That : information is already exported via sysctl and if we have to export the : same thing different ways, I think the interface is not optimal. Capacity isnt' exported via a sysctl. 'life' is, but it doesn't export anything more than a percentage. : In general, I'd like to move away from acpi-specific ioctls. There : should be just one way of getting the battery info and it shouldn't : refer to the underlying method names (_BST and _BIF) like the current : ones do. Mike made a good case for eliminating the dev_t entirely since : there is never any IO for acpi, it's all control traffic. Sysctl seems : more appropriate for that than creating a device that will never see a : read, write, or other access other than ioctl(). But this is a : complaint about the current design and the half-ioctl, half-sysctl : implementation. The amount of information exported is certainly parsimonious at best. I was mostly interested in 'Rate' to see if the various things I was doing was having any effect on the amount of power being eaten from the batteries.... I'm not entirely sure I agree with a device needing read/write methods to be legit. Especially after I saw sysctl abused for the devinfo interface, which likely should have been read instead :-)... : You're not making it worse so go ahead and commit. I'm just hoping : someone will consider improving the interface in the future. True.... It should be one or the other... Warner