acpi_system76: Add support for battary charge thresholds
ClosedPublic
Actions

Authored by pouria on Sat, Mar 7, 7:31 PM.

Details

Reviewers

wulf
imp

Commits

rGf87ba4522ec9: acpi_system76: Add support for battary charge thresholds

Summary

Before this patch, I have to use the repo below.
https://github.com/brokentoaster/freebsd-system76-charge-thresholds/

Test Plan

In order to set the charging thresholds

% sysctl hw.acpi.s76.battary_thresholds_high
hw.acpi.s76.battary_thresholds_high: 95
% sysctl hw.acpi.s76.battary_thresholds_low
hw.acpi.s76.battary_thresholds_low: 60
% sysctl hw.acpi.s76.battary_thresholds_low=10
hw.acpi.s76.battary_thresholds_low: 60 -> 10
% sysctl hw.acpi.s76.battary_thresholds_high=70
hw.acpi.s76.battary_thresholds_high: 95 -> 70
% sysctl hw.acpi.s76
hw.acpi.s76.battary_thresholds_high: 70
hw.acpi.s76.battary_thresholds_low: 10
hw.acpi.s76.keyboard_color: 16777215
hw.acpi.s76.keyboard_backlight: 48

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

pouria requested review of this revision.Sat, Mar 7, 7:31 PM

pouria created this revision.

Harbormaster completed remote builds in B71244: Diff 173311.Sat, Mar 7, 7:31 PM

pouria added a child revision: D55716: acpi_system76: Add backlight(9) support for keyboard.Sat, Mar 7, 11:05 PM

wulf added inline comments.Sun, Mar 8, 8:26 AM

sys/dev/acpi_support/acpi_system76.c
334	If you removed default case than convert #defines to enum. It will enable case completeness checks in C compiler

Convert methods from macro to enum.
@wulf done.

Harbormaster completed remote builds in B71256: Diff 173360.Sun, Mar 8, 1:32 PM

pouria mentioned this in D55716: acpi_system76: Add backlight(9) support for keyboard.Sun, Mar 8, 1:34 PM

wulf accepted this revision.Sun, Mar 8, 8:34 PM

wulf added inline comments.

sys/dev/acpi_support/acpi_system76.c
82	S76_CTRL_MAX should not be in enum

This revision is now accepted and ready to land.Sun, Mar 8, 8:34 PM

pouria added inline comments.Sun, Mar 8, 11:30 PM

sys/dev/acpi_support/acpi_system76.c
82	S76_CTRL_MAX should not be in enum Can I ask you why? I want to learn because we have A LOT of `_MAX` members at the end of our enums inside netlink implementation. (see `sys/netlink/route/interface.h`)

It will trigger enumeration value ‘S76_CTRL_MAX’ not specifically handled in switch if -Wswitch option is passed to compiler.
From [1]:

-Wswitch

Warn whenever a switch statement has an index of enumerated type and lacks a case for one or more of the named codes of that enumeration. (The presence of a default label prevents this warning.) case labels that do not correspond to enumerators also provoke warnings when this option is used, unless the enumeration is marked with the flag_enum attribute. This warning is enabled by -Wall.

Obviously case S76_CTRL_MAX is not generally handled in switch() statements.

It seems that both ACPI and netlink builds without -Wswitch so we see no negative effects now.

[1] https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

put S76_CTRL_MAX back into macro.
@wulf done.

This revision now requires review to proceed.Mon, Mar 9, 9:10 AM

Harbormaster completed remote builds in B71278: Diff 173407.Mon, Mar 9, 9:10 AM

This revision was not accepted when it landed; it landed in state Needs Review.Mon, Mar 9, 9:22 AM

Closed by commit rGf87ba4522ec9: acpi_system76: Add support for battary charge thresholds (authored by pouria). · Explain Why

This revision was automatically updated to reflect the committed changes.

pouria added a commit: rGf87ba4522ec9: acpi_system76: Add support for battary charge thresholds.