[Fwd: cvs commit: src/sys/dev/acpica acpi_cmbat.c]

From: Nate Lawson (nate_at_root.org)
Date: 11/23/05

  • Next message: Scott Long: "Re: [Fwd: cvs commit: src/sys/dev/acpica acpi_cmbat.c]"
    Date: Tue, 22 Nov 2005 17:06:11 -0800
    To: acpi@FreeBSD.org
    
    

    Here is a patch that should fix the battery hangs on RELENG_6. It was
    tested to work fine, although I need testing from an affected user to
    verify it fixes the problem. It was committed to HEAD and will be MFCed
    if it fixes the problem.

    I'm a bit disappointed that no one reported this problem in the 2 weeks
    it was present in 7-current. If you have the time to run -current on at
    least one partition of your laptop, that would assist me greatly.

    Thanks,
    -Nate

    -------- Original Message --------
    Subject: cvs commit: src/sys/dev/acpica acpi_cmbat.c
    Date: Wed, 23 Nov 2005 00:58:05 +0000 (GMT)
    From: Nate Lawson <njl@FreeBSD.org>
    To: njl@FreeBSD.ORG

    njl 2005-11-23 00:57:51 UTC

       FreeBSD src repository

       Modified files:
         sys/dev/acpica acpi_cmbat.c
       Log:
       Try to fix problems with periodic hangs by never directly calling _BIF.
       Instead, re-evaluate _BIF only when we get a notify and use the cached
       results. We also still evaluate _BIF once on boot. Also, optimize the
       init loop a little by only querying for a particular info if it's not
    valid.

       MFC after: 2 days

       Revision Changes Path
       1.42 +34 -22 src/sys/dev/acpica/acpi_cmbat.c

    Index: src/sys/dev/acpica/acpi_cmbat.c
    diff -u src/sys/dev/acpica/acpi_cmbat.c:1.41
    src/sys/dev/acpica/acpi_cmbat.c:1.42
    --- src/sys/dev/acpica/acpi_cmbat.c:1.41 Sun Sep 11 18:39:01 2005
    +++ src/sys/dev/acpica/acpi_cmbat.c Wed Nov 23 00:57:51 2005
    @@ -66,7 +66,6 @@

          struct acpi_bif bif;
          struct acpi_bst bst;
    - struct timespec bif_lastupdated;
          struct timespec bst_lastupdated;
      };

    @@ -80,8 +79,8 @@
                                  void *context);
      static int acpi_cmbat_info_expired(struct timespec *lastupdated);
      static void acpi_cmbat_info_updated(struct timespec *lastupdated);
    -static void acpi_cmbat_get_bst(device_t dev);
    -static void acpi_cmbat_get_bif(device_t dev);
    +static void acpi_cmbat_get_bst(void *arg);
    +static void acpi_cmbat_get_bif(void *arg);
      static int acpi_cmbat_bst(device_t dev, struct acpi_bst *bstp);
      static int acpi_cmbat_bif(device_t dev, struct acpi_bif *bifp);
      static void acpi_cmbat_init_battery(void *arg);
    @@ -134,7 +133,6 @@
          handle = acpi_get_handle(dev);
          sc->dev = dev;

    - timespecclear(&sc->bif_lastupdated);
          timespecclear(&sc->bst_lastupdated);

          error = acpi_battery_register(dev);
    @@ -180,20 +178,22 @@
          dev = (device_t)context;
          sc = device_get_softc(dev);

    - /*
    - * Clear the appropriate last updated time. The next call to retrieve
    - * the battery status will get the new value for us. We don't need to
    - * acquire a lock since we are only clearing the time stamp and since
    - * calling _BST/_BIF can trigger a notify, we could deadlock also.
    - */
          switch (notify) {
          case ACPI_NOTIFY_DEVICE_CHECK:
          case ACPI_BATTERY_BST_CHANGE:
    + /*
    + * Clear the last updated time. The next call to retrieve the
    + * battery status will get the new value for us.
    + */
              timespecclear(&sc->bst_lastupdated);
              break;
          case ACPI_NOTIFY_BUS_CHECK:
          case ACPI_BATTERY_BIF_CHANGE:
    - timespecclear(&sc->bif_lastupdated);
    + /*
    + * Queue a callback to get the current battery info from thread
    + * context. It's not safe to block in a notify handler.
    + */
    + AcpiOsQueueForExecution(OSD_PRIORITY_LO, acpi_cmbat_get_bif, dev);
              break;
          }

    @@ -229,16 +229,18 @@
      }

      static void
    -acpi_cmbat_get_bst(device_t dev)
    +acpi_cmbat_get_bst(void *arg)
      {
          struct acpi_cmbat_softc *sc;
          ACPI_STATUS as;
          ACPI_OBJECT *res;
          ACPI_HANDLE h;
          ACPI_BUFFER bst_buffer;
    + device_t dev;

          ACPI_SERIAL_ASSERT(cmbat);

    + dev = arg;
          sc = device_get_softc(dev);
          h = acpi_get_handle(dev);
          bst_buffer.Pointer = NULL;
    @@ -287,24 +289,23 @@
      }

      static void
    -acpi_cmbat_get_bif(device_t dev)
    +acpi_cmbat_get_bif(void *arg)
      {
          struct acpi_cmbat_softc *sc;
          ACPI_STATUS as;
          ACPI_OBJECT *res;
          ACPI_HANDLE h;
          ACPI_BUFFER bif_buffer;
    + device_t dev;

          ACPI_SERIAL_ASSERT(cmbat);

    + dev = arg;
          sc = device_get_softc(dev);
          h = acpi_get_handle(dev);
          bif_buffer.Pointer = NULL;
          bif_buffer.Length = ACPI_ALLOCATE_BUFFER;

    - if (!acpi_cmbat_info_expired(&sc->bif_lastupdated))
    - goto end;
    -
          as = AcpiEvaluateObject(h, "_BIF", NULL, &bif_buffer);
          if (ACPI_FAILURE(as)) {
              ACPI_VPRINT(dev, acpi_device_get_parent_softc(dev),
    @@ -346,7 +347,6 @@
              goto end;
          if (acpi_PkgStr(res, 12, sc->bif.oeminfo, ACPI_CMBAT_MAXSTRLEN) != 0)
              goto end;
    - acpi_cmbat_info_updated(&sc->bif_lastupdated);

      end:
          if (bif_buffer.Pointer != NULL)
    @@ -360,8 +360,13 @@

          sc = device_get_softc(dev);

    + /*
    + * Just copy the data. The only value that should change is the
    + * last-full capacity, so we only update when we get a notify that says
    + * the info has changed. Many systems apparently take a long time to
    + * process a _BIF call so we avoid it if possible.
    + */
          ACPI_SERIAL_BEGIN(cmbat);
    - acpi_cmbat_get_bif(dev);
          bifp->units = sc->bif.units;
          bifp->dcap = sc->bif.dcap;
          bifp->lfcap = sc->bif.lfcap;
    @@ -422,11 +427,18 @@
              if (!acpi_BatteryIsPresent(dev))
                  continue;

    + /*
    + * Only query the battery if this is the first try or the specific
    + * type of info is still invalid.
    + */
              ACPI_SERIAL_BEGIN(cmbat);
    - timespecclear(&sc->bst_lastupdated);
    - timespecclear(&sc->bif_lastupdated);
    - acpi_cmbat_get_bst(dev);
    - acpi_cmbat_get_bif(dev);
    + if (retry == 0 || !acpi_battery_bst_valid(&sc->bst)) {
    + timespecclear(&sc->bst_lastupdated);
    + acpi_cmbat_get_bst(dev);
    + }
    + if (retry == 0 || !acpi_battery_bif_valid(&sc->bif))
    + acpi_cmbat_get_bif(dev);
    +
              valid = acpi_battery_bst_valid(&sc->bst) &&
                  acpi_battery_bif_valid(&sc->bif);
              ACPI_SERIAL_END(cmbat);

    -- 
    Nate
    _______________________________________________
    freebsd-current@freebsd.org mailing list
    http://lists.freebsd.org/mailman/listinfo/freebsd-current
    To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"
    

  • Next message: Scott Long: "Re: [Fwd: cvs commit: src/sys/dev/acpica acpi_cmbat.c]"

    Relevant Pages

    • [Fwd: cvs commit: src/sys/dev/acpica acpi_cmbat.c]
      ... Here is a patch that should fix the battery hangs on RELENG_6. ... Try to fix problems with periodic hangs by never directly calling _BIF. ... -acpi_cmbat_get_bst(device_t dev) ...
      (freebsd-stable)
    • Re: [linux-usb-devel] Bug creating USB endpoints in 2.6.20.x (kernel bug 8198)
      ... >> Okay, here's a half-assed fix. ... The problem is that dev->devt_attr (class dev has ... sysfs attribute has users at that point, the dev sysfs node is left with ... I think it's better to risk module unload / sysfs race ...
      (Linux-Kernel)
    • Re: RPC Terminated
      ... information and how to fix this please see these links: ... Marc Reynolds ... Microsoft Technical Support ... > Thanks for any help - Dev ...
      (microsoft.public.windowsxp.setup_deployment)
    • [Patch] [mm] More driver core fixes for -mm
      ... I've looked some more into the __must_check stuff in the driver core, ... Fix missing checks of return codes for driver model functions called in ... int bus_add_device(struct device * dev) ... goto AttrsError; ...
      (Linux-Kernel)