Thread (11 messages) 11 messages, 5 authors, 2026-01-18

Re: [PATCH v3 1/2] power: supply: Add macsmc-power driver for Apple Silicon

From: Janne Grunau <j@jannau.net>
Date: 2026-01-18 12:49:47
Also in: asahi, linux-pm, lkml

On Sun, Jan 18, 2026 at 08:19:38PM +0800, Nick Chan wrote:

On 17/1/2026 20:26, Janne Grunau wrote:
quoted
On Thu, Jan 15, 2026 at 06:08:15PM +1100, Michael Reeves via B4 Relay wrote:
quoted
From: Michael Reeves <redacted>
I think the driver is overall similar to the downstream AsahiLinux
driver so please keep Hector as author.
quoted
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 <redacted>
Signed-off-by: Hector Martin <redacted>
The downstream driver a quite a few more Co-developed-by:/Sobs. When I
squashed the commits I decided to err on the safe side and included
commit authors from incremental patches as Co-developed-by: Why did you
drop those?
quoted
Reviewed-by: Neal Gompa <neal@gompa.dev>
Signed-off-by: Michael Reeves <redacted>
---
 MAINTAINERS                         |   1 +
 drivers/power/supply/Kconfig        |  11 +
 drivers/power/supply/Makefile       |   1 +
 drivers/power/supply/macsmc-power.c | 834 ++++++++++++++++++++++++++++++++++++
 4 files changed, 847 insertions(+)
diff --git a/drivers/power/supply/macsmc-power.c b/drivers/power/supply/macsmc-power.c
new file mode 100644
index 000000000000..9b3faefe7a45
--- /dev/null
+++ b/drivers/power/supply/macsmc-power.c
@@ -0,0 +1,834 @@
...
quoted
quoted
+static int macsmc_power_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct apple_smc *smc = dev_get_drvdata(pdev->dev.parent);
+	struct power_supply_config psy_cfg = {};
+	struct macsmc_power *power;
+	bool has_battery = false;
+	bool has_ac_adapter = false;
+	int ret = -ENODEV;
+	bool flag;
+	u16 vu16;
+	u32 val32;
+	enum power_supply_property *props;
+	size_t nprops;
+
+	if (!smc)
+		return -ENODEV;
+
+	power = devm_kzalloc(dev, sizeof(*power), GFP_KERNEL);
+	if (!power)
+		return -ENOMEM;
+
+	power->dev = dev;
+	power->smc = smc;
+	dev_set_drvdata(dev, power);
+
+	INIT_WORK(&power->critical_work, macsmc_power_critical_work);
+	ret = devm_work_autocancel(dev, &power->critical_work, macsmc_power_critical_work);
+	if (ret)
+		return ret;
+
+	/*
+	 * 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;
+
I think a short circuit check for !(has_battery || has_ac_adapter) would
make sense here. The setup code for props is quite long. It should
return -ENODEV. ret is not -ENODEV. anymore since it was overwritten
with the return value of devm_work_autocancel()
quoted
+	if (has_battery) {
+		power->batt_desc = macsmc_battery_desc_template;
+		props = devm_kcalloc(dev, MACSMC_MAX_BATT_PROPS,
+				     sizeof(enum power_supply_property),
+				     GFP_KERNEL);
I don't like the dynamic allocation for the props. I think we can
currently get way with static property arrays and so we should do that.
See my comments below
Dynamic allocation is needed to properly add support pre-M1 support,
as the SMCs in them misses a few keys, yet in iOS 16 the charge limit
(CHWA) keys are added. In a similar way, there really is no sane way
to add any new properties for M1/M2/M3 with static allocation. So I
would prefer if dynamic allocation is kept.
ack, I wasn't really thinking beyond the code in front of me. Let's go
with dynamic allocations. A check that nprops doesn't exceed
MACSMC_MAX_BATT_PROPS should be added then.

Janne
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help