* [PATCH 1/2] ACPI: sbs: add battery hook mechanism for SBS-registered batteries
2026-09-13 23:14 [PATCH 0/2] hwmon: (applesmc) add native charge_control_end_threshold support Jordan Brough
@ 2026-09-13 23:14 ` Jordan Brough
2026-09-16 13:00 ` Rafael J. Wysocki (Intel)
2026-09-16 17:22 ` Thomas Weißschuh
2026-09-13 23:14 ` [PATCH 2/2] hwmon: (applesmc) add charge_control_end_threshold support Jordan Brough
1 sibling, 2 replies; 5+ messages in thread
From: Jordan Brough @ 2026-09-13 23:14 UTC (permalink / raw)
To: rafael, rydberg, linux
Cc: lenb, maciej.wieczor-retman, pawel.chmielewski, linux-acpi,
acpica-devel, linux-hwmon, linux-kernel
drivers/acpi/battery.c provides a battery_hook_register()/unregister()
mechanism that lets other drivers (e.g. hwmon drivers exposing vendor-
specific charge control) attach extra power_supply properties to an
ACPI Control Method Battery (HID PNP0C0A, "CmBatt").
Some machines instead expose their battery through the ACPI Smart
Battery System (HID ACPI0002, "SBS") driver in drivers/acpi/sbs.c,
which has no equivalent hook point. On these machines a hwmon driver
has no supported way to attach additional properties to the battery's
power_supply device.
Add a parallel sbs_battery_hook_register()/unregister() mechanism to
drivers/acpi/sbs.c, mirroring the existing CmBatt hook API in name and
behavior so that callers can support both battery registration paths
symmetrically. This is purely additive: no existing hook consumer or
code path in drivers/acpi/battery.c is touched, and sbs.c's own
behavior is unchanged for any driver that does not call the new API.
This is a prerequisite for hwmon/applesmc gaining
charge_control_end_threshold support, since the Intel MacBook hardware
being targeted registers its battery via SBS, not CmBatt.
Signed-off-by: Jordan Brough <jordan@brough.org>
---
drivers/acpi/sbs.c | 147 ++++++++++++++++++++++++++++++++++++++++-
include/acpi/battery.h | 16 +++++
2 files changed, 162 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c
index 86b7c797585..305bf62eae3 100644
--- a/drivers/acpi/sbs.c
+++ b/drivers/acpi/sbs.c
@@ -10,6 +10,7 @@
#define pr_fmt(fmt) "ACPI: " fmt
#include <linux/init.h>
+#include <linux/list.h>
#include <linux/slab.h>
#include <linux/module.h>
#include <linux/moduleparam.h>
@@ -54,6 +55,7 @@ struct acpi_battery {
struct power_supply *bat;
struct power_supply_desc bat_desc;
struct acpi_sbs *sbs;
+ struct list_head list; /* node in sbs_hook_battery_list */
unsigned long update_time;
char name[8];
char manufacturer_name[ACPI_SBS_BLOCK_MAX];
@@ -518,6 +520,145 @@ static int acpi_battery_read(struct acpi_battery *battery)
return result;
}
+/*
+ * Battery hook support, mirroring the mechanism in drivers/acpi/battery.c
+ * for batteries registered there. That mechanism only sees batteries
+ * added via drivers/acpi/battery.c, not the ones this driver registers,
+ * so drivers wanting to attach optional functionality (e.g. an extra
+ * power_supply_ext) to an SBS battery need this separate copy; see the
+ * comment above the declarations in include/acpi/battery.h.
+ */
+static LIST_HEAD(sbs_hook_list);
+static LIST_HEAD(sbs_hook_battery_list);
+static DEFINE_MUTEX(sbs_hook_mutex);
+
+static void sbs_battery_hook_unregister_unlocked(struct acpi_battery_hook *hook)
+{
+ struct acpi_battery *battery;
+
+ /*
+ * In order to remove a hook, we first need to
+ * de-register all the batteries that are registered.
+ */
+ list_for_each_entry(battery, &sbs_hook_battery_list, list) {
+ if (!hook->remove_battery(battery->bat, hook))
+ power_supply_changed(battery->bat);
+ }
+ list_del_init(&hook->list);
+}
+
+void sbs_battery_hook_unregister(struct acpi_battery_hook *hook)
+{
+ mutex_lock(&sbs_hook_mutex);
+ /*
+ * Ignore already unregistered battery hooks. This might happen
+ * if a battery hook was previously unloaded due to an error when
+ * adding a new battery.
+ */
+ if (!list_empty(&hook->list))
+ sbs_battery_hook_unregister_unlocked(hook);
+ mutex_unlock(&sbs_hook_mutex);
+}
+EXPORT_SYMBOL_GPL(sbs_battery_hook_unregister);
+
+void sbs_battery_hook_register(struct acpi_battery_hook *hook)
+{
+ struct acpi_battery *battery;
+
+ mutex_lock(&sbs_hook_mutex);
+ list_add(&hook->list, &sbs_hook_list);
+ /*
+ * Now that the driver is registered, we need
+ * to notify the hook that a battery is available
+ * for each battery, so that the driver may add
+ * its attributes.
+ */
+ list_for_each_entry(battery, &sbs_hook_battery_list, list) {
+ if (hook->add_battery(battery->bat, hook)) {
+ /*
+ * If a add-battery returns non-zero,
+ * the registration of the hook has failed,
+ * and we will not add it to the list of loaded
+ * hooks.
+ */
+ pr_err("hook failed to load: %s", hook->name);
+ sbs_battery_hook_unregister_unlocked(hook);
+ goto end;
+ }
+
+ power_supply_changed(battery->bat);
+ }
+ pr_info("new hook: %s\n", hook->name);
+end:
+ mutex_unlock(&sbs_hook_mutex);
+}
+EXPORT_SYMBOL_GPL(sbs_battery_hook_register);
+
+static void devm_sbs_battery_hook_unregister(void *data)
+{
+ struct acpi_battery_hook *hook = data;
+
+ sbs_battery_hook_unregister(hook);
+}
+
+int devm_sbs_battery_hook_register(struct device *dev, struct acpi_battery_hook *hook)
+{
+ sbs_battery_hook_register(hook);
+
+ return devm_add_action_or_reset(dev, devm_sbs_battery_hook_unregister, hook);
+}
+EXPORT_SYMBOL_GPL(devm_sbs_battery_hook_register);
+
+/*
+ * This function gets called right after the battery sysfs
+ * attributes have been added, so that the drivers that
+ * define custom sysfs attributes can add their own.
+ */
+static void sbs_battery_hook_add_battery(struct acpi_battery *battery)
+{
+ struct acpi_battery_hook *hook_node, *tmp;
+
+ mutex_lock(&sbs_hook_mutex);
+ INIT_LIST_HEAD(&battery->list);
+ list_add(&battery->list, &sbs_hook_battery_list);
+ /*
+ * Since we added a new battery to the list, we need to
+ * iterate over the hooks and call add_battery for each
+ * hook that was registered. This usually happens
+ * when a battery gets hotplugged or initialized
+ * during the battery module initialization.
+ */
+ list_for_each_entry_safe(hook_node, tmp, &sbs_hook_list, list) {
+ if (hook_node->add_battery(battery->bat, hook_node)) {
+ /*
+ * The notification of the hook has failed, to
+ * prevent further errors we will unload the hook.
+ */
+ pr_err("error in hook, unloading: %s",
+ hook_node->name);
+ sbs_battery_hook_unregister_unlocked(hook_node);
+ }
+ }
+ mutex_unlock(&sbs_hook_mutex);
+}
+
+static void sbs_battery_hook_remove_battery(struct acpi_battery *battery)
+{
+ struct acpi_battery_hook *hook;
+
+ mutex_lock(&sbs_hook_mutex);
+ /*
+ * Before removing the hook, we need to remove all
+ * custom attributes from the battery.
+ */
+ list_for_each_entry(hook, &sbs_hook_list, list) {
+ hook->remove_battery(battery->bat, hook);
+ }
+ /* Then, just remove the battery from the list */
+ list_del(&battery->list);
+ mutex_unlock(&sbs_hook_mutex);
+}
+
/* Smart Battery */
static int acpi_battery_add(struct acpi_sbs *sbs, int id)
{
@@ -555,6 +696,8 @@ static int acpi_battery_add(struct acpi_sbs *sbs, int id)
goto end;
}
+ sbs_battery_hook_add_battery(battery);
+
end:
pr_info("%s [%s]: Battery Slot [%s] (battery %s)\n",
ACPI_SBS_DEVICE_NAME, acpi_device_bid(sbs->device),
@@ -566,8 +709,10 @@ static void acpi_battery_remove(struct acpi_sbs *sbs, int id)
{
struct acpi_battery *battery = &sbs->battery[id];
- if (battery->bat)
+ if (battery->bat) {
+ sbs_battery_hook_remove_battery(battery);
power_supply_unregister(battery->bat);
+ }
}
static int acpi_charger_add(struct acpi_sbs *sbs)
diff --git a/include/acpi/battery.h b/include/acpi/battery.h
index c93f16dfb94..ddbfa4ed135 100644
--- a/include/acpi/battery.h
+++ b/include/acpi/battery.h
@@ -18,8 +18,24 @@ struct acpi_battery_hook {
struct list_head list;
};
+/*
+ * battery_hook_register() and friends only see batteries registered by
+ * drivers/acpi/battery.c, the ACPI Control Method Battery driver (ACPI HID
+ * "PNP0C0A"). Batteries registered by drivers/acpi/sbs.c, the ACPI Smart
+ * Battery System driver (ACPI HID "ACPI0002", common on hardware with
+ * SMBus/SBS fuel-gauge chips such as many Intel MacBooks), are invisible
+ * to them; use the sbs_battery_hook_* equivalents below for those. A
+ * caller wanting to support both kinds of hardware needs two separate
+ * struct acpi_battery_hook instances, one per registration call, since a
+ * given instance's embedded list node can only belong to one list at a
+ * time.
+ */
void battery_hook_register(struct acpi_battery_hook *hook);
void battery_hook_unregister(struct acpi_battery_hook *hook);
int devm_battery_hook_register(struct device *dev, struct acpi_battery_hook *hook);
+void sbs_battery_hook_register(struct acpi_battery_hook *hook);
+void sbs_battery_hook_unregister(struct acpi_battery_hook *hook);
+int devm_sbs_battery_hook_register(struct device *dev, struct acpi_battery_hook *hook);
+
#endif
--
2.55.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 2/2] hwmon: (applesmc) add charge_control_end_threshold support
2026-09-13 23:14 [PATCH 0/2] hwmon: (applesmc) add native charge_control_end_threshold support Jordan Brough
2026-09-13 23:14 ` [PATCH 1/2] ACPI: sbs: add battery hook mechanism for SBS-registered batteries Jordan Brough
@ 2026-09-13 23:14 ` Jordan Brough
1 sibling, 0 replies; 5+ messages in thread
From: Jordan Brough @ 2026-09-13 23:14 UTC (permalink / raw)
To: rafael, rydberg, linux
Cc: lenb, maciej.wieczor-retman, pawel.chmielewski, linux-acpi,
acpica-devel, linux-hwmon, linux-kernel
Apple's SMC firmware exposes a real charging-cutoff control via the
BCLM (Battery Charge Limit Max) key, and a separate cosmetic BFCL
(Battery Final Charge Level) key that only controls when the MagSafe
LED switches from orange to green. Userspace tools and out-of-tree
kernel patches have historically conflated the two or exposed neither
through a standard interface, leaving desktop environments unable to
offer a native charge-limit control on Intel MacBooks.
Add native charge_control_end_threshold support to applesmc using the
power_supply_ext extension API, so that generic userspace (UPower,
KDE Powerdevil, GNOME Settings) can read and write the threshold
through the standard sysfs attribute with no vendor-specific daemon.
BFCL is kept in sync automatically (BCLM - 5) purely so the MagSafe
LED still changes color at a sensible point relative to the real
cutoff; it is not independently exposed.
Apple MacBooks register their battery through either the ACPI Control
Method Battery driver (CmBatt, drivers/acpi/battery.c) or the ACPI
Smart Battery System driver (SBS, drivers/acpi/sbs.c), depending on
model. Register against both hook mechanisms, guarded at compile/link
time with IS_REACHABLE() so this stays a no-op when the corresponding
subsystem isn't built in, and at runtime with a probe of the BCLM SMC
key so this is a no-op on any Mac (or non-Mac, given applesmc's DMI
match table) that doesn't actually implement it.
Tested on a MacBookAir6,2 (SBS-registered battery): the sysfs
attribute appears, reads/writes correctly propagate to the real SMC
BCLM key end-to-end via KDE's charge-limit slider, and BFCL tracks
BCLM - 5 as designed. Tested on a non-Mac laptop (CmBatt-registered
battery, no BCLM key) to confirm both new hook registrations are
true no-ops there: no new sysfs attributes, no behavioral change,
clean module load/unload.
Signed-off-by: Jordan Brough <jordan@brough.org>
---
drivers/hwmon/applesmc.c | 164 +++++++++++++++++++++++++++++++++++++--
1 file changed, 159 insertions(+), 5 deletions(-)
diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index d0baa10502f..39276cdba3d 100644
--- a/drivers/hwmon/applesmc.c
+++ b/drivers/hwmon/applesmc.c
@@ -33,6 +33,8 @@
#include <linux/workqueue.h>
#include <linux/err.h>
#include <linux/bits.h>
+#include <linux/power_supply.h>
+#include <acpi/battery.h>
#include <asm/barrier.h>
/* data port used by Apple SMC */
@@ -76,6 +78,26 @@
#define TEMP_SENSOR_TYPE "sp78"
+/*
+ * BCLM caps charging at a percentage; it is the only key that affects
+ * charging behavior. BFCL only controls when the charging LED switches
+ * from orange to green and has no effect on charging itself.
+ */
+#define BATTERY_CHARGE_LIMIT_KEY "BCLM" /* r/w ui8 */
+#define BATTERY_CHARGE_LIMIT_LED_KEY "BFCL" /* r/w ui8 */
+
+/*
+ * Margin kept between BCLM and BFCL so the charging LED still reports
+ * "done" once BCLM halts charging below 100%. Left equal to BCLM, the
+ * two can race: charging can stop a moment before the SMC's internal
+ * (sub-percent) charge counter actually reaches BFCL, leaving the LED
+ * stuck on "charging" indefinitely since no further current ever flows
+ * to push it over the threshold. A margin of 1 was sufficient to avoid
+ * this on a MacBookAir6,2; 5 matches Apple's own firmware convention and
+ * gives headroom on hardware we haven't tested.
+ */
+#define APPLESMC_BATTERY_CHARGE_LIMIT_LED_MARGIN 5
+
/* List of keys used to read/write fan speeds */
static const char *const fan_speed_fmt[] = {
"F%dAc", /* actual speed */
@@ -131,6 +153,7 @@ static struct applesmc_registers {
int num_light_sensors; /* number of light sensors */
bool has_accelerometer; /* has motion sensor */
bool has_key_backlight; /* has keyboard backlight */
+ bool has_battery_charge_limit; /* has BCLM battery charge limit */
bool init_complete; /* true when fully initialized */
struct applesmc_entry *cache; /* cached key entries */
const char **index; /* temperature key index */
@@ -633,17 +656,21 @@ static int applesmc_init_smcreg_try(void)
if (ret)
return ret;
ret = applesmc_has_key(BACKLIGHT_KEY, &s->has_key_backlight);
+ if (ret)
+ return ret;
+ ret = applesmc_has_key(BATTERY_CHARGE_LIMIT_KEY, &s->has_battery_charge_limit);
if (ret)
return ret;
s->num_light_sensors = left_light_sensor + right_light_sensor;
s->init_complete = true;
- pr_info("key=%d fan=%d temp=%d index=%d acc=%d lux=%d kbd=%d\n",
- s->key_count, s->fan_count, s->temp_count, s->index_count,
- s->has_accelerometer,
- s->num_light_sensors,
- s->has_key_backlight);
+ pr_info("key=%d fan=%d temp=%d index=%d acc=%d lux=%d kbd=%d bat=%d\n",
+ s->key_count, s->fan_count, s->temp_count, s->index_count,
+ s->has_accelerometer,
+ s->num_light_sensors,
+ s->has_key_backlight,
+ s->has_battery_charge_limit);
return 0;
}
@@ -724,6 +751,120 @@ static struct platform_driver applesmc_driver = {
},
};
+static int applesmc_bat_get_property(struct power_supply *psy,
+ const struct power_supply_ext *ext,
+ void *data,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ u8 limit;
+ int ret;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
+ ret = applesmc_read_key(BATTERY_CHARGE_LIMIT_KEY, &limit, 1);
+ if (ret)
+ return ret;
+ val->intval = limit;
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int applesmc_bat_set_property(struct power_supply *psy,
+ const struct power_supply_ext *ext,
+ void *data,
+ enum power_supply_property psp,
+ const union power_supply_propval *val)
+{
+ u8 limit, led_limit;
+ int ret;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD:
+ if (val->intval < 20 || val->intval > 100)
+ return -EINVAL;
+
+ limit = val->intval;
+ ret = applesmc_write_key(BATTERY_CHARGE_LIMIT_KEY, &limit, 1);
+ if (ret)
+ return ret;
+
+ led_limit = limit > APPLESMC_BATTERY_CHARGE_LIMIT_LED_MARGIN ?
+ limit - APPLESMC_BATTERY_CHARGE_LIMIT_LED_MARGIN : limit;
+ applesmc_write_key(BATTERY_CHARGE_LIMIT_LED_KEY, &led_limit, 1);
+ return 0;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int applesmc_bat_property_is_writeable(struct power_supply *psy,
+ const struct power_supply_ext *ext,
+ void *data,
+ enum power_supply_property psp)
+{
+ return psp == POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD;
+}
+
+static const enum power_supply_property applesmc_bat_props[] = {
+ POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD,
+};
+
+static const struct power_supply_ext applesmc_bat_ext = {
+ .name = "applesmc-charge-control",
+ .properties = applesmc_bat_props,
+ .num_properties = ARRAY_SIZE(applesmc_bat_props),
+ .get_property = applesmc_bat_get_property,
+ .set_property = applesmc_bat_set_property,
+ .property_is_writeable = applesmc_bat_property_is_writeable,
+};
+
+static struct power_supply *applesmc_hooked_battery;
+
+static int applesmc_battery_add(struct power_supply *battery, struct acpi_battery_hook *hook)
+{
+ if (applesmc_hooked_battery)
+ return 0;
+
+ applesmc_hooked_battery = battery;
+ return power_supply_register_extension(battery, &applesmc_bat_ext, &pdev->dev, NULL);
+}
+
+static int applesmc_battery_remove(struct power_supply *battery, struct acpi_battery_hook *hook)
+{
+ if (applesmc_hooked_battery != battery)
+ return 0;
+
+ power_supply_unregister_extension(battery, &applesmc_bat_ext);
+ applesmc_hooked_battery = NULL;
+ return 0;
+}
+
+/*
+ * Two separate hook registrations are required: Intel Macs register
+ * their battery through either drivers/acpi/battery.c (ACPI Control
+ * Method Battery) or drivers/acpi/sbs.c (ACPI Smart Battery System,
+ * common on hardware with SMBus/SBS fuel-gauge chips), depending on
+ * model, and each mechanism only sees batteries registered through
+ * itself. A single struct acpi_battery_hook cannot be registered with
+ * both, since its embedded list node can only belong to one list at a
+ * time. Whichever mechanism matches the actual hardware will hook the
+ * battery; the other simply never finds one to call back for.
+ */
+static struct acpi_battery_hook applesmc_battery_hook = {
+ .name = "Apple SMC Battery Charge Control",
+ .add_battery = applesmc_battery_add,
+ .remove_battery = applesmc_battery_remove,
+};
+
+static struct acpi_battery_hook applesmc_sbs_battery_hook = {
+ .name = "Apple SMC Battery Charge Control (SBS)",
+ .add_battery = applesmc_battery_add,
+ .remove_battery = applesmc_battery_remove,
+};
+
/*
* applesmc_calibrate - Set our "resting" values. Callers must
* hold applesmc_lock.
@@ -1563,6 +1704,13 @@ static int __init applesmc_init(void)
goto out_light_ledclass;
}
+ if (smcreg.has_battery_charge_limit) {
+ if (IS_REACHABLE(CONFIG_ACPI_BATTERY))
+ battery_hook_register(&applesmc_battery_hook);
+ if (IS_REACHABLE(CONFIG_ACPI_SBS))
+ sbs_battery_hook_register(&applesmc_sbs_battery_hook);
+ }
+
return 0;
out_light_ledclass:
@@ -1589,6 +1737,12 @@ static int __init applesmc_init(void)
static void __exit applesmc_exit(void)
{
+ if (smcreg.has_battery_charge_limit) {
+ if (IS_REACHABLE(CONFIG_ACPI_BATTERY))
+ battery_hook_unregister(&applesmc_battery_hook);
+ if (IS_REACHABLE(CONFIG_ACPI_SBS))
+ sbs_battery_hook_unregister(&applesmc_sbs_battery_hook);
+ }
hwmon_device_unregister(hwmon_dev);
applesmc_release_key_backlight();
applesmc_release_light_sensor();
--
2.55.0
^ permalink raw reply [flat|nested] 5+ messages in thread