Hajimu UMEMOTO wrote:

Hi,

On Mon, 14 Nov 2005 12:40:36 -0500
Pierre-Luc Drouin <[EMAIL PROTECTED]> said:

pldrouin> Yep, smart battery is definately the problem. The performance of my pldrouin> laptop is back to normal when I remove the xfce4-battery-plugin. pldrouin> acpiconf -i loop reproduces the problem for me too. So it looks like pldrouin> there is something wrong in smart battery.

The cmbat has similar issue on some laptops.  So, acpi_cmbat.c uses
cache for retrieval to reduce its influence, and its expiration
time is set by hw.acpi.battery.info_expire.
However, acpi_smbat.c doesn't use cache.  So, I made a patch.  Since I
don't have a laptop which has smbat, I cannot test it by myself.
Please test it and let me know the result.

Index: sys/dev/acpica/acpi_smbat.c
diff -u -p sys/dev/acpica/acpi_smbat.c.orig sys/dev/acpica/acpi_smbat.c
--- sys/dev/acpica/acpi_smbat.c.orig    Sun Nov  6 08:55:56 2005
+++ sys/dev/acpica/acpi_smbat.c Tue Nov 15 16:41:00 2005
@@ -44,11 +44,18 @@ __FBSDID("$FreeBSD: src/sys/dev/acpica/a
struct acpi_smbat_softc {
        uint8_t         sb_base_addr;
        device_t        ec_dev;
+
+       struct acpi_bif bif;
+       struct acpi_bst bst;
+       struct timespec bif_lastupdated;
+       struct timespec bst_lastupdated;
};

static int      acpi_smbat_probe(device_t dev);
static int      acpi_smbat_attach(device_t dev);
static int      acpi_smbat_shutdown(device_t dev);
+static int     acpi_smbat_info_expired(struct timespec *lastupdated);
+static void    acpi_smbat_info_updated(struct timespec *lastupdated);
static int      acpi_smbat_get_bif(device_t dev, struct acpi_bif *bif);
static int      acpi_smbat_get_bst(device_t dev, struct acpi_bst *bst);

@@ -114,6 +121,9 @@ acpi_smbat_attach(device_t dev)
                return (ENXIO);
        }

+       timespecclear(&sc->bif_lastupdated);
+       timespecclear(&sc->bst_lastupdated);
+
        if (acpi_battery_register(dev) != 0) {
                device_printf(dev, "cannot register battery\n");
                return (ENXIO);
@@ -132,6 +142,34 @@ acpi_smbat_shutdown(device_t dev)
}

