From owner-freebsd-acpi@FreeBSD.ORG Tue Nov 16 06:47:02 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 3383516A4CE for ; Tue, 16 Nov 2004 06:47:02 +0000 (GMT) Received: from ylpvm15.prodigy.net (ylpvm15-ext.prodigy.net [207.115.57.46]) by mx1.FreeBSD.org (Postfix) with ESMTP id D894543D1D for ; Tue, 16 Nov 2004 06:47:01 +0000 (GMT) (envelope-from nate@root.org) Received: from [10.0.5.50] (adsl-64-171-186-185.dsl.snfc21.pacbell.net [64.171.186.185])iAG6l7re003194; Tue, 16 Nov 2004 01:47:08 -0500 Message-ID: <4199A260.3020001@root.org> Date: Mon, 15 Nov 2004 22:46:56 -0800 From: Nate Lawson User-Agent: Mozilla Thunderbird 0.7.3 (X11/20040901) X-Accept-Language: en-us, en MIME-Version: 1.0 To: "M. Warner Losh" References: <20041115.231816.133541642.imp@bsdimp.com> In-Reply-To: <20041115.231816.133541642.imp@bsdimp.com> Content-Type: text/plain; charset=us-ascii; format=flowed 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 06:47:02 -0000 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? > 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. > + 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. 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. You're not making it worse so go ahead and commit. I'm just hoping someone will consider improving the interface in the future. -Nate