fa.openbsd.tech
[Top] [All Lists]

Re: [PATCH] fix recent regression in acpibat(4)

Subject: Re: [PATCH] fix recent regression in acpibat(4)
From: Stefan Sperling <stsp@xxxxxxxxx>
Date: Thu, 31 Jul 2008 21:31:54 UTC
Newsgroups: fa.openbsd.tech

On Fri, Jul 18, 2008 at 07:31:49AM -0500, Marco Peereboom wrote:
> try this:

Marco,

I could finally try this, but it does not solve the issue.
Battery readings via APM still drop to zero from time to time:

$ while true; do apm; sleep 1; done
Battery state: high, 99% remaining, unknown life estimate
A/C adapter state: connected
Performance adjustment mode: auto (1667 MHz)
Battery state: high, 99% remaining, unknown life estimate
A/C adapter state: connected
Performance adjustment mode: auto (1667 MHz)
Battery state: high, 99% remaining, unknown life estimate
A/C adapter state: connected
Performance adjustment mode: auto (1667 MHz)
Battery state: absent, 0% remaining, unknown life estimate
A/C adapter state: connected
Performance adjustment mode: auto (1667 MHz)
Battery state: CRITICAL, 0% remaining, unknown life estimate
A/C adapter state: connected
Performance adjustment mode: auto (1667 MHz)
Battery state: high, 99% remaining, unknown life estimate
A/C adapter state: connected
Performance adjustment mode: auto (1667 MHz)


Note that I had to tweak the diff slightly to apply to current HEAD,
because aml_register_notify has been moved around.

This is what I used to test:

Index: acpiac.c
===================================================================
RCS file: /usr/OpenBSD-CVS/src/sys/dev/acpi/acpiac.c,v
retrieving revision 1.25
diff -u -p -r1.25 acpiac.c
--- acpiac.c    23 Jul 2008 00:20:35 -0000      1.25
+++ acpiac.c    31 Jul 2008 20:56:50 -0000
@@ -87,7 +87,7 @@ acpiac_attach(struct device *parent, str
        sc->sc_sens[0].value = sc->sc_ac_stat;
 
        aml_register_notify(sc->sc_devnode, aa->aaa_dev,
-           acpiac_notify, sc, ACPIDEV_NOPOLL);
+           acpiac_notify, sc, ACPIDEV_POLL);
 }
 
 void
