[PATCH v2 2/2] hwmon: (applesmc) add charge_control_end_threshold support

[Jordan Brough <jordan@brough.org>]

Intel MacBooks with Apple SMC expose battery charge control threshold
registers (BCLM) and MagSafe LED thresholds (BFCL).

Add native charge_control_end_threshold support to the applesmc driver
using the power_supply_ext extension API and the ACPI battery hook
infrastructure.

Hardware testing on Intel MacBooks shows that setting BCLM to a given
threshold requires keeping BFCL set to a margin (5%) below BCLM to
prevent a race where charging halts just before the SMC's sub-percent
counter reaches BFCL, leaving the MagSafe LED stuck in amber ("charging").

Serialize SMC register writes and power_supply extension registration under
applesmc_bat_lock to prevent concurrent write desynchronization and ensure
clean rollback if extension registration fails.

Signed-off-by: Jordan Brough <jordan@brough.org>
---
 drivers/hwmon/applesmc.c | 157 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 152 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index d0baa10502f..2444bea8a0f 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,119 @@ static struct platform_driver applesmc_driver = {
 	},
 };
 
+static DEFINE_MUTEX(applesmc_bat_lock);
+
+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:
+		scoped_guard(mutex, &applesmc_bat_lock) {
+			ret = applesmc_read_key(BATTERY_CHARGE_LIMIT_KEY, &limit, 1);
+			if (ret)
+				return ret;
+			val->intval = limit;
+			return 0;
+		}
+		break;
+	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;
+		led_limit = limit > APPLESMC_BATTERY_CHARGE_LIMIT_LED_MARGIN ?
+			limit - APPLESMC_BATTERY_CHARGE_LIMIT_LED_MARGIN : limit;
+
+		scoped_guard(mutex, &applesmc_bat_lock) {
+			ret = applesmc_write_key(BATTERY_CHARGE_LIMIT_KEY, &limit, 1);
+			if (ret)
+				return ret;
+
+			return applesmc_write_key(BATTERY_CHARGE_LIMIT_LED_KEY, &led_limit, 1);
+		}
+		break;
+	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)
+{
+	int ret;
+
+	guard(mutex)(&applesmc_bat_lock);
+	if (applesmc_hooked_battery)
+		return 0;
+
+	ret = power_supply_register_extension(battery, &applesmc_bat_ext, &pdev->dev, NULL);
+	if (ret)
+		return ret;
+
+	applesmc_hooked_battery = battery;
+	return 0;
+}
+
+static int applesmc_battery_remove(struct power_supply *battery, struct acpi_battery_hook *hook)
+{
+	guard(mutex)(&applesmc_bat_lock);
+	if (applesmc_hooked_battery != battery)
+		return 0;
+
+	power_supply_unregister_extension(battery, &applesmc_bat_ext);
+	applesmc_hooked_battery = NULL;
+	return 0;
+}
+
+static struct acpi_battery_hook applesmc_battery_hook = {
+	.name = "Apple SMC Battery Charge Control",
+	.add_battery = applesmc_battery_add,
+	.remove_battery = applesmc_battery_remove,
+};
+
 /*
  * applesmc_calibrate - Set our "resting" values.  Callers must
  * hold applesmc_lock.
@@ -1563,6 +1703,10 @@ static int __init applesmc_init(void)
 		goto out_light_ledclass;
 	}
 
+	if (smcreg.has_battery_charge_limit &&
+	    (IS_REACHABLE(CONFIG_ACPI_BATTERY) || IS_REACHABLE(CONFIG_ACPI_SBS)))
+		battery_hook_register(&applesmc_battery_hook);
+
 	return 0;
 
 out_light_ledclass:
@@ -1589,6 +1733,9 @@ static int __init applesmc_init(void)
 
 static void __exit applesmc_exit(void)
 {
+	if (smcreg.has_battery_charge_limit &&
+	    (IS_REACHABLE(CONFIG_ACPI_BATTERY) || IS_REACHABLE(CONFIG_ACPI_SBS)))
+		battery_hook_unregister(&applesmc_battery_hook);
 	hwmon_device_unregister(hwmon_dev);
 	applesmc_release_key_backlight();
 	applesmc_release_light_sensor();
-- 
2.55.0