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