[Bug 283683] [PATCH] dev/acpica/acpi_cmbat.c: Add battery trip point (_BTP)

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=283683

Warner Losh <imp@FreeBSD.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|New                         |Open
                 CC|                            |imp@FreeBSD.org
           Assignee|acpi@FreeBSD.org            |imp@FreeBSD.org

--- Comment #1 from Warner Losh <imp@FreeBSD.org> ---
So there's a number of problems with the attached patch, though I think they
are all style(9) issues:

No space between if and ( in a few places.

+       if(ACPI_SUCCESS(acpi_GetHandleInScope(acpi_get_handle(dev), "_BTP",
&tmp)))
+       {
+               sc->btp_warning_level = ACPI_BATTERY_BTP_WARNING_LEVEL;
+       
+               struct sysctl_oid *cmbat_oid = device_get_sysctl_tree(dev);
+               SYSCTL_ADD_PROC(NULL, SYSCTL_CHILDREN(cmbat_oid), OID_AUTO,
+               "warning_level", CTLTYPE_INT | CTLFLAG_RW, dev, 0,
+               acpi_cmbat_btp_sysctl, "I" ,"battery warning level");
+       }

So here the struct sysctl_oid line should be the first line of this, with a
blank line after it before the rest.

Also the { for this should be at the end of the if line (which is one example
of the if complaint above)

Also, the continuation of the SYSCTL_ADD_PROC macro should be indented by 4
spaces.

+       if(req->newptr && acpi_BatteryIsPresent(dev)) {
+       /* Write request. */
+
+               SYSCTL_IN(req, &sc->btp_warning_level,
sizeof(sc->btp_warning_level));
+               
+               /* Correct bogus writes. */
+               if(sc->btp_warning_level < 0 || sc->btp_warning_level > 100)
+                       sc->btp_warning_level = ACPI_BATTERY_BTP_WARNING_LEVEL;
+       
+               /* Call _BTP method */
+               newtp = sc->bix.lfcap * sc->btp_warning_level / 100;
+               as = acpi_SetInteger(acpi_get_handle(dev), "_BTP", (uint32_t)
newtp);
+               
+               /* Error checking. */
+               if (ACPI_FAILURE(as)) {
+                       ACPI_VPRINT(dev, acpi_device_get_parent_softc(dev),
+                       "error setting _BTP --%s\n", AcpiFormatException(as));
+                        sc->btp_warning_level = 0;
+               }
+    }
+
+       else if(req->newptr) /* Write request w/o battery. */
+               sc->btp_warning_level = 0;
+
+       else /* Read request. */
+               SYSCTL_OUT(req, &sc->btp_warning_level,
sizeof(sc->btp_warning_level));
+

(same if complaints)

The last two clauses in the if else chain should have { } around them.

The /* Write request. */ comment should be indented one more tab

I think that the error case should cause a return of EINVAL instead of 0 when
the ACPI call fails.

+}
\ No newline at end of file

The file needs to end in a newline

Would you be able to correct these issues with the patch?

-- 
You are receiving this mail because:
You are the assignee for the bug.