Minor improvement to acpiconf

[M. Warner Losh]

In message: <4199A260.3020001 at root.org>
            Nate Lawson <nate at 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