Hi, On Fri, Jan 09, 2026 at 12:18:48AM +1100, Michael Reeves via B4 Relay wrote: > From: Michael Reeves > > This driver provides battery and AC status monitoring for Apple Silicon > Macs via the SMC (System Management Controller). It supports > reporting capacity, voltage, current, and charging status. > > Co-developed-by: Hector Martin > Signed-off-by: Hector Martin > Reviewed-by: Neal Gompa > Signed-off-by: Michael Reeves > --- > MAINTAINERS | 1 + > drivers/power/supply/Kconfig | 11 + > drivers/power/supply/Makefile | 1 + > drivers/power/supply/macsmc-power.c | 883 ++++++++++++++++++++++++++++++++++++ > 4 files changed, 896 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 0dbf349fc1ed..c18da2295477 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2507,6 +2507,7 @@ F: drivers/nvmem/apple-efuses.c > F: drivers/nvmem/apple-spmi-nvmem.c > F: drivers/pinctrl/pinctrl-apple-gpio.c > F: drivers/power/reset/macsmc-reboot.c > +F: drivers/power/supply/macsmc-power.c > F: drivers/pwm/pwm-apple.c > F: drivers/rtc/rtc-macsmc.c > F: drivers/soc/apple/* > diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig > index 92f9f7aae92f..3a5b7d9234c2 100644 > --- a/drivers/power/supply/Kconfig > +++ b/drivers/power/supply/Kconfig > @@ -1132,4 +1132,15 @@ config FUEL_GAUGE_MM8013 > the state of charge, temperature, cycle count, actual and design > capacity, etc. > > +config MACSMC_POWER > + tristate "Apple SMC Battery and Power Driver" > + depends on MFD_MACSMC > + help > + This driver provides support for the battery and AC adapter on > + Apple Silicon machines. It exposes battery telemetry (voltage, > + current, health) and AC adapter status through the standard Linux > + power supply framework. > + > + Say Y or M here if you have an Apple Silicon based Mac. > + > endif # POWER_SUPPLY > diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile > index 4b79d5abc49a..d14420b606d8 100644 > --- a/drivers/power/supply/Makefile > +++ b/drivers/power/supply/Makefile > @@ -128,3 +128,4 @@ obj-$(CONFIG_CHARGER_SURFACE) += surface_charger.o > obj-$(CONFIG_BATTERY_UG3105) += ug3105_battery.o > obj-$(CONFIG_CHARGER_QCOM_SMB2) += qcom_smbx.o > obj-$(CONFIG_FUEL_GAUGE_MM8013) += mm8013.o > +obj-$(CONFIG_MACSMC_POWER) += macsmc-power.o > diff --git a/drivers/power/supply/macsmc-power.c b/drivers/power/supply/macsmc-power.c > new file mode 100644 > index 000000000000..4ff90da73fa0 > --- /dev/null > +++ b/drivers/power/supply/macsmc-power.c > @@ -0,0 +1,883 @@ > +// SPDX-License-Identifier: GPL-2.0-only OR MIT > +/* > + * Apple SMC Power/Battery Management Driver > + * > + * This driver exposes battery telemetry (voltage, current, temperature, health) > + * and AC adapter status provided by the Apple SMC (System Management Controller) > + * on Apple Silicon systems. > + * > + * Copyright The Asahi Linux Contributors > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define MAX_STRING_LENGTH 256 > + > +/* > + * The SMC reports charge in mAh (Coulombs) but energy in mWh (Joules). > + * We lack a register for "Nominal Voltage" or "Energy Accumulator". > + * We use a fixed 3.8V/cell constant to approximate energy stats for userspace, > + * derived from empirical data across supported MacBook models. > + */ > +#define MACSMC_NOMINAL_CELL_VOLTAGE_MV 3800 > + > +/* SMC Key Flags */ > +#define CHNC_BATTERY_FULL BIT(0) > +#define CHNC_NO_CHARGER BIT(7) > +#define CHNC_NOCHG_CH0C BIT(14) > +#define CHNC_NOCHG_CH0B_CH0K BIT(15) > +#define CHNC_BATTERY_FULL_2 BIT(18) > +#define CHNC_BMS_BUSY BIT(23) > +#define CHNC_CHLS_LIMIT BIT(24) > +#define CHNC_NOAC_CH0J BIT(53) > +#define CHNC_NOAC_CH0I BIT(54) > + > +#define CH0R_LOWER_FLAGS GENMASK(15, 0) > +#define CH0R_NOAC_CH0I BIT(0) > +#define CH0R_NOAC_DISCONNECTED BIT(4) > +#define CH0R_NOAC_CH0J BIT(5) > +#define CH0R_BMS_BUSY BIT(8) > +#define CH0R_NOAC_CH0K BIT(9) > +#define CH0R_NOAC_CHWA BIT(11) > + > +#define CH0X_CH0C BIT(0) > +#define CH0X_CH0B BIT(1) > + > +#define ACSt_CAN_BOOT_AP BIT(2) > +#define ACSt_CAN_BOOT_IBOOT BIT(1) > + > +#define CHWA_CHLS_FIXED_START_OFFSET 5 > +#define CHLS_MIN_END_THRESHOLD 10 > +#define CHLS_FORCE_DISCHARGE 0x100 > +#define CHWA_FIXED_END_THRESHOLD 80 > +#define CHWA_PROP_WRITE_THRESHOLD 95 > + > +#define MACSMC_MAX_BATT_PROPS 50 > +#define MACSMC_MAX_AC_PROPS 10 > + > +struct macsmc_power { > + struct device *dev; > + struct apple_smc *smc; > + > + struct power_supply_desc ac_desc; > + struct power_supply_desc batt_desc; > + > + enum power_supply_property *ac_props; > + enum power_supply_property *batt_props; > + > + struct power_supply *batt; > + struct power_supply *ac; > + > + char model_name[MAX_STRING_LENGTH]; > + char serial_number[MAX_STRING_LENGTH]; > + char mfg_date[MAX_STRING_LENGTH]; > + > + bool has_chwa; > + bool has_chls; > + bool has_ch0i; > + bool has_ch0c; > + bool has_chte; > + > + u8 num_cells; > + int nominal_voltage_mv; > + > + struct notifier_block nb; > + struct work_struct critical_work; > + bool shutdown_started; > +}; > + > +static int macsmc_battery_get_status(struct macsmc_power *power) > +{ > + u64 nocharge_flags; > + u32 nopower_flags; > + u16 ac_current; > + int charge_limit = 0; > + bool limited = false; > + bool flag; > + int ret; > + > + /* > + * Fallbacks exist for keys that may disappear in future hardware. > + * CHCE/CHCC/BSFC/CHSC are considered fundamental; absence is an error. > + */ > + > + /* Check if power input is inhibited (e.g. BMS balancing cycle) */ > + ret = apple_smc_read_u32(power->smc, SMC_KEY(CH0R), &nopower_flags); > + if (!ret && (nopower_flags & CH0R_LOWER_FLAGS & ~CH0R_BMS_BUSY)) > + return POWER_SUPPLY_STATUS_DISCHARGING; > + > + /* Check if charger is present */ > + ret = apple_smc_read_flag(power->smc, SMC_KEY(CHCE), &flag); > + if (ret < 0) > + return ret; > + if (!flag) > + return POWER_SUPPLY_STATUS_DISCHARGING; > + > + /* Check if AC is charge capable */ > + ret = apple_smc_read_flag(power->smc, SMC_KEY(CHCC), &flag); > + if (ret < 0) > + return ret; > + if (!flag) > + return POWER_SUPPLY_STATUS_DISCHARGING; > + > + /* Check if AC input limit is too low */ > + ret = apple_smc_read_u16(power->smc, SMC_KEY(AC-i), &ac_current); > + if (!ret && ac_current < 100) > + return POWER_SUPPLY_STATUS_DISCHARGING; > + > + /* Check if battery is full */ > + ret = apple_smc_read_flag(power->smc, SMC_KEY(BSFC), &flag); > + if (ret < 0) > + return ret; > + if (flag) > + return POWER_SUPPLY_STATUS_FULL; > + > + /* Check for user-defined charge limits */ > + if (power->has_chls) { > + u16 vu16; > + > + ret = apple_smc_read_u16(power->smc, SMC_KEY(CHLS), &vu16); > + if (ret == 0 && (vu16 & 0xff) >= CHLS_MIN_END_THRESHOLD) > + charge_limit = (vu16 & 0xff) - CHWA_CHLS_FIXED_START_OFFSET; > + } else if (power->has_chwa) { > + ret = apple_smc_read_flag(power->smc, SMC_KEY(CHWA), &flag); > + if (ret == 0 && flag) > + charge_limit = CHWA_FIXED_END_THRESHOLD - CHWA_CHLS_FIXED_START_OFFSET; > + } > + > + if (charge_limit > 0) { > + u8 buic = 0; > + > + if (apple_smc_read_u8(power->smc, SMC_KEY(BUIC), &buic) >= 0 && > + buic >= charge_limit) > + limited = true; > + } > + > + /* Check charging inhibitors */ > + ret = apple_smc_read_u64(power->smc, SMC_KEY(CHNC), &nocharge_flags); > + if (!ret) { > + if (nocharge_flags & CHNC_BATTERY_FULL) > + return POWER_SUPPLY_STATUS_FULL; > + /* BMS busy shows up as inhibit, but we treat it as charging */ > + else if (nocharge_flags == CHNC_BMS_BUSY && !limited) > + return POWER_SUPPLY_STATUS_CHARGING; > + else if (nocharge_flags) > + return POWER_SUPPLY_STATUS_NOT_CHARGING; > + else > + return POWER_SUPPLY_STATUS_CHARGING; > + } > + > + /* Fallback: System charging flag */ > + ret = apple_smc_read_flag(power->smc, SMC_KEY(CHSC), &flag); > + if (ret < 0) > + return ret; > + if (!flag) > + return POWER_SUPPLY_STATUS_NOT_CHARGING; > + > + return POWER_SUPPLY_STATUS_CHARGING; > +} > + > +static int macsmc_battery_get_charge_behaviour(struct macsmc_power *power) > +{ > + int ret; > + u8 val8; > + u8 chte_buf[4]; > + > + if (power->has_ch0i) { > + ret = apple_smc_read_u8(power->smc, SMC_KEY(CH0I), &val8); > + if (ret) > + return ret; > + if (val8 & CH0R_NOAC_CH0I) > + return POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE; > + } > + > + if (power->has_chte) { > + ret = apple_smc_read(power->smc, SMC_KEY(CHTE), chte_buf, 4); > + if (ret < 0) > + return ret; > + > + if (chte_buf[0] == 0x01) > + return POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE; > + } else if (power->has_ch0c) { > + ret = apple_smc_read_u8(power->smc, SMC_KEY(CH0C), &val8); > + if (ret) > + return ret; > + if (val8 & CH0X_CH0C) > + return POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE; > + } > + > + return POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO; > +} > + > +static int macsmc_battery_set_charge_behaviour(struct macsmc_power *power, int val) > +{ > + int ret; > + > + /* First, reset all inhibitors to a known-good 'auto' state */ > + if (power->has_ch0i) { > + ret = apple_smc_write_u8(power->smc, SMC_KEY(CH0I), 0); > + if (ret) > + return ret; > + } > + > + if (power->has_chte) { > + ret = apple_smc_write_u32(power->smc, SMC_KEY(CHTE), 0); > + if (ret) > + return ret; > + } else if (power->has_ch0c) { > + ret = apple_smc_write_u8(power->smc, SMC_KEY(CH0C), 0); > + if (ret) > + return ret; > + } > + > + switch (val) { > + case POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO: > + return 0; > + > + case POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE: > + if (power->has_chte) > + return apple_smc_write_u32(power->smc, SMC_KEY(CHTE), 1); > + else if (power->has_ch0c) > + return apple_smc_write_u8(power->smc, SMC_KEY(CH0C), 1); > + else > + return -EOPNOTSUPP; > + break; Indentation? > + > + case POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE: > + if (!power->has_ch0i) > + return -EOPNOTSUPP; > + return apple_smc_write_u8(power->smc, SMC_KEY(CH0I), 1); Indentation? > + default: > + return -EINVAL; > + } > +} > + > +static int macsmc_battery_get_date(const char *s, int *out) > +{ > + if (!isdigit(s[0]) || !isdigit(s[1])) > + return -EOPNOTSUPP; > + > + *out = (s[0] - '0') * 10 + s[1] - '0'; > + return 0; > +} > + > +static int macsmc_battery_get_capacity_level(struct macsmc_power *power) > +{ > + bool flag; > + u32 val; > + int ret; > + > + /* Check for emergency shutdown condition */ > + if (apple_smc_read_u32(power->smc, SMC_KEY(BCF0), &val) >= 0 && val) > + return POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL; > + > + /* Check AC status for whether we could boot in this state */ > + if (apple_smc_read_u32(power->smc, SMC_KEY(ACSt), &val) >= 0) { > + if (!(val & ACSt_CAN_BOOT_IBOOT)) > + return POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL; > + > + if (!(val & ACSt_CAN_BOOT_AP)) > + return POWER_SUPPLY_CAPACITY_LEVEL_LOW; > + } > + > + ret = apple_smc_read_flag(power->smc, SMC_KEY(BSFC), &flag); > + if (ret < 0) > + return POWER_SUPPLY_CAPACITY_LEVEL_UNKNOWN; > + > + if (flag) > + return POWER_SUPPLY_CAPACITY_LEVEL_FULL; > + else > + return POWER_SUPPLY_CAPACITY_LEVEL_NORMAL; > +} > + > +static int macsmc_battery_get_property(struct power_supply *psy, > + enum power_supply_property psp, > + union power_supply_propval *val) > +{ > + struct macsmc_power *power = power_supply_get_drvdata(psy); > + int ret = 0; > + u8 vu8; > + u16 vu16; > + s16 vs16; > + s32 vs32; > + s64 vs64; > + bool flag; > + > + switch (psp) { > + case POWER_SUPPLY_PROP_STATUS: > + val->intval = macsmc_battery_get_status(power); > + ret = val->intval < 0 ? val->intval : 0; > + break; > + case POWER_SUPPLY_PROP_PRESENT: > + val->intval = 1; > + break; > + case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR: > + val->intval = macsmc_battery_get_charge_behaviour(power); > + ret = val->intval < 0 ? val->intval : 0; > + break; > + case POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(B0TE), &vu16); > + val->intval = vu16 == 0xffff ? 0 : vu16 * 60; > + break; > + case POWER_SUPPLY_PROP_TIME_TO_FULL_NOW: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(B0TF), &vu16); > + val->intval = vu16 == 0xffff ? 0 : vu16 * 60; > + break; > + case POWER_SUPPLY_PROP_CAPACITY: > + ret = apple_smc_read_u8(power->smc, SMC_KEY(BUIC), &vu8); > + val->intval = vu8; > + break; > + case POWER_SUPPLY_PROP_CAPACITY_LEVEL: > + val->intval = macsmc_battery_get_capacity_level(power); > + ret = val->intval < 0 ? val->intval : 0; > + break; > + case POWER_SUPPLY_PROP_VOLTAGE_NOW: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(B0AV), &vu16); > + val->intval = vu16 * 1000; > + break; > + case POWER_SUPPLY_PROP_CURRENT_NOW: > + ret = apple_smc_read_s16(power->smc, SMC_KEY(B0AC), &vs16); > + val->intval = vs16 * 1000; > + break; > + case POWER_SUPPLY_PROP_POWER_NOW: > + ret = apple_smc_read_s32(power->smc, SMC_KEY(B0AP), &vs32); > + val->intval = vs32 * 1000; > + break; > + case POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(BITV), &vu16); > + val->intval = vu16 * 1000; > + break; > + case POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN: > + /* Calculate total max design voltage from per-cell nominal voltage */ Using the nominal voltage here doesn't make much sense. As the comment above mentions not having nominal voltage data for the battery, I suppose the comment is wrong and this uses the per-cell **maximum** voltage? > + ret = apple_smc_read_u16(power->smc, SMC_KEY(BVVN), &vu16); > + val->intval = vu16 * 1000 * power->num_cells; > + break; > + case POWER_SUPPLY_PROP_VOLTAGE_MIN: > + /* Lifetime min */ > + ret = apple_smc_read_s16(power->smc, SMC_KEY(BLPM), &vs16); > + val->intval = vs16 * 1000; > + break; > + case POWER_SUPPLY_PROP_VOLTAGE_MAX: > + /* Lifetime max */ > + ret = apple_smc_read_s16(power->smc, SMC_KEY(BLPX), &vs16); > + val->intval = vs16 * 1000; > + break; > + case POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(B0RC), &vu16); > + val->intval = vu16 * 1000; > + break; > + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(B0RI), &vu16); > + val->intval = vu16 * 1000; > + break; > + case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(B0RV), &vu16); > + val->intval = vu16 * 1000; > + break; > + case POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(B0DC), &vu16); > + val->intval = vu16 * 1000; > + break; > + case POWER_SUPPLY_PROP_CHARGE_FULL: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(B0FC), &vu16); > + val->intval = vu16 * 1000; > + break; > + case POWER_SUPPLY_PROP_CHARGE_NOW: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(B0RM), &vu16); > + /* B0RM is Big Endian, likely pass through from TI gas gauge */ > + val->intval = (s16)swab16(vu16) * 1000; > + break; > + case POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(B0DC), &vu16); > + val->intval = vu16 * power->nominal_voltage_mv; > + break; > + case POWER_SUPPLY_PROP_ENERGY_FULL: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(B0FC), &vu16); > + val->intval = vu16 * power->nominal_voltage_mv; > + break; > + case POWER_SUPPLY_PROP_ENERGY_NOW: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(B0RM), &vu16); > + /* B0RM is Big Endian, likely pass through from TI gas gauge */ > + val->intval = (s16)swab16(vu16) * power->nominal_voltage_mv; > + break; > + case POWER_SUPPLY_PROP_TEMP: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(B0AT), &vu16); > + val->intval = vu16 - 2732; /* Kelvin x10 to Celsius x10 */ > + break; > + case POWER_SUPPLY_PROP_CHARGE_COUNTER: > + ret = apple_smc_read_s64(power->smc, SMC_KEY(BAAC), &vs64); > + val->intval = vs64; > + break; > + case POWER_SUPPLY_PROP_CYCLE_COUNT: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(B0CT), &vu16); > + val->intval = vu16; > + break; > + case POWER_SUPPLY_PROP_SCOPE: > + val->intval = POWER_SUPPLY_SCOPE_SYSTEM; > + break; > + case POWER_SUPPLY_PROP_HEALTH: > + flag = false; > + ret = apple_smc_read_flag(power->smc, SMC_KEY(BBAD), &flag); > + val->intval = flag ? POWER_SUPPLY_HEALTH_DEAD : POWER_SUPPLY_HEALTH_GOOD; > + break; > + case POWER_SUPPLY_PROP_MODEL_NAME: > + val->strval = power->model_name; > + break; > + case POWER_SUPPLY_PROP_SERIAL_NUMBER: > + val->strval = power->serial_number; > + break; > + case POWER_SUPPLY_PROP_MANUFACTURE_YEAR: > + ret = macsmc_battery_get_date(&power->mfg_date[0], &val->intval); > + /* The SMC reports the manufacture year as an offset from 1992. */ > + val->intval += 1992; > + break; > + case POWER_SUPPLY_PROP_MANUFACTURE_MONTH: > + ret = macsmc_battery_get_date(&power->mfg_date[2], &val->intval); > + break; > + case POWER_SUPPLY_PROP_MANUFACTURE_DAY: > + ret = macsmc_battery_get_date(&power->mfg_date[4], &val->intval); > + break; > + case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD: > + case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD: > + if (power->has_chls) { > + ret = apple_smc_read_u16(power->smc, SMC_KEY(CHLS), &vu16); > + val->intval = vu16 & 0xff; > + if (val->intval < CHLS_MIN_END_THRESHOLD || val->intval >= 100) > + val->intval = 100; > + } else if (power->has_chwa) { > + flag = false; > + ret = apple_smc_read_flag(power->smc, SMC_KEY(CHWA), &flag); > + val->intval = flag ? CHWA_FIXED_END_THRESHOLD : 100; > + } else { > + return -EINVAL; > + } > + if (psp == POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD && > + ret >= 0 && val->intval < 100 && val->intval >= CHLS_MIN_END_THRESHOLD) > + val->intval -= CHWA_CHLS_FIXED_START_OFFSET; > + break; > + default: > + return -EINVAL; > + } > + > + return ret; > +} > + > +static int macsmc_battery_set_property(struct power_supply *psy, > + enum power_supply_property psp, > + const union power_supply_propval *val) > +{ > + struct macsmc_power *power = power_supply_get_drvdata(psy); > + > + switch (psp) { > + case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR: > + return macsmc_battery_set_charge_behaviour(power, val->intval); > + case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD: > + /* > + * Read-only reflection of end threshold logic. > + * Allowed to be written to avoid userspace confusion, but ignored. > + */ > + return 0; I think it's better to fix userspace and expose the value read-only. In any case this creates an ABI and requires proper documentation in Documentation/ABI/testing/sysfs-class-power. My suggestion would be to drop this for now and handle it in a separate patchset, so that ABI discussion don't block the remaining driver. > + case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD: > + if (power->has_chls) { > + u16 kval = 0; > + /* Configurable logic for future expansion */ > + if (val->intval < CHLS_MIN_END_THRESHOLD) > + kval = CHLS_FORCE_DISCHARGE | CHLS_MIN_END_THRESHOLD; > + else if (val->intval < 100) > + kval = CHLS_FORCE_DISCHARGE | (val->intval & 0xff); > + > + return apple_smc_write_u16(power->smc, SMC_KEY(CHLS), kval); > + } else if (power->has_chwa) { > + return apple_smc_write_flag(power->smc, SMC_KEY(CHWA), > + val->intval <= CHWA_PROP_WRITE_THRESHOLD); > + } else { > + return -EINVAL; > + } > + default: > + return -EINVAL; > + } > +} > + > +static int macsmc_battery_property_is_writeable(struct power_supply *psy, > + enum power_supply_property psp) > +{ > + struct macsmc_power *power = power_supply_get_drvdata(psy); > + > + switch (psp) { > + case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR: > + return true; > + case POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD: > + case POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD: > + return power->has_chwa || power->has_chls; > + default: > + return false; > + } > +} > + > +static const struct power_supply_desc macsmc_battery_desc_template = { > + .name = "macsmc-battery", > + .type = POWER_SUPPLY_TYPE_BATTERY, > + .get_property = macsmc_battery_get_property, > + .set_property = macsmc_battery_set_property, > + .property_is_writeable = macsmc_battery_property_is_writeable, > +}; > + > +static int macsmc_ac_get_property(struct power_supply *psy, > + enum power_supply_property psp, > + union power_supply_propval *val) > +{ > + struct macsmc_power *power = power_supply_get_drvdata(psy); > + int ret = 0; > + u16 vu16; > + u32 vu32; > + > + switch (psp) { > + case POWER_SUPPLY_PROP_ONLINE: > + ret = apple_smc_read_u32(power->smc, SMC_KEY(CHIS), &vu32); > + val->intval = !!vu32; > + break; > + case POWER_SUPPLY_PROP_VOLTAGE_NOW: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(AC-n), &vu16); > + val->intval = vu16 * 1000; > + break; > + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT: > + ret = apple_smc_read_u16(power->smc, SMC_KEY(AC-i), &vu16); > + val->intval = vu16 * 1000; > + break; > + case POWER_SUPPLY_PROP_INPUT_POWER_LIMIT: > + ret = apple_smc_read_u32(power->smc, SMC_KEY(ACPW), &vu32); > + val->intval = vu32 * 1000; > + break; > + default: > + return -EINVAL; > + } > + > + return ret; > +} > + > +static const struct power_supply_desc macsmc_ac_desc_template = { > + .name = "macsmc-ac", > + .type = POWER_SUPPLY_TYPE_MAINS, > + .get_property = macsmc_ac_get_property, > +}; > + > +static void macsmc_power_critical_work(struct work_struct *wrk) > +{ > + struct macsmc_power *power = container_of(wrk, struct macsmc_power, critical_work); > + u16 bitv, b0av; > + u32 bcf0; > + > + if (!power->batt) > + return; > + > + /* > + * EMERGENCY: Check voltage vs design minimum. > + * If we are below BITV, the battery is physically exhausted. > + * We must shut down NOW to protect the filesystem. > + */ > + if (apple_smc_read_u16(power->smc, SMC_KEY(BITV), &bitv) >= 0 && > + apple_smc_read_u16(power->smc, SMC_KEY(B0AV), &b0av) >= 0 && > + b0av < bitv) { > + dev_emerg(power->dev, > + "Battery voltage (%d mV) below design minimum (%d mV)! Emergency shutdown.\n", > + b0av, bitv); > + kernel_power_off(); > + return; > + } > + > + /* > + * Avoid duplicate attempts at orderly shutdown. > + * Voltage check is above this as we may want to > + * "upgrade" an orderly shutdown to a critical power > + * off if voltage drops. > + */ > + if (power->shutdown_started || system_state > SYSTEM_RUNNING) > + return; > + > + /* > + * Check if SMC flagged the battery as empty. > + * We trigger a graceful shutdown to let the OS save data. > + */ > + if (apple_smc_read_u32(power->smc, SMC_KEY(BCF0), &bcf0) == 0 && bcf0 != 0) { > + power->shutdown_started = true; > + dev_crit(power->dev, "Battery critical (empty flag set). Triggering orderly shutdown.\n"); > + orderly_poweroff(true); > + } > +} > + > +static int macsmc_power_event(struct notifier_block *nb, unsigned long event, void *data) > +{ > + struct macsmc_power *power = container_of(nb, struct macsmc_power, nb); > + > + /* > + * SMC Event IDs are reverse-engineered. > + * 0x71... indicates power/battery events. > + */ > + if ((event & 0xffffff00) == 0x71010100 || /* Charger status change */ > + (event & 0xffff0000) == 0x71060000 || /* Port charge state change */ > + (event & 0xffff0000) == 0x71130000) { /* Connector insert/remove event */ > + if (power->batt) > + power_supply_changed(power->batt); > + if (power->ac) > + power_supply_changed(power->ac); > + return NOTIFY_OK; > + } else if (event == 0x71020000) { > + /* Critical battery warning */ > + if (power->batt) > + schedule_work(&power->critical_work); > + return NOTIFY_OK; > + } > + > + return NOTIFY_DONE; > +} > + > +static int macsmc_power_probe(struct platform_device *pdev) > +{ > + struct apple_smc *smc = dev_get_drvdata(pdev->dev.parent); You have a bunch of &pdev->dev further ahead, so I think it's sensible to create 'struct dev = &pdev->dev;' and use it everywhere in the probe function. > + struct power_supply_config psy_cfg = {}; > + struct macsmc_power *power; > + bool has_battery = false; > + bool has_ac_adapter = false; > + int ret; > + bool flag; > + u16 vu16; > + u32 val32; > + enum power_supply_property *props; > + size_t nprops; > + > + if (!smc) > + return -ENODEV; > + > + power = devm_kzalloc(&pdev->dev, sizeof(*power), GFP_KERNEL); > + if (!power) > + return -ENOMEM; > + > + power->dev = &pdev->dev; > + power->smc = smc; > + dev_set_drvdata(&pdev->dev, power); > + > + /* > + * Check for battery presence. > + * B0AV is a fundamental key. > + */ > + if (apple_smc_read_u16(power->smc, SMC_KEY(B0AV), &vu16) == 0 && > + macsmc_battery_get_status(power) > POWER_SUPPLY_STATUS_UNKNOWN) > + has_battery = true; > + > + /* > + * Check for AC adapter presence. > + * CHIS is a fundamental key. > + */ > + if (apple_smc_key_exists(smc, SMC_KEY(CHIS))) > + has_ac_adapter = true; > + > + if (has_battery) { > + power->batt_desc = macsmc_battery_desc_template; > + power->batt_props = devm_kcalloc(&pdev->dev, MACSMC_MAX_BATT_PROPS, > + sizeof(enum power_supply_property), > + GFP_KERNEL); > + if (!power->batt_props) > + return -ENOMEM; > + > + props = power->batt_props; > + nprops = 0; > + > + /* Fundamental properties */ > + props[nprops++] = POWER_SUPPLY_PROP_STATUS; > + props[nprops++] = POWER_SUPPLY_PROP_PRESENT; > + props[nprops++] = POWER_SUPPLY_PROP_VOLTAGE_NOW; > + props[nprops++] = POWER_SUPPLY_PROP_CURRENT_NOW; > + props[nprops++] = POWER_SUPPLY_PROP_POWER_NOW; > + props[nprops++] = POWER_SUPPLY_PROP_CAPACITY; > + props[nprops++] = POWER_SUPPLY_PROP_CAPACITY_LEVEL; > + props[nprops++] = POWER_SUPPLY_PROP_TEMP; > + props[nprops++] = POWER_SUPPLY_PROP_CYCLE_COUNT; > + props[nprops++] = POWER_SUPPLY_PROP_HEALTH; > + props[nprops++] = POWER_SUPPLY_PROP_SCOPE; > + props[nprops++] = POWER_SUPPLY_PROP_MODEL_NAME; > + props[nprops++] = POWER_SUPPLY_PROP_SERIAL_NUMBER; > + props[nprops++] = POWER_SUPPLY_PROP_MANUFACTURE_YEAR; > + props[nprops++] = POWER_SUPPLY_PROP_MANUFACTURE_MONTH; > + props[nprops++] = POWER_SUPPLY_PROP_MANUFACTURE_DAY; > + > + /* Extended properties usually present */ > + props[nprops++] = POWER_SUPPLY_PROP_TIME_TO_EMPTY_NOW; > + props[nprops++] = POWER_SUPPLY_PROP_TIME_TO_FULL_NOW; > + props[nprops++] = POWER_SUPPLY_PROP_VOLTAGE_MIN_DESIGN; > + props[nprops++] = POWER_SUPPLY_PROP_VOLTAGE_MAX_DESIGN; > + props[nprops++] = POWER_SUPPLY_PROP_VOLTAGE_MIN; > + props[nprops++] = POWER_SUPPLY_PROP_VOLTAGE_MAX; > + props[nprops++] = POWER_SUPPLY_PROP_CHARGE_TERM_CURRENT; > + props[nprops++] = POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX; > + props[nprops++] = POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE; > + props[nprops++] = POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN; > + props[nprops++] = POWER_SUPPLY_PROP_CHARGE_FULL; > + props[nprops++] = POWER_SUPPLY_PROP_CHARGE_NOW; > + props[nprops++] = POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN; > + props[nprops++] = POWER_SUPPLY_PROP_ENERGY_FULL; > + props[nprops++] = POWER_SUPPLY_PROP_ENERGY_NOW; > + props[nprops++] = POWER_SUPPLY_PROP_CHARGE_COUNTER; > + > + /* Detect features based on key availability */ > + if (apple_smc_key_exists(smc, SMC_KEY(CHTE))) > + power->has_chte = true; > + if (apple_smc_key_exists(smc, SMC_KEY(CH0C))) > + power->has_ch0c = true; > + if (apple_smc_key_exists(smc, SMC_KEY(CH0I))) > + power->has_ch0i = true; > + > + /* Reset "Optimised Battery Charging" flags to default state */ > + if (power->has_chte) > + apple_smc_write_u32(smc, SMC_KEY(CHTE), 0); > + else if (power->has_ch0c) > + apple_smc_write_u8(smc, SMC_KEY(CH0C), 0); > + > + if (power->has_ch0i) > + apple_smc_write_u8(smc, SMC_KEY(CH0I), 0); > + > + apple_smc_write_u8(smc, SMC_KEY(CH0K), 0); > + apple_smc_write_u8(smc, SMC_KEY(CH0B), 0); > + > + /* Configure charge behaviour if supported */ > + if (power->has_ch0i || power->has_ch0c || power->has_chte) { > + props[nprops++] = POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR; > + > + power->batt_desc.charge_behaviours = > + BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO); > + > + if (power->has_ch0i) > + power->batt_desc.charge_behaviours |= > + BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_FORCE_DISCHARGE); > + > + if (power->has_chte || power->has_ch0c) > + power->batt_desc.charge_behaviours |= > + BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE); > + } > + > + /* Detect charge limit method (CHWA vs CHLS) */ > + if (apple_smc_read_flag(power->smc, SMC_KEY(CHWA), &flag) == 0) > + power->has_chwa = true; > + else if (apple_smc_read_u16(power->smc, SMC_KEY(CHLS), &vu16) >= 0) > + power->has_chls = true; > + > + if (power->has_chwa || power->has_chls) { > + props[nprops++] = POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD; > + props[nprops++] = POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD; > + } > + > + power->batt_desc.properties = power->batt_props; It seems 'power->batt_props' is not used anywhere and you can just use 'power->batt_desc.properties' directly? > + power->batt_desc.num_properties = nprops; > + > + /* Fetch identity strings */ > + apple_smc_read(smc, SMC_KEY(BMDN), power->model_name, > + sizeof(power->model_name) - 1); > + apple_smc_read(smc, SMC_KEY(BMSN), power->serial_number, > + sizeof(power->serial_number) - 1); > + apple_smc_read(smc, SMC_KEY(BMDT), power->mfg_date, > + sizeof(power->mfg_date) - 1); > + > + apple_smc_read_u8(power->smc, SMC_KEY(BNCB), &power->num_cells); > + power->nominal_voltage_mv = MACSMC_NOMINAL_CELL_VOLTAGE_MV * power->num_cells; > + > + /* Enable critical shutdown notifications by reading status once */ > + apple_smc_read_u32(power->smc, SMC_KEY(BCF0), &val32); > + > + psy_cfg.drv_data = power; > + power->batt = devm_power_supply_register(&pdev->dev, &power->batt_desc, &psy_cfg); > + if (IS_ERR(power->batt)) { > + ret = dev_err_probe(&pdev->dev, PTR_ERR(power->batt), > + "Failed to register battery\n"); No need to assign ret; it is unused. But it becomes necessary with a follow-up suggestion from me. > + /* Don't return failure yet; try AC registration first */ > + power->batt = NULL; > + } > + } else { > + dev_dbg(&pdev->dev, "No battery detected.\n"); > + } > + > + if (has_ac_adapter) { > + power->ac_desc = macsmc_ac_desc_template; > + power->ac_props = devm_kcalloc(&pdev->dev, MACSMC_MAX_AC_PROPS, > + sizeof(enum power_supply_property), > + GFP_KERNEL); > + if (!power->ac_props) > + return -ENOMEM; > + > + props = power->ac_props; > + nprops = 0; > + > + /* Online status is fundamental */ > + props[nprops++] = POWER_SUPPLY_PROP_ONLINE; > + > + /* Input power limits are usually available */ > + if (apple_smc_key_exists(power->smc, SMC_KEY(ACPW))) > + props[nprops++] = POWER_SUPPLY_PROP_INPUT_POWER_LIMIT; > + > + /* macOS 15.4+ firmware dropped legacy AC keys (AC-n, AC-i) */ > + if (apple_smc_read_u16(power->smc, SMC_KEY(AC-n), &vu16) >= 0) { > + props[nprops++] = POWER_SUPPLY_PROP_VOLTAGE_NOW; > + props[nprops++] = POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT; > + } > + > + power->ac_desc.properties = power->ac_props; Just like 'power->batt_props': You can drop power->ac_props and use power->ac_desc.properties directly. > + power->ac_desc.num_properties = nprops; > + > + psy_cfg.drv_data = power; > + power->ac = devm_power_supply_register(&pdev->dev, &power->ac_desc, &psy_cfg); > + if (IS_ERR(power->ac)) { > + ret = dev_err_probe(&pdev->dev, PTR_ERR(power->ac), > + "Failed to register AC adapter\n"); > + /* If battery also failed or didn't exist, this is a fatal error */ > + if (!power->batt) > + return ret; You can just drop this check and instead rely on the "Final check" > + power->ac = NULL; > + } > + } else { > + dev_dbg(&pdev->dev, "No A/C adapter detected.\n"); > + } > + > + /* Final check: did we register anything? */ > + if (!power->batt && !power->ac) > + return -ENODEV; You can just return ret here, if you assign -ENODEV as initial value. In that case the correct error code will be returned in all cases. > + power->nb.notifier_call = macsmc_power_event; > + blocking_notifier_chain_register(&smc->event_handlers, &power->nb); > + > + INIT_WORK(&power->critical_work, macsmc_power_critical_work); This must happen before registering the event handler, which in theory might instant-trigger an event creating a race condition. You already have the correct order in the remove handler (which also hints about this problem :)). After reordering you can further simplify by just using devm_work_autocancel(). > + > + return 0; > +} > + > +static void macsmc_power_remove(struct platform_device *pdev) > +{ > + struct macsmc_power *power = dev_get_drvdata(&pdev->dev); > + > + blocking_notifier_chain_unregister(&power->smc->event_handlers, &power->nb); > + cancel_work_sync(&power->critical_work); > +} > + > +static struct platform_driver macsmc_power_driver = { > + .driver = { > + .name = "macsmc-power", > + .owner = THIS_MODULE, There is no need to set platform_driver.driver.owner manually. It is handled automatically, so please drop. > + }, > + .probe = macsmc_power_probe, > + .remove = macsmc_power_remove, > +}; > +module_platform_driver(macsmc_power_driver); > + > +MODULE_LICENSE("Dual MIT/GPL"); > +MODULE_DESCRIPTION("Apple SMC battery and power management driver"); > +MODULE_AUTHOR("Hector Martin "); > +MODULE_AUTHOR("Michael Reeves "); > +MODULE_ALIAS("platform:macsmc-power"); Drop MODULE_ALIAS and instead add a platform_device_id table with MODULE_DEVICE_TABLE(platform, ); Greetings, -- Sebastian