@@ -123,10 +123,15 @@ int
 acpiac_notify(struct aml_node *node, int notify_type, void *arg)
 {
        struct acpiac_softc *sc = arg;
+       static int in_progress = 0;
 
        dnprintf(10, "acpiac_notify: %.2x %s\n", notify_type,
            sc->sc_devnode->name);
 
+       if (in_progress)
+               return (0);
+       in_progress = 1;
+
        switch (notify_type) {
        case 0x00:
        case 0x01:
@@ -141,5 +146,8 @@ acpiac_notify(struct aml_node *node, int
                dnprintf(10, "A/C status: %d\n", sc->sc_ac_stat);
                break;
        }
+
+       in_progress = 0;
+
        return (0);
 }
Index: acpibat.c
===================================================================
RCS file: /usr/OpenBSD-CVS/src/sys/dev/acpi/acpibat.c,v
retrieving revision 1.50
diff -u -p -r1.50 acpibat.c
--- acpibat.c   13 Jun 2008 05:50:21 -0000      1.50
+++ acpibat.c   31 Jul 2008 20:53:46 -0000
@@ -400,11 +400,16 @@ int
 acpibat_notify(struct aml_node *node, int notify_type, void *arg)
 {
        struct acpibat_softc    *sc = arg;
-       struct aml_value res;
+       struct aml_value        res;
+       static int              in_progress = 0;
 
        dnprintf(10, "acpibat_notify: %.2x %s\n", notify_type,
            sc->sc_devnode->name);
 
+       if (in_progress)
+               return (0);
+       in_progress = 1;
+
        /* Check if installed state of battery has changed */
        memset(&res, 0, sizeof(res));
        if (aml_evalname(sc->sc_acpi, node, "_STA", 0, NULL, &res) == 0) {
@@ -431,6 +436,8 @@ acpibat_notify(struct aml_node *node, in
        }
 
        acpibat_refresh(sc);
+
+       in_progress = 0;
 
        return (0);
 }

> 
> opencvs diff: Diffing inside .
> Index: acpiac.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/acpiac.c,v
> retrieving revision 1.24
> diff -u -N -p -u -N -p acpiac.c
> --- acpiac.c  18 Jul 2008 03:54:18 -0000      1.24
> +++ acpiac.c  18 Jul 2008 12:24:34 -0000
> @@ -68,9 +68,6 @@ acpiac_attach(struct device *parent, struct device *se
>       sc->sc_acpi = (struct acpi_softc *)parent;
>       sc->sc_devnode = aa->aaa_node;
>  
> -     aml_register_notify(sc->sc_devnode, aa->aaa_dev,
> -         acpiac_notify, sc, ACPIDEV_NOPOLL);
> -
>       acpiac_getsta(sc);
>       printf(": AC unit ");
>       if (sc->sc_ac_stat == PSR_ONLINE)
> @@ -88,6 +85,9 @@ acpiac_attach(struct device *parent, struct device *se
>       sensor_attach(&sc->sc_sensdev, &sc->sc_sens[0]);
>       sensordev_install(&sc->sc_sensdev);
>       sc->sc_sens[0].value = sc->sc_ac_stat;
> +
> +     aml_register_notify(sc->sc_devnode, aa->aaa_dev,
> +         acpiac_notify, sc, ACPIDEV_POLL);
>  }
>  
>  void
> @@ -123,10 +123,15 @@ int
>  acpiac_notify(struct aml_node *node, int notify_type, void *arg)
>  {
>       struct acpiac_softc *sc = arg;
> +     static int in_progress = 0;
>  
>       dnprintf(10, "acpiac_notify: %.2x %s\n", notify_type,
>           sc->sc_devnode->name);
>  
> +     if (in_progress)
> +             return (0);
> +     in_progress = 1;
> +
>       switch (notify_type) {
>       case 0x00:
>       case 0x01:
> @@ -141,5 +146,8 @@ acpiac_notify(struct aml_node *node, int notify_type, 
>               dnprintf(10, "A/C status: %d\n", sc->sc_ac_stat);
>               break;
>       }
> +
> +     in_progress = 0;
> +
>       return (0);
>  }
> Index: acpibat.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/acpibat.c,v
> retrieving revision 1.50
> diff -u -N -p -u -N -p acpibat.c
> --- acpibat.c 13 Jun 2008 05:50:21 -0000      1.50
> +++ acpibat.c 18 Jul 2008 12:33:29 -0000
> @@ -400,11 +400,16 @@ int
>  acpibat_notify(struct aml_node *node, int notify_type, void *arg)
>  {
>       struct acpibat_softc    *sc = arg;
> -     struct aml_value res;
> +     struct aml_value        res;
> +     static int              in_progress = 0;
>  
>       dnprintf(10, "acpibat_notify: %.2x %s\n", notify_type,
>           sc->sc_devnode->name);
>  
> +     if (in_progress)
> +             return (0);
> +     in_progress = 1;
> +
>       /* Check if installed state of battery has changed */
>       memset(&res, 0, sizeof(res));
>       if (aml_evalname(sc->sc_acpi, node, "_STA", 0, NULL, &res) == 0) {
> @@ -431,6 +436,8 @@ acpibat_notify(struct aml_node *node, int notify_type,
>       }
>  
>       acpibat_refresh(sc);
> +
> +     in_progress = 0;
>  
>       return (0);
>  }


<Prev in Thread] Current Thread [Next in Thread>