static int
+acpi_smbat_info_expired(struct timespec *lastupdated)
+{
+       struct timespec curtime;
+
+       ACPI_SERIAL_ASSERT(smbat);
+
+       if (lastupdated == NULL)
+               return (TRUE);
+       if (!timespecisset(lastupdated))
+               return (TRUE);
+
+       getnanotime(&curtime);
+       timespecsub(&curtime, lastupdated);
+       return (curtime.tv_sec < 0 ||
+           curtime.tv_sec > acpi_battery_get_info_expire());
+}
+
+static void
+acpi_smbat_info_updated(struct timespec *lastupdated)
+{
+
+       ACPI_SERIAL_ASSERT(smbat);
+
+       if (lastupdated != NULL)
+               getnanotime(lastupdated);
+}
+
+static int
acpi_smbus_read_2(struct acpi_smbat_softc *sc, uint8_t addr, uint8_t cmd,
    uint16_t *ptr)
{
@@ -284,6 +322,11 @@ acpi_smbat_get_bst(device_t dev, struct error = ENXIO;
        sc = device_get_softc(dev);

+       if (!acpi_smbat_info_expired(&sc->bst_lastupdated)) {
+               error = 0;
+               goto out;
+       }
+
        if (acpi_smbus_read_2(sc, addr, SMBATT_CMD_BATTERY_MODE, &val))
                goto out;
        if (val & SMBATT_BM_CAPACITY_MODE) {
@@ -299,7 +342,7 @@ acpi_smbat_get_bst(device_t dev, struct goto out;

        if (val & SMBATT_BS_DISCHARGING) {
-               bst->state |= ACPI_BATT_STAT_DISCHARG;
+               sc->bst.state |= ACPI_BATT_STAT_DISCHARG;

                /*
                 * If the rate is negative, it is discharging.  Otherwise,
@@ -308,27 +351,31 @@ acpi_smbat_get_bst(device_t dev, struct if (acpi_smbus_read_2(sc, addr, SMBATT_CMD_AT_RATE, &val))
                        goto out;
                if (val < 0)
-                       bst->rate = (-val) * factor;
+                       sc->bst.rate = (-val) * factor;
                else
-                       bst->rate = -1;
+                       sc->bst.rate = -1;
        } else {
-               bst->state |= ACPI_BATT_STAT_CHARGING;
-               bst->rate = -1;
+               sc->bst.state |= ACPI_BATT_STAT_CHARGING;
+               sc->bst.rate = -1;
        }

        if (val & SMBATT_BS_REMAINING_CAPACITY_ALARM)
-               bst->state |= ACPI_BATT_STAT_CRITICAL;
+               sc->bst.state |= ACPI_BATT_STAT_CRITICAL;

        if (acpi_smbus_read_2(sc, addr, SMBATT_CMD_REMAINING_CAPACITY, &val))
                goto out;
-       bst->cap = val * factor;
+       sc->bst.cap = val * factor;

        if (acpi_smbus_read_2(sc, addr, SMBATT_CMD_VOLTAGE, &val))
                goto out;
-       bst->volt = val;
+       sc->bst.volt = val;
+
+       acpi_smbat_info_updated(&sc->bst_lastupdated);
+
        error = 0;

out:
+       memcpy(bst, &sc->bst, sizeof(sc->bst));
        ACPI_SERIAL_END(smbat);
        return (error);
}
@@ -348,55 +395,63 @@ acpi_smbat_get_bif(device_t dev, struct error = ENXIO;
        sc = device_get_softc(dev);

+       if (!acpi_smbat_info_expired(&sc->bif_lastupdated)) {
+               error = 0;
+               goto out;
+       }
+
        if (acpi_smbus_read_2(sc, addr, SMBATT_CMD_BATTERY_MODE, &val))
                goto out;
        if (val & SMBATT_BM_CAPACITY_MODE) {
                factor = 10;
-               bif->units = ACPI_BIF_UNITS_MW;
+               sc->bif.units = ACPI_BIF_UNITS_MW;
        } else {
                factor = 1;
-               bif->units = ACPI_BIF_UNITS_MA;
+               sc->bif.units = ACPI_BIF_UNITS_MA;
        }

        if (acpi_smbus_read_2(sc, addr, SMBATT_CMD_DESIGN_CAPACITY, &val))
                goto out;
-       bif->dcap = val * factor;
+       sc->bif.dcap = val * factor;

        if (acpi_smbus_read_2(sc, addr, SMBATT_CMD_FULL_CHARGE_CAPACITY, &val))
                goto out;
-       bif->lfcap = val * factor;
-       bif->btech = 1;              /* secondary (rechargeable) */
+       sc->bif.lfcap = val * factor;
+       sc->bif.btech = 1;           /* secondary (rechargeable) */

        if (acpi_smbus_read_2(sc, addr, SMBATT_CMD_DESIGN_VOLTAGE, &val))
                goto out;
-       bif->dvol = val;
+       sc->bif.dvol = val;

-       bif->wcap = bif->dcap / 10;
-       bif->lcap = bif->dcap / 10;
+       sc->bif.wcap = sc->bif.dcap / 10;
+       sc->bif.lcap = sc->bif.dcap / 10;

-       bif->gra1 = factor;  /* not supported */
-       bif->gra2 = factor;  /* not supported */
+       sc->bif.gra1 = factor;       /* not supported */
+       sc->bif.gra2 = factor;       /* not supported */

        if (acpi_smbus_read_multi_1(sc, addr, SMBATT_CMD_DEVICE_NAME,
-           bif->model, sizeof(bif->model)))
+           sc->bif.model, sizeof(sc->bif.model)))
                goto out;

        if (acpi_smbus_read_2(sc, addr, SMBATT_CMD_SERIAL_NUMBER, &val))
                goto out;
-       snprintf(bif->serial, sizeof(bif->serial), "0x%04x", val);
+       snprintf(sc->bif.serial, sizeof(sc->bif.serial), "0x%04x", val);

        if (acpi_smbus_read_multi_1(sc, addr, SMBATT_CMD_DEVICE_CHEMISTRY,
-           bif->type, sizeof(bif->type)))
+           sc->bif.type, sizeof(sc->bif.type)))
                goto out;

        if (acpi_smbus_read_multi_1(sc, addr, SMBATT_CMD_MANUFACTURER_DATA,
-           bif->oeminfo, sizeof(bif->oeminfo)))
+           sc->bif.oeminfo, sizeof(sc->bif.oeminfo)))
                goto out;

+       acpi_smbat_info_updated(&sc->bif_lastupdated);
+
        /* XXX check if device was replugged during read? */
        error = 0;

out:
+       memcpy(bif, &sc->bif, sizeof(sc->bif));
        ACPI_SERIAL_END(smbat);
        return (error);
}


Sincerely,

--
Hajimu UMEMOTO @ Internet Mutual Aid Society Yokohama, Japan
[EMAIL PROTECTED]  [EMAIL PROTECTED],jp.}FreeBSD.org
http://www.imasy.org/~ume/

The patch seams to do its job correctly, but it is still very annoying to have the whole computer to freeze for 1 second when the cache expires. What does make the whole system to freeze? Before the code was changed in 6.0-stable, FreeBSD was able to read the battery status without freezing my laptop... I have been running 3 OSes (FreeBSD, Ubuntu and Win XP) on my laptop for a while and never experienced that kind of problem with either Linux or Win XP. I guess there is something wrong in the new code added after 6.0-release.

Thank you!

Pierre-Luc Drouin
_______________________________________________
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "[EMAIL PROTECTED]"

Reply via email to