Date: Mon, 15 Nov 2004 22:46:56 -0800 From: Nate Lawson <nate@root.org> To: "M. Warner Losh" <imp@bsdimp.com> Cc: acpi@FreeBSD.org Subject: Re: Minor improvement to acpiconf Message-ID: <4199A260.3020001@root.org> In-Reply-To: <20041115.231816.133541642.imp@bsdimp.com> References: <20041115.231816.133541642.imp@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4199A260.3020